Re: [PATCH v4 0/6] KVM: x86: Prep KVM hypercall handling for TDX

[Binbin Wu <binbin.wu@linux.intel.com>]

On 12/19/2024 10:40 AM, Sean Christopherson wrote:
> On Wed, 27 Nov 2024 16:43:38 -0800, Sean Christopherson wrote:
>> Effectively v4 of Binbin's series to handle hypercall exits to userspace in
>> a generic manner, so that TDX
>>
>> Binbin and Kai, this is fairly different that what we last discussed.  While
>> sorting through Binbin's latest patch, I stumbled on what I think/hope is an
>> approach that will make life easier for TDX.  Rather than have common code
>> set the return value, _and_ have TDX implement a callback to do the same for
>> user return MSRs, just use the callback for all paths.
>>
>> [...]
> Applied patch 1 to kvm-x86 fixes.  I'm going to hold off on the rest until the
> dust settles on the SEAMCALL interfaces, e.g. in case TDX ends up marshalling
> state into the "normal" GPRs.
Hi Sean, Based on your suggestions in the link https://lore.kernel.org/kvm/Z1suNzg2Or743a7e@google.com, the v2 of "KVM: TDX: TDX hypercalls may exit to userspace" is planned to morph the TDG.VP.VMCALL with KVM hypercall to EXIT_REASON_VMCALL and marshall r10~r14 from vp_enter_args in struct vcpu_tdx to the appropriate x86 registers for KVM hypercall handling.

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index ef66985ddc91..d5aaf66af835 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -935,6 +935,23 @@ fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu, bool force_immediate_exit)
         return tdx_exit_handlers_fastpath(vcpu);
  }
+static int complete_hypercall_exit(struct kvm_vcpu *vcpu)
+{
+       tdvmcall_set_return_code(vcpu, vcpu->run->hypercall.ret);
+       return 1;
+}
+
+static int tdx_emulate_vmcall(struct kvm_vcpu *vcpu)
+{
+       kvm_rax_write(vcpu, to_tdx(vcpu)->vp_enter_args.r10);
+       kvm_rbx_write(vcpu, to_tdx(vcpu)->vp_enter_args.r11);
+       kvm_rcx_write(vcpu, to_tdx(vcpu)->vp_enter_args.r12);
+       kvm_rdx_write(vcpu, to_tdx(vcpu)->vp_enter_args.r13);
+       kvm_rsi_write(vcpu, to_tdx(vcpu)->vp_enter_args.r14);
+
+       return __kvm_emulate_hypercall(vcpu, 0, complete_hypercall_exit);
+}
+
  static int handle_tdvmcall(struct kvm_vcpu *vcpu)
  {
         switch (tdvmcall_leaf(vcpu)) {
@@ -1286,6 +1303,8 @@ int tdx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t fastpath)
                 return 0;
         case EXIT_REASON_TDCALL:
                 return handle_tdvmcall(vcpu);
+       case EXIT_REASON_VMCALL:
+               return tdx_emulate_vmcall(vcpu);
         default:
                 break;
         }


To test TDX, I made some modifications to your patch
"KVM: x86: Refactor __kvm_emulate_hypercall() into a macro"
Are the following changes make sense to you?

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index a2198807290b..2c5df57ad799 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10088,9 +10088,7 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
         if (kvm_hv_hypercall_enabled(vcpu))
                 return kvm_hv_hypercall(vcpu);
-       return __kvm_emulate_hypercall(vcpu, rax, rbx, rcx, rdx, rsi,
-                                      is_64_bit_hypercall(vcpu),
-                                      kvm_x86_call(get_cpl)(vcpu),
+       return __kvm_emulate_hypercall(vcpu, kvm_x86_call(get_cpl)(vcpu),
                                        complete_hypercall_exit);
  }
  EXPORT_SYMBOL_GPL(kvm_emulate_hypercall);
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index b00ecbfef000..989bed5b48b0 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -623,19 +623,18 @@ int ____kvm_emulate_hypercall(struct kvm_vcpu *vcpu, unsigned long nr,
                               int op_64_bit, int cpl,
                               int (*complete_hypercall)(struct kvm_vcpu *));
-#define __kvm_emulate_hypercall(_vcpu, nr, a0, a1, a2, a3, op_64_bit, cpl, complete_hypercall) \
-({                                                                                             \
-       int __ret;                                                                              \
-                                                                                               \
-       __ret = ____kvm_emulate_hypercall(_vcpu,                                                \
-                                         kvm_##nr##_read(_vcpu), kvm_##a0##_read(_vcpu),       \
-                                         kvm_##a1##_read(_vcpu), kvm_##a2##_read(_vcpu),       \
-                                         kvm_##a3##_read(_vcpu), op_64_bit, cpl,               \
-                                         complete_hypercall);                                  \
-                                                                                               \
-       if (__ret > 0)                                                                          \
-               __ret = complete_hypercall(_vcpu);                                              \
-       __ret;                                                                                  \
+#define __kvm_emulate_hypercall(_vcpu, cpl, complete_hypercall)                                \
+({                                                                                     \
+       int __ret;                                                                      \
+       __ret = ____kvm_emulate_hypercall(_vcpu, kvm_rax_read(_vcpu),                   \
+                                         kvm_rbx_read(_vcpu), kvm_rcx_read(_vcpu),     \
+                                         kvm_rdx_read(_vcpu), kvm_rsi_read(_vcpu),     \
+                                         is_64_bit_hypercall(_vcpu), cpl,              \
+                                         complete_hypercall);                          \
+                                                                                       \
+       if (__ret > 0)                                                                  \
+               __ret = complete_hypercall(_vcpu);                                      \
+       __ret;                                                                          \
  })
  int kvm_emulate_hypercall(struct kvm_vcpu *vcpu);