Prev: [PATCH 1/4] posix-timers: Refactoring of CPUCLOCK* macros
Next: x86, xsave: some code cleanups and reworks, -v2
From: Suresh Siddha on 21 Jul 2010 14:20 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 22 Jul 2010 08:40 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 23 Jul 2010 14:00 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 26 Jul 2010 12:50 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 26 Jul 2010 14:30
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/ |