Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[Greg KH]

On Tue, Dec 09, 2008 at 04:50:21PM -0800, Zachary Amsden wrote:
> Patches backported into 2.6.27.4 caused a regression with VMI kernels
> running on VMware which ends in a page fault during boot.  I have a fix
> which still allows DMI checks to be done early.

Is this regression also present on 2.6.28-rc kernels as well?

thanks,

greg k-h

[PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[Zachary Amsden]

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

Patches backported into 2.6.27.4 caused a regression with VMI kernels
running on VMware which ends in a page fault during boot.  I have a fix
which still allows DMI checks to be done early.

VMI initialiation can relocate the fixmap, causing early_ioremap
to malfunction if it is initialized before the relocation.  The
ioremap area is low enough in virtual address space that no actual
collision occurs, however, because the pagetables for it were not
allocated under VMI mode, the pagetable updates are dropped by
the hypervisor as irrelevant, resulting in a crash on boot.

The best fix is perhaps to move early_ioremap_init() after vmi_init().
The only things done before VMI init are basic memory access, things
like collating the memory map, collecting boot CPUID capabilities, and
parsing the early command line options... which vmi_init needs.

Since this went back into 2.6.27, it needs to go to both 2.6.28 and
eventually to stable.  I didn't add any comments or anything as there
could be some debate what the proper ordering should be.  In case that
becomes an interesting discussion, there are two relevant facts in git
today:

1) no clients of early_ioremap occur before DMI.
2) VMI requires access to early boot params.

If any can suggest a better ordering, I am certainly open to that as
well.

Thanks,

Zach

[-- Attachment #2: x86-vmi-boot-ioremap-fix.patch --]
[-- Type: text/x-patch, Size: 1002 bytes --]

VMI initialiation can relocate the fixmap, causing early_ioremap
to malfunction if it is initialized before the relocation.  The
ioremap area is low enough in virtual address space that no actual
collision occurs, however, because the pagetables for it were not
allocated under VMI mode, the pagetable updates are dropped by
the hypervisor as irrelevant, resulting in a crash on boot.

Signed-off-by: Zachary Amsden <zach@vmware.com>

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 9d5674f..9627753 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -795,7 +795,6 @@ void __init setup_arch(char **cmdline_p)
 #endif
 
 	early_cpu_init();
-	early_ioremap_init();
 
 	ROOT_DEV = old_decode_dev(boot_params.hdr.root_dev);
 	screen_info = boot_params.screen_info;
@@ -888,6 +887,8 @@ void __init setup_arch(char **cmdline_p)
 	vmi_init();
 #endif
 
+	early_ioremap_init();
+
 	/* after early param, so could get panic from serial */
 	reserve_early_setup_data();
 

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[Yinghai Lu]

On Tue, Dec 9, 2008 at 4:50 PM, Zachary Amsden <zach@vmware.com> wrote:
> Patches backported into 2.6.27.4 caused a regression with VMI kernels
> running on VMware which ends in a page fault during boot.  I have a fix
> which still allows DMI checks to be done early.
>
> The best fix is perhaps to move early_ioremap_init() after vmi_init().
> The only things done before VMI init are basic memory access, things
> like collating the memory map, collecting boot CPUID capabilities, and
> parsing the early command line options... which vmi_init needs.
>
> Since this went back into 2.6.27, it needs to go to both 2.6.28 and
> eventually to stable.  I didn't add any comments or anything as there
> could be some debate what the proper ordering should be.  In case that
> becomes an interesting discussion, there are two relevant facts in git
> today:
>
> 1) no clients of early_ioremap occur before DMI.
> 2) VMI requires access to early boot params.
>
> If any can suggest a better ordering, I am certainly open to that as
> well.

VMI initialiation can relocate the fixmap, causing early_ioremap
to malfunction if it is initialized before the relocation.  The
ioremap area is low enough in virtual address space that no actual
collision occurs, however, because the pagetables for it were not
allocated under VMI mode, the pagetable updates are dropped by
the hypervisor as irrelevant, resulting in a crash on boot.

