Prev: x86, xsave: reduce cpu_has_xsave checks
Next: x86, xsave: do not initialize xsave in fpu_init()
From: Suresh Siddha on 20 Jul 2010 18:30
On Tue, 2010-07-20 at 11:50 -0700, Robert Richter wrote:
> The patch renames xsave_cntxt_init() and __xsave_init() into
> xstate_enable_boot_cpu() and xstate_enable() as this names are more
> meaningful.
>
> It also removes the duplicate xcr setup for the boot cpu.
>
> Signed-off-by: Robert Richter <robert.richter(a)amd.com>

Acked-by: Suresh Siddha <suresh.b.siddha(a)intel.com>

> ---
> arch/x86/kernel/xsave.c | 19 ++++++++-----------
> 1 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/kernel/xsave.c b/arch/x86/kernel/xsave.c
> index 550bf45..2322f58 100644
> --- a/arch/x86/kernel/xsave.c
> +++ b/arch/x86/kernel/xsave.c
> @@ -360,15 +360,10 @@ unsigned int sig_xstate_size = sizeof(struct _fpstate);
> /*
> * Enable the extended processor state save/restore feature
> */
> -static void __cpuinit __xsave_init(void)
> +static inline void xstate_enable(u64 mask)
> {
> set_in_cr4(X86_CR4_OSXSAVE);
> -
> - /*
> - * Enable all the features that the HW is capable of
> - * and the Linux kernel is aware of.
> - */
> - xsetbv(XCR_XFEATURE_ENABLED_MASK, pcntxt_mask);
> + xsetbv(XCR_XFEATURE_ENABLED_MASK, mask);
> }
>
> /*
> @@ -426,7 +421,7 @@ static void __init setup_xstate_init(void)
> /*
> * Enable and initialize the xsave feature.
> */
> -static void __cpuinit xsave_cntxt_init(void)
> +static void __cpuinit xstate_enable_boot_cpu(void)
> {
> unsigned int eax, ebx, ecx, edx;
>
> @@ -443,7 +438,8 @@ static void __cpuinit xsave_cntxt_init(void)
> * Support only the state known to OS.
> */
> pcntxt_mask = pcntxt_mask & XCNTXT_MASK;
> - __xsave_init();
> +
> + xstate_enable(pcntxt_mask);
>
> /*
> * Recompute the context size for enabled features
> @@ -470,6 +466,7 @@ void __cpuinit xsave_init(void)
> * Boot processor to setup the FP and extended state context info.
> */
> if (!smp_processor_id())
> - xsave_cntxt_init();
> - __xsave_init();
> + xstate_enable_boot_cpu();
> + else
> + xstate_enable(pcntxt_mask);
> }

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 21 Jul 2010 17:20
On 07/21/2010 10:03 AM, Robert Richter wrote:
> The patch renames xsave_cntxt_init() and __xsave_init() into
> xstate_enable_boot_cpu() and xstate_enable() as this names are more
> meaningful.
>
> It also removes the duplicate xcr setup for the boot cpu.
>
> Signed-off-by: Robert Richter <robert.richter(a)amd.com>

> -static void __cpuinit xsave_cntxt_init(void)
> +static void __cpuinit xstate_enable_boot_cpu(void)
> {
> unsigned int eax, ebx, ecx, edx;
>
> @@ -443,7 +438,8 @@ static void __cpuinit xsave_cntxt_init(void)
> * Support only the state known to OS.
> */
> pcntxt_mask = pcntxt_mask & XCNTXT_MASK;
> - __xsave_init();
> +
> + xstate_enable(pcntxt_mask);
>
> /*
> * Recompute the context size for enabled features

This one should be __init rather than __cpuinit, right? As written, I get:


WARNING: vmlinux.o(.cpuinit.text+0x824): Section mismatch in reference
from the function xstate_enable_boot_cpu() to the function
..init.text:__alloc_bootmem()
The function __cpuinit xstate_enable_boot_cpu() references
a function __init __alloc_bootmem().
If __alloc_bootmem is only used by xstate_enable_boot_cpu then
annotate __alloc_bootmem with a matching annotation.

[No need to resend the patch, but if either Suresh or Robert could ACK
this change I'd appreciate it.]


-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Suresh Siddha on 21 Jul 2010 17:30
On Wed, 2010-07-21 at 14:10 -0700, H. Peter Anvin wrote:
> On 07/21/2010 10:03 AM, Robert Richter wrote:
> > The patch renames xsave_cntxt_init() and __xsave_init() into
> > xstate_enable_boot_cpu() and xstate_enable() as this names are more
> > meaningful.
> >
> > It also removes the duplicate xcr setup for the boot cpu.
> >
> > Signed-off-by: Robert Richter <robert.richter(a)amd.com>
>
> > -static void __cpuinit xsave_cntxt_init(void)
> > +static void __cpuinit xstate_enable_boot_cpu(void)
> > {
> > unsigned int eax, ebx, ecx, edx;
> >
> > @@ -443,7 +438,8 @@ static void __cpuinit xsave_cntxt_init(void)
> > * Support only the state known to OS.
> > */
> > pcntxt_mask = pcntxt_mask & XCNTXT_MASK;
> > - __xsave_init();
> > +
> > + xstate_enable(pcntxt_mask);
> >
> > /*
> > * Recompute the context size for enabled features
>
> This one should be __init rather than __cpuinit, right? As written, I get:
>
>
> WARNING: vmlinux.o(.cpuinit.text+0x824): Section mismatch in reference
> from the function xstate_enable_boot_cpu() to the function
> .init.text:__alloc_bootmem()
> The function __cpuinit xstate_enable_boot_cpu() references
> a function __init __alloc_bootmem().
> If __alloc_bootmem is only used by xstate_enable_boot_cpu then
> annotate __alloc_bootmem with a matching annotation.
>
> [No need to resend the patch, but if either Suresh or Robert could ACK
> this change I'd appreciate it.]

Yes, it should be __init.

thanks,
suresh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: H. Peter Anvin on 21 Jul 2010 18:00
On 07/21/2010 02:20 PM, Suresh Siddha wrote:
>
> Yes, it should be __init.
>

OK, here is the proposed fix for that. It's a bit uglier than I would
have liked.

It also fixes the assumption that "we are on the boot CPU so we are
early in the boot", which is true now but will not be true in the future
when we can offline (and later re-online) the boot CPU.

Comments appreciated...

-hpa
From: Suresh Siddha on 21 Jul 2010 18:40
On Wed, 2010-07-21 at 14:53 -0700, H. Peter Anvin wrote:
> On 07/21/2010 02:20 PM, Suresh Siddha wrote:
> >
> > Yes, it should be __init.
> >
>
> OK, here is the proposed fix for that. It's a bit uglier than I would
> have liked.
>
> It also fixes the assumption that "we are on the boot CPU so we are
> early in the boot", which is true now but will not be true in the future
> when we can offline (and later re-online) the boot CPU.
>
> Comments appreciated...

I am ok with this fix. Thanks for taking a stab at this.

suresh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2
Prev: x86, xsave: reduce cpu_has_xsave checks
Next: x86, xsave: do not initialize xsave in fpu_init()