Re: [PATCH v4 0/6] KVM: x86: Prep KVM hypercall handling for TDX
From: Binbin Wu
Date: Sun Jan 19 2025 - 19:38:15 EST
On 1/18/2025 3:31 AM, Sean Christopherson wrote:
On Wed, Jan 15, 2025, Binbin Wu wrote:
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@xxxxxxxxxx, 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.
...
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?
Yes, but I think we can go a step further and effectively revert the bulk of commit
e913ef159fad ("KVM: x86: Split core of hypercall emulation to helper function"),
i.e. have ____kvm_emulate_hypercall() read the GPRs instead of passing them in
via the macro.
Sure.
Are you OK if I sent the change (as a prep patch) along with v2 of
"TDX hypercalls may exit to userspace"?
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);