Re: [PATCH 17/21] x86/fpu: Use proper mask to replace full instruction mask

[Liang, Kan]

On 6/22/2020 11:02 AM, Dave Hansen wrote:
> On 6/22/20 7:52 AM, Liang, Kan wrote:
> > > > --- a/arch/x86/kernel/fpu/xstate.c
> > > > +++ b/arch/x86/kernel/fpu/xstate.c
> > > > @@ -58,6 +58,7 @@ static short xsave_cpuid_features[] __initdata = {
> > > >  ÂÂ * XSAVE buffer, both supervisor and user xstates.
> > > >  ÂÂ */
> > > >  Â u64 xfeatures_mask_all __read_mostly;
> > > > +EXPORT_SYMBOL_GPL(xfeatures_mask_all);
> > >
> > > *groan*...
> > >
> > > AFAICT KVM doesn't actually use any of those functions,
> >
> > It seems KVM may eventually invoke copy_xregs_to_kernel() as below.
> >
> > kvm_save_current_fpu()
> >  ÂÂÂ copy_fpregs_to_fpstate()
> >  ÂÂÂÂÂÂÂ copy_xregs_to_kernel()
> >
> > I think we have to export the xfeatures_mask_all.
>
> I'm wondering if we should just take these copy_*regs_to_*() functions
> and uninline them.  Yeah, they are basically wrapping one instruction,
> but it might literally be the most heavyweight instruction in the whole ISA.

Thanks for the suggestions, but I'm not sure if I follow these methods.

I don't think simply removing the "inline" key word for the 
copy_xregs_to_kernel() functions would help here.
Do you mean exporting the copy_*regs_to_*()?


> Or, maybe just make an out-of-line version for KVM to call?

I think the out-of-line version for KVM still needs the 
xfeatures_mask_all. Because the size of vcpu's XSAVE buffer 
(&vcpu->arch.guest_fpu) is the same as other kernel XSAVE buffers, such 
as task->fpu. The xfeatures_mask_all is required for KVM to filter out 
the dynamic supervisor feature as well. I think even if we make an 
out-of-line version for KVM, we still have to export the 
xfeatures_mask_all for KVM.


Thanks,
Kan