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?).