Signed-off-by: Zachary Amsden <zach@vmware.com>

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 9d5674f..9627753 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -795,7 +795,6 @@ void __init setup_arch(char **cmdline_p)
 #endif

 	early_cpu_init();
-	early_ioremap_init();

 	ROOT_DEV = old_decode_dev(boot_params.hdr.root_dev);
 	screen_info = boot_params.screen_info;
@@ -888,6 +887,8 @@ void __init setup_arch(char **cmdline_p)
 	vmi_init();
 #endif

+	early_ioremap_init();
+
 	/* after early param, so could get panic from serial */
 	reserve_early_setup_data();


you can not move that late,

parse_setup_data==>early_memremap==>__early_ioremap

YH

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[H. Peter Anvin]

Yinghai Lu wrote:
> 
> VMI initialiation can relocate the fixmap, causing early_ioremap
> to malfunction if it is initialized before the relocation.  The
> ioremap area is low enough in virtual address space that no actual
> collision occurs, however, because the pagetables for it were not
> allocated under VMI mode, the pagetable updates are dropped by
> the hypervisor as irrelevant, resulting in a crash on boot.
> 

I have mentioned in the past that I think the very concept of relocating
the fixmap to be utterly braindead.  Instead, I believe we should locate
it low in kernel space so it doesn't have to be relocated.  It's
unfortunately a relatively large change.

	-hpa

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[Zachary Amsden]

On Tue, 2008-12-09 at 16:44 -0800, Greg KH wrote:
> On Tue, Dec 09, 2008 at 04:50:21PM -0800, Zachary Amsden wrote:
> > Patches backported into 2.6.27.4 caused a regression with VMI kernels
> > running on VMware which ends in a page fault during boot.  I have a fix
> > which still allows DMI checks to be done early.
> 
> Is this regression also present on 2.6.28-rc kernels as well?

Yes, I reproduced and fixed this on a git tree synced to mainline today.

Thanks,

Zach

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[Zachary Amsden]

     A. On Tue, 2008-12-09 at 17:48 -0800, H. Peter Anvin wrote:
> Yinghai Lu wrote:
> > 
> > VMI initialiation can relocate the fixmap, causing early_ioremap
> > to malfunction if it is initialized before the relocation.  The
> > ioremap area is low enough in virtual address space that no actual
> > collision occurs, however, because the pagetables for it were not
> > allocated under VMI mode, the pagetable updates are dropped by
> > the hypervisor as irrelevant, resulting in a crash on boot.
> > 
> 
> I have mentioned in the past that I think the very concept of relocating
> the fixmap to be utterly braindead.  Instead, I believe we should locate
> it low in kernel space so it doesn't have to be relocated.  It's
> unfortunately a relatively large change.

I agree, but a bit too late for 2.6.27 stable and 2.6.28-rc.

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[Zachary Amsden]

On Tue, 2008-12-09 at 17:15 -0800, Yinghai Lu wrote:

> you can not move that late,
> 
> parse_setup_data==>early_memremap==>__early_ioremap

Eww.  Let me find a way to fix that.  It may be as simple as detecting
the required fixmap relocation first, then initializing VMI later, when
we can parse options, but it will need testing...

Thanks for the quick response, I realize it is late in the 2.6.28-rc
phase here and I will send an updated fix tomorrow.

Zach

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[H. Peter Anvin]

Zachary Amsden wrote:
>>>
>> I have mentioned in the past that I think the very concept of relocating
>> the fixmap to be utterly braindead.  Instead, I believe we should locate
>> it low in kernel space so it doesn't have to be relocated.  It's
>> unfortunately a relatively large change.
> 
> I agree, but a bit too late for 2.6.27 stable and 2.6.28-rc.
> 

Of course, no argument there.

	-hpa

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[Zachary Amsden]

