Messages in this thread | | | Date | Thu, 13 Oct 2022 10:33:29 -0700 | Subject | Re: [PATCH] x86/fpu: Remove dynamic features from xcomp_bv for init_fpstate | From | "Chang S. Bae" <> |
| |
On 10/13/2022 10:21 AM, Dave Hansen wrote: > On 10/13/22 09:23, Chang S. Bae wrote: >> >> --- a/arch/x86/kernel/fpu/xstate.c >> +++ b/arch/x86/kernel/fpu/xstate.c >> @@ -1127,8 +1127,12 @@ void __copy_xstate_to_uabi_buf(struct membuf to, >> struct fpstate *fpstate, >> * non-compacted format disabled features still occupy state space, >> * but there is no state to copy from in the compacted >> * init_fpstate. The gap tracking will zero these states. >> + * >> + * In the case of guest fpstate, this user_xfeatures does not >> + * dynamically reflect the capacity of the XSAVE buffer but >> + * xfeatures does. So AND them together. >> */ >> - mask = fpstate->user_xfeatures; >> + mask = fpstate->user_xfeatures & fpstate->xfeatures; > > I'm not sure this is quite right either. > > Doesn't kvm expect that all of the ->user_xfeatures will end up being > copied out? We surely can't copy them from 'fpstate' if the feature > isn't there, but we can't skip them entirely, can we?
No, we can't skip them. IIUC, the code will zero out:
/* * ... The gap tracking will zero these states. */ mask = fpstate->user_xfeatures;
for_each_extended_xfeature(i, mask) { /* * If there was a feature or alignment gap, zero the space * in the destination buffer. */ if (zerofrom < xstate_offsets[i]) membuf_zero(&to, xstate_offsets[i] - zerofrom);
<snip>
/* * Keep track of the last copied state in the non-compacted * target buffer for gap zeroing. */ zerofrom = xstate_offsets[i] + xstate_sizes[i]; }
out: if (to.left) membuf_zero(&to, to.left);
Thanks, Chang
| |