* [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx
@ 2025-03-18 6:35 Vishal Verma
2025-03-18 6:35 ` [PATCH v2 1/4] KVM: TDX: Fix definition of tdx_guest_nr_guest_keyids() Vishal Verma
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Vishal Verma @ 2025-03-18 6:35 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Binbin Wu, Rick Edgecombe, Vishal Verma
This is a cleanup that should follow the initial TDX base support (i.e.
not an immediate fix needed for kvm-coco-queue).
Patch 1 is a precursory fix for a build warning/error found when
manually testing the CONFIG_INTEL_TDX_HOST=n case.
For Patches 2-4:
In [1], Sean points out that the kvm_x86_ops structure and its
associated helpers and wrappers can be cleaned up a lot by -
1. Putting the wrappers under CONFIG_KVM_INTEL_TDX, and
2. Defining the helpers with macros that switch between the tdx and
non-tdx case, as well as NULL out the TDX-only stubs when needed.
This cleans up the generated code by completely removing trampolines
that would otherwise be left behind in the CONFIG_KVM_INTEL_TDX=n case.
[1]: https://lore.kernel.org/kvm/Z6v9yjWLNTU6X90d@google.com/
For example, looking at vt_refresh_apicv_exec_ctrl(), before this cleanup,
when CONFIG_KVM_INTEL_TDX=n, the following asm is generated:
0000000000036490 <vt_refresh_apicv_exec_ctrl>:
36490: f3 0f 1e fa endbr64
36494: e8 00 00 00 00 call 36499 <vt_refresh_apicv_exec_ctrl+0x9>
36495: R_X86_64_PLT32 __fentry__-0x4
36499: e9 00 00 00 00 jmp 3649e <vt_refresh_apicv_exec_ctrl+0xe>
3649a: R_X86_64_PLT32 vmx_refresh_apicv_exec_ctrl-0x4
3649e: 66 90 xchg %ax,%ax
But with these patches, it goes away completely.
These patches have been tested with TDX kvm-unit-tests, booting a Linux
TD, TDX enhanced KVM selftests, and building and examining the generated
assembly (or lack thereof) with both CONFIG_KVM_INTEL_TDX=y and
CONFIG_KVM_INTEL_TDX=n
Based on a patch by Sean Christopherson <seanjc@google.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
Changes in v2:
- Collect review tags (Binbin)
- Add a new patch (patch 1) as a precursor that fixes a build problem
- Squash the config change into patch 4 that converts ops to macros to
avoid breaking the build when CONFIG_KVM_INTEL_TDX=n (Binbin)
- Link to v1: https://lore.kernel.org/r/20250313-vverma7-cleanup_x86_ops-v1-0-0346c8211a0c@intel.com
---
Vishal Verma (4):
KVM: TDX: Fix definition of tdx_guest_nr_guest_keyids()
KVM: VMX: Move apicv_pre_state_restore to posted_intr.c
KVM: VMX: Make naming consistent for kvm_complete_insn_gp via define
KVM: VMX: Clean up and macrofy x86_ops
arch/x86/include/asm/tdx.h | 2 +-
arch/x86/kvm/vmx/posted_intr.h | 1 +
arch/x86/kvm/vmx/tdx.h | 2 +-
arch/x86/kvm/vmx/x86_ops.h | 68 +-------------
arch/x86/kvm/vmx/main.c | 204 ++++++++++++++++++++---------------------
arch/x86/kvm/vmx/posted_intr.c | 8 ++
6 files changed, 114 insertions(+), 171 deletions(-)
---
base-commit: 85c9490bbed74b006a614e542da404a55ff5938f
change-id: 20250311-vverma7-cleanup_x86_ops-c62e50e47126
Best regards,
--
Vishal Verma <vishal.l.verma@intel.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v2 1/4] KVM: TDX: Fix definition of tdx_guest_nr_guest_keyids() 2025-03-18 6:35 [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx Vishal Verma @ 2025-03-18 6:35 ` Vishal Verma 2025-03-18 15:33 ` Edgecombe, Rick P 2025-03-18 6:35 ` [PATCH v2 2/4] KVM: VMX: Move apicv_pre_state_restore to posted_intr.c Vishal Verma ` (3 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Vishal Verma @ 2025-03-18 6:35 UTC (permalink / raw) To: Sean Christopherson, Paolo Bonzini Cc: kvm, linux-kernel, Binbin Wu, Rick Edgecombe, Vishal Verma When CONFIG_INTEL_TDX_HOST=n, the above definition produced an unused-function warning with gcc. error: ‘tdx_get_nr_guest_keyids’ defined but not used [-Werror=unused-function] 198 | static u32 tdx_get_nr_guest_keyids(void) { return 0; } | ^~~~~~~~~~~~~~~~~~~~~~~ Make the definition 'inline' so that in the config disabled case, the whole thing can be optimized away. Cc: Sean Christopherson <seanjc@google.com> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- arch/x86/include/asm/tdx.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h index e6b003fe7f5e..fbc22bf39cfd 100644 --- a/arch/x86/include/asm/tdx.h +++ b/arch/x86/include/asm/tdx.h @@ -195,7 +195,7 @@ u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct page *page); static inline void tdx_init(void) { } static inline int tdx_cpu_enable(void) { return -ENODEV; } static inline int tdx_enable(void) { return -ENODEV; } -static u32 tdx_get_nr_guest_keyids(void) { return 0; } +static inline u32 tdx_get_nr_guest_keyids(void) { return 0; } static inline const char *tdx_dump_mce_info(struct mce *m) { return NULL; } static inline const struct tdx_sys_info *tdx_get_sysinfo(void) { return NULL; } #endif /* CONFIG_INTEL_TDX_HOST */ -- 2.48.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/4] KVM: TDX: Fix definition of tdx_guest_nr_guest_keyids() 2025-03-18 6:35 ` [PATCH v2 1/4] KVM: TDX: Fix definition of tdx_guest_nr_guest_keyids() Vishal Verma @ 2025-03-18 15:33 ` Edgecombe, Rick P 2025-03-18 16:31 ` Verma, Vishal L 0 siblings, 1 reply; 10+ messages in thread From: Edgecombe, Rick P @ 2025-03-18 15:33 UTC (permalink / raw) To: pbonzini@redhat.com, Verma, Vishal L, seanjc@google.com Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, binbin.wu@linxu.intel.com On Tue, 2025-03-18 at 00:35 -0600, Vishal Verma wrote: > When CONFIG_INTEL_TDX_HOST=n, the above definition produced an > unused-function warning with gcc. > > error: ‘tdx_get_nr_guest_keyids’ defined but not used [-Werror=unused-function] > 198 | static u32 tdx_get_nr_guest_keyids(void) { return 0; } > | ^~~~~~~~~~~~~~~~~~~~~~~ > > Make the definition 'inline' so that in the config disabled case, the > whole thing can be optimized away. This looks to be fixed in the current kvm-coco-queue. Can you double check? > > Cc: Sean Christopherson <seanjc@google.com> > Cc: Rick Edgecombe <rick.p.edgecombe@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > arch/x86/include/asm/tdx.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h > index e6b003fe7f5e..fbc22bf39cfd 100644 > --- a/arch/x86/include/asm/tdx.h > +++ b/arch/x86/include/asm/tdx.h > @@ -195,7 +195,7 @@ u64 tdh_phymem_page_wbinvd_hkid(u64 hkid, struct page *page); > static inline void tdx_init(void) { } > static inline int tdx_cpu_enable(void) { return -ENODEV; } > static inline int tdx_enable(void) { return -ENODEV; } > -static u32 tdx_get_nr_guest_keyids(void) { return 0; } > +static inline u32 tdx_get_nr_guest_keyids(void) { return 0; } > static inline const char *tdx_dump_mce_info(struct mce *m) { return NULL; } > static inline const struct tdx_sys_info *tdx_get_sysinfo(void) { return NULL; } > #endif /* CONFIG_INTEL_TDX_HOST */ > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/4] KVM: TDX: Fix definition of tdx_guest_nr_guest_keyids() 2025-03-18 15:33 ` Edgecombe, Rick P @ 2025-03-18 16:31 ` Verma, Vishal L 0 siblings, 0 replies; 10+ messages in thread From: Verma, Vishal L @ 2025-03-18 16:31 UTC (permalink / raw) To: pbonzini@redhat.com, seanjc@google.com, Edgecombe, Rick P Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, binbin.wu@linxu.intel.com On Tue, 2025-03-18 at 15:33 +0000, Edgecombe, Rick P wrote: > On Tue, 2025-03-18 at 00:35 -0600, Vishal Verma wrote: > > When CONFIG_INTEL_TDX_HOST=n, the above definition produced an > > unused-function warning with gcc. > > > > error: ‘tdx_get_nr_guest_keyids’ defined but not used [- > > Werror=unused-function] > > 198 | static u32 tdx_get_nr_guest_keyids(void) { return 0; } > > | ^~~~~~~~~~~~~~~~~~~~~~~ > > > > Make the definition 'inline' so that in the config disabled case, > > the > > whole thing can be optimized away. > > This looks to be fixed in the current kvm-coco-queue. Can you double > check? > > Indeed it is, I will drop it for the next revision. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/4] KVM: VMX: Move apicv_pre_state_restore to posted_intr.c 2025-03-18 6:35 [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx Vishal Verma 2025-03-18 6:35 ` [PATCH v2 1/4] KVM: TDX: Fix definition of tdx_guest_nr_guest_keyids() Vishal Verma @ 2025-03-18 6:35 ` Vishal Verma 2025-04-29 8:17 ` Chao Gao 2025-03-18 6:35 ` [PATCH v2 3/4] KVM: VMX: Make naming consistent for kvm_complete_insn_gp via define Vishal Verma ` (2 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Vishal Verma @ 2025-03-18 6:35 UTC (permalink / raw) To: Sean Christopherson, Paolo Bonzini Cc: kvm, linux-kernel, Binbin Wu, Rick Edgecombe, Vishal Verma In preparation for a cleanup of the x86_ops struct for TDX, which turns several of the ops definitions to macros, move the vt_apicv_pre_state_restore() helper into posted_intr.c. Based on a patch by Sean Christopherson <seanjc@google.com> Link: https://lore.kernel.org/kvm/Z6v9yjWLNTU6X90d@google.com/ Cc: Sean Christopherson <seanjc@google.com> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com> Reviewed-by: Binbin Wu <binbin.wu@linxu.intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- arch/x86/kvm/vmx/posted_intr.h | 1 + arch/x86/kvm/vmx/main.c | 10 +--------- arch/x86/kvm/vmx/posted_intr.c | 8 ++++++++ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/arch/x86/kvm/vmx/posted_intr.h b/arch/x86/kvm/vmx/posted_intr.h index 68605ca7ef68..9d0677a2ba0e 100644 --- a/arch/x86/kvm/vmx/posted_intr.h +++ b/arch/x86/kvm/vmx/posted_intr.h @@ -11,6 +11,7 @@ void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int cpu); void vmx_vcpu_pi_put(struct kvm_vcpu *vcpu); void pi_wakeup_handler(void); void __init pi_init_cpu(int cpu); +void pi_apicv_pre_state_restore(struct kvm_vcpu *vcpu); bool pi_has_pending_interrupt(struct kvm_vcpu *vcpu); int vmx_pi_update_irte(struct kvm *kvm, unsigned int host_irq, uint32_t guest_irq, bool set); diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c index 320c96e1e80a..9d201ddb794a 100644 --- a/arch/x86/kvm/vmx/main.c +++ b/arch/x86/kvm/vmx/main.c @@ -315,14 +315,6 @@ static void vt_set_virtual_apic_mode(struct kvm_vcpu *vcpu) return vmx_set_virtual_apic_mode(vcpu); } -static void vt_apicv_pre_state_restore(struct kvm_vcpu *vcpu) -{ - struct pi_desc *pi = vcpu_to_pi_desc(vcpu); - - pi_clear_on(pi); - memset(pi->pir, 0, sizeof(pi->pir)); -} - static void vt_hwapic_isr_update(struct kvm_vcpu *vcpu, int max_isr) { if (is_td_vcpu(vcpu)) @@ -983,7 +975,7 @@ struct kvm_x86_ops vt_x86_ops __initdata = { .set_apic_access_page_addr = vt_set_apic_access_page_addr, .refresh_apicv_exec_ctrl = vt_refresh_apicv_exec_ctrl, .load_eoi_exitmap = vt_load_eoi_exitmap, - .apicv_pre_state_restore = vt_apicv_pre_state_restore, + .apicv_pre_state_restore = pi_apicv_pre_state_restore, .required_apicv_inhibits = VMX_REQUIRED_APICV_INHIBITS, .hwapic_isr_update = vt_hwapic_isr_update, .sync_pir_to_irr = vt_sync_pir_to_irr, diff --git a/arch/x86/kvm/vmx/posted_intr.c b/arch/x86/kvm/vmx/posted_intr.c index f2ca37b3f606..a140af060bb8 100644 --- a/arch/x86/kvm/vmx/posted_intr.c +++ b/arch/x86/kvm/vmx/posted_intr.c @@ -241,6 +241,14 @@ void __init pi_init_cpu(int cpu) raw_spin_lock_init(&per_cpu(wakeup_vcpus_on_cpu_lock, cpu)); } +void pi_apicv_pre_state_restore(struct kvm_vcpu *vcpu) +{ + struct pi_desc *pi = vcpu_to_pi_desc(vcpu); + + pi_clear_on(pi); + memset(pi->pir, 0, sizeof(pi->pir)); +} + bool pi_has_pending_interrupt(struct kvm_vcpu *vcpu) { struct pi_desc *pi_desc = vcpu_to_pi_desc(vcpu); -- 2.48.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/4] KVM: VMX: Move apicv_pre_state_restore to posted_intr.c 2025-03-18 6:35 ` [PATCH v2 2/4] KVM: VMX: Move apicv_pre_state_restore to posted_intr.c Vishal Verma @ 2025-04-29 8:17 ` Chao Gao 2025-04-29 17:25 ` Verma, Vishal L 0 siblings, 1 reply; 10+ messages in thread From: Chao Gao @ 2025-04-29 8:17 UTC (permalink / raw) To: Vishal Verma Cc: Sean Christopherson, Paolo Bonzini, kvm, linux-kernel, Binbin Wu, Rick Edgecombe On Tue, Mar 18, 2025 at 12:35:07AM -0600, Vishal Verma wrote: >In preparation for a cleanup of the x86_ops struct for TDX, which turns >several of the ops definitions to macros, move the >vt_apicv_pre_state_restore() helper into posted_intr.c. This doesn't explain how the movement is related to that cleanup. how about: In preparation for a cleanup of the kvm_x86_ops struct for TDX, all vt_* functions are expected to act as glue functions that route to either tdx_* or vmx_* based on the VM type. Specifically, the pattern is: vt_abc: if (is_td()) return tdx_abc(); return vmx_abc(); But vt_apicv_pre_state_restore() does not follow this pattern. To facilitate that cleanup, rename and move vt_apicv_pre_state_restore() into posted_intr.c. > >Based on a patch by Sean Christopherson <seanjc@google.com> You can consider adding his Suggested-by. >-static void vt_apicv_pre_state_restore(struct kvm_vcpu *vcpu) >-{ >- struct pi_desc *pi = vcpu_to_pi_desc(vcpu); With this removal, vcpu_to_pi_desc() is only used within posted_intr.c. no need to expose it. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/4] KVM: VMX: Move apicv_pre_state_restore to posted_intr.c 2025-04-29 8:17 ` Chao Gao @ 2025-04-29 17:25 ` Verma, Vishal L 0 siblings, 0 replies; 10+ messages in thread From: Verma, Vishal L @ 2025-04-29 17:25 UTC (permalink / raw) To: Gao, Chao Cc: Edgecombe, Rick P, kvm@vger.kernel.org, pbonzini@redhat.com, seanjc@google.com, linux-kernel@vger.kernel.org, binbin.wu@linxu.intel.com On Tue, 2025-04-29 at 16:17 +0800, Chao Gao wrote: > On Tue, Mar 18, 2025 at 12:35:07AM -0600, Vishal Verma wrote: > > In preparation for a cleanup of the x86_ops struct for TDX, which turns > > several of the ops definitions to macros, move the > > vt_apicv_pre_state_restore() helper into posted_intr.c. > > This doesn't explain how the movement is related to that cleanup. > > how about: > > In preparation for a cleanup of the kvm_x86_ops struct for TDX, all vt_* > functions are expected to act as glue functions that route to either tdx_* > or vmx_* based on the VM type. Specifically, the pattern is: > > vt_abc: > if (is_td()) > return tdx_abc(); > return vmx_abc(); > > But vt_apicv_pre_state_restore() does not follow this pattern. To > facilitate that cleanup, rename and move vt_apicv_pre_state_restore() into > posted_intr.c. Hi Chao, Thanks for the suggestion, I've added this for the next revision. > > > > > Based on a patch by Sean Christopherson <seanjc@google.com> > > You can consider adding his Suggested-by. Done. > > > -static void vt_apicv_pre_state_restore(struct kvm_vcpu *vcpu) > > -{ > > - struct pi_desc *pi = vcpu_to_pi_desc(vcpu); > > With this removal, vcpu_to_pi_desc() is only used within posted_intr.c. no > need to expose it. Good find, done. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/4] KVM: VMX: Make naming consistent for kvm_complete_insn_gp via define 2025-03-18 6:35 [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx Vishal Verma 2025-03-18 6:35 ` [PATCH v2 1/4] KVM: TDX: Fix definition of tdx_guest_nr_guest_keyids() Vishal Verma 2025-03-18 6:35 ` [PATCH v2 2/4] KVM: VMX: Move apicv_pre_state_restore to posted_intr.c Vishal Verma @ 2025-03-18 6:35 ` Vishal Verma 2025-03-18 6:35 ` [PATCH v2 4/4] KVM: VMX: Clean up and macrofy x86_ops Vishal Verma 2025-05-02 21:51 ` [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx Sean Christopherson 4 siblings, 0 replies; 10+ messages in thread From: Vishal Verma @ 2025-03-18 6:35 UTC (permalink / raw) To: Sean Christopherson, Paolo Bonzini Cc: kvm, linux-kernel, Binbin Wu, Rick Edgecombe, Vishal Verma In preparation for defining x86_ops using macros, make the naming of kvm_complete_insn_gp() in the non TDX case more consistent with other vmx_ops - i.e. use a #define to allow it to be referred as vmx_complete_emulated_msr(). Based on a patch by Sean Christopherson <seanjc@google.com> Link: https://lore.kernel.org/kvm/Z6v9yjWLNTU6X90d@google.com/ Cc: Sean Christopherson <seanjc@google.com> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- arch/x86/kvm/vmx/x86_ops.h | 1 + arch/x86/kvm/vmx/main.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h index 19f770b0fc81..97fcbcb0a503 100644 --- a/arch/x86/kvm/vmx/x86_ops.h +++ b/arch/x86/kvm/vmx/x86_ops.h @@ -58,6 +58,7 @@ void vmx_prepare_switch_to_guest(struct kvm_vcpu *vcpu); void vmx_update_exception_bitmap(struct kvm_vcpu *vcpu); int vmx_get_feature_msr(u32 msr, u64 *data); int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info); +#define vmx_complete_emulated_msr kvm_complete_insn_gp u64 vmx_get_segment_base(struct kvm_vcpu *vcpu, int seg); void vmx_get_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg); void vmx_set_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int seg); diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c index 9d201ddb794a..a1388adffa1e 100644 --- a/arch/x86/kvm/vmx/main.c +++ b/arch/x86/kvm/vmx/main.c @@ -240,7 +240,7 @@ static int vt_complete_emulated_msr(struct kvm_vcpu *vcpu, int err) if (is_td_vcpu(vcpu)) return tdx_complete_emulated_msr(vcpu, err); - return kvm_complete_insn_gp(vcpu, err); + return vmx_complete_emulated_msr(vcpu, err); } #ifdef CONFIG_KVM_SMM -- 2.48.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 4/4] KVM: VMX: Clean up and macrofy x86_ops 2025-03-18 6:35 [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx Vishal Verma ` (2 preceding siblings ...) 2025-03-18 6:35 ` [PATCH v2 3/4] KVM: VMX: Make naming consistent for kvm_complete_insn_gp via define Vishal Verma @ 2025-03-18 6:35 ` Vishal Verma 2025-05-02 21:51 ` [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx Sean Christopherson 4 siblings, 0 replies; 10+ messages in thread From: Vishal Verma @ 2025-03-18 6:35 UTC (permalink / raw) To: Sean Christopherson, Paolo Bonzini Cc: kvm, linux-kernel, Binbin Wu, Rick Edgecombe, Vishal Verma Eliminate a lot of stub definitions by using macros to define the TDX vs non-TDX versions of various x86_ops. Moving the x86_ops wrappers under CONFIG_KVM_INTEL_TDX also allows nearly all of vmx/main.c to go under a single #ifdef, eliminating trampolines in the generated code, and almost all of the stubs. For example, with CONFIG_KVM_INTEL_TDX=n, before this cleanup, vt_refresh_apicv_exec_ctrl() would produce: 0000000000036490 <vt_refresh_apicv_exec_ctrl>: 36490: f3 0f 1e fa endbr64 36494: e8 00 00 00 00 call 36499 <vt_refresh_apicv_exec_ctrl+0x9> 36495: R_X86_64_PLT32 __fentry__-0x4 36499: e9 00 00 00 00 jmp 3649e <vt_refresh_apicv_exec_ctrl+0xe> 3649a: R_X86_64_PLT32 vmx_refresh_apicv_exec_ctrl-0x4 3649e: 66 90 xchg %ax,%ax After this patch, this is completely eliminated. Based on a patch by Sean Christopherson <seanjc@google.com> Link: https://lore.kernel.org/kvm/Z6v9yjWLNTU6X90d@google.com/ Cc: Sean Christopherson <seanjc@google.com> Cc: Rick Edgecombe <rick.p.edgecombe@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- arch/x86/kvm/vmx/tdx.h | 2 +- arch/x86/kvm/vmx/x86_ops.h | 67 +--------------- arch/x86/kvm/vmx/main.c | 192 +++++++++++++++++++++++---------------------- 3 files changed, 101 insertions(+), 160 deletions(-) diff --git a/arch/x86/kvm/vmx/tdx.h b/arch/x86/kvm/vmx/tdx.h index 8f8070d0f55e..b43d7a7c8f1c 100644 --- a/arch/x86/kvm/vmx/tdx.h +++ b/arch/x86/kvm/vmx/tdx.h @@ -5,7 +5,7 @@ #include "tdx_arch.h" #include "tdx_errno.h" -#ifdef CONFIG_INTEL_TDX_HOST +#ifdef CONFIG_KVM_INTEL_TDX #include "common.h" int tdx_bringup(void); diff --git a/arch/x86/kvm/vmx/x86_ops.h b/arch/x86/kvm/vmx/x86_ops.h index 97fcbcb0a503..e628318fc3fc 100644 --- a/arch/x86/kvm/vmx/x86_ops.h +++ b/arch/x86/kvm/vmx/x86_ops.h @@ -122,7 +122,7 @@ void vmx_cancel_hv_timer(struct kvm_vcpu *vcpu); #endif void vmx_setup_mce(struct kvm_vcpu *vcpu); -#ifdef CONFIG_INTEL_TDX_HOST +#ifdef CONFIG_KVM_INTEL_TDX void tdx_disable_virtualization_cpu(void); int tdx_vm_init(struct kvm *kvm); void tdx_mmu_release_hkid(struct kvm *kvm); @@ -165,71 +165,6 @@ void tdx_flush_tlb_current(struct kvm_vcpu *vcpu); void tdx_flush_tlb_all(struct kvm_vcpu *vcpu); void tdx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, int root_level); int tdx_gmem_private_max_mapping_level(struct kvm *kvm, kvm_pfn_t pfn); -#else -static inline void tdx_disable_virtualization_cpu(void) {} -static inline int tdx_vm_init(struct kvm *kvm) { return -EOPNOTSUPP; } -static inline void tdx_mmu_release_hkid(struct kvm *kvm) {} -static inline void tdx_vm_destroy(struct kvm *kvm) {} -static inline int tdx_vm_ioctl(struct kvm *kvm, void __user *argp) { return -EOPNOTSUPP; } - -static inline int tdx_vcpu_create(struct kvm_vcpu *vcpu) { return -EOPNOTSUPP; } -static inline void tdx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) {} -static inline void tdx_vcpu_free(struct kvm_vcpu *vcpu) {} -static inline void tdx_vcpu_load(struct kvm_vcpu *vcpu, int cpu) {} -static inline int tdx_vcpu_pre_run(struct kvm_vcpu *vcpu) { return -EOPNOTSUPP; } -static inline fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu, bool force_immediate_exit) -{ - return EXIT_FASTPATH_NONE; -} -static inline void tdx_prepare_switch_to_guest(struct kvm_vcpu *vcpu) {} -static inline void tdx_vcpu_put(struct kvm_vcpu *vcpu) {} -static inline bool tdx_protected_apic_has_interrupt(struct kvm_vcpu *vcpu) { return false; } -static inline int tdx_handle_exit(struct kvm_vcpu *vcpu, - enum exit_fastpath_completion fastpath) { return 0; } - -static inline void tdx_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode, - int trig_mode, int vector) {} -static inline void tdx_inject_nmi(struct kvm_vcpu *vcpu) {} -static inline void tdx_get_exit_info(struct kvm_vcpu *vcpu, u32 *reason, u64 *info1, - u64 *info2, u32 *intr_info, u32 *error_code) {} -static inline bool tdx_has_emulated_msr(u32 index) { return false; } -static inline int tdx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) { return 1; } -static inline int tdx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) { return 1; } - -static inline int tdx_vcpu_ioctl(struct kvm_vcpu *vcpu, void __user *argp) { return -EOPNOTSUPP; } - -static inline int tdx_sept_link_private_spt(struct kvm *kvm, gfn_t gfn, - enum pg_level level, - void *private_spt) -{ - return -EOPNOTSUPP; -} - -static inline int tdx_sept_free_private_spt(struct kvm *kvm, gfn_t gfn, - enum pg_level level, - void *private_spt) -{ - return -EOPNOTSUPP; -} - -static inline int tdx_sept_set_private_spte(struct kvm *kvm, gfn_t gfn, - enum pg_level level, - kvm_pfn_t pfn) -{ - return -EOPNOTSUPP; -} - -static inline int tdx_sept_remove_private_spte(struct kvm *kvm, gfn_t gfn, - enum pg_level level, - kvm_pfn_t pfn) -{ - return -EOPNOTSUPP; -} - -static inline void tdx_flush_tlb_current(struct kvm_vcpu *vcpu) {} -static inline void tdx_flush_tlb_all(struct kvm_vcpu *vcpu) {} -static inline void tdx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, int root_level) {} -static inline int tdx_gmem_private_max_mapping_level(struct kvm *kvm, kvm_pfn_t pfn) { return 0; } #endif #endif /* __KVM_X86_VMX_X86_OPS_H */ diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c index a1388adffa1e..218078ba039f 100644 --- a/arch/x86/kvm/vmx/main.c +++ b/arch/x86/kvm/vmx/main.c @@ -10,9 +10,8 @@ #include "tdx.h" #include "tdx_arch.h" -#ifdef CONFIG_INTEL_TDX_HOST +#ifdef CONFIG_KVM_INTEL_TDX static_assert(offsetof(struct vcpu_vmx, vt) == offsetof(struct vcpu_tdx, vt)); -#endif static void vt_disable_virtualization_cpu(void) { @@ -880,6 +879,13 @@ static int vt_gmem_private_max_mapping_level(struct kvm *kvm, kvm_pfn_t pfn) return 0; } +#define vt_op(name) vt_##name +#define vt_op_tdx_only(name) vt_##name +#else /* CONFIG_KVM_INTEL_TDX */ +#define vt_op(name) vmx_##name +#define vt_op_tdx_only(name) NULL +#endif /* CONFIG_KVM_INTEL_TDX */ + #define VMX_REQUIRED_APICV_INHIBITS \ (BIT(APICV_INHIBIT_REASON_DISABLED) | \ BIT(APICV_INHIBIT_REASON_ABSENT) | \ @@ -897,113 +903,113 @@ struct kvm_x86_ops vt_x86_ops __initdata = { .hardware_unsetup = vmx_hardware_unsetup, .enable_virtualization_cpu = vmx_enable_virtualization_cpu, - .disable_virtualization_cpu = vt_disable_virtualization_cpu, + .disable_virtualization_cpu = vt_op(disable_virtualization_cpu), .emergency_disable_virtualization_cpu = vmx_emergency_disable_virtualization_cpu, - .has_emulated_msr = vt_has_emulated_msr, + .has_emulated_msr = vt_op(has_emulated_msr), .vm_size = sizeof(struct kvm_vmx), - .vm_init = vt_vm_init, - .vm_pre_destroy = vt_vm_pre_destroy, - .vm_destroy = vt_vm_destroy, + .vm_init = vt_op(vm_init), + .vm_destroy = vt_op(vm_destroy), + .vm_pre_destroy = vt_op_tdx_only(vm_pre_destroy), - .vcpu_precreate = vt_vcpu_precreate, - .vcpu_create = vt_vcpu_create, - .vcpu_free = vt_vcpu_free, - .vcpu_reset = vt_vcpu_reset, + .vcpu_precreate = vt_op(vcpu_precreate), + .vcpu_create = vt_op(vcpu_create), + .vcpu_free = vt_op(vcpu_free), + .vcpu_reset = vt_op(vcpu_reset), - .prepare_switch_to_guest = vt_prepare_switch_to_guest, - .vcpu_load = vt_vcpu_load, - .vcpu_put = vt_vcpu_put, + .prepare_switch_to_guest = vt_op(prepare_switch_to_guest), + .vcpu_load = vt_op(vcpu_load), + .vcpu_put = vt_op(vcpu_put), - .update_exception_bitmap = vt_update_exception_bitmap, + .update_exception_bitmap = vt_op(update_exception_bitmap), .get_feature_msr = vmx_get_feature_msr, - .get_msr = vt_get_msr, - .set_msr = vt_set_msr, + .get_msr = vt_op(get_msr), + .set_msr = vt_op(set_msr), - .get_segment_base = vt_get_segment_base, - .get_segment = vt_get_segment, - .set_segment = vt_set_segment, - .get_cpl = vt_get_cpl, - .get_cpl_no_cache = vt_get_cpl_no_cache, - .get_cs_db_l_bits = vt_get_cs_db_l_bits, - .is_valid_cr0 = vt_is_valid_cr0, - .set_cr0 = vt_set_cr0, - .is_valid_cr4 = vt_is_valid_cr4, - .set_cr4 = vt_set_cr4, - .set_efer = vt_set_efer, - .get_idt = vt_get_idt, - .set_idt = vt_set_idt, - .get_gdt = vt_get_gdt, - .set_gdt = vt_set_gdt, - .set_dr6 = vt_set_dr6, - .set_dr7 = vt_set_dr7, - .sync_dirty_debug_regs = vt_sync_dirty_debug_regs, - .cache_reg = vt_cache_reg, - .get_rflags = vt_get_rflags, - .set_rflags = vt_set_rflags, - .get_if_flag = vt_get_if_flag, + .get_segment_base = vt_op(get_segment_base), + .get_segment = vt_op(get_segment), + .set_segment = vt_op(set_segment), + .get_cpl = vt_op(get_cpl), + .get_cpl_no_cache = vt_op(get_cpl_no_cache), + .get_cs_db_l_bits = vt_op(get_cs_db_l_bits), + .is_valid_cr0 = vt_op(is_valid_cr0), + .set_cr0 = vt_op(set_cr0), + .is_valid_cr4 = vt_op(is_valid_cr4), + .set_cr4 = vt_op(set_cr4), + .set_efer = vt_op(set_efer), + .get_idt = vt_op(get_idt), + .set_idt = vt_op(set_idt), + .get_gdt = vt_op(get_gdt), + .set_gdt = vt_op(set_gdt), + .set_dr6 = vt_op(set_dr6), + .set_dr7 = vt_op(set_dr7), + .sync_dirty_debug_regs = vt_op(sync_dirty_debug_regs), + .cache_reg = vt_op(cache_reg), + .get_rflags = vt_op(get_rflags), + .set_rflags = vt_op(set_rflags), + .get_if_flag = vt_op(get_if_flag), - .flush_tlb_all = vt_flush_tlb_all, - .flush_tlb_current = vt_flush_tlb_current, - .flush_tlb_gva = vt_flush_tlb_gva, - .flush_tlb_guest = vt_flush_tlb_guest, + .flush_tlb_all = vt_op(flush_tlb_all), + .flush_tlb_current = vt_op(flush_tlb_current), + .flush_tlb_gva = vt_op(flush_tlb_gva), + .flush_tlb_guest = vt_op(flush_tlb_guest), - .vcpu_pre_run = vt_vcpu_pre_run, - .vcpu_run = vt_vcpu_run, - .handle_exit = vt_handle_exit, + .vcpu_pre_run = vt_op(vcpu_pre_run), + .vcpu_run = vt_op(vcpu_run), + .handle_exit = vt_op(handle_exit), .skip_emulated_instruction = vmx_skip_emulated_instruction, .update_emulated_instruction = vmx_update_emulated_instruction, - .set_interrupt_shadow = vt_set_interrupt_shadow, - .get_interrupt_shadow = vt_get_interrupt_shadow, - .patch_hypercall = vt_patch_hypercall, - .inject_irq = vt_inject_irq, - .inject_nmi = vt_inject_nmi, - .inject_exception = vt_inject_exception, - .cancel_injection = vt_cancel_injection, - .interrupt_allowed = vt_interrupt_allowed, - .nmi_allowed = vt_nmi_allowed, - .get_nmi_mask = vt_get_nmi_mask, - .set_nmi_mask = vt_set_nmi_mask, - .enable_nmi_window = vt_enable_nmi_window, - .enable_irq_window = vt_enable_irq_window, - .update_cr8_intercept = vt_update_cr8_intercept, + .set_interrupt_shadow = vt_op(set_interrupt_shadow), + .get_interrupt_shadow = vt_op(get_interrupt_shadow), + .patch_hypercall = vt_op(patch_hypercall), + .inject_irq = vt_op(inject_irq), + .inject_nmi = vt_op(inject_nmi), + .inject_exception = vt_op(inject_exception), + .cancel_injection = vt_op(cancel_injection), + .interrupt_allowed = vt_op(interrupt_allowed), + .nmi_allowed = vt_op(nmi_allowed), + .get_nmi_mask = vt_op(get_nmi_mask), + .set_nmi_mask = vt_op(set_nmi_mask), + .enable_nmi_window = vt_op(enable_nmi_window), + .enable_irq_window = vt_op(enable_irq_window), + .update_cr8_intercept = vt_op(update_cr8_intercept), .x2apic_icr_is_split = false, - .set_virtual_apic_mode = vt_set_virtual_apic_mode, - .set_apic_access_page_addr = vt_set_apic_access_page_addr, - .refresh_apicv_exec_ctrl = vt_refresh_apicv_exec_ctrl, - .load_eoi_exitmap = vt_load_eoi_exitmap, + .set_virtual_apic_mode = vt_op(set_virtual_apic_mode), + .set_apic_access_page_addr = vt_op(set_apic_access_page_addr), + .refresh_apicv_exec_ctrl = vt_op(refresh_apicv_exec_ctrl), + .load_eoi_exitmap = vt_op(load_eoi_exitmap), .apicv_pre_state_restore = pi_apicv_pre_state_restore, .required_apicv_inhibits = VMX_REQUIRED_APICV_INHIBITS, - .hwapic_isr_update = vt_hwapic_isr_update, - .sync_pir_to_irr = vt_sync_pir_to_irr, - .deliver_interrupt = vt_deliver_interrupt, + .hwapic_isr_update = vt_op(hwapic_isr_update), + .sync_pir_to_irr = vt_op(sync_pir_to_irr), + .deliver_interrupt = vt_op(deliver_interrupt), .dy_apicv_has_pending_interrupt = pi_has_pending_interrupt, - .set_tss_addr = vt_set_tss_addr, - .set_identity_map_addr = vt_set_identity_map_addr, + .set_tss_addr = vt_op(set_tss_addr), + .set_identity_map_addr = vt_op(set_identity_map_addr), .get_mt_mask = vmx_get_mt_mask, - .get_exit_info = vt_get_exit_info, - .get_entry_info = vt_get_entry_info, + .get_exit_info = vt_op(get_exit_info), + .get_entry_info = vt_op(get_entry_info), - .vcpu_after_set_cpuid = vt_vcpu_after_set_cpuid, + .vcpu_after_set_cpuid = vt_op(vcpu_after_set_cpuid), .has_wbinvd_exit = cpu_has_vmx_wbinvd_exit, - .get_l2_tsc_offset = vt_get_l2_tsc_offset, - .get_l2_tsc_multiplier = vt_get_l2_tsc_multiplier, - .write_tsc_offset = vt_write_tsc_offset, - .write_tsc_multiplier = vt_write_tsc_multiplier, + .get_l2_tsc_offset = vt_op(get_l2_tsc_offset), + .get_l2_tsc_multiplier = vt_op(get_l2_tsc_multiplier), + .write_tsc_offset = vt_op(write_tsc_offset), + .write_tsc_multiplier = vt_op(write_tsc_multiplier), - .load_mmu_pgd = vt_load_mmu_pgd, + .load_mmu_pgd = vt_op(load_mmu_pgd), .check_intercept = vmx_check_intercept, .handle_exit_irqoff = vmx_handle_exit_irqoff, - .update_cpu_dirty_logging = vt_update_cpu_dirty_logging, + .update_cpu_dirty_logging = vt_op(update_cpu_dirty_logging), .nested_ops = &vmx_nested_ops, @@ -1011,38 +1017,38 @@ struct kvm_x86_ops vt_x86_ops __initdata = { .pi_start_assignment = vmx_pi_start_assignment, #ifdef CONFIG_X86_64 - .set_hv_timer = vt_set_hv_timer, - .cancel_hv_timer = vt_cancel_hv_timer, + .set_hv_timer = vt_op(set_hv_timer), + .cancel_hv_timer = vt_op(cancel_hv_timer), #endif - .setup_mce = vt_setup_mce, + .setup_mce = vt_op(setup_mce), #ifdef CONFIG_KVM_SMM - .smi_allowed = vt_smi_allowed, - .enter_smm = vt_enter_smm, - .leave_smm = vt_leave_smm, - .enable_smi_window = vt_enable_smi_window, + .smi_allowed = vt_op(smi_allowed), + .enter_smm = vt_op(enter_smm), + .leave_smm = vt_op(leave_smm), + .enable_smi_window = vt_op(enable_smi_window), #endif - .check_emulate_instruction = vt_check_emulate_instruction, - .apic_init_signal_blocked = vt_apic_init_signal_blocked, + .check_emulate_instruction = vt_op(check_emulate_instruction), + .apic_init_signal_blocked = vt_op(apic_init_signal_blocked), .migrate_timers = vmx_migrate_timers, - .msr_filter_changed = vt_msr_filter_changed, - .complete_emulated_msr = vt_complete_emulated_msr, + .msr_filter_changed = vt_op(msr_filter_changed), + .complete_emulated_msr = vt_op(complete_emulated_msr), .vcpu_deliver_sipi_vector = kvm_vcpu_deliver_sipi_vector, .get_untagged_addr = vmx_get_untagged_addr, - .mem_enc_ioctl = vt_mem_enc_ioctl, - .vcpu_mem_enc_ioctl = vt_vcpu_mem_enc_ioctl, + .mem_enc_ioctl = vt_op_tdx_only(mem_enc_ioctl), + .vcpu_mem_enc_ioctl = vt_op_tdx_only(vcpu_mem_enc_ioctl), - .private_max_mapping_level = vt_gmem_private_max_mapping_level + .private_max_mapping_level = vt_op_tdx_only(gmem_private_max_mapping_level) }; struct kvm_x86_init_ops vt_init_ops __initdata = { - .hardware_setup = vt_hardware_setup, + .hardware_setup = vt_op(hardware_setup), .handle_intel_pt_intr = NULL, .runtime_ops = &vt_x86_ops, -- 2.48.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx 2025-03-18 6:35 [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx Vishal Verma ` (3 preceding siblings ...) 2025-03-18 6:35 ` [PATCH v2 4/4] KVM: VMX: Clean up and macrofy x86_ops Vishal Verma @ 2025-05-02 21:51 ` Sean Christopherson 4 siblings, 0 replies; 10+ messages in thread From: Sean Christopherson @ 2025-05-02 21:51 UTC (permalink / raw) To: Sean Christopherson, Paolo Bonzini, Vishal Verma Cc: kvm, linux-kernel, Binbin Wu, Rick Edgecombe On Tue, 18 Mar 2025 00:35:05 -0600, Vishal Verma wrote: > This is a cleanup that should follow the initial TDX base support (i.e. > not an immediate fix needed for kvm-coco-queue). > > Patch 1 is a precursory fix for a build warning/error found when > manually testing the CONFIG_INTEL_TDX_HOST=n case. > > For Patches 2-4: > > [...] Applied 2-4 to kvm-x86 vmx, with Chao's feedback incorporated, along with a few other minor cleanups. [1/4] KVM: TDX: Fix definition of tdx_guest_nr_guest_keyids() (no commit info) [2/4] KVM: VMX: Move apicv_pre_state_restore to posted_intr.c https://github.com/kvm-x86/linux/commit/84ad4d834ce9 [3/4] KVM: VMX: Make naming consistent for kvm_complete_insn_gp via define https://github.com/kvm-x86/linux/commit/1a81d9d5a1da [4/4] KVM: VMX: Clean up and macrofy x86_ops https://github.com/kvm-x86/linux/commit/907092bf7cbd -- https://github.com/kvm-x86/linux/tree/next ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-05-02 21:53 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-18 6:35 [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx Vishal Verma 2025-03-18 6:35 ` [PATCH v2 1/4] KVM: TDX: Fix definition of tdx_guest_nr_guest_keyids() Vishal Verma 2025-03-18 15:33 ` Edgecombe, Rick P 2025-03-18 16:31 ` Verma, Vishal L 2025-03-18 6:35 ` [PATCH v2 2/4] KVM: VMX: Move apicv_pre_state_restore to posted_intr.c Vishal Verma 2025-04-29 8:17 ` Chao Gao 2025-04-29 17:25 ` Verma, Vishal L 2025-03-18 6:35 ` [PATCH v2 3/4] KVM: VMX: Make naming consistent for kvm_complete_insn_gp via define Vishal Verma 2025-03-18 6:35 ` [PATCH v2 4/4] KVM: VMX: Clean up and macrofy x86_ops Vishal Verma 2025-05-02 21:51 ` [PATCH v2 0/4] KVM: TDX: Cleanup the kvm_x86_ops structure for vmx/tdx Sean Christopherson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox