All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86: Execute WP test after interrupts are enabled
@ 2009-08-25 19:16 Pekka Enberg
  2009-08-25 19:34 ` Jeremy Fitzhardinge
  2009-08-25 19:37 ` Brian Gerst
  0 siblings, 2 replies; 9+ messages in thread
From: Pekka Enberg @ 2009-08-25 19:16 UTC (permalink / raw)
  To: mingo; +Cc: linux-kernel, penberg, Jeremy Fitzhardinge

Commit 83b519e8b9572c319c8e0c615ee5dd7272856090 ("slab: setup allocators
earlier in the boot sequence") changed the boot sequence to call
mem_init() early. Unfortunately Xen is not prepared to handle the WP test at
that point so we need to make the test run later.

This patch fixes the Xen boot failures reported by Arnd Hannemann.

Reported-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
Tested-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
---
 arch/x86/Kconfig      |    5 +++++
 arch/x86/mm/init_32.c |   10 +++++++---
 include/linux/mm.h    |    8 ++++++++
 init/main.c           |    1 +
 4 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 13ffa5d..b6ff185 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -50,6 +50,7 @@ config X86
 	select HAVE_KERNEL_BZIP2
 	select HAVE_KERNEL_LZMA
 	select HAVE_ARCH_KMEMCHECK
+	select HAVE_ARCH_MEM_INIT_LATE if X86_32
 
 config OUTPUT_FORMAT
 	string
@@ -86,6 +87,10 @@ config STACKTRACE_SUPPORT
 config HAVE_LATENCYTOP_SUPPORT
 	def_bool y
 
+config HAVE_ARCH_MEM_INIT_LATE
+	def_bool y
+	depends on X86_32
+
 config FAST_CMPXCHG_LOCAL
 	bool
 	default y
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index 3cd7711..488ed4b 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -956,13 +956,17 @@ void __init mem_init(void)
 	BUG_ON(VMALLOC_START				>= VMALLOC_END);
 	BUG_ON((unsigned long)high_memory		> VMALLOC_START);
 
-	if (boot_cpu_data.wp_works_ok < 0)
-		test_wp_bit();
-
 	save_pg_dir();
 	zap_low_mappings(true);
 }
 
+void __init mem_init_late(void)
+{
+	/* Interrupts are enabled at this point. */
+	if (boot_cpu_data.wp_works_ok < 0)
+		test_wp_bit();
+}
+
 #ifdef CONFIG_MEMORY_HOTPLUG
 int arch_add_memory(int nid, u64 start, u64 size)
 {
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 9a72cc7..eefcfbe 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1052,6 +1052,14 @@ extern void si_meminfo(struct sysinfo * val);
 extern void si_meminfo_node(struct sysinfo *val, int nid);
 extern int after_bootmem;
 
+#ifdef CONFIG_HAVE_ARCH_MEM_INIT_LATE
+extern void mem_init_late(void);
+#else
+static inline void mem_init_late(void)
+{
+}
+#endif
+
 #ifdef CONFIG_NUMA
 extern void setup_per_cpu_pageset(void);
 #else
diff --git a/init/main.c b/init/main.c
index 2d9d6bd..45d8dbd 100644
--- a/init/main.c
+++ b/init/main.c
@@ -643,6 +643,7 @@ asmlinkage void __init start_kernel(void)
 	set_gfp_allowed_mask(__GFP_BITS_MASK);
 
 	kmem_cache_init_late();
+	mem_init_late();
 
 	/*
 	 * HACK ALERT! This is early. We're enabling the console before
-- 
1.5.6.3


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] x86: Execute WP test after interrupts are enabled
  2009-08-25 19:16 [PATCH] x86: Execute WP test after interrupts are enabled Pekka Enberg
@ 2009-08-25 19:34 ` Jeremy Fitzhardinge
  2009-08-25 19:38   ` Pekka Enberg
  2009-08-25 19:37 ` Brian Gerst
  1 sibling, 1 reply; 9+ messages in thread
From: Jeremy Fitzhardinge @ 2009-08-25 19:34 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: mingo, linux-kernel

On 08/25/09 12:16, Pekka Enberg wrote:
> Commit 83b519e8b9572c319c8e0c615ee5dd7272856090 ("slab: setup allocators
> earlier in the boot sequence") changed the boot sequence to call
> mem_init() early. Unfortunately Xen is not prepared to handle the WP test at
> that point so we need to make the test run later.
>
> This patch fixes the Xen boot failures reported by Arnd Hannemann.
>   

I don't think this is the real fix, and it seems a bit ugly.  I'm OK
with it as a workaround, but I think it will end up getting reverted if
applied.

    J

> Reported-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
> Tested-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
> Cc: Jeremy Fitzhardinge <jeremy@goop.org>
> Signed-off-by: Pekka Enberg <penberg@cs.helsinki.fi>
> ---
>  arch/x86/Kconfig      |    5 +++++
>  arch/x86/mm/init_32.c |   10 +++++++---
>  include/linux/mm.h    |    8 ++++++++
>  init/main.c           |    1 +
>  4 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 13ffa5d..b6ff185 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -50,6 +50,7 @@ config X86
>  	select HAVE_KERNEL_BZIP2
>  	select HAVE_KERNEL_LZMA
>  	select HAVE_ARCH_KMEMCHECK
> +	select HAVE_ARCH_MEM_INIT_LATE if X86_32
>  
>  config OUTPUT_FORMAT
>  	string
> @@ -86,6 +87,10 @@ config STACKTRACE_SUPPORT
>  config HAVE_LATENCYTOP_SUPPORT
>  	def_bool y
>  
> +config HAVE_ARCH_MEM_INIT_LATE
> +	def_bool y
> +	depends on X86_32
> +
>  config FAST_CMPXCHG_LOCAL
>  	bool
>  	default y
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index 3cd7711..488ed4b 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -956,13 +956,17 @@ void __init mem_init(void)
>  	BUG_ON(VMALLOC_START				>= VMALLOC_END);
>  	BUG_ON((unsigned long)high_memory		> VMALLOC_START);
>  
> -	if (boot_cpu_data.wp_works_ok < 0)
> -		test_wp_bit();
> -
>  	save_pg_dir();
>  	zap_low_mappings(true);
>  }
>  
> +void __init mem_init_late(void)
> +{
> +	/* Interrupts are enabled at this point. */
> +	if (boot_cpu_data.wp_works_ok < 0)
> +		test_wp_bit();
> +}
> +
>  #ifdef CONFIG_MEMORY_HOTPLUG
>  int arch_add_memory(int nid, u64 start, u64 size)
>  {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 9a72cc7..eefcfbe 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1052,6 +1052,14 @@ extern void si_meminfo(struct sysinfo * val);
>  extern void si_meminfo_node(struct sysinfo *val, int nid);
>  extern int after_bootmem;
>  
> +#ifdef CONFIG_HAVE_ARCH_MEM_INIT_LATE
> +extern void mem_init_late(void);
> +#else
> +static inline void mem_init_late(void)
> +{
> +}
> +#endif
> +
>  #ifdef CONFIG_NUMA
>  extern void setup_per_cpu_pageset(void);
>  #else
> diff --git a/init/main.c b/init/main.c
> index 2d9d6bd..45d8dbd 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -643,6 +643,7 @@ asmlinkage void __init start_kernel(void)
>  	set_gfp_allowed_mask(__GFP_BITS_MASK);
>  
>  	kmem_cache_init_late();
> +	mem_init_late();
>  
>  	/*
>  	 * HACK ALERT! This is early. We're enabling the console before
>   


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] x86: Execute WP test after interrupts are enabled
  2009-08-25 19:16 [PATCH] x86: Execute WP test after interrupts are enabled Pekka Enberg
  2009-08-25 19:34 ` Jeremy Fitzhardinge
@ 2009-08-25 19:37 ` Brian Gerst
  2009-08-25 19:53   ` Jeremy Fitzhardinge
  1 sibling, 1 reply; 9+ messages in thread
From: Brian Gerst @ 2009-08-25 19:37 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: mingo, linux-kernel, Jeremy Fitzhardinge

Is there any reason to run this test at all on Xen?  It's only needed
for real 386 and some broken 486 processors.

On Tue, Aug 25, 2009 at 3:16 PM, Pekka Enberg<penberg@cs.helsinki.fi> wrote:
> Commit 83b519e8b9572c319c8e0c615ee5dd7272856090 ("slab: setup allocators
> earlier in the boot sequence") changed the boot sequence to call
> mem_init() early. Unfortunately Xen is not prepared to handle the WP test at
> that point so we need to make the test run later.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] x86: Execute WP test after interrupts are enabled
  2009-08-25 19:34 ` Jeremy Fitzhardinge
@ 2009-08-25 19:38   ` Pekka Enberg
  2009-08-25 19:53     ` Jeremy Fitzhardinge
  0 siblings, 1 reply; 9+ messages in thread
From: Pekka Enberg @ 2009-08-25 19:38 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: mingo, linux-kernel

On 08/25/09 12:16, Pekka Enberg wrote:
> > Commit 83b519e8b9572c319c8e0c615ee5dd7272856090 ("slab: setup allocators
> > earlier in the boot sequence") changed the boot sequence to call
> > mem_init() early. Unfortunately Xen is not prepared to handle the WP test at
> > that point so we need to make the test run later.
> >
> > This patch fixes the Xen boot failures reported by Arnd Hannemann.

On Tue, 2009-08-25 at 12:34 -0700, Jeremy Fitzhardinge wrote:
> I don't think this is the real fix, and it seems a bit ugly.  I'm OK
> with it as a workaround, but I think it will end up getting reverted if
> applied.

Yeah, I'm fine with that. It's just that we're really late in the
release cycle now so I'd like to keep the fix as simple as possible. Do
you think the real bug lies in Xen or...?

			Pekka


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] x86: Execute WP test after interrupts are enabled
  2009-08-25 19:38   ` Pekka Enberg
@ 2009-08-25 19:53     ` Jeremy Fitzhardinge
  2009-08-25 19:58       ` Pekka Enberg
  2009-08-25 20:03       ` Arnd Hannemann
  0 siblings, 2 replies; 9+ messages in thread
From: Jeremy Fitzhardinge @ 2009-08-25 19:53 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: mingo, linux-kernel, Arnd Hannemann, Brian Gerst

On 08/25/09 12:38, Pekka Enberg wrote:
> On Tue, 2009-08-25 at 12:34 -0700, Jeremy Fitzhardinge wrote:
>   
>> I don't think this is the real fix, and it seems a bit ugly.  I'm OK
>> with it as a workaround, but I think it will end up getting reverted if
>> applied.
>>     
> Yeah, I'm fine with that. It's just that we're really late in the
> release cycle now so I'd like to keep the fix as simple as possible.

Yeah, that's fine, but I think something like this would be simpler. 
Arnd, could you test it please?

From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Date: Tue, 25 Aug 2009 12:47:17 -0700
Subject: [PATCH] xen: suppress WP test

Xen always runs on CPUs which properly support WP enforcement in
privileged mode, so there's no need to test for it.

This also works around a crash reported by Arnd Hannemann, though
I think its just a band-aid for that case.

Reported-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 16d0d70..e5b903b 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1069,6 +1069,7 @@ asmlinkage void __init xen_start_kernel(void)
 	/* set up basic CPUID stuff */
 	cpu_detect(&new_cpu_data);
 	new_cpu_data.hard_math = 1;
+	new_cpu_data.wp_works_ok = 1;
 	new_cpu_data.x86_capability[0] = cpuid_edx(1);
 #endif



>  Do you think the real bug lies in Xen or...?
>   

I'm unsure.  If handling a trap ends up enabling interrupts for some
reason, then things would break horribly all over the place, so it would
have to be something a bit more subtle.  I'm looking into it.

    J

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] x86: Execute WP test after interrupts are enabled
  2009-08-25 19:37 ` Brian Gerst
@ 2009-08-25 19:53   ` Jeremy Fitzhardinge
  0 siblings, 0 replies; 9+ messages in thread
