Re: [PATCH v8 03/20] KVM: x86: Support APX state for XSAVE ABI

[Jim Mattson]

On Thu, Nov 5, 2026 at 9:33 PM Chang S. Bae <chang.seok.bae@xxxxxxxxx> wrote:
>
> On 11/5/26 16:13, Jim Mattson wrote:
> > On Wed, Sep 30, 2026 at 4:02 PM Chang S. Bae <chang.seok.bae@xxxxxxxxx> wrote:
> >>
> >
> >> +static int kvm_copy_uabi_to_vcpu_regs(struct kvm_vcpu *vcpu, void *buf, u64 supported_xcr0)
> >> +{
> >> +       union fpregs_state *xstate = (union fpregs_state *)buf;
> >> +
> >> +       if (!(xstate->xsave.header.xfeatures & XFEATURE_MASK_APX))
> >> +               return 0;
> >> +
> >> +       if (!(supported_xcr0 & XFEATURE_MASK_APX))
> >> +               return -EINVAL;
> >
> > Sashiko is right that this early return leaves stale values in R16-R31 when
> > XSTATE_BV[19] is 0.
> >
> > In your reply to Sashiko, you wrote:
> >
> >    This matches the existing XSAVE uABI semantics. XSTATE_BV specifies
> >    which state components are being updated. If userspace wants to zero
> >    the APX state, it must include the APX bit in XSTATE_BV; omitting the
> >    bit means "leave this component unchanged", just like the generic
> >    XSAVE copy path.
> >
> > That is not how XRSTOR or KVM_SET_XSAVE works. In the XSAVE architecture,
> > XSTATE_BV[i] = 0 indicates that state component i is in its initial
> > configuration (typically zeros), not that it should be left unchanged. In
> > copy_uabi_to_xstate(), clearing bit i in xsave->header.xfeatures
>
> But copy_uabi_to_xstate() does not overwrite XSTATE_BV. Instead,
>
>         /*
>          * Add back in the features that came in from userspace:
>          */
>         xsave->header.xfeatures |= hdr.xfeatures;
>
> This means that even if userspace clears a bit, the bit in the kernel
> XSAVE buffer remains set if it was already set. Consequently, clearing
> XSTATE_BV[i] in the userspace buffer does not necessarily reset the
> state component.

Look at the lines immediately preceding the ones you quoted in
copy_uabi_to_xstate():

    /*
     * The state that came in from userspace was user-state only.
     * Mask all the user states out of 'xfeatures':
     */
    xsave->header.xfeatures &= XFEATURE_MASK_SUPERVISOR_ALL;

    /*
     * Add back in the features that came in from userspace:
     */
    xsave->header.xfeatures |= hdr.xfeatures;

All user-state bits in xsave->header.xfeatures are cleared before
hdr.xfeatures is ORed in. If userspace clears bit i for any user state
component in hdr.xfeatures, bit i in xsave->header.xfeatures is cleared,
and the component is reset to its initial state.