diff mbox series

[v2,08/17] KVM: TDX: Complete interrupts after TD exit

Message ID 20250211025828.3072076-9-binbin.wu@linux.intel.com (mailing list archive)
State New
Headers show
Series KVM: TDX: TDX interrupts | expand

Commit Message

Binbin Wu Feb. 11, 2025, 2:58 a.m. UTC 
From: Isaku Yamahata <isaku.yamahata@intel.com>

Complete NMI injection by updating the status of NMI injection for TDX.

Because TDX virtualizes vAPIC, and non-NMI interrupts are delivered
using posted-interrupt mechanism, KVM only needs to care about NMI
injection.

For VMX, KVM injects an NMI by setting VM_ENTRY_INTR_INFO_FIELD via
vector-event injection mechanism.  For TDX, KVM needs to request TDX
module to inject an NMI into a guest TD vCPU when the vCPU is not
active by setting PEND_NMI field within the TDX vCPU scope metadata
(Trust Domain Virtual Processor State (TDVPS)).  TDX module will attempt
to inject an NMI as soon as possible on TD entry.  KVM can read PEND_NMI
to get the status of NMI injection.  A value of 0 indicates the NMI has
been injected into the guest TD vCPU.

Update KVM's NMI status on TD exit by checking whether a requested NMI
has been injected into the TD.  Reading the PEND_NMI field via SEAMCALL
is expensive so only perform the check if an NMI was requested to inject.
If the read back value is 0, the NMI has been injected, update the NMI
status.  If the read back value is 1, no action needed since the PEND_NMI
is still set.

Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
Signed-off-by: Binbin Wu <binbin.wu@linux.intel.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
TDX interrupts v2:
- No change.

TDX interrupts v1:
- Shortlog "tdexit" -> "TD exit" (Reinette)
- Update change log as following suggested by Reinette with a little
  supplement.
  https://lore.kernel.org/lkml/fe9cec78-36ee-4a20-81df-ec837a45f69f@linux.intel.com/
- Fix comment, "nmi" -> "NMI" and add a missing period. (Reinette)
- Add a comment to explain why no need to request KVM_REQ_EVENT.

v19:
- move tdvps_management_check() to this patch
- typo: complete -> Complete in short log
---
 arch/x86/kvm/vmx/tdx.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Chao Gao Feb. 13, 2025, 8:20 a.m. UTC | #1 
>+static void tdx_complete_interrupts(struct kvm_vcpu *vcpu)
>+{
>+	/* Avoid costly SEAMCALL if no NMI was injected. */
>+	if (vcpu->arch.nmi_injected) {
>+		/*
>+		 * No need to request KVM_REQ_EVENT because PEND_NMI is still
>+		 * set if NMI re-injection needed.  No other event types need
>+		 * to be handled because TDX doesn't support injection of
>+		 * exception, SMI or interrupt (via event injection).
>+		 */
>+		vcpu->arch.nmi_injected = td_management_read8(to_tdx(vcpu),
>+							      TD_VCPU_PEND_NMI);
>+	}

Why does KVM care whether/when an NMI is injected by the TDX module?

I think we can simply set nmi_injected to false unconditionally here, or even in
tdx_inject_nmi(). From KVM's perspective, NMI injection is complete right after
writing to PEND_NMI. It is the TDX module that should inject the NMI at the
right time and do the re-injection.


>+}
>+
Binbin Wu Feb. 13, 2025, 8:55 a.m. UTC | #2 
On 2/13/2025 4:20 PM, Chao Gao wrote:
>> +static void tdx_complete_interrupts(struct kvm_vcpu *vcpu)
>> +{
>> +	/* Avoid costly SEAMCALL if no NMI was injected. */
>> +	if (vcpu->arch.nmi_injected) {
>> +		/*
>> +		 * No need to request KVM_REQ_EVENT because PEND_NMI is still
>> +		 * set if NMI re-injection needed.  No other event types need
>> +		 * to be handled because TDX doesn't support injection of
>> +		 * exception, SMI or interrupt (via event injection).
>> +		 */
>> +		vcpu->arch.nmi_injected = td_management_read8(to_tdx(vcpu),
>> +							      TD_VCPU_PEND_NMI);
>> +	}
> Why does KVM care whether/when an NMI is injected by the TDX module?
>
> I think we can simply set nmi_injected to false unconditionally here, or even in
> tdx_inject_nmi(). From KVM's perspective, NMI injection is complete right after
> writing to PEND_NMI. It is the TDX module that should inject the NMI at the
> right time and do the re-injection.
Yes, it can/should be cleared unconditionally here.

Previously (v19 and before), nmi_injected will impact the limit of pending nmi.
Now, we don't care the limit of pending nmi because more pending NMIs will be
collapsed to the one pending in the TDX module.

Will update it.
Thanks!

>
>
>> +}
>> +
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
index ba9038ac5bf7..9737574b8049 100644
--- a/arch/x86/kvm/vmx/tdx.c
+++ b/arch/x86/kvm/vmx/tdx.c
@@ -803,6 +803,21 @@  int tdx_vcpu_pre_run(struct kvm_vcpu *vcpu)
 	return 1;
 }
 
+static void tdx_complete_interrupts(struct kvm_vcpu *vcpu)
+{
+	/* Avoid costly SEAMCALL if no NMI was injected. */
+	if (vcpu->arch.nmi_injected) {
+		/*
+		 * No need to request KVM_REQ_EVENT because PEND_NMI is still
+		 * set if NMI re-injection needed.  No other event types need
+		 * to be handled because TDX doesn't support injection of
+		 * exception, SMI or interrupt (via event injection).
+		 */
+		vcpu->arch.nmi_injected = td_management_read8(to_tdx(vcpu),
+							      TD_VCPU_PEND_NMI);
+	}
+}
+
 struct tdx_uret_msr {
 	u32 msr;
 	unsigned int slot;
@@ -985,6 +1000,8 @@  fastpath_t tdx_vcpu_run(struct kvm_vcpu *vcpu, bool force_immediate_exit)
 	if (unlikely(tdx_failed_vmentry(vcpu)))
 		return EXIT_FASTPATH_NONE;
 
+	tdx_complete_interrupts(vcpu);
+
 	return tdx_exit_handlers_fastpath(vcpu);
 }