Re: [PATCH 6/8] KVM: x86/xen: Enforce alignment of vcpu_info registration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Enforcing hypercall-time alignment checks inside KVM_XEN_VCPU_SET_ATTR breaks live migration for guests that registered a 4-byte aligned vcpu_info in 32-bit mode and subsequently switched to 64-bit mode.
--

commit cfaeaf400a74663fc7a7498659d4e3aee30b0640
Author: David Woodhouse <dwmw@amazon.co.uk>
Subject: KVM: x86/xen: Enforce alignment of vcpu_info registration

This commit enforces that the vcpu_info GPA and HVA are aligned to the size
of a xen_ulong_t based on the guest's bitness. It returns -ENXIO for
requests that do not meet the alignment requirements.

Link: https://lore.kernel.org/all/20260604193554.1BA311F00893@smtp.kernel.org/

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 40b98e70c2dfe..9a6bb094a44f4 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -926,6 +926,12 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
>  				break;
>  			}
>  
> +			r = -ENXIO;
> +			if (!IS_ALIGNED(data->u.gpa,
> +					kvm_xen_has_64bit_shinfo(vcpu->kvm) ?
> +					sizeof(unsigned long) : sizeof(u32)))
> +				break;
> +

[Severity: High]
The commit message mentions that a guest can validly register a vcpu_info at
a 4-byte aligned address while in 32-bit mode and then switch to 64-bit mode.

If a VM does this, what happens during live migration?

If the VMM restores the VM-wide LONG_MODE state before restoring the vCPU
state via kvm_xen_vcpu_set_attr(), kvm_xen_has_64bit_shinfo() will evaluate to
true. 

Could this cause KVM to incorrectly enforce 8-byte alignment and reject
the valid 4-byte aligned GPA with -ENXIO, breaking the migration?

>  			r = kvm_gpc_activate(&vcpu->arch.xen.vcpu_info_cache,
>  					     data->u.gpa, sizeof(struct vcpu_info));
>  		} else {
> @@ -935,6 +941,12 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
>  				break;
>  			}
>  
> +			r = -ENXIO;
> +			if (!IS_ALIGNED(data->u.hva,
> +					kvm_xen_has_64bit_shinfo(vcpu->kvm) ?
> +					sizeof(unsigned long) : sizeof(u32)))
> +				break;
> +

[Severity: High]
Does this HVA alignment check suffer from the same state restoration ordering
issue during live migration?

>  			r = kvm_gpc_activate_hva(&vcpu->arch.xen.vcpu_info_cache,
>  						 data->u.hva, sizeof(struct vcpu_info));
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605143034.3603-1-dwmw2@infradead.org?part=6