Re: [PATCH 6/7] x86, xsave: disable xsave in i387 emulation mode

[Robert Richter <robert.richter@amd.com>]

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@intel.com>
> 
> thanks.
> 
> > 
> > Signed-off-by: Robert Richter <robert.richter@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