Re: [PATCH v3 01/13] KVM: x86/xen: Rename 'longmode' to 'is_64bit' in hypercall handling

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 3191 bytes --]

On Wed, 2026-09-02 at 11:57 -0700, Sean Christopherson wrote:
> On Wed, Sep 02, 2026, David Woodhouse wrote:
> > On Wed, 2026-09-02 at 13:18 +0100, Paul Durrant wrote:
> > > On 31/08/2026 22:26, David Woodhouse wrote:
> > > > From: David Woodhouse <dwmw@amazon.co.uk>
> > > > 
> > > > Rename the local 'longmode' variable and function parameter to
> > > > 'is_64bit' throughout the Xen hypercall handling code. This
> > > > distinguishes it from the VM-wide kvm->arch.xen.long_mode which
> > > > represents the Xen shared_info layout mode.
> > > > 
> > > > The 'is_64bit' parameter indicates whether the vCPU was in 64-bit
> > > > mode when it made the hypercall, which determines how to parse the
> > > > hypercall arguments. The UAPI field name (vcpu->run->xen.u.hcall.longmode)
> > > > is unchanged.
> > > > 
> > > 
> > > Given that 'longmode' is the term used in the UAPI I'm not sure I really 
> > > see the point in this change (particularly since there is not even a 
> > > name clash with 'long_mode').
> > 
> > The difference between 'longmode' and 'long_mode' is subtle, and *has*
> > caused confusion which IIRC is what led to part of this series.
> > 
> > Having to keep 'longmode' in the UAPI for KVM_EXIT_XEN_HCALL is sad,
> > but at least the context is very clear there (xen.u.hcall.longmode).
> 
> We can actually "fix" that, if we want.  And given that the only "longmode"
> reference left in KVM is one in kvm_hv_hypercall_set_result() that can and should
> be nuked, I think it make sense to purge longmode from KVM's source.
> 
> We already did something very similar in e65733b5c59a ("KVM: x86: Redefine 'longmode'
> as a flag for KVM_EXIT_HYPERCALL").  And if we expose both names to userspace, we can
> even purge the misleading name from selftests without forcing existing VMMs to
> rebuild.
> 
> E.g. (completely untested)
> 
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 718396340d3c..043a61e2409d 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -1804,7 +1804,7 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu)
>  handle_in_userspace:
>  	vcpu->run->exit_reason = KVM_EXIT_XEN;
>  	vcpu->run->xen.type = KVM_EXIT_XEN_HCALL;
> -	vcpu->run->xen.u.hcall.longmode = is_64bit;
> +	vcpu->run->xen.u.hcall.is_64bit = is_64bit;
>  	vcpu->run->xen.u.hcall.cpl = cpl;
>  	vcpu->run->xen.u.hcall.input = input;
>  	vcpu->run->xen.u.hcall.params[0] = params[0];
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 9fc8dfdfd65f..5a74765732e3 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -131,7 +131,12 @@ struct kvm_xen_exit {
>  	__u32 type;
>  	union {
>  		struct {
> -			__u32 longmode;
> +			union {
> +#ifndef __KERNEL__
> +				__u32 longmode;
> +#endif
> +				__u32 is_64bit;
> +			};
>  			__u32 cpl;
>  			__u64 input;
>  			__u64 result;

I worry that this kind of thing would leave me grepping the kernel for
"what even *sets* longmode?" after seeing userspace consuming that
field. In the context of a structure that is explicitly about the
hypercall, I think the existing name is fine.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]