| Message ID | 20250211025442.3071607-2-binbin.wu@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: TDX: TDX hypercalls may exit to userspace | expand |
On Tue, 2025-02-11 at 10:54 +0800, Binbin Wu wrote: > Have ____kvm_emulate_hypercall() read the GPRs instead of passing them > in via the macro. > > When emulating KVM hypercalls via TDVMCALL, TDX will marshall registers of > TDVMCALL ABI into KVM's x86 registers to match the definition of KVM > hypercall ABI _before_ ____kvm_emulate_hypercall() gets called. Therefore, > ____kvm_emulate_hypercall() can just read registers internally based on KVM > hypercall ABI, and those registers can be removed from the > __kvm_emulate_hypercall() macro. > > Also, op_64_bit can be determined inside ____kvm_emulate_hypercall(), > remove it from the __kvm_emulate_hypercall() macro as well. > > No functional change intended. > > Reviewed-by: Kai Huang <kai.huang@intel.com>
On 2/11/2025 10:54 AM, Binbin Wu wrote: > Have ____kvm_emulate_hypercall() read the GPRs instead of passing them > in via the macro. > > When emulating KVM hypercalls via TDVMCALL, TDX will marshall registers of > TDVMCALL ABI into KVM's x86 registers to match the definition of KVM > hypercall ABI _before_ ____kvm_emulate_hypercall() gets called. Therefore, > ____kvm_emulate_hypercall() can just read registers internally based on KVM > hypercall ABI, and those registers can be removed from the > __kvm_emulate_hypercall() macro. > > Also, op_64_bit can be determined inside ____kvm_emulate_hypercall(), > remove it from the __kvm_emulate_hypercall() macro as well. After this patch, __kvm_emulate_hypercall() becomes superfluous. we can just put the logic to call the "complete_hypercall" into ____kvm_emulate_hypercall() and rename it to __kvm_emulate_hypercall()
On 2/11/2025 6:23 PM, Xiaoyao Li wrote: > On 2/11/2025 10:54 AM, Binbin Wu wrote: >> Have ____kvm_emulate_hypercall() read the GPRs instead of passing them >> in via the macro. >> >> When emulating KVM hypercalls via TDVMCALL, TDX will marshall registers of >> TDVMCALL ABI into KVM's x86 registers to match the definition of KVM >> hypercall ABI _before_ ____kvm_emulate_hypercall() gets called. Therefore, >> ____kvm_emulate_hypercall() can just read registers internally based on KVM >> hypercall ABI, and those registers can be removed from the >> __kvm_emulate_hypercall() macro. >> >> Also, op_64_bit can be determined inside ____kvm_emulate_hypercall(), >> remove it from the __kvm_emulate_hypercall() macro as well. > > After this patch, __kvm_emulate_hypercall() becomes superfluous. > we can just put the logic to call the "complete_hypercall" into ____kvm_emulate_hypercall() and rename it to __kvm_emulate_hypercall() > > According to the commit message of "KVM: x86: Refactor __kvm_emulate_hypercall() into a macro": "Rework __kvm_emulate_hypercall() into a macro so that completion of hypercalls that don't exit to userspace use direct function calls to the completion helper, i.e. don't trigger a retpoline when RETPOLINE=y." So I kept the macro.
On 2/12/2025 9:32 AM, Binbin Wu wrote: > > > On 2/11/2025 6:23 PM, Xiaoyao Li wrote: >> On 2/11/2025 10:54 AM, Binbin Wu wrote: >>> Have ____kvm_emulate_hypercall() read the GPRs instead of passing them >>> in via the macro. >>> >>> When emulating KVM hypercalls via TDVMCALL, TDX will marshall >>> registers of >>> TDVMCALL ABI into KVM's x86 registers to match the definition of KVM >>> hypercall ABI _before_ ____kvm_emulate_hypercall() gets called. >>> Therefore, >>> ____kvm_emulate_hypercall() can just read registers internally based >>> on KVM >>> hypercall ABI, and those registers can be removed from the >>> __kvm_emulate_hypercall() macro. >>> >>> Also, op_64_bit can be determined inside ____kvm_emulate_hypercall(), >>> remove it from the __kvm_emulate_hypercall() macro as well. >> >> After this patch, __kvm_emulate_hypercall() becomes superfluous. >> we can just put the logic to call the "complete_hypercall" into >> ____kvm_emulate_hypercall() and rename it to __kvm_emulate_hypercall() >> >> > According to the commit message of > "KVM: x86: Refactor __kvm_emulate_hypercall() into a macro": > "Rework __kvm_emulate_hypercall() into a macro so that completion of > hypercalls that don't exit to userspace use direct function calls to the > completion helper, i.e. don't trigger a retpoline when RETPOLINE=y." I see. I thought the purpose of introducing the macro was for TDX usage. My fault that didn't checking the commit message of that change. It makes sense for retpoline reason. > So I kept the macro.
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 6ace11303f90..29f33f7c9da9 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -10022,13 +10022,16 @@ static int complete_hypercall_exit(struct kvm_vcpu *vcpu) return kvm_skip_emulated_instruction(vcpu); } -int ____kvm_emulate_hypercall(struct kvm_vcpu *vcpu, unsigned long nr, - unsigned long a0, unsigned long a1, - unsigned long a2, unsigned long a3, - int op_64_bit, int cpl, +int ____kvm_emulate_hypercall(struct kvm_vcpu *vcpu, int cpl, int (*complete_hypercall)(struct kvm_vcpu *)) { unsigned long ret; + unsigned long nr = kvm_rax_read(vcpu); + unsigned long a0 = kvm_rbx_read(vcpu); + unsigned long a1 = kvm_rcx_read(vcpu); + unsigned long a2 = kvm_rdx_read(vcpu); + unsigned long a3 = kvm_rsi_read(vcpu); + int op_64_bit = is_64_bit_hypercall(vcpu); ++vcpu->stat.hypercalls; @@ -10131,9 +10134,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 91e50a513100..8b27f70c6321 100644 --- a/arch/x86/kvm/x86.h +++ b/arch/x86/kvm/x86.h @@ -621,25 +621,17 @@ static inline bool user_exit_on_hypercall(struct kvm *kvm, unsigned long hc_nr) return kvm->arch.hypercall_exit_enabled & BIT(hc_nr); } -int ____kvm_emulate_hypercall(struct kvm_vcpu *vcpu, unsigned long nr, - unsigned long a0, unsigned long a1, - unsigned long a2, unsigned long a3, - int op_64_bit, int cpl, +int ____kvm_emulate_hypercall(struct kvm_vcpu *vcpu, 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, cpl, complete_hypercall); \ + \ + if (__ret > 0) \ + __ret = complete_hypercall(_vcpu); \ + __ret; \ }) int kvm_emulate_hypercall(struct kvm_vcpu *vcpu);
Have ____kvm_emulate_hypercall() read the GPRs instead of passing them in via the macro. When emulating KVM hypercalls via TDVMCALL, TDX will marshall registers of TDVMCALL ABI into KVM's x86 registers to match the definition of KVM hypercall ABI _before_ ____kvm_emulate_hypercall() gets called. Therefore, ____kvm_emulate_hypercall() can just read registers internally based on KVM hypercall ABI, and those registers can be removed from the __kvm_emulate_hypercall() macro. Also, op_64_bit can be determined inside ____kvm_emulate_hypercall(), remove it from the __kvm_emulate_hypercall() macro as well. No functional change intended. Suggested-by: Sean Christopherson <seanjc@google.com> Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com> --- arch/x86/kvm/x86.c | 15 ++++++++------- arch/x86/kvm/x86.h | 26 +++++++++----------------- 2 files changed, 17 insertions(+), 24 deletions(-)