From: Jeremy Fitzhardinge @ 2009-08-25 19:53 UTC (permalink / raw)
  To: Brian Gerst; +Cc: Pekka Enberg, mingo, linux-kernel

On 08/25/09 12:37, Brian Gerst wrote:
> Is there any reason to run this test at all on Xen?  It's only needed
> for real 386 and some broken 486 processors.
>   

Yeah.  We could easily set boot_cpu_data.wp_works_ok and bypass the
test, but I think its highlighting some other bug.

    J

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] x86: Execute WP test after interrupts are enabled
  2009-08-25 19:53     ` Jeremy Fitzhardinge
@ 2009-08-25 19:58       ` Pekka Enberg
  2009-08-25 20:03         ` Ingo Molnar
  2009-08-25 20:03       ` Arnd Hannemann
  1 sibling, 1 reply; 9+ messages in thread
From: Pekka Enberg @ 2009-08-25 19:58 UTC (permalink / raw)
  To: Jeremy Fitzhardinge; +Cc: mingo, linux-kernel, Arnd Hannemann, Brian Gerst

On Tue, Aug 25, 2009 at 10:53 PM, Jeremy Fitzhardinge<jeremy@goop.org> wrote:
> On 08/25/09 12:38, Pekka Enberg wrote:
>> On Tue, 2009-08-25 at 12:34 -0700, Jeremy Fitzhardinge wrote:
>>
>>> I don't think this is the real fix, and it seems a bit ugly.  I'm OK
>>> with it as a workaround, but I think it will end up getting reverted if
>>> applied.
>>>
>> Yeah, I'm fine with that. It's just that we're really late in the
>> release cycle now so I'd like to keep the fix as simple as possible.
>
> Yeah, that's fine, but I think something like this would be simpler.
> Arnd, could you test it please?
>
> From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> Date: Tue, 25 Aug 2009 12:47:17 -0700
> Subject: [PATCH] xen: suppress WP test
>
> Xen always runs on CPUs which properly support WP enforcement in
> privileged mode, so there's no need to test for it.
>
> This also works around a crash reported by Arnd Hannemann, though
> I think its just a band-aid for that case.
>
> Reported-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> Cc: Pekka Enberg <penberg@cs.helsinki.fi>
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 16d0d70..e5b903b 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -1069,6 +1069,7 @@ asmlinkage void __init xen_start_kernel(void)
>        /* set up basic CPUID stuff */
>        cpu_detect(&new_cpu_data);
>        new_cpu_data.hard_math = 1;
> +       new_cpu_data.wp_works_ok = 1;
>        new_cpu_data.x86_capability[0] = cpuid_edx(1);
>  #endif

