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

["Chang S. Bae" <chang.seok.bae@intel.com>]

On 7/20/2026 11:22 AM, sashiko-bot@kernel.org 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;
> 
> [Severity: High]
> Does this early return fail to reset the APX registers to their architectural
> INIT state?
> 
> If a VMM restores a vCPU's state using KVM_SET_XSAVE and the
> XFEATURE_MASK_APX bit is clear, the APX component should be initialized to
> zero. Because this returns early without modifying vcpu->arch.regs, could
> this leave stale data in the VCPU's APX registers and leak state across
> resets or migrations?

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.

>> @@ -3161,6 +3211,7 @@ static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
>>   					struct kvm_xsave *guest_xsave)
>>   {
>>   	union fpregs_state *xstate = (union fpregs_state *)guest_xsave->region;
> 
> [Severity: High]
> Will adding APX as a static FPU feature inflate KVM's base uABI size beyond
> 4096 bytes on AMX-capable hosts, unconditionally breaking legacy KVM_GET_XSAVE?

APX reuses the XSAVE area formerly allocated to MPX, so the layout does 
not grow beyond the existing ABI size.

>> @@ -3172,6 +3223,16 @@ static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
>>   	 */
>>   	xstate->xsave.header.xfeatures &= ~vcpu->arch.guest_fpu.fpstate->xfd;
>>   
>> +	/*
>> +	 * This and the following copy functions copy APX state into each
>> +	 * storage. The VCPU cache is the single source of truth but avoiding
>> +	 * this redundant copy tends to introduce special-case handling. Since
>> +	 * this lies in a slow path, keep the implementation simple.
>> +	 */
>> +	err = kvm_copy_uabi_to_vcpu_regs(vcpu, guest_xsave->region, kvm_caps.supported_xcr0);
>> +	if (err)
>> +		return err;
> 
> [Severity: High]
> Does modifying the APX state before validating the rest of the FPU state
> lead to a corrupted vCPU state if the ioctl fails?

This follows the existing behavior. copy_uabi_to_xstate() also updates
state incrementally before later validation:
  
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/fpu/xstate.c#n1363

This patch still preserves those semantics.

Thanks,
Chang