Re: [PATCH v2 08/15] KVM: x86: Add mode-aware versions of kvm_<reg>_{read,write}() helpers
From: Sean Christopherson
Date: Mon May 18 2026 - 16:51:26 EST
On Mon, May 18, 2026, Kai Huang wrote:
>
> > @@ -10413,29 +10413,30 @@ static int complete_hypercall_exit(struct kvm_vcpu *vcpu)
> >
> > if (!is_64_bit_hypercall(vcpu))
> > ret = (u32)ret;
> > - kvm_rax_write(vcpu, ret);
> > + kvm_rax_write_raw(vcpu, ret);
> > return kvm_skip_emulated_instruction(vcpu);
> > }
> >
>
> Nit: AFAICT if we use kvm_rax_write(vcpu, ret) instead of the "raw" version
> here, we can then remove the
>
> if (!is_64_bit_hypercall(vcpu))
> ret = (u32)ret;
No, because sneakily, is_64_bit_hypercall() != is_64_bit_mode(vcpu). And because
we also need to avoid calling is_64_bit_mode(). If we use kvm_rax_write(), then
the unpacked code will be:
WARN_ON_ONCE(vcpu->arch.guest_state_protected);
if (is_long_mode(vcpu))
kvm_x86_call(get_cs_db_l_bits)(vcpu, &cs_db, &cs_l);
else
cs_l = 0;
if (cs_l)
vcpu->arch.regs[VCPU_REGS_RAX] = ret;
else
vcpu->arch.regs[VCPU_REGS_RAX] = (u32)ret;
whereas the (correct) behavior here is:
if (vcpu->arch.guest_state_protected)
cs_l = 1;
else if (is_long_mode(vcpu))
kvm_x86_call(get_cs_db_l_bits)(vcpu, &cs_db, &cs_l);
else
cs_l = 0;
if (cs_l)
vcpu->arch.regs[VCPU_REGS_RAX] = ret;
else
vcpu->arch.regs[VCPU_REGS_RAX] = (u32)ret;
I.e. using the non-raw version will trigger the WARN_ON_ONCE(), and will incorrectly
truncate "ret" whenever cs_l is stale (which might be always?).