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

["Huang, Kai" <kai.huang@intel.com>]

> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -757,7 +757,7 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
>  
>  	svm->vcpu.arch.cr2 = save->cr2;
>  
> -	kvm_rax_write(vcpu, save->rax);
> +	kvm_rax_write_raw(vcpu, save->rax);
>  	kvm_rsp_write(vcpu, save->rsp);
>  	kvm_rip_write(vcpu, save->rip);
>  
> @@ -1238,7 +1238,7 @@ static int nested_svm_vmexit_update_vmcb12(struct kvm_vcpu *vcpu)
>  	vmcb12->save.rflags = kvm_get_rflags(vcpu);
>  	vmcb12->save.rip    = kvm_rip_read(vcpu);
>  	vmcb12->save.rsp    = kvm_rsp_read(vcpu);
> -	vmcb12->save.rax    = kvm_rax_read(vcpu);
> +	vmcb12->save.rax    = kvm_rax_read_raw(vcpu);

Not sure whether it matters, I think there's an inconsistency here:

The "rax" one has "raw" postfix, but "rsp" doesn't, despite in practice it
is also a "raw" operation.  Ditto for "rip", although it will be moved out
of the "regs[]" GPR array.

But maybe they are different?

[...]

>  	case EXIT_REASON_MSR_WRITE:
> -		kvm_rcx_write(vcpu, tdx->vp_enter_args.r12);
> -		kvm_rax_write(vcpu, tdx->vp_enter_args.r13 & -1u);
> -		kvm_rdx_write(vcpu, tdx->vp_enter_args.r13 >> 32);
> +		kvm_ecx_write(vcpu, tdx->vp_enter_args.r12);
> +		kvm_eax_write(vcpu, tdx->vp_enter_args.r13 & -1u);

Nit:  the "& -1u" isn't needed anymore with using kvm_eax_write(), but maybe
we should just focus on replacing the functions in this patch but leave
cleanup in the future.

[...]


> @@ -12184,23 +12185,23 @@ static void __set_regs(struct kvm_vcpu *vcpu, struct kvm_regs *regs)
>  	vcpu->arch.emulate_regs_need_sync_from_vcpu = true;
>  	vcpu->arch.emulate_regs_need_sync_to_vcpu = false;
>  
> -	kvm_rax_write(vcpu, regs->rax);
> -	kvm_rbx_write(vcpu, regs->rbx);
> -	kvm_rcx_write(vcpu, regs->rcx);
> -	kvm_rdx_write(vcpu, regs->rdx);
> -	kvm_rsi_write(vcpu, regs->rsi);
> -	kvm_rdi_write(vcpu, regs->rdi);
> +	kvm_rax_write_raw(vcpu, regs->rax);
> +	kvm_rbx_write_raw(vcpu, regs->rbx);
> +	kvm_rcx_write_raw(vcpu, regs->rcx);
> +	kvm_rdx_write_raw(vcpu, regs->rdx);
> +	kvm_rsi_write_raw(vcpu, regs->rsi);
> +	kvm_rdi_write_raw(vcpu, regs->rdi);
>  	kvm_rsp_write(vcpu, regs->rsp);
> -	kvm_rbp_write(vcpu, regs->rbp);
> +	kvm_rbp_write_raw(vcpu, regs->rbp);
> 

Ditto, the "rsp" one stands out. :-)

[...]

> +static __always_inline void kvm_e##lname##_write(struct kvm_vcpu *vcpu,	u32 val)	\

It seems tab is used before the 'u32 val'.  Seems no need.