| Message ID | 20250612214849.3950094-3-sohil.mehta@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Add support for NMI-source reporting with FRED | expand |
On Thu, Jun 12, 2025, Sohil Mehta wrote: > Extend the FRED NMI entry point from KVM to take an extra argument to > allow KVM to invoke the FRED event dispatch framework with event data. > > This API is used to pass the NMI-source bitmap for NMI-induced VM exits. > Read the VMCS exit qualification field to get the NMI-source information > and store it as event data precisely in the format expected by the FRED > event framework. > > Read the VMCS exit qualification unconditionally since almost all > upcoming CPUs are expected to enable FRED and NMI-source together. In > the rare case that NMI-source isn't enabled, the extra VMREAD would be > harmless since the exit qualification is expected to be zero. Nit, instead of "is expected to be zero", something like this harmless since the exit qualification is architecturally guaranteed to be zero on CPUs that don't support NMI-source reporting. Per the SDM's "Exit qualification" subsection of "Basic VM-Exit Information": For all other VM exits, this field is cleared. -- to make it very explicit that reading the exit qualification on older CPUs is 100% safe, e.g. even on non-FRED CPUs (see https://lore.kernel.org/all/aBUiwLV4ZY2HdRbz@google.com). > Suggested-by: Sean Christopherson <seanjc@google.com> > Originally-by: Zeng Guang <guang.zeng@intel.com> > Signed-off-by: Sohil Mehta <sohil.mehta@intel.com> > --- Acked-by: Sean Christopherson <seanjc@google.com>
On 6/12/2025 5:18 PM, Sean Christopherson wrote: >> Read the VMCS exit qualification unconditionally since almost all >> upcoming CPUs are expected to enable FRED and NMI-source together. In >> the rare case that NMI-source isn't enabled, the extra VMREAD would be >> harmless since the exit qualification is expected to be zero. > > Nit, instead of "is expected to be zero", something like this > > harmless since the exit qualification is architecturally guaranteed to be > zero on CPUs that don't support NMI-source reporting. Per the SDM's > "Exit qualification" subsection of "Basic VM-Exit Information": > > For all other VM exits, this field is cleared. > -- > Looks good. Clarifying it explicitly reduces ambiguity. If this patchset gets applied directly, I am hoping the clarification can be included while applying. If we end up doing another version, I'll add it to the log. > to make it very explicit that reading the exit qualification on older CPUs is 100% > safe, e.g. even on non-FRED CPUs (see https://lore.kernel.org/all/aBUiwLV4ZY2HdRbz@google.com). > That's interesting. Thanks for the link. >> Suggested-by: Sean Christopherson <seanjc@google.com> >> Originally-by: Zeng Guang <guang.zeng@intel.com> >> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com> >> --- > > Acked-by: Sean Christopherson <seanjc@google.com> Thank you!
diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S index 29c5c32c16c3..1c9c6e036233 100644 --- a/arch/x86/entry/entry_64_fred.S +++ b/arch/x86/entry/entry_64_fred.S @@ -93,7 +93,7 @@ SYM_FUNC_START(asm_fred_entry_from_kvm) * +--------+-----------------+ */ push $0 /* Reserved, must be 0 */ - push $0 /* Event data, 0 for IRQ/NMI */ + push %rsi /* Event data for NMI */ push %rdi /* fred_ss handed in by the caller */ push %rbp pushf diff --git a/arch/x86/include/asm/fred.h b/arch/x86/include/asm/fred.h index 552332ca060c..bccf4a3c06b8 100644 --- a/arch/x86/include/asm/fred.h +++ b/arch/x86/include/asm/fred.h @@ -66,14 +66,14 @@ static __always_inline unsigned long fred_event_data(struct pt_regs *regs) void asm_fred_entrypoint_user(void); void asm_fred_entrypoint_kernel(void); -void asm_fred_entry_from_kvm(struct fred_ss); +void asm_fred_entry_from_kvm(struct fred_ss ss, unsigned long edata); __visible void fred_entry_from_user(struct pt_regs *regs); __visible void fred_entry_from_kernel(struct pt_regs *regs); __visible void __fred_entry_from_kvm(struct pt_regs *regs); /* Must be called from noinstr code, thus __always_inline */ -static __always_inline void fred_nmi_from_kvm(void) +static __always_inline void fred_nmi_from_kvm(unsigned long edata) { struct fred_ss ss = { .ss = __KERNEL_DS, @@ -83,7 +83,7 @@ static __always_inline void fred_nmi_from_kvm(void) .lm = 1, }; - asm_fred_entry_from_kvm(ss); + asm_fred_entry_from_kvm(ss, edata); } static inline void fred_irq_from_kvm(unsigned int vector) @@ -95,7 +95,8 @@ static inline void fred_irq_from_kvm(unsigned int vector) .lm = 1, }; - asm_fred_entry_from_kvm(ss); + /* Event data is always zero for IRQ */ + asm_fred_entry_from_kvm(ss, 0); } void cpu_init_fred_exceptions(void); @@ -123,7 +124,7 @@ static __always_inline unsigned long fred_event_data(struct pt_regs *regs) { ret static inline void cpu_init_fred_exceptions(void) { } static inline void cpu_init_fred_rsps(void) { } static inline void fred_complete_exception_setup(void) { } -static __always_inline void fred_nmi_from_kvm(void) { } +static __always_inline void fred_nmi_from_kvm(unsigned long edata) { } static inline void fred_irq_from_kvm(unsigned int vector) { } static inline void fred_sync_rsp0(unsigned long rsp0) { } static inline void fred_update_rsp0(void) { } diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index 8183886eda3d..69832ca31e5b 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -7264,7 +7264,7 @@ noinstr void vmx_handle_nmi(struct kvm_vcpu *vcpu) kvm_before_interrupt(vcpu, KVM_HANDLING_NMI); if (cpu_feature_enabled(X86_FEATURE_FRED)) - fred_nmi_from_kvm(); + fred_nmi_from_kvm(vmx_get_exit_qual(vcpu)); else vmx_do_nmi_irqoff(); kvm_after_interrupt(vcpu);
Extend the FRED NMI entry point from KVM to take an extra argument to allow KVM to invoke the FRED event dispatch framework with event data. This API is used to pass the NMI-source bitmap for NMI-induced VM exits. Read the VMCS exit qualification field to get the NMI-source information and store it as event data precisely in the format expected by the FRED event framework. Read the VMCS exit qualification unconditionally since almost all upcoming CPUs are expected to enable FRED and NMI-source together. In the rare case that NMI-source isn't enabled, the extra VMREAD would be harmless since the exit qualification is expected to be zero. Suggested-by: Sean Christopherson <seanjc@google.com> Originally-by: Zeng Guang <guang.zeng@intel.com> Signed-off-by: Sohil Mehta <sohil.mehta@intel.com> --- v7: Pass the event data from KVM only for NMI. (Sean) v6: No change v5: Read the VMCS exit qualification unconditionally. (Sean) Combine related patches into one. --- arch/x86/entry/entry_64_fred.S | 2 +- arch/x86/include/asm/fred.h | 11 ++++++----- arch/x86/kvm/vmx/vmx.c | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-)