[-- Attachment #1: Type: text/plain, Size: 162 bytes --]

On Tue, 2008-12-09 at 17:15 -0800, Yinghai Lu wrote:

> you can not move that late,
> 
> parse_setup_data==>early_memremap==>__early_ioremap

How does this look?

[-- Attachment #2: x86-vmi-boot-ioremap-fix-take-2.patch --]
[-- Type: text/x-patch, Size: 2449 bytes --]

VMI initialiation can relocate the fixmap, causing early_ioremap
to malfunction if it is initialized before the relocation.
To fix this, VMI activation is split into two phases; the detection,
which must happen before setting up ioremap, and the activation,
which must happen after parsing early boot parameters.

This fixes a crash on boot when VMI is enabled under VMware.

Signed-off-by: Zachary Amsden <zach@vmware.com>

diff --git a/arch/x86/include/asm/vmi.h b/arch/x86/include/asm/vmi.h
index b7c0dea..31dd52d 100644
--- a/arch/x86/include/asm/vmi.h
+++ b/arch/x86/include/asm/vmi.h
@@ -224,6 +224,7 @@ struct pci_header {
 
 /* Function prototypes for bootstrapping */
 extern void vmi_init(void);
+extern void vmi_activate(void);
 extern void vmi_bringup(void);
 extern void vmi_apply_boot_page_allocations(void);
 
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 9d5674f..4c381cb 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -794,6 +794,11 @@ void __init setup_arch(char **cmdline_p)
 	printk(KERN_INFO "Command line: %s\n", boot_command_line);
 #endif
 
+#ifdef CONFIG_VMI
+	/* VMI may relocate the fixmap; do this before touching ioremap area */
+	vmi_init();
+#endif
+
 	early_cpu_init();
 	early_ioremap_init();
 
@@ -880,12 +885,9 @@ void __init setup_arch(char **cmdline_p)
 	check_efer();
 #endif
 
-#if defined(CONFIG_VMI) && defined(CONFIG_X86_32)
-	/*
-	 * Must be before kernel pagetables are setup
-	 * or fixmap area is touched.
-	 */
-	vmi_init();
+#if defined(CONFIG_VMI)
+	/* Must be before kernel pagetables are setup */
+	vmi_activate();
 #endif
 
 	/* after early param, so could get panic from serial */
diff --git a/arch/x86/kernel/vmi_32.c b/arch/x86/kernel/vmi_32.c
index 8b6c393..22fd657 100644
--- a/arch/x86/kernel/vmi_32.c
+++ b/arch/x86/kernel/vmi_32.c
@@ -960,8 +960,6 @@ static inline int __init activate_vmi(void)
 
 void __init vmi_init(void)
 {
-	unsigned long flags;
-
 	if (!vmi_rom)
 		probe_vmi_rom();
 	else
@@ -973,13 +971,21 @@ void __init vmi_init(void)
 
 	reserve_top_address(-vmi_rom->virtual_top);
 
-	local_irq_save(flags);
-	activate_vmi();
-
 #ifdef CONFIG_X86_IO_APIC
 	/* This is virtual hardware; timer routing is wired correctly */
 	no_timer_check = 1;
 #endif
+}
+
+void vmi_activate(void)
+{
+	unsigned long flags;
+
+	if (!vmi_rom)
+		return;
+
+	local_irq_save(flags);
+	activate_vmi();
 	local_irq_restore(flags & X86_EFLAGS_IF);
 }
 

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[Yinghai Lu]

Zachary Amsden wrote:
> On Tue, 2008-12-09 at 17:15 -0800, Yinghai Lu wrote:
> 
>> you can not move that late,
>>
>> parse_setup_data==>early_memremap==>__early_ioremap
> 
> How does this look?
> 

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 9d5674f..4c381cb 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -794,6 +794,11 @@ void __init setup_arch(char **cmdline_p)
 	printk(KERN_INFO "Command line: %s\n", boot_command_line);
 #endif
 
+#ifdef CONFIG_VMI
+	/* VMI may relocate the fixmap; do this before touching ioremap area */
+	vmi_init();
+#endif
+
 	early_cpu_init();
 	early_ioremap_init();
 
@@ -880,12 +885,9 @@ void __init setup_arch(char **cmdline_p)
 	check_efer();
 #endif
 
-#if defined(CONFIG_VMI) && defined(CONFIG_X86_32)
-	/*
-	 * Must be before kernel pagetables are setup
-	 * or fixmap area is touched.
-	 */
-	vmi_init();
+#if defined(CONFIG_VMI)
+	/* Must be before kernel pagetables are setup */
+	vmi_activate();
 #endif
 
 	/* after early param, so could get panic from serial */
diff --git a/arch/x86/kernel/vmi_32.c b/arch/x86/kernel/vmi_32.c
index 8b6c393..22fd657 100644
--- a/arch/x86/kernel/vmi_32.c
+++ b/arch/x86/kernel/vmi_32.c
@@ -960,8 +960,6 @@ static inline int __init activate_vmi(void)
 
 void __init vmi_init(void)
 {
-	unsigned long flags;
-
 	if (!vmi_rom)
 		probe_vmi_rom();
 	else
@@ -973,13 +971,21 @@ void __init vmi_init(void)
 
 	reserve_top_address(-vmi_rom->virtual_top);


it seems still have some problem.
you moved reserve_top_address before parse_parameter...

so
void __init reserve_top_address(unsigned long reserve)
{
        BUG_ON(fixmaps_set > 0);
        printk(KERN_INFO "Reserving virtual address space above 0x%08x\n",
               (int)-reserve);
        __FIXADDR_TOP = -reserve - PAGE_SIZE;
        __VMALLOC_RESERVE += reserve;
}

/*
 * vmalloc=size forces the vmalloc area to be exactly 'size'
 * bytes. This can be used to increase (or decrease) the
 * vmalloc area - the default is 128m.
 */
static int __init parse_vmalloc(char *arg)
{
        if (!arg)
                return -EINVAL;

        /* Add VMALLOC_OFFSET to the parsed value due to vm area guard hole*/
        __VMALLOC_RESERVE = memparse(arg, &arg) + VMALLOC_OFFSET;
        return 0;
}
early_param("vmalloc", parse_vmalloc);

__VMALLOC_RESERVE will be overwriten by vmalloc=...

you may need to split reserve_top_address() to two functions...

YH

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[Greg KH]

On Wed, Dec 10, 2008 at 04:20:06PM -0800, Yinghai Lu wrote:
> Zachary Amsden wrote:
> > On Tue, 2008-12-09 at 17:15 -0800, Yinghai Lu wrote:
> > 
> >> you can not move that late,
> >>
> >> parse_setup_data==>early_memremap==>__early_ioremap
> > 
> > How does this look?
> > 
> 
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 9d5674f..4c381cb 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -794,6 +794,11 @@ void __init setup_arch(char **cmdline_p)
>  	printk(KERN_INFO "Command line: %s\n", boot_command_line);
>  #endif
>  
> +#ifdef CONFIG_VMI
> +	/* VMI may relocate the fixmap; do this before touching ioremap area */
> +	vmi_init();
> +#endif

Shouldn't the #ifdef not be needed here if the .h files are set up
properly for the vmi_init prototype?  Please try to keep them out of .c
files wherever possible.

thanks,

greg k-h

Re: [PATCH] Fix VMI crash on boot in 2.6.27+ kernels

[Zachary Amsden]

On Wed, 2008-12-10 at 16:20 -0800, Yinghai Lu wrote:
> it seems still have some problem.
> you moved reserve_top_address before parse_parameter...

I really only care about resetting fixmap_top.  Adding more vmalloc
space to accomodate what we stole is just being nice... in practice this
should not be a problem.

> __VMALLOC_RESERVE will be overwriten by vmalloc=...
> 
> you may need to split reserve_top_address() to two functions...

For now, misuse vmalloc=XXX at your own risk - why the need for it
anyway?

I agree, it should be cleaned up, as HPA suggests, we should move the
fixmap to a fixed place anyways, and have vmalloc area from the top of
linear memory down to the end of the physical memory mapping.  But there
really isn't time to do anything better for 2.6.28.

Zach

Re: [PATCH] Fix VMI crash on boot in 2.6.27, 2.6.28 kernels

[Greg KH]

On Thu, Dec 11, 2008 at 02:23:11PM -0800, Zachary Amsden wrote:
> On Wed, 2008-12-10 at 19:31 -0800, Greg KH wrote:
> 
> > > +#ifdef CONFIG_VMI
> > > +	/* VMI may relocate the fixmap; do this before touching ioremap area */
> > > +	vmi_init();
> > > +#endif
> > 
> > Shouldn't the #ifdef not be needed here if the .h files are set up
> > properly for the vmi_init prototype?  Please try to keep them out of .c
> > files wherever possible.
> 
> Yes, they should.  Judging by setup.c though, you would think the
> opposite... in any case I fixed it.  Please apply - and yes, I tested
> compile both ways.

> VMI initialiation can relocate the fixmap, causing early_ioremap
> to malfunction if it is initialized before the relocation.
> To fix this, VMI activation is split into two phases; the detection,
> which must happen before setting up ioremap, and the activation,
> which must happen after parsing early boot parameters.
> 
> This fixes a crash on boot when VMI is enabled under VMware.
> 
> Signed-off-by: Zachary Amsden <zach@vmware.com>
> 
> diff --git a/arch/x86/include/asm/vmi.h b/arch/x86/include/asm/vmi.h
> index b7c0dea..128958a 100644
> --- a/arch/x86/include/asm/vmi.h
> +++ b/arch/x86/include/asm/vmi.h
> @@ -223,9 +223,15 @@ struct pci_header {
>  } __attribute__((packed));
>  
>  /* Function prototypes for bootstrapping */
> +#ifdef CONFIG_VMI
>  extern void vmi_init(void);
> +extern void vmi_activate(void);
>  extern void vmi_bringup(void);
> -extern void vmi_apply_boot_page_allocations(void);
> +#else
> +#define vmi_init()
> +#define vmi_activate()
> +#define vmi_bringup()
> +#endif

static inline please, don't use #defines for function prototypes, it's
not nice.  See Andrew's previous rants about this for details :)

thanks,

greg k-h

Re: [PATCH] Fix VMI crash on boot in 2.6.27, 2.6.28 kernels

[Zachary Amsden]

[-- Attachment #1: Type: text/plain, Size: 511 bytes --]

On Wed, 2008-12-10 at 19:31 -0800, Greg KH wrote:

> > +#ifdef CONFIG_VMI
> > +	/* VMI may relocate the fixmap; do this before touching ioremap area */
> > +	vmi_init();
> > +#endif
> 
> Shouldn't the #ifdef not be needed here if the .h files are set up
> properly for the vmi_init prototype?  Please try to keep them out of .c
> files wherever possible.

Yes, they should.  Judging by setup.c though, you would think the
opposite... in any case I fixed it.  Please apply - and yes, I tested
compile both ways.

[-- Attachment #2: x86-vmi-boot-ioremap-fix-take-3.patch --]
[-- Type: text/x-patch, Size: 3076 bytes --]

VMI initialiation can relocate the fixmap, causing early_ioremap
to malfunction if it is initialized before the relocation.
To fix this, VMI activation is split into two phases; the detection,
which must happen before setting up ioremap, and the activation,
which must happen after parsing early boot parameters.

This fixes a crash on boot when VMI is enabled under VMware.

Signed-off-by: Zachary Amsden <zach@vmware.com>

diff --git a/arch/x86/include/asm/vmi.h b/arch/x86/include/asm/vmi.h
index b7c0dea..128958a 100644
--- a/arch/x86/include/asm/vmi.h
+++ b/arch/x86/include/asm/vmi.h
@@ -223,9 +223,15 @@ struct pci_header {
 } __attribute__((packed));
 
 /* Function prototypes for bootstrapping */
+#ifdef CONFIG_VMI
 extern void vmi_init(void);
+extern void vmi_activate(void);
 extern void vmi_bringup(void);
-extern void vmi_apply_boot_page_allocations(void);
+#else
+#define vmi_init()
+#define vmi_activate()
+#define vmi_bringup()
+#endif
 
 /* State needed to start an application processor in an SMP system. */
 struct vmi_ap_state {
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 9d5674f..bdec76e 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -794,6 +794,9 @@ void __init setup_arch(char **cmdline_p)
 	printk(KERN_INFO "Command line: %s\n", boot_command_line);
 #endif
 
+	/* VMI may relocate the fixmap; do this before touching ioremap area */
+	vmi_init();
+
 	early_cpu_init();
 	early_ioremap_init();
 
@@ -880,13 +883,8 @@ void __init setup_arch(char **cmdline_p)
 	check_efer();
 #endif
 
-#if defined(CONFIG_VMI) && defined(CONFIG_X86_32)
-	/*
-	 * Must be before kernel pagetables are setup
-	 * or fixmap area is touched.
-	 */
-	vmi_init();
-#endif
+	/* Must be before kernel pagetables are setup */
+	vmi_activate();
 
 	/* after early param, so could get panic from serial */
 	reserve_early_setup_data();
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 7b10933..f71f96f 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -294,9 +294,7 @@ static void __cpuinit start_secondary(void *unused)
 	 * fragile that we want to limit the things done here to the
 	 * most necessary things.
 	 */
-#ifdef CONFIG_VMI
 	vmi_bringup();
-#endif
 	cpu_init();
 	preempt_disable();
 	smp_callin();
diff --git a/arch/x86/kernel/vmi_32.c b/arch/x86/kernel/vmi_32.c
index 8b6c393..22fd657 100644
--- a/arch/x86/kernel/vmi_32.c
+++ b/arch/x86/kernel/vmi_32.c
@@ -960,8 +960,6 @@ static inline int __init activate_vmi(void)
 
 void __init vmi_init(void)
 {
-	unsigned long flags;
-
 	if (!vmi_rom)
 		probe_vmi_rom();
 	else
@@ -973,13 +971,21 @@ void __init vmi_init(void)
 
 	reserve_top_address(-vmi_rom->virtual_top);
 
-	local_irq_save(flags);
-	activate_vmi();
-
 #ifdef CONFIG_X86_IO_APIC
 	/* This is virtual hardware; timer routing is wired correctly */
 	no_timer_check = 1;
 #endif
+}
+
+void vmi_activate(void)
+{
+	unsigned long flags;
+
+	if (!vmi_rom)
+		return;
+
+	local_irq_save(flags);
+	activate_vmi();
 	local_irq_restore(flags & X86_EFLAGS_IF);
 }
 

Re: [PATCH] Fix VMI crash on boot in 2.6.27, 2.6.28 kernels

[H. Peter Anvin]

Greg KH wrote:
>> +#else
>> +#define vmi_init()
>> +#define vmi_activate()
>> +#define vmi_bringup()
>> +#endif
> 
> static inline please, don't use #defines for function prototypes, it's
> not nice.  See Andrew's previous rants about this for details :)

And if it is not possible, technically, for whatever reason, the proper
forms look like:

#define foo()    ((void)0)
#define bar(x)   ((void)(x))
#define baz(x,y) ((void)((x),(y)))

... which preserve side effects, even if they don't guarantee type safety.

	-hpa

Re: [PATCH] Fix VMI crash on boot in 2.6.27, 2.6.28 kernels

[Zachary Amsden]

[-- Attachment #1: Type: text/plain, Size: 210 bytes --]

On Thu, 2008-12-11 at 13:45 -0800, Greg KH wrote:

> 
> static inline please, don't use #defines for function prototypes, it's
> not nice.  See Andrew's previous rants about this for details :)

Fixed, thanks.

[-- Attachment #2: x86-vmi-boot-ioremap-fix-take-4.patch --]
[-- Type: text/x-patch, Size: 3130 bytes --]

VMI initialiation can relocate the fixmap, causing early_ioremap
to malfunction if it is initialized before the relocation.
To fix this, VMI activation is split into two phases; the detection,
which must happen before setting up ioremap, and the activation,
which must happen after parsing early boot parameters.

This fixes a crash on boot when VMI is enabled under VMware.

Signed-off-by: Zachary Amsden <zach@vmware.com>

diff --git a/arch/x86/include/asm/vmi.h b/arch/x86/include/asm/vmi.h
index b7c0dea..61e08c0 100644
--- a/arch/x86/include/asm/vmi.h
+++ b/arch/x86/include/asm/vmi.h
@@ -223,9 +223,15 @@ struct pci_header {
 } __attribute__((packed));
 
 /* Function prototypes for bootstrapping */
+#ifdef CONFIG_VMI
 extern void vmi_init(void);
+extern void vmi_activate(void);
 extern void vmi_bringup(void);
-extern void vmi_apply_boot_page_allocations(void);
+#else
+static inline void vmi_init(void) {}
+static inline void vmi_activate(void) {}
+static inline void vmi_bringup(void) {}
+#endif
 
 /* State needed to start an application processor in an SMP system. */
 struct vmi_ap_state {
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 9d5674f..bdec76e 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -794,6 +794,9 @@ void __init setup_arch(char **cmdline_p)
 	printk(KERN_INFO "Command line: %s\n", boot_command_line);
 #endif
 
+	/* VMI may relocate the fixmap; do this before touching ioremap area */
+	vmi_init();
+
 	early_cpu_init();
 	early_ioremap_init();
 
@@ -880,13 +883,8 @@ void __init setup_arch(char **cmdline_p)
 	check_efer();
 #endif
 
-#if defined(CONFIG_VMI) && defined(CONFIG_X86_32)
-	/*
-	 * Must be before kernel pagetables are setup
-	 * or fixmap area is touched.
-	 */
-	vmi_init();
-#endif
+	/* Must be before kernel pagetables are setup */
+	vmi_activate();
 
 	/* after early param, so could get panic from serial */
 	reserve_early_setup_data();
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 7b10933..f71f96f 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -294,9 +294,7 @@ static void __cpuinit start_secondary(void *unused)
 	 * fragile that we want to limit the things done here to the
 	 * most necessary things.
 	 */
-#ifdef CONFIG_VMI
 	vmi_bringup();
-#endif
 	cpu_init();
 	preempt_disable();
 	smp_callin();
diff --git a/arch/x86/kernel/vmi_32.c b/arch/x86/kernel/vmi_32.c
index 8b6c393..22fd657 100644
--- a/arch/x86/kernel/vmi_32.c
+++ b/arch/x86/kernel/vmi_32.c
@@ -960,8 +960,6 @@ static inline int __init activate_vmi(void)
 
 void __init vmi_init(void)
 {
-	unsigned long flags;
-
 	if (!vmi_rom)
 		probe_vmi_rom();
 	else
@@ -973,13 +971,21 @@ void __init vmi_init(void)
 
 	reserve_top_address(-vmi_rom->virtual_top);
 
-	local_irq_save(flags);
-	activate_vmi();
-
 #ifdef CONFIG_X86_IO_APIC
 	/* This is virtual hardware; timer routing is wired correctly */
 	no_timer_check = 1;
 #endif
+}
+
+void vmi_activate(void)
+{
+	unsigned long flags;
+
+	if (!vmi_rom)
+		return;
+
+	local_irq_save(flags);
+	activate_vmi();
 	local_irq_restore(flags & X86_EFLAGS_IF);
 }