From: Suresh Siddha on
On Wed, 2010-07-21 at 10:03 -0700, Robert Richter wrote:
> xsave is broken for (!HAVE_HWFP). This is the case if config
> MATH_EMULATION is enabled, 'no387' kernel parameter is set and xsave
> exists. xsave will not work because x86/math-emu and xsave share the
> same memory. As this case can be treated as corner case we simply
> disable xsave then.

I think it is cleaner to clear these cpu capabilities in the function
which handles no387 boot parameter.

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

thanks.

>
> Signed-off-by: Robert Richter <robert.richter(a)amd.com>
> ---
> arch/x86/kernel/i387.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
> index e73c54e..ff81143 100644
> --- a/arch/x86/kernel/i387.c
> +++ b/arch/x86/kernel/i387.c
> @@ -67,6 +67,12 @@ static void __cpuinit init_thread_xstate(void)
> */
>
> if (!HAVE_HWFP) {
> + /*
> + * Disable xsave as we do not support it if i387
> + * emulation is enabled.
> + */
> + setup_clear_cpu_cap(X86_FEATURE_XSAVE);
> + setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT);
> xstate_size = sizeof(struct i387_soft_struct);
> return;
> }

--
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: Robert Richter on
On 21.07.10 14:16:02, Suresh Siddha wrote:

> I think it is cleaner to clear these cpu capabilities in the function
> which handles no387 boot parameter.

This does not cover all (of course weird but potentially existing)
cases. Disabling xsave in the no387 setup would only work if an fpu
exists. The implementation below disables it if the soft fpu is actual
used. An artificial condition that would break your approach would be
no fpu but xsave. There is no hardware like this but maybe virtual
machines configurations.

So I think it does not hurt to deactivate xsave directly when enabling
soft fpu. The only drawback here is if fpu and xsave initialization
order changes. Hmm...

-Robert

>
> Otherwise Acked-by: Suresh Siddha <suresh.b.siddha(a)intel.com>
>
> thanks.
>
> >
> > Signed-off-by: Robert Richter <robert.richter(a)amd.com>
> > ---
> > arch/x86/kernel/i387.c | 6 ++++++
> > 1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
> > index e73c54e..ff81143 100644
> > --- a/arch/x86/kernel/i387.c
> > +++ b/arch/x86/kernel/i387.c
> > @@ -67,6 +67,12 @@ static void __cpuinit init_thread_xstate(void)
> > */
> >
> > if (!HAVE_HWFP) {
> > + /*
> > + * Disable xsave as we do not support it if i387
> > + * emulation is enabled.
> > + */
> > + setup_clear_cpu_cap(X86_FEATURE_XSAVE);
> > + setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT);
> > xstate_size = sizeof(struct i387_soft_struct);
> > return;
> > }
>
>

--
Advanced Micro Devices, Inc.
Operating System Research Center

--
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
On Thu, 2010-07-22 at 05:36 -0700, Robert Richter wrote:
> On 21.07.10 14:16:02, Suresh Siddha wrote:
>
> > I think it is cleaner to clear these cpu capabilities in the function
> > which handles no387 boot parameter.
>
> This does not cover all (of course weird but potentially existing)
> cases. Disabling xsave in the no387 setup would only work if an fpu
> exists. The implementation below disables it if the soft fpu is actual
> used. An artificial condition that would break your approach would be
> no fpu but xsave. There is no hardware like this but maybe virtual
> machines configurations.
>
> So I think it does not hurt to deactivate xsave directly when enabling
> soft fpu. The only drawback here is if fpu and xsave initialization
> order changes. Hmm...

Then the more appropriate place for this is to check at the beginning of
the xsave init code.

thanks,
suresh

>
> -Robert
>
> >
> > Otherwise Acked-by: Suresh Siddha <suresh.b.siddha(a)intel.com>
> >
> > thanks.
> >
> > >
> > > Signed-off-by: Robert Richter <robert.richter(a)amd.com>
> > > ---
> > > arch/x86/kernel/i387.c | 6 ++++++
> > > 1 files changed, 6 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
> > > index e73c54e..ff81143 100644
> > > --- a/arch/x86/kernel/i387.c
> > > +++ b/arch/x86/kernel/i387.c
> > > @@ -67,6 +67,12 @@ static void __cpuinit init_thread_xstate(void)
> > > */
> > >
> > > if (!HAVE_HWFP) {
> > > + /*
> > > + * Disable xsave as we do not support it if i387
> > > + * emulation is enabled.
> > > + */
> > > + setup_clear_cpu_cap(X86_FEATURE_XSAVE);
> > > + setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT);
> > > xstate_size = sizeof(struct i387_soft_struct);
> > > return;
> > > }
> >
> >
>

--
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: Robert Richter on
On 23.07.10 13:50:05, Suresh Siddha wrote:

> Then the more appropriate place for this is to check at the beginning of
> the xsave init code.

No it isn't, HAVE_HWFP is an fpu implementation and the soft fpu check
would be duplicate in xsave.c. Also, the HAVE_HWFP macro is local in
i387.c and would have to be exported in a header file then, which
somehow taints the api.

So I rather tend to this patch than implementing fpu code in xsave for
a small or no benefit.

-Robert

--
Advanced Micro Devices, Inc.
Operating System Research Center

--
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
On 07/22/2010 05:36 AM, Robert Richter wrote:
> On 21.07.10 14:16:02, Suresh Siddha wrote:
>
>> I think it is cleaner to clear these cpu capabilities in the function
>> which handles no387 boot parameter.
>
> This does not cover all (of course weird but potentially existing)
> cases. Disabling xsave in the no387 setup would only work if an fpu
> exists.

If the fpu doesn't exist, then xsave can't exist, either.

-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/