Re: [PATCH v2 08/15] KVM: x86: Add mode-aware versions of kvm_<reg>_{read,write}() helpers

[Huang, Kai]

On Mon, 2026-05-18 at 13:51 -0700, Sean Christopherson wrote:
> 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?).

FWIW, I sanity tested that booting/destroying both TD and VMX guests worked
fine.  I have no environment to test SVM and Xen related parts, though.