On 04/07/2010 03:00 PM, [email protected] wrote:
> The patch titled
> x86: fix handling of the 'reservetop' boot option
> has been added to the -mm tree. Its filename is
> x86-fix-handling-of-the-reservetop-boot-option.patch
>
> Before you just go and hit "reply", please:
> a) Consider who else should be cc'ed
> b) Prefer to cc a suitable mailing list as well
> c) Ideally: find the original patch on the mailing list and do a
> reply-to-all to that, adding suitable additional cc's
>
> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>
> See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
> out what to do about this
>
> The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
>
> ------------------------------------------------------
> Subject: x86: fix handling of the 'reservetop' boot option
> From: Liang Li <[email protected]>
>
> When specifying the 'reservetop=0xbadc0de' kernel parameter, the kernel
> will stop booting due to a early_ioremap bug that relate to commit
> 8827247ff ("x86: don't define __this_fixmap_does_not_exist()").
>
> The root cause of boot failure problem is the value of 'slot_virt[i]' was
> initialized in setup_arch->early_ioremap_init. But later in setup_arch,
> the function 'parse_early_param' will modify 'FIXADDR_TOP' when
> 'reservetop=0xbadc0de' being specified.
>
> The simplest fix might be use __fix_to_virt(idx0) to get updated value
> of 'FIXADDR_TOP' in '__early_ioremap' instead of reference old value
> from slot_virt[slot] directly.
>
> Signed-off-by: Liang Li <[email protected]>
> Cc: Wang Chen <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Yinghai Lu <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> arch/x86/mm/ioremap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff -puN arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option arch/x86/mm/ioremap.c
> --- a/arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option
> +++ a/arch/x86/mm/ioremap.c
> @@ -537,9 +537,9 @@ __early_ioremap(resource_size_t phys_add
> --nrpages;
> }
> if (early_ioremap_debug)
> - printk(KERN_CONT "%08lx + %08lx\n", offset, slot_virt[slot]);
> + printk(KERN_CONT "%08lx + %08lx\n", offset, __fix_to_virt(idx0));
>
> - prev_map[slot] = (void __iomem *)(offset + slot_virt[slot]);
> + prev_map[slot] = (void __iomem *)(offset + __fix_to_virt(idx0));
> return prev_map[slot];
> }
>
> _
not that simple. but it looks like correct direction.
please consider:
when early_parsing reserve_top, double check if there is left over in prev_map[], and
reinitialize slot_virt[] and clear old PMD and setup new PMD if needed.
Thanks
Yinghai
On Wed, Apr 07, 2010 at 04:52:03PM -0700, Yinghai wrote:
> On 04/07/2010 03:00 PM, [email protected] wrote:
> > The patch titled
> > x86: fix handling of the 'reservetop' boot option
> > has been added to the -mm tree. Its filename is
> > x86-fix-handling-of-the-reservetop-boot-option.patch
> >
> > Before you just go and hit "reply", please:
> > a) Consider who else should be cc'ed
> > b) Prefer to cc a suitable mailing list as well
> > c) Ideally: find the original patch on the mailing list and do a
> > reply-to-all to that, adding suitable additional cc's
> >
> > *** Remember to use Documentation/SubmitChecklist when testing your code ***
> >
> > See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
> > out what to do about this
> >
> > The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
> >
> > ------------------------------------------------------
> > Subject: x86: fix handling of the 'reservetop' boot option
> > From: Liang Li <[email protected]>
> >
> > When specifying the 'reservetop=0xbadc0de' kernel parameter, the kernel
> > will stop booting due to a early_ioremap bug that relate to commit
> > 8827247ff ("x86: don't define __this_fixmap_does_not_exist()").
> >
> > The root cause of boot failure problem is the value of 'slot_virt[i]' was
> > initialized in setup_arch->early_ioremap_init. But later in setup_arch,
> > the function 'parse_early_param' will modify 'FIXADDR_TOP' when
> > 'reservetop=0xbadc0de' being specified.
> >
> > The simplest fix might be use __fix_to_virt(idx0) to get updated value
> > of 'FIXADDR_TOP' in '__early_ioremap' instead of reference old value
> > from slot_virt[slot] directly.
> >
> > Signed-off-by: Liang Li <[email protected]>
> > Cc: Wang Chen <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: "H. Peter Anvin" <[email protected]>
> > Cc: Yinghai Lu <[email protected]>
> > Signed-off-by: Andrew Morton <[email protected]>
> > ---
> >
> > arch/x86/mm/ioremap.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff -puN arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option arch/x86/mm/ioremap.c
> > --- a/arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option
> > +++ a/arch/x86/mm/ioremap.c
> > @@ -537,9 +537,9 @@ __early_ioremap(resource_size_t phys_add
> > --nrpages;
> > }
> > if (early_ioremap_debug)
> > - printk(KERN_CONT "%08lx + %08lx\n", offset, slot_virt[slot]);
> > + printk(KERN_CONT "%08lx + %08lx\n", offset, __fix_to_virt(idx0));
> >
> > - prev_map[slot] = (void __iomem *)(offset + slot_virt[slot]);
> > + prev_map[slot] = (void __iomem *)(offset + __fix_to_virt(idx0));
> > return prev_map[slot];
> > }
> >
> > _
>
> not that simple. but it looks like correct direction.
>
> please consider:
> when early_parsing reserve_top, double check if there is left over in prev_map[], and
> reinitialize slot_virt[] and clear old PMD and setup new PMD if needed.
Hi Yinghai,
Thanks for your reply, its better to have eyes on then being ignored. :)
Your suggestions were considered before the patch to public, let me try
to explain:
#1 check/adjust prev_map[]?
In my tests, seems early_ioremap is untouched between early_ioremap_init
and parse_early_param so I did not check prev_map. Even its get touched,
I think we could do nothing to this mapping, since prev_map[i] just
record virt addr for clients of early_ioremap. We can check and adjust
prev_map but clients of early_ioremap won't realize the fact so nothing
being fixed or broken.
#2 reinitialize slot_virt and update PMD
I actually tried this approach, call early_ioremap_init again after
parse_early_param will do that work, it also works but I am not sure
that is the better solution or too heavy for solve the problem? So I
tend to say 'simplest' solution in git commit log.
Thanks and best regards,
-Liang Li
On 04/07/2010 06:05 PM, Liang Li wrote:
> On Wed, Apr 07, 2010 at 04:52:03PM -0700, Yinghai wrote:
>> On 04/07/2010 03:00 PM, [email protected] wrote:
>>> The patch titled
>>> x86: fix handling of the 'reservetop' boot option
>>> has been added to the -mm tree. Its filename is
>>> x86-fix-handling-of-the-reservetop-boot-option.patch
>>>
>>> Before you just go and hit "reply", please:
>>> a) Consider who else should be cc'ed
>>> b) Prefer to cc a suitable mailing list as well
>>> c) Ideally: find the original patch on the mailing list and do a
>>> reply-to-all to that, adding suitable additional cc's
>>>
>>> *** Remember to use Documentation/SubmitChecklist when testing your code ***
>>>
>>> See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
>>> out what to do about this
>>>
>>> The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
>>>
>>> ------------------------------------------------------
>>> Subject: x86: fix handling of the 'reservetop' boot option
>>> From: Liang Li <[email protected]>
>>>
>>> When specifying the 'reservetop=0xbadc0de' kernel parameter, the kernel
>>> will stop booting due to a early_ioremap bug that relate to commit
>>> 8827247ff ("x86: don't define __this_fixmap_does_not_exist()").
>>>
>>> The root cause of boot failure problem is the value of 'slot_virt[i]' was
>>> initialized in setup_arch->early_ioremap_init. But later in setup_arch,
>>> the function 'parse_early_param' will modify 'FIXADDR_TOP' when
>>> 'reservetop=0xbadc0de' being specified.
>>>
>>> The simplest fix might be use __fix_to_virt(idx0) to get updated value
>>> of 'FIXADDR_TOP' in '__early_ioremap' instead of reference old value
>>> from slot_virt[slot] directly.
>>>
>>> Signed-off-by: Liang Li <[email protected]>
>>> Cc: Wang Chen <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: Thomas Gleixner <[email protected]>
>>> Cc: "H. Peter Anvin" <[email protected]>
>>> Cc: Yinghai Lu <[email protected]>
>>> Signed-off-by: Andrew Morton <[email protected]>
>>> ---
>>>
>>> arch/x86/mm/ioremap.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff -puN arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option arch/x86/mm/ioremap.c
>>> --- a/arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option
>>> +++ a/arch/x86/mm/ioremap.c
>>> @@ -537,9 +537,9 @@ __early_ioremap(resource_size_t phys_add
>>> --nrpages;
>>> }
>>> if (early_ioremap_debug)
>>> - printk(KERN_CONT "%08lx + %08lx\n", offset, slot_virt[slot]);
>>> + printk(KERN_CONT "%08lx + %08lx\n", offset, __fix_to_virt(idx0));
>>>
>>> - prev_map[slot] = (void __iomem *)(offset + slot_virt[slot]);
>>> + prev_map[slot] = (void __iomem *)(offset + __fix_to_virt(idx0));
>>> return prev_map[slot];
>>> }
>>>
>>> _
>>
>> not that simple. but it looks like correct direction.
>>
>> please consider:
>> when early_parsing reserve_top, double check if there is left over in prev_map[], and
>> reinitialize slot_virt[] and clear old PMD and setup new PMD if needed.
>
> Hi Yinghai,
>
> Thanks for your reply, its better to have eyes on then being ignored. :)
>
> Your suggestions were considered before the patch to public, let me try
> to explain:
>
> #1 check/adjust prev_map[]?
> In my tests, seems early_ioremap is untouched between early_ioremap_init
> and parse_early_param so I did not check prev_map. Even its get touched,
> I think we could do nothing to this mapping, since prev_map[i] just
> record virt addr for clients of early_ioremap. We can check and adjust
> prev_map but clients of early_ioremap won't realize the fact so nothing
> being fixed or broken.
efi related code need them
dmi
you need to add bug_on if there is still have left over, and need the caller to re map it again later.
>
> #2 reinitialize slot_virt and update PMD
> I actually tried this approach, call early_ioremap_init again after
> parse_early_param will do that work, it also works but I am not sure
> that is the better solution or too heavy for solve the problem? So I
> tend to say 'simplest' solution in git commit log.
how about PMD? you don't need set PMD again.
YH
On Wed, Apr 07, 2010 at 06:13:56PM -0700, Yinghai wrote:
> On 04/07/2010 06:05 PM, Liang Li wrote:
> > On Wed, Apr 07, 2010 at 04:52:03PM -0700, Yinghai wrote:
> >> On 04/07/2010 03:00 PM, [email protected] wrote:
> >>> The patch titled
> >>> x86: fix handling of the 'reservetop' boot option
> >>> has been added to the -mm tree. Its filename is
> >>> x86-fix-handling-of-the-reservetop-boot-option.patch
> >>>
> >>> Before you just go and hit "reply", please:
> >>> a) Consider who else should be cc'ed
> >>> b) Prefer to cc a suitable mailing list as well
> >>> c) Ideally: find the original patch on the mailing list and do a
> >>> reply-to-all to that, adding suitable additional cc's
> >>>
> >>> *** Remember to use Documentation/SubmitChecklist when testing your code ***
> >>>
> >>> See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
> >>> out what to do about this
> >>>
> >>> The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
> >>>
> >>> ------------------------------------------------------
> >>> Subject: x86: fix handling of the 'reservetop' boot option
> >>> From: Liang Li <[email protected]>
> >>>
> >>> When specifying the 'reservetop=0xbadc0de' kernel parameter, the kernel
> >>> will stop booting due to a early_ioremap bug that relate to commit
> >>> 8827247ff ("x86: don't define __this_fixmap_does_not_exist()").
> >>>
> >>> The root cause of boot failure problem is the value of 'slot_virt[i]' was
> >>> initialized in setup_arch->early_ioremap_init. But later in setup_arch,
> >>> the function 'parse_early_param' will modify 'FIXADDR_TOP' when
> >>> 'reservetop=0xbadc0de' being specified.
> >>>
> >>> The simplest fix might be use __fix_to_virt(idx0) to get updated value
> >>> of 'FIXADDR_TOP' in '__early_ioremap' instead of reference old value
> >>> from slot_virt[slot] directly.
> >>>
> >>> Signed-off-by: Liang Li <[email protected]>
> >>> Cc: Wang Chen <[email protected]>
> >>> Cc: Ingo Molnar <[email protected]>
> >>> Cc: Thomas Gleixner <[email protected]>
> >>> Cc: "H. Peter Anvin" <[email protected]>
> >>> Cc: Yinghai Lu <[email protected]>
> >>> Signed-off-by: Andrew Morton <[email protected]>
> >>> ---
> >>>
> >>> arch/x86/mm/ioremap.c | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff -puN arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option arch/x86/mm/ioremap.c
> >>> --- a/arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option
> >>> +++ a/arch/x86/mm/ioremap.c
> >>> @@ -537,9 +537,9 @@ __early_ioremap(resource_size_t phys_add
> >>> --nrpages;
> >>> }
> >>> if (early_ioremap_debug)
> >>> - printk(KERN_CONT "%08lx + %08lx\n", offset, slot_virt[slot]);
> >>> + printk(KERN_CONT "%08lx + %08lx\n", offset, __fix_to_virt(idx0));
> >>>
> >>> - prev_map[slot] = (void __iomem *)(offset + slot_virt[slot]);
> >>> + prev_map[slot] = (void __iomem *)(offset + __fix_to_virt(idx0));
> >>> return prev_map[slot];
> >>> }
> >>>
> >>> _
> >>
> >> not that simple. but it looks like correct direction.
> >>
> >> please consider:
> >> when early_parsing reserve_top, double check if there is left over in prev_map[], and
> >> reinitialize slot_virt[] and clear old PMD and setup new PMD if needed.
> >
> > Hi Yinghai,
> >
> > Thanks for your reply, its better to have eyes on then being ignored. :)
> >
> > Your suggestions were considered before the patch to public, let me try
> > to explain:
> >
> > #1 check/adjust prev_map[]?
> > In my tests, seems early_ioremap is untouched between early_ioremap_init
> > and parse_early_param so I did not check prev_map. Even its get touched,
> > I think we could do nothing to this mapping, since prev_map[i] just
> > record virt addr for clients of early_ioremap. We can check and adjust
> > prev_map but clients of early_ioremap won't realize the fact so nothing
> > being fixed or broken.
>
> efi related code need them
>
> dmi
>
> you need to add bug_on if there is still have left over, and need the caller to re map it again later.
>
> >
> > #2 reinitialize slot_virt and update PMD
> > I actually tried this approach, call early_ioremap_init again after
> > parse_early_param will do that work, it also works but I am not sure
> > that is the better solution or too heavy for solve the problem? So I
> > tend to say 'simplest' solution in git commit log.
>
> how about PMD? you don't need set PMD again.
>
> YH
Hi Yinghai,
Does this similar modification like this is more preferred?
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index a1dcfa3..30a3e97 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
extern void __iomem *early_memremap(resource_size_t phys_addr,
unsigned long size);
extern void early_iounmap(void __iomem *addr, unsigned long size);
+extern void fixup_early_ioremap(void);
#define IO_SPACE_LIMIT 0xffff
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index ea82ef0..fe06296 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -448,6 +448,23 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
+void __init fixup_early_ioremap(void)
+{
+ int i;
+ for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
+ if (prev_map[i])
+ break;
+ }
+
+ if (i == FIX_BTMAPS_SLOTS)
+ WARN_ON(1);
+
+ for (i = 0; i < FIX_BTMAPS_SLOTS; i++)
+ slot_virt[i] = __fix_to_virt(FIX_BTMAP_BEGIN - NR_FIX_BTMAPS * i);
+
+ return;
+}
+
static int __init check_early_ioremap_leak(void)
{
int count = 0;
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index 5c4ee42..ea4d54c 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -4,6 +4,7 @@
#include <asm/pgtable.h>
#include <asm/tlb.h>
#include <asm/fixmap.h>
+#include <asm/io.h>
#define PGALLOC_GFP GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO
@@ -351,6 +352,7 @@ void __init reserve_top_address(unsigned long reserve)
printk(KERN_INFO "Reserving virtual address space above 0x%08x\n",
(int)-reserve);
__FIXADDR_TOP = -reserve - PAGE_SIZE;
+ fixup_early_ioremap();
#endif
}
Thanks,
-Liang Li
On Thu, Apr 08, 2010 at 09:53:53AM +0800, Liang Li wrote:
> On Wed, Apr 07, 2010 at 06:13:56PM -0700, Yinghai wrote:
> > On 04/07/2010 06:05 PM, Liang Li wrote:
> > > On Wed, Apr 07, 2010 at 04:52:03PM -0700, Yinghai wrote:
> > >> On 04/07/2010 03:00 PM, [email protected] wrote:
> > >>> The patch titled
> > >>> x86: fix handling of the 'reservetop' boot option
> > >>> has been added to the -mm tree. Its filename is
> > >>> x86-fix-handling-of-the-reservetop-boot-option.patch
> > >>>
> > >>> Before you just go and hit "reply", please:
> > >>> a) Consider who else should be cc'ed
> > >>> b) Prefer to cc a suitable mailing list as well
> > >>> c) Ideally: find the original patch on the mailing list and do a
> > >>> reply-to-all to that, adding suitable additional cc's
> > >>>
> > >>> *** Remember to use Documentation/SubmitChecklist when testing your code ***
> > >>>
> > >>> See http://userweb.kernel.org/~akpm/stuff/added-to-mm.txt to find
> > >>> out what to do about this
> > >>>
> > >>> The current -mm tree may be found at http://userweb.kernel.org/~akpm/mmotm/
> > >>>
> > >>> ------------------------------------------------------
> > >>> Subject: x86: fix handling of the 'reservetop' boot option
> > >>> From: Liang Li <[email protected]>
> > >>>
> > >>> When specifying the 'reservetop=0xbadc0de' kernel parameter, the kernel
> > >>> will stop booting due to a early_ioremap bug that relate to commit
> > >>> 8827247ff ("x86: don't define __this_fixmap_does_not_exist()").
> > >>>
> > >>> The root cause of boot failure problem is the value of 'slot_virt[i]' was
> > >>> initialized in setup_arch->early_ioremap_init. But later in setup_arch,
> > >>> the function 'parse_early_param' will modify 'FIXADDR_TOP' when
> > >>> 'reservetop=0xbadc0de' being specified.
> > >>>
> > >>> The simplest fix might be use __fix_to_virt(idx0) to get updated value
> > >>> of 'FIXADDR_TOP' in '__early_ioremap' instead of reference old value
> > >>> from slot_virt[slot] directly.
> > >>>
> > >>> Signed-off-by: Liang Li <[email protected]>
> > >>> Cc: Wang Chen <[email protected]>
> > >>> Cc: Ingo Molnar <[email protected]>
> > >>> Cc: Thomas Gleixner <[email protected]>
> > >>> Cc: "H. Peter Anvin" <[email protected]>
> > >>> Cc: Yinghai Lu <[email protected]>
> > >>> Signed-off-by: Andrew Morton <[email protected]>
> > >>> ---
> > >>>
> > >>> arch/x86/mm/ioremap.c | 4 ++--
> > >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> > >>>
> > >>> diff -puN arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option arch/x86/mm/ioremap.c
> > >>> --- a/arch/x86/mm/ioremap.c~x86-fix-handling-of-the-reservetop-boot-option
> > >>> +++ a/arch/x86/mm/ioremap.c
> > >>> @@ -537,9 +537,9 @@ __early_ioremap(resource_size_t phys_add
> > >>> --nrpages;
> > >>> }
> > >>> if (early_ioremap_debug)
> > >>> - printk(KERN_CONT "%08lx + %08lx\n", offset, slot_virt[slot]);
> > >>> + printk(KERN_CONT "%08lx + %08lx\n", offset, __fix_to_virt(idx0));
> > >>>
> > >>> - prev_map[slot] = (void __iomem *)(offset + slot_virt[slot]);
> > >>> + prev_map[slot] = (void __iomem *)(offset + __fix_to_virt(idx0));
> > >>> return prev_map[slot];
> > >>> }
> > >>>
> > >>> _
> > >>
> > >> not that simple. but it looks like correct direction.
> > >>
> > >> please consider:
> > >> when early_parsing reserve_top, double check if there is left over in prev_map[], and
> > >> reinitialize slot_virt[] and clear old PMD and setup new PMD if needed.
> > >
> > > Hi Yinghai,
> > >
> > > Thanks for your reply, its better to have eyes on then being ignored. :)
> > >
> > > Your suggestions were considered before the patch to public, let me try
> > > to explain:
> > >
> > > #1 check/adjust prev_map[]?
> > > In my tests, seems early_ioremap is untouched between early_ioremap_init
> > > and parse_early_param so I did not check prev_map. Even its get touched,
> > > I think we could do nothing to this mapping, since prev_map[i] just
> > > record virt addr for clients of early_ioremap. We can check and adjust
> > > prev_map but clients of early_ioremap won't realize the fact so nothing
> > > being fixed or broken.
> >
> > efi related code need them
> >
> > dmi
> >
> > you need to add bug_on if there is still have left over, and need the caller to re map it again later.
> >
> > >
> > > #2 reinitialize slot_virt and update PMD
> > > I actually tried this approach, call early_ioremap_init again after
> > > parse_early_param will do that work, it also works but I am not sure
> > > that is the better solution or too heavy for solve the problem? So I
> > > tend to say 'simplest' solution in git commit log.
> >
> > how about PMD? you don't need set PMD again.
> >
> > YH
>
> Hi Yinghai,
>
> Does this similar modification like this is more preferred?
>
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index a1dcfa3..30a3e97 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
> extern void __iomem *early_memremap(resource_size_t phys_addr,
> unsigned long size);
> extern void early_iounmap(void __iomem *addr, unsigned long size);
> +extern void fixup_early_ioremap(void);
>
> #define IO_SPACE_LIMIT 0xffff
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index ea82ef0..fe06296 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -448,6 +448,23 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>
> +void __init fixup_early_ioremap(void)
> +{
> + int i;
> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
> + if (prev_map[i])
> + break;
> + }
> +
> + if (i == FIX_BTMAPS_SLOTS)
> + WARN_ON(1);
Should be:
if (i < FIX_BTMAPS_SLOTS)
WARN_ON(1);
On 04/07/2010 06:53 PM, Liang Li wrote:
> Does this similar modification like this is more preferred?
>
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index a1dcfa3..30a3e97 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
> extern void __iomem *early_memremap(resource_size_t phys_addr,
> unsigned long size);
> extern void early_iounmap(void __iomem *addr, unsigned long size);
> +extern void fixup_early_ioremap(void);
>
> #define IO_SPACE_LIMIT 0xffff
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index ea82ef0..fe06296 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -448,6 +448,23 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>
> +void __init fixup_early_ioremap(void)
> +{
> + int i;
> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
> + if (prev_map[i])
> + break;
> + }
> +
> + if (i == FIX_BTMAPS_SLOTS)
<
> + WARN_ON(1);
BUG_ON()
> +
> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++)
> + slot_virt[i] = __fix_to_virt(FIX_BTMAP_BEGIN - NR_FIX_BTMAPS * i);
> +
need to clear the old PMD, and set new PMD.
so you can clear old PMD and call early_ioremap_init() in fixup_early_ioremap()
Thanks
Yinghai
On Wed, Apr 07, 2010 at 09:30:39PM -0700, Yinghai wrote:
> On 04/07/2010 06:53 PM, Liang Li wrote:
> > Does this similar modification like this is more preferred?
> >
> > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > index a1dcfa3..30a3e97 100644
> > --- a/arch/x86/include/asm/io.h
> > +++ b/arch/x86/include/asm/io.h
> > @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
> > extern void __iomem *early_memremap(resource_size_t phys_addr,
> > unsigned long size);
> > extern void early_iounmap(void __iomem *addr, unsigned long size);
> > +extern void fixup_early_ioremap(void);
> >
> > #define IO_SPACE_LIMIT 0xffff
> >
> > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> > index ea82ef0..fe06296 100644
> > --- a/arch/x86/mm/ioremap.c
> > +++ b/arch/x86/mm/ioremap.c
> > @@ -448,6 +448,23 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
> > static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
> > static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
> >
> > +void __init fixup_early_ioremap(void)
> > +{
> > + int i;
> > + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
> > + if (prev_map[i])
> > + break;
> > + }
> > +
> > + if (i == FIX_BTMAPS_SLOTS)
> <
> > + WARN_ON(1);
> BUG_ON()
> > +
> > + for (i = 0; i < FIX_BTMAPS_SLOTS; i++)
> > + slot_virt[i] = __fix_to_virt(FIX_BTMAP_BEGIN - NR_FIX_BTMAPS * i);
> > +
> need to clear the old PMD, and set new PMD.
>
> so you can clear old PMD and call early_ioremap_init() in fixup_early_ioremap()
Call early_ioremap_init will do the update PMD work. So the preferred
patch would be:
---------------
>From 61fe7a116cbbf6eef98a49b88ed5861ed9ebd32d Mon Sep 17 00:00:00 2001
From: Liang Li <[email protected]>
Date: Mon, 22 Mar 2010 18:38:14 +0800
Subject: [PATCH] x86: let 'reservetop' functioning right
When specify 'reservetop=0xbadc0de' kernel parameter, the kernel will
stop booting due to a early_ioremap bug that relate to commit 8827247ff.
The root cause of boot failure problem is the value of 'slot_virt[i]'
was initialized in setup_arch->early_ioremap_init. But later in
setup_arch, the function 'parse_early_param' will modify 'FIXADDR_TOP'
when 'reservetop=0xbadc0de' being specified.
When reservetop being handled then FIXADDR_TOP get adjusted, Hence check
prev_map then re-initialize slot_virt and PMD based on new FIXADDR_TOP.
Signed-off-by: Liang Li <[email protected]>
Cc: Wang Chen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: Andrew Morton <[email protected]>
---
arch/x86/include/asm/io.h | 1 +
arch/x86/mm/ioremap.c | 15 +++++++++++++++
arch/x86/mm/pgtable.c | 2 ++
3 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index a1dcfa3..30a3e97 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
extern void __iomem *early_memremap(resource_size_t phys_addr,
unsigned long size);
extern void early_iounmap(void __iomem *addr, unsigned long size);
+extern void fixup_early_ioremap(void);
#define IO_SPACE_LIMIT 0xffff
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 5eb1ba7..e4ab706 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -448,6 +448,21 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
+void __init fixup_early_ioremap(void)
+{
+ int i;
+ for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
+ if (prev_map[i])
+ break;
+ }
+
+ if (i < FIX_BTMAPS_SLOTS)
+ BUG_ON(1);
+
+ early_ioremap_init();
+ return;
+}
+
static int __init check_early_ioremap_leak(void)
{
int count = 0;
diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
index 5c4ee42..ea4d54c 100644
--- a/arch/x86/mm/pgtable.c
+++ b/arch/x86/mm/pgtable.c
@@ -4,6 +4,7 @@
#include <asm/pgtable.h>
#include <asm/tlb.h>
#include <asm/fixmap.h>
+#include <asm/io.h>
#define PGALLOC_GFP GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO
@@ -351,6 +352,7 @@ void __init reserve_top_address(unsigned long reserve)
printk(KERN_INFO "Reserving virtual address space above 0x%08x\n",
(int)-reserve);
__FIXADDR_TOP = -reserve - PAGE_SIZE;
+ fixup_early_ioremap();
#endif
}
-----------
Acceptable?
Thanks and regards,
-Liang Li
On 04/07/2010 09:59 PM, Liang Li wrote:
> On Wed, Apr 07, 2010 at 09:30:39PM -0700, Yinghai wrote:
>> On 04/07/2010 06:53 PM, Liang Li wrote:
>>> Does this similar modification like this is more preferred?
>>>
>>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
>>> index a1dcfa3..30a3e97 100644
>>> --- a/arch/x86/include/asm/io.h
>>> +++ b/arch/x86/include/asm/io.h
>>> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
>>> extern void __iomem *early_memremap(resource_size_t phys_addr,
>>> unsigned long size);
>>> extern void early_iounmap(void __iomem *addr, unsigned long size);
>>> +extern void fixup_early_ioremap(void);
>>>
>>> #define IO_SPACE_LIMIT 0xffff
>>>
>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>> index ea82ef0..fe06296 100644
>>> --- a/arch/x86/mm/ioremap.c
>>> +++ b/arch/x86/mm/ioremap.c
>>> @@ -448,6 +448,23 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
>>> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
>>> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>>>
>>> +void __init fixup_early_ioremap(void)
>>> +{
>>> + int i;
>>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
>>> + if (prev_map[i])
>>> + break;
>>> + }
>>> +
>>> + if (i == FIX_BTMAPS_SLOTS)
>> <
>>> + WARN_ON(1);
>> BUG_ON()
>>> +
>>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++)
>>> + slot_virt[i] = __fix_to_virt(FIX_BTMAP_BEGIN - NR_FIX_BTMAPS * i);
>>> +
>> need to clear the old PMD, and set new PMD.
>>
>> so you can clear old PMD and call early_ioremap_init() in fixup_early_ioremap()
>
> Call early_ioremap_init will do the update PMD work. So the preferred
> patch would be:
> ---------------
> From 61fe7a116cbbf6eef98a49b88ed5861ed9ebd32d Mon Sep 17 00:00:00 2001
> From: Liang Li <[email protected]>
> Date: Mon, 22 Mar 2010 18:38:14 +0800
> Subject: [PATCH] x86: let 'reservetop' functioning right
>
> When specify 'reservetop=0xbadc0de' kernel parameter, the kernel will
> stop booting due to a early_ioremap bug that relate to commit 8827247ff.
>
> The root cause of boot failure problem is the value of 'slot_virt[i]'
> was initialized in setup_arch->early_ioremap_init. But later in
> setup_arch, the function 'parse_early_param' will modify 'FIXADDR_TOP'
> when 'reservetop=0xbadc0de' being specified.
>
> When reservetop being handled then FIXADDR_TOP get adjusted, Hence check
> prev_map then re-initialize slot_virt and PMD based on new FIXADDR_TOP.
>
> Signed-off-by: Liang Li <[email protected]>
> Cc: Wang Chen <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Yinghai Lu <[email protected]>
> Cc: Andrew Morton <[email protected]>
> ---
> arch/x86/include/asm/io.h | 1 +
> arch/x86/mm/ioremap.c | 15 +++++++++++++++
> arch/x86/mm/pgtable.c | 2 ++
> 3 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index a1dcfa3..30a3e97 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
> extern void __iomem *early_memremap(resource_size_t phys_addr,
> unsigned long size);
> extern void early_iounmap(void __iomem *addr, unsigned long size);
> +extern void fixup_early_ioremap(void);
>
> #define IO_SPACE_LIMIT 0xffff
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 5eb1ba7..e4ab706 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -448,6 +448,21 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>
> +void __init fixup_early_ioremap(void)
> +{
> + int i;
> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
> + if (prev_map[i])
> + break;
> + }
> +
> + if (i < FIX_BTMAPS_SLOTS)
> + BUG_ON(1);
> +
> + early_ioremap_init();
> + return;
> +}
> +
> static int __init check_early_ioremap_leak(void)
> {
> int count = 0;
> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> index 5c4ee42..ea4d54c 100644
> --- a/arch/x86/mm/pgtable.c
> +++ b/arch/x86/mm/pgtable.c
> @@ -4,6 +4,7 @@
> #include <asm/pgtable.h>
> #include <asm/tlb.h>
> #include <asm/fixmap.h>
> +#include <asm/io.h>
>
> #define PGALLOC_GFP GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO
>
> @@ -351,6 +352,7 @@ void __init reserve_top_address(unsigned long reserve)
> printk(KERN_INFO "Reserving virtual address space above 0x%08x\n",
> (int)-reserve);
> __FIXADDR_TOP = -reserve - PAGE_SIZE;
> + fixup_early_ioremap();
> #endif
> }
>
> -----------
> Acceptable?
>
good to me.
may need to ask xen/lguest/vmi related to check that too.
arch/x86/kernel/vmi_32.c: reserve_top_address(-vmi_rom->virtual_top);
arch/x86/lguest/boot.c: reserve_top_address(lguest_data.reserve_mem);
arch/x86/mm/pgtable_32.c: reserve_top_address(address);
arch/x86/xen/mmu.c: reserve_top_address(-top);
YH
On Wed, Apr 07, 2010 at 11:57:22PM -0700, Yinghai wrote:
> On 04/07/2010 09:59 PM, Liang Li wrote:
> > On Wed, Apr 07, 2010 at 09:30:39PM -0700, Yinghai wrote:
> >> On 04/07/2010 06:53 PM, Liang Li wrote:
> >>> Does this similar modification like this is more preferred?
> >>>
> >>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> >>> index a1dcfa3..30a3e97 100644
> >>> --- a/arch/x86/include/asm/io.h
> >>> +++ b/arch/x86/include/asm/io.h
> >>> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
> >>> extern void __iomem *early_memremap(resource_size_t phys_addr,
> >>> unsigned long size);
> >>> extern void early_iounmap(void __iomem *addr, unsigned long size);
> >>> +extern void fixup_early_ioremap(void);
> >>>
> >>> #define IO_SPACE_LIMIT 0xffff
> >>>
> >>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> >>> index ea82ef0..fe06296 100644
> >>> --- a/arch/x86/mm/ioremap.c
> >>> +++ b/arch/x86/mm/ioremap.c
> >>> @@ -448,6 +448,23 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
> >>> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
> >>> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
> >>>
> >>> +void __init fixup_early_ioremap(void)
> >>> +{
> >>> + int i;
> >>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
> >>> + if (prev_map[i])
> >>> + break;
> >>> + }
> >>> +
> >>> + if (i == FIX_BTMAPS_SLOTS)
> >> <
> >>> + WARN_ON(1);
> >> BUG_ON()
> >>> +
> >>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++)
> >>> + slot_virt[i] = __fix_to_virt(FIX_BTMAP_BEGIN - NR_FIX_BTMAPS * i);
> >>> +
> >> need to clear the old PMD, and set new PMD.
> >>
> >> so you can clear old PMD and call early_ioremap_init() in fixup_early_ioremap()
> >
> > Call early_ioremap_init will do the update PMD work. So the preferred
> > patch would be:
> > ---------------
> > From 61fe7a116cbbf6eef98a49b88ed5861ed9ebd32d Mon Sep 17 00:00:00 2001
> > From: Liang Li <[email protected]>
> > Date: Mon, 22 Mar 2010 18:38:14 +0800
> > Subject: [PATCH] x86: let 'reservetop' functioning right
> >
> > When specify 'reservetop=0xbadc0de' kernel parameter, the kernel will
> > stop booting due to a early_ioremap bug that relate to commit 8827247ff.
> >
> > The root cause of boot failure problem is the value of 'slot_virt[i]'
> > was initialized in setup_arch->early_ioremap_init. But later in
> > setup_arch, the function 'parse_early_param' will modify 'FIXADDR_TOP'
> > when 'reservetop=0xbadc0de' being specified.
> >
> > When reservetop being handled then FIXADDR_TOP get adjusted, Hence check
> > prev_map then re-initialize slot_virt and PMD based on new FIXADDR_TOP.
> >
> > Signed-off-by: Liang Li <[email protected]>
> > Cc: Wang Chen <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: "H. Peter Anvin" <[email protected]>
> > Cc: Yinghai Lu <[email protected]>
> > Cc: Andrew Morton <[email protected]>
> > ---
> > arch/x86/include/asm/io.h | 1 +
> > arch/x86/mm/ioremap.c | 15 +++++++++++++++
> > arch/x86/mm/pgtable.c | 2 ++
> > 3 files changed, 18 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> > index a1dcfa3..30a3e97 100644
> > --- a/arch/x86/include/asm/io.h
> > +++ b/arch/x86/include/asm/io.h
> > @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
> > extern void __iomem *early_memremap(resource_size_t phys_addr,
> > unsigned long size);
> > extern void early_iounmap(void __iomem *addr, unsigned long size);
> > +extern void fixup_early_ioremap(void);
> >
> > #define IO_SPACE_LIMIT 0xffff
> >
> > diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> > index 5eb1ba7..e4ab706 100644
> > --- a/arch/x86/mm/ioremap.c
> > +++ b/arch/x86/mm/ioremap.c
> > @@ -448,6 +448,21 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
> > static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
> > static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
> >
> > +void __init fixup_early_ioremap(void)
> > +{
> > + int i;
> > + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
> > + if (prev_map[i])
> > + break;
> > + }
> > +
> > + if (i < FIX_BTMAPS_SLOTS)
> > + BUG_ON(1);
> > +
> > + early_ioremap_init();
> > + return;
> > +}
> > +
> > static int __init check_early_ioremap_leak(void)
> > {
> > int count = 0;
> > diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
> > index 5c4ee42..ea4d54c 100644
> > --- a/arch/x86/mm/pgtable.c
> > +++ b/arch/x86/mm/pgtable.c
> > @@ -4,6 +4,7 @@
> > #include <asm/pgtable.h>
> > #include <asm/tlb.h>
> > #include <asm/fixmap.h>
> > +#include <asm/io.h>
> >
> > #define PGALLOC_GFP GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO
> >
> > @@ -351,6 +352,7 @@ void __init reserve_top_address(unsigned long reserve)
> > printk(KERN_INFO "Reserving virtual address space above 0x%08x\n",
> > (int)-reserve);
> > __FIXADDR_TOP = -reserve - PAGE_SIZE;
> > + fixup_early_ioremap();
> > #endif
> > }
> >
> > -----------
> > Acceptable?
> >
> good to me.
>
> may need to ask xen/lguest/vmi related to check that too.
>
> arch/x86/kernel/vmi_32.c: reserve_top_address(-vmi_rom->virtual_top);
> arch/x86/lguest/boot.c: reserve_top_address(lguest_data.reserve_mem);
> arch/x86/mm/pgtable_32.c: reserve_top_address(address);
> arch/x86/xen/mmu.c: reserve_top_address(-top);
When linux as vmi/xen/lguest guest OS, kernel call reserve_top_address
before start_kernel. It is far before the start_kernel hence far before
setup_arch->early_ioremap_init. So it is unsafe to place
fixup_early_ioremap inside reserve_top_address. So I think the patch
should be:
>From 7cefa9a80c4434f2941a7072d39b1f1ffc08a40f Mon Sep 17 00:00:00 2001
From: Liang Li <[email protected]>
Date: Mon, 22 Mar 2010 18:38:14 +0800
Subject: [PATCH] x86: let 'reservetop' functioning right
When specify 'reservetop=0xbadc0de' kernel parameter, the kernel will
stop booting due to a early_ioremap bug that relate to commit 8827247ff.
The root cause of boot failure problem is the value of 'slot_virt[i]'
was initialized in setup_arch->early_ioremap_init. But later in
setup_arch, the function 'parse_early_param' will modify 'FIXADDR_TOP'
when 'reservetop=0xbadc0de' being specified.
When reservetop being handled then FIXADDR_TOP get adjusted, Hence check
prev_map then re-initialize slot_virt and PMD based on new FIXADDR_TOP.
Signed-off-by: Liang Li <[email protected]>
Cc: Wang Chen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Yinghai Lu <[email protected]>
Cc: Andrew Morton <[email protected]>
---
arch/x86/include/asm/io.h | 1 +
arch/x86/mm/ioremap.c | 15 +++++++++++++++
arch/x86/mm/pgtable_32.c | 1 +
3 files changed, 17 insertions(+), 0 deletions(-)
diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index a1dcfa3..30a3e97 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
extern void __iomem *early_memremap(resource_size_t phys_addr,
unsigned long size);
extern void early_iounmap(void __iomem *addr, unsigned long size);
+extern void fixup_early_ioremap(void);
#define IO_SPACE_LIMIT 0xffff
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 5eb1ba7..e4ab706 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -448,6 +448,21 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
+void __init fixup_early_ioremap(void)
+{
+ int i;
+ for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
+ if (prev_map[i])
+ break;
+ }
+
+ if (i < FIX_BTMAPS_SLOTS)
+ BUG_ON(1);
+
+ early_ioremap_init();
+ return;
+}
+
static int __init check_early_ioremap_leak(void)
{
int count = 0;
diff --git a/arch/x86/mm/pgtable_32.c b/arch/x86/mm/pgtable_32.c
index 1a8faf0..26eadaa 100644
--- a/arch/x86/mm/pgtable_32.c
+++ b/arch/x86/mm/pgtable_32.c
@@ -128,6 +128,7 @@ static int __init parse_reservetop(char *arg)
address = memparse(arg, &arg);
reserve_top_address(address);
+ fixup_early_ioremap();
return 0;
}
early_param("reservetop", parse_reservetop);
--
1.6.6
Thanks and best regards,
-Liang Li
>
>
> YH
On 04/07/2010 11:57 PM, Yinghai wrote:
> On 04/07/2010 09:59 PM, Liang Li wrote:
>
>> On Wed, Apr 07, 2010 at 09:30:39PM -0700, Yinghai wrote:
>>
>>> On 04/07/2010 06:53 PM, Liang Li wrote:
>>>
>>>> Does this similar modification like this is more preferred?
>>>>
>>>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
>>>> index a1dcfa3..30a3e97 100644
>>>> --- a/arch/x86/include/asm/io.h
>>>> +++ b/arch/x86/include/asm/io.h
>>>> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
>>>> extern void __iomem *early_memremap(resource_size_t phys_addr,
>>>> unsigned long size);
>>>> extern void early_iounmap(void __iomem *addr, unsigned long size);
>>>> +extern void fixup_early_ioremap(void);
>>>>
>>>> #define IO_SPACE_LIMIT 0xffff
>>>>
>>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>>> index ea82ef0..fe06296 100644
>>>> --- a/arch/x86/mm/ioremap.c
>>>> +++ b/arch/x86/mm/ioremap.c
>>>> @@ -448,6 +448,23 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
>>>> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
>>>> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>>>>
>>>> +void __init fixup_early_ioremap(void)
>>>> +{
>>>> + int i;
>>>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
>>>> + if (prev_map[i])
>>>> + break;
>>>> + }
>>>> +
>>>> + if (i == FIX_BTMAPS_SLOTS)
>>>>
>>> <
>>>
>>>> + WARN_ON(1);
>>>>
>>> BUG_ON()
>>>
>>>> +
>>>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++)
>>>> + slot_virt[i] = __fix_to_virt(FIX_BTMAP_BEGIN - NR_FIX_BTMAPS * i);
>>>> +
>>>>
>>> need to clear the old PMD, and set new PMD.
>>>
>>> so you can clear old PMD and call early_ioremap_init() in fixup_early_ioremap()
>>>
>> Call early_ioremap_init will do the update PMD work. So the preferred
>> patch would be:
>> ---------------
>> From 61fe7a116cbbf6eef98a49b88ed5861ed9ebd32d Mon Sep 17 00:00:00 2001
>> From: Liang Li <[email protected]>
>> Date: Mon, 22 Mar 2010 18:38:14 +0800
>> Subject: [PATCH] x86: let 'reservetop' functioning right
>>
>> When specify 'reservetop=0xbadc0de' kernel parameter, the kernel will
>> stop booting due to a early_ioremap bug that relate to commit 8827247ff.
>>
>> The root cause of boot failure problem is the value of 'slot_virt[i]'
>> was initialized in setup_arch->early_ioremap_init. But later in
>> setup_arch, the function 'parse_early_param' will modify 'FIXADDR_TOP'
>> when 'reservetop=0xbadc0de' being specified.
>>
>> When reservetop being handled then FIXADDR_TOP get adjusted, Hence check
>> prev_map then re-initialize slot_virt and PMD based on new FIXADDR_TOP.
>>
>> Signed-off-by: Liang Li <[email protected]>
>> Cc: Wang Chen <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: "H. Peter Anvin" <[email protected]>
>> Cc: Yinghai Lu <[email protected]>
>> Cc: Andrew Morton <[email protected]>
>> ---
>> arch/x86/include/asm/io.h | 1 +
>> arch/x86/mm/ioremap.c | 15 +++++++++++++++
>> arch/x86/mm/pgtable.c | 2 ++
>> 3 files changed, 18 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
>> index a1dcfa3..30a3e97 100644
>> --- a/arch/x86/include/asm/io.h
>> +++ b/arch/x86/include/asm/io.h
>> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
>> extern void __iomem *early_memremap(resource_size_t phys_addr,
>> unsigned long size);
>> extern void early_iounmap(void __iomem *addr, unsigned long size);
>> +extern void fixup_early_ioremap(void);
>>
>> #define IO_SPACE_LIMIT 0xffff
>>
>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>> index 5eb1ba7..e4ab706 100644
>> --- a/arch/x86/mm/ioremap.c
>> +++ b/arch/x86/mm/ioremap.c
>> @@ -448,6 +448,21 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
>> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
>> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>>
>> +void __init fixup_early_ioremap(void)
>> +{
>> + int i;
>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
>> + if (prev_map[i])
>> + break;
>> + }
>> +
>> + if (i < FIX_BTMAPS_SLOTS)
>> + BUG_ON(1);
>> +
>> + early_ioremap_init();
>> + return;
>> +}
>> +
>> static int __init check_early_ioremap_leak(void)
>> {
>> int count = 0;
>> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
>> index 5c4ee42..ea4d54c 100644
>> --- a/arch/x86/mm/pgtable.c
>> +++ b/arch/x86/mm/pgtable.c
>> @@ -4,6 +4,7 @@
>> #include <asm/pgtable.h>
>> #include <asm/tlb.h>
>> #include <asm/fixmap.h>
>> +#include <asm/io.h>
>>
>> #define PGALLOC_GFP GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO
>>
>> @@ -351,6 +352,7 @@ void __init reserve_top_address(unsigned long reserve)
>> printk(KERN_INFO "Reserving virtual address space above 0x%08x\n",
>> (int)-reserve);
>> __FIXADDR_TOP = -reserve - PAGE_SIZE;
>> + fixup_early_ioremap();
>> #endif
>> }
>>
>> -----------
>> Acceptable?
>>
>>
> good to me.
>
> may need to ask xen/lguest/vmi related to check that too.
>
> arch/x86/kernel/vmi_32.c: reserve_top_address(-vmi_rom->virtual_top);
> arch/x86/lguest/boot.c: reserve_top_address(lguest_data.reserve_mem);
> arch/x86/mm/pgtable_32.c: reserve_top_address(address);
> arch/x86/xen/mmu.c: reserve_top_address(-top);
>
This looks troublesome for us. We're calling reserve_top_address() very
early - before start_kernel - to make sure the address space for the
hypervisor has been reserved. Calling fixup_early_ioremap() will
probably fail horribly.
Can you make it so that it only calls fixup_early_ioremap() if ioremap
init has already happened?
Also, do we actually need reservetop= any more. It looks like Zach
added it for VMI, but VMI has been deprecated. Are there any other use
cases?
Thanks,
J
On 04/08/2010 01:58 AM, Liang Li wrote:
> On Wed, Apr 07, 2010 at 11:57:22PM -0700, Yinghai wrote:
>
>> On 04/07/2010 09:59 PM, Liang Li wrote:
>>
>>> On Wed, Apr 07, 2010 at 09:30:39PM -0700, Yinghai wrote:
>>>
>>>> On 04/07/2010 06:53 PM, Liang Li wrote:
>>>>
>>>>> Does this similar modification like this is more preferred?
>>>>>
>>>>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
>>>>> index a1dcfa3..30a3e97 100644
>>>>> --- a/arch/x86/include/asm/io.h
>>>>> +++ b/arch/x86/include/asm/io.h
>>>>> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
>>>>> extern void __iomem *early_memremap(resource_size_t phys_addr,
>>>>> unsigned long size);
>>>>> extern void early_iounmap(void __iomem *addr, unsigned long size);
>>>>> +extern void fixup_early_ioremap(void);
>>>>>
>>>>> #define IO_SPACE_LIMIT 0xffff
>>>>>
>>>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>>>> index ea82ef0..fe06296 100644
>>>>> --- a/arch/x86/mm/ioremap.c
>>>>> +++ b/arch/x86/mm/ioremap.c
>>>>> @@ -448,6 +448,23 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
>>>>> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
>>>>> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>>>>>
>>>>> +void __init fixup_early_ioremap(void)
>>>>> +{
>>>>> + int i;
>>>>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
>>>>> + if (prev_map[i])
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + if (i == FIX_BTMAPS_SLOTS)
>>>>>
>>>> <
>>>>
>>>>> + WARN_ON(1);
>>>>>
>>>> BUG_ON()
>>>>
>>>>> +
>>>>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++)
>>>>> + slot_virt[i] = __fix_to_virt(FIX_BTMAP_BEGIN - NR_FIX_BTMAPS * i);
>>>>> +
>>>>>
>>>> need to clear the old PMD, and set new PMD.
>>>>
>>>> so you can clear old PMD and call early_ioremap_init() in fixup_early_ioremap()
>>>>
>>> Call early_ioremap_init will do the update PMD work. So the preferred
>>> patch would be:
>>> ---------------
>>> From 61fe7a116cbbf6eef98a49b88ed5861ed9ebd32d Mon Sep 17 00:00:00 2001
>>> From: Liang Li <[email protected]>
>>> Date: Mon, 22 Mar 2010 18:38:14 +0800
>>> Subject: [PATCH] x86: let 'reservetop' functioning right
>>>
>>> When specify 'reservetop=0xbadc0de' kernel parameter, the kernel will
>>> stop booting due to a early_ioremap bug that relate to commit 8827247ff.
>>>
>>> The root cause of boot failure problem is the value of 'slot_virt[i]'
>>> was initialized in setup_arch->early_ioremap_init. But later in
>>> setup_arch, the function 'parse_early_param' will modify 'FIXADDR_TOP'
>>> when 'reservetop=0xbadc0de' being specified.
>>>
>>> When reservetop being handled then FIXADDR_TOP get adjusted, Hence check
>>> prev_map then re-initialize slot_virt and PMD based on new FIXADDR_TOP.
>>>
>>> Signed-off-by: Liang Li <[email protected]>
>>> Cc: Wang Chen <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: Thomas Gleixner <[email protected]>
>>> Cc: "H. Peter Anvin" <[email protected]>
>>> Cc: Yinghai Lu <[email protected]>
>>> Cc: Andrew Morton <[email protected]>
>>> ---
>>> arch/x86/include/asm/io.h | 1 +
>>> arch/x86/mm/ioremap.c | 15 +++++++++++++++
>>> arch/x86/mm/pgtable.c | 2 ++
>>> 3 files changed, 18 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
>>> index a1dcfa3..30a3e97 100644
>>> --- a/arch/x86/include/asm/io.h
>>> +++ b/arch/x86/include/asm/io.h
>>> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
>>> extern void __iomem *early_memremap(resource_size_t phys_addr,
>>> unsigned long size);
>>> extern void early_iounmap(void __iomem *addr, unsigned long size);
>>> +extern void fixup_early_ioremap(void);
>>>
>>> #define IO_SPACE_LIMIT 0xffff
>>>
>>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>>> index 5eb1ba7..e4ab706 100644
>>> --- a/arch/x86/mm/ioremap.c
>>> +++ b/arch/x86/mm/ioremap.c
>>> @@ -448,6 +448,21 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
>>> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
>>> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>>>
>>> +void __init fixup_early_ioremap(void)
>>> +{
>>> + int i;
>>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
>>> + if (prev_map[i])
>>> + break;
>>> + }
>>> +
>>> + if (i < FIX_BTMAPS_SLOTS)
>>> + BUG_ON(1);
>>> +
>>> + early_ioremap_init();
>>> + return;
>>> +}
>>> +
>>> static int __init check_early_ioremap_leak(void)
>>> {
>>> int count = 0;
>>> diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
>>> index 5c4ee42..ea4d54c 100644
>>> --- a/arch/x86/mm/pgtable.c
>>> +++ b/arch/x86/mm/pgtable.c
>>> @@ -4,6 +4,7 @@
>>> #include <asm/pgtable.h>
>>> #include <asm/tlb.h>
>>> #include <asm/fixmap.h>
>>> +#include <asm/io.h>
>>>
>>> #define PGALLOC_GFP GFP_KERNEL | __GFP_NOTRACK | __GFP_REPEAT | __GFP_ZERO
>>>
>>> @@ -351,6 +352,7 @@ void __init reserve_top_address(unsigned long reserve)
>>> printk(KERN_INFO "Reserving virtual address space above 0x%08x\n",
>>> (int)-reserve);
>>> __FIXADDR_TOP = -reserve - PAGE_SIZE;
>>> + fixup_early_ioremap();
>>> #endif
>>> }
>>>
>>> -----------
>>> Acceptable?
>>>
>>>
>> good to me.
>>
>> may need to ask xen/lguest/vmi related to check that too.
>>
>> arch/x86/kernel/vmi_32.c: reserve_top_address(-vmi_rom->virtual_top);
>> arch/x86/lguest/boot.c: reserve_top_address(lguest_data.reserve_mem);
>> arch/x86/mm/pgtable_32.c: reserve_top_address(address);
>> arch/x86/xen/mmu.c: reserve_top_address(-top);
>>
> When linux as vmi/xen/lguest guest OS, kernel call reserve_top_address
> before start_kernel. It is far before the start_kernel hence far before
> setup_arch->early_ioremap_init. So it is unsafe to place
> fixup_early_ioremap inside reserve_top_address. So I think the patch
> should be:
>
Yes indeed. That looks better.
Acked-by: Jeremy Fitzhardinge <[email protected]>
Thanks,
J
> From 7cefa9a80c4434f2941a7072d39b1f1ffc08a40f Mon Sep 17 00:00:00 2001
> From: Liang Li <[email protected]>
> Date: Mon, 22 Mar 2010 18:38:14 +0800
> Subject: [PATCH] x86: let 'reservetop' functioning right
>
> When specify 'reservetop=0xbadc0de' kernel parameter, the kernel will
> stop booting due to a early_ioremap bug that relate to commit 8827247ff.
>
> The root cause of boot failure problem is the value of 'slot_virt[i]'
> was initialized in setup_arch->early_ioremap_init. But later in
> setup_arch, the function 'parse_early_param' will modify 'FIXADDR_TOP'
> when 'reservetop=0xbadc0de' being specified.
>
> When reservetop being handled then FIXADDR_TOP get adjusted, Hence check
> prev_map then re-initialize slot_virt and PMD based on new FIXADDR_TOP.
>
> Signed-off-by: Liang Li <[email protected]>
> Cc: Wang Chen <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Yinghai Lu <[email protected]>
> Cc: Andrew Morton <[email protected]>
> ---
> arch/x86/include/asm/io.h | 1 +
> arch/x86/mm/ioremap.c | 15 +++++++++++++++
> arch/x86/mm/pgtable_32.c | 1 +
> 3 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index a1dcfa3..30a3e97 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
> extern void __iomem *early_memremap(resource_size_t phys_addr,
> unsigned long size);
> extern void early_iounmap(void __iomem *addr, unsigned long size);
> +extern void fixup_early_ioremap(void);
>
> #define IO_SPACE_LIMIT 0xffff
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index 5eb1ba7..e4ab706 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -448,6 +448,21 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>
> +void __init fixup_early_ioremap(void)
> +{
> + int i;
> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
> + if (prev_map[i])
> + break;
> + }
> +
> + if (i < FIX_BTMAPS_SLOTS)
> + BUG_ON(1);
> +
> + early_ioremap_init();
> + return;
> +}
> +
> static int __init check_early_ioremap_leak(void)
> {
> int count = 0;
> diff --git a/arch/x86/mm/pgtable_32.c b/arch/x86/mm/pgtable_32.c
> index 1a8faf0..26eadaa 100644
> --- a/arch/x86/mm/pgtable_32.c
> +++ b/arch/x86/mm/pgtable_32.c
> @@ -128,6 +128,7 @@ static int __init parse_reservetop(char *arg)
>
> address = memparse(arg, &arg);
> reserve_top_address(address);
> + fixup_early_ioremap();
> return 0;
> }
> early_param("reservetop", parse_reservetop);
>
On 04/08/2010 10:12 AM, Jeremy Fitzhardinge wrote:
> On 04/08/2010 01:58 AM, Liang Li wrote:
>>>
>> When linux as vmi/xen/lguest guest OS, kernel call reserve_top_address
>> before start_kernel. It is far before the start_kernel hence far before
>> setup_arch->early_ioremap_init. So it is unsafe to place
>> fixup_early_ioremap inside reserve_top_address. So I think the patch
>> should be:
>>
>
> Yes indeed. That looks better.
>
> Acked-by: Jeremy Fitzhardinge <[email protected]>
>
good.
Liang please resubmit with clear version changelog.
like
-v2: ...
-v3: move fixup_early_ioremap out of reserve_top_address..
Yinghai
> Thanks,
> J
>
>> From 7cefa9a80c4434f2941a7072d39b1f1ffc08a40f Mon Sep 17 00:00:00 2001
>> From: Liang Li <[email protected]>
>> Date: Mon, 22 Mar 2010 18:38:14 +0800
>> Subject: [PATCH] x86: let 'reservetop' functioning right
>>
>> When specify 'reservetop=0xbadc0de' kernel parameter, the kernel will
>> stop booting due to a early_ioremap bug that relate to commit 8827247ff.
>>
>> The root cause of boot failure problem is the value of 'slot_virt[i]'
>> was initialized in setup_arch->early_ioremap_init. But later in
>> setup_arch, the function 'parse_early_param' will modify 'FIXADDR_TOP'
>> when 'reservetop=0xbadc0de' being specified.
>>
>> When reservetop being handled then FIXADDR_TOP get adjusted, Hence check
>> prev_map then re-initialize slot_virt and PMD based on new FIXADDR_TOP.
>>
>> Signed-off-by: Liang Li <[email protected]>
>> Cc: Wang Chen <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: "H. Peter Anvin" <[email protected]>
>> Cc: Yinghai Lu <[email protected]>
>> Cc: Andrew Morton <[email protected]>
>> ---
>> arch/x86/include/asm/io.h | 1 +
>> arch/x86/mm/ioremap.c | 15 +++++++++++++++
>> arch/x86/mm/pgtable_32.c | 1 +
>> 3 files changed, 17 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
>> index a1dcfa3..30a3e97 100644
>> --- a/arch/x86/include/asm/io.h
>> +++ b/arch/x86/include/asm/io.h
>> @@ -347,6 +347,7 @@ extern void __iomem *early_ioremap(resource_size_t phys_addr,
>> extern void __iomem *early_memremap(resource_size_t phys_addr,
>> unsigned long size);
>> extern void early_iounmap(void __iomem *addr, unsigned long size);
>> +extern void fixup_early_ioremap(void);
>>
>> #define IO_SPACE_LIMIT 0xffff
>>
>> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
>> index 5eb1ba7..e4ab706 100644
>> --- a/arch/x86/mm/ioremap.c
>> +++ b/arch/x86/mm/ioremap.c
>> @@ -448,6 +448,21 @@ static inline void __init early_clear_fixmap(enum fixed_addresses idx)
>> static void __iomem *prev_map[FIX_BTMAPS_SLOTS] __initdata;
>> static unsigned long prev_size[FIX_BTMAPS_SLOTS] __initdata;
>>
>> +void __init fixup_early_ioremap(void)
>> +{
>> + int i;
>> + for (i = 0; i < FIX_BTMAPS_SLOTS; i++) {
>> + if (prev_map[i])
>> + break;
>> + }
>> +
>> + if (i < FIX_BTMAPS_SLOTS)
>> + BUG_ON(1);
>> +
>> + early_ioremap_init();
>> + return;
>> +}
>> +
>> static int __init check_early_ioremap_leak(void)
>> {
>> int count = 0;
>> diff --git a/arch/x86/mm/pgtable_32.c b/arch/x86/mm/pgtable_32.c
>> index 1a8faf0..26eadaa 100644
>> --- a/arch/x86/mm/pgtable_32.c
>> +++ b/arch/x86/mm/pgtable_32.c
>> @@ -128,6 +128,7 @@ static int __init parse_reservetop(char *arg)
>>
>> address = memparse(arg, &arg);
>> reserve_top_address(address);
>> + fixup_early_ioremap();
>> return 0;
>> }
>> early_param("reservetop", parse_reservetop);
>>
>
On Thu, Apr 08, 2010 at 11:03:22AM -0700, Yinghai wrote:
> On 04/08/2010 10:12 AM, Jeremy Fitzhardinge wrote:
> > On 04/08/2010 01:58 AM, Liang Li wrote:
> >>>
> >> When linux as vmi/xen/lguest guest OS, kernel call reserve_top_address
> >> before start_kernel. It is far before the start_kernel hence far before
> >> setup_arch->early_ioremap_init. So it is unsafe to place
> >> fixup_early_ioremap inside reserve_top_address. So I think the patch
> >> should be:
> >>
> >
> > Yes indeed. That looks better.
> >
> > Acked-by: Jeremy Fitzhardinge <[email protected]>
I've also tested it today as PV Xen guest:
Tested-by: Konrad Rzeszutek Wilk <[email protected]>
On Thu, Apr 08, 2010 at 02:38:06PM -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Apr 08, 2010 at 11:03:22AM -0700, Yinghai wrote:
> > On 04/08/2010 10:12 AM, Jeremy Fitzhardinge wrote:
> > > On 04/08/2010 01:58 AM, Liang Li wrote:
> > >>>
> > >> When linux as vmi/xen/lguest guest OS, kernel call reserve_top_address
> > >> before start_kernel. It is far before the start_kernel hence far before
> > >> setup_arch->early_ioremap_init. So it is unsafe to place
> > >> fixup_early_ioremap inside reserve_top_address. So I think the patch
> > >> should be:
> > >>
> > >
> > > Yes indeed. That looks better.
> > >
> > > Acked-by: Jeremy Fitzhardinge <[email protected]>
>
> I've also tested it today as PV Xen guest:
>
> Tested-by: Konrad Rzeszutek Wilk <[email protected]>
Thanks all. Updated patch will be sent out soon.
Best regards,
-Liang Li
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/