Yeah, this is even better assuming it works. :-)

Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] x86: Execute WP test after interrupts are enabled
  2009-08-25 19:58       ` Pekka Enberg
@ 2009-08-25 20:03         ` Ingo Molnar
  0 siblings, 0 replies; 9+ messages in thread
From: Ingo Molnar @ 2009-08-25 20:03 UTC (permalink / raw)
  To: Pekka Enberg, H. Peter Anvin
  Cc: Jeremy Fitzhardinge, linux-kernel, Arnd Hannemann, Brian Gerst


* Pekka Enberg <penberg@cs.helsinki.fi> wrote:

> On Tue, Aug 25, 2009 at 10:53 PM, Jeremy Fitzhardinge<jeremy@goop.org> wrote:
> > On 08/25/09 12:38, Pekka Enberg wrote:
> >> On Tue, 2009-08-25 at 12:34 -0700, Jeremy Fitzhardinge wrote:
> >>
> >>> I don't think this is the real fix, and it seems a bit ugly. ?I'm OK
> >>> with it as a workaround, but I think it will end up getting reverted if
> >>> applied.
> >>>
> >> Yeah, I'm fine with that. It's just that we're really late in the
> >> release cycle now so I'd like to keep the fix as simple as possible.
> >
> > Yeah, that's fine, but I think something like this would be simpler.
> > Arnd, could you test it please?
> >
> > From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> > Date: Tue, 25 Aug 2009 12:47:17 -0700
> > Subject: [PATCH] xen: suppress WP test
> >
> > Xen always runs on CPUs which properly support WP enforcement in
> > privileged mode, so there's no need to test for it.
> >
> > This also works around a crash reported by Arnd Hannemann, though
> > I think its just a band-aid for that case.
> >
> > Reported-by: Arnd Hannemann <hannemann@nets.rwth-aachen.de>
> > Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
> > Cc: Pekka Enberg <penberg@cs.helsinki.fi>
> >
> > diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> > index 16d0d70..e5b903b 100644
> > --- a/arch/x86/xen/enlighten.c
> > +++ b/arch/x86/xen/enlighten.c
> > @@ -1069,6 +1069,7 @@ asmlinkage void __init xen_start_kernel(void)
> > ? ? ? ?/* set up basic CPUID stuff */
> > ? ? ? ?cpu_detect(&new_cpu_data);
> > ? ? ? ?new_cpu_data.hard_math = 1;
> > + ? ? ? new_cpu_data.wp_works_ok = 1;
> > ? ? ? ?new_cpu_data.x86_capability[0] = cpuid_edx(1);
> > ?#endif
> 
> Yeah, this is even better assuming it works. :-)
> 
> Acked-by: Pekka Enberg <penberg@cs.helsinki.fi>

yeah, we should do this one for .31.

	Ingo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] x86: Execute WP test after interrupts are enabled
  2009-08-25 19:53     ` Jeremy Fitzhardinge
  2009-08-25 19:58       ` Pekka Enberg
@ 2009-08-25 20:03       ` Arnd Hannemann
  1 sibling, 0 replies; 9+ messages in thread
From: Arnd Hannemann @ 2009-08-25 20:03 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: Pekka Enberg, mingo, linux-kernel, Arnd Hannemann, Brian Gerst

Jeremy Fitzhardinge schrieb:
> On 08/25/09 12:38, Pekka Enberg wrote:
>> On Tue, 2009-08-25 at 12:34 -0700, Jeremy Fitzhardinge wrote:
>>
>>> I don't think this is the real fix, and it seems a bit ugly.  I'm
>>> OK with it as a workaround, but I think it will end up getting
>>> reverted if applied.
>>>
>> Yeah, I'm fine with that. It's just that we're really late in the
>> release cycle now so I'd like to keep the fix as simple as
>> possible.
>
> Yeah, that's fine, but I think something like this would be simpler.
>  Arnd, could you test it please?

Yes, this indeed also fixes my problem.


Best regards,
Arnd

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2009-08-25 20:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-25 19:16 [PATCH] x86: Execute WP test after interrupts are enabled Pekka Enberg
2009-08-25 19:34 ` Jeremy Fitzhardinge
2009-08-25 19:38   ` Pekka Enberg
2009-08-25 19:53     ` Jeremy Fitzhardinge
2009-08-25 19:58       ` Pekka Enberg
2009-08-25 20:03         ` Ingo Molnar
2009-08-25 20:03       ` Arnd Hannemann
2009-08-25 19:37 ` Brian Gerst
2009-08-25 19:53   ` Jeremy Fitzhardinge

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.