From: Zeng Guang <guang.zeng@intel.com>
If the "NMI exiting" VM-execution control is 1, the value of the 16-bit NMI
source vector is saved in the exit-qualification field in the VMCS when VM
exits occur on CPUs that support NMI source.
KVM that is aware of NMI-source reporting will push the bitmask of NMI source
vectors as the exceptoin event data field on the stack for then entry of FRED
exception. Subsequently, the host NMI exception handler is invoked which
will process NMI source information in the event data. This operation is
independent of vCPU FRED enabling status.
Signed-off-by: Zeng Guang <guang.zeng@intel.com>
Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
---
arch/x86/entry/entry_64_fred.S | 2 +-
arch/x86/kvm/vmx/vmx.c | 11 ++++++++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
index a02bc6f3d2e6..0d934a3fcaf8 100644
--- a/arch/x86/entry/entry_64_fred.S
+++ b/arch/x86/entry/entry_64_fred.S
@@ -92,7 +92,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 IRQ/NMI */
push %rdi /* fred_ss handed in by the caller */
push %rbp
pushf
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 4e7b36081b76..6719c598fa5f 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7331,10 +7331,15 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
if ((u16)vmx->exit_reason.basic == EXIT_REASON_EXCEPTION_NMI &&
is_nmi(vmx_get_intr_info(vcpu))) {
kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
- if (cpu_feature_enabled(X86_FEATURE_FRED))
- fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR, 0);
- else
+ if (cpu_feature_enabled(X86_FEATURE_FRED)) {
+ unsigned long edata = 0;
+
+ if (cpu_feature_enabled(X86_FEATURE_NMI_SOURCE))
+ edata = vmx_get_exit_qual(vcpu);
+ fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR, edata);
+ } else {
vmx_do_nmi_irqoff();
+ }
kvm_after_interrupt(vcpu);
}
--
2.25.1
On 6/29/2024 4:18 AM, Jacob Pan wrote:
> From: Zeng Guang <guang.zeng@intel.com>
>
> If the "NMI exiting" VM-execution control is 1, the value of the 16-bit NMI
> source vector is saved in the exit-qualification field in the VMCS when VM
> exits occur on CPUs that support NMI source.
>
> KVM that is aware of NMI-source reporting will push the bitmask of NMI source
> vectors as the exceptoin event data field on the stack for then entry of FRED
> exception. Subsequently, the host NMI exception handler is invoked which
> will process NMI source information in the event data. This operation is
> independent of vCPU FRED enabling status.
>
> Signed-off-by: Zeng Guang <guang.zeng@intel.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
> arch/x86/entry/entry_64_fred.S | 2 +-
> arch/x86/kvm/vmx/vmx.c | 11 ++++++++---
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
> index a02bc6f3d2e6..0d934a3fcaf8 100644
> --- a/arch/x86/entry/entry_64_fred.S
> +++ b/arch/x86/entry/entry_64_fred.S
> @@ -92,7 +92,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 IRQ/NMI */
> push %rdi /* fred_ss handed in by the caller */
> push %rbp
> pushf
Move this part to previous patch as it changes the common FRED api and
prepares for nmi handling in case of nmi
source enabled in this patch.
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 4e7b36081b76..6719c598fa5f 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7331,10 +7331,15 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
> if ((u16)vmx->exit_reason.basic == EXIT_REASON_EXCEPTION_NMI &&
> is_nmi(vmx_get_intr_info(vcpu))) {
> kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
> - if (cpu_feature_enabled(X86_FEATURE_FRED))
> - fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR, 0);
> - else
> + if (cpu_feature_enabled(X86_FEATURE_FRED)) {
> + unsigned long edata = 0;
> +
> + if (cpu_feature_enabled(X86_FEATURE_NMI_SOURCE))
> + edata = vmx_get_exit_qual(vcpu);
> + fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR, edata);
> + } else {
> vmx_do_nmi_irqoff();
> + }
> kvm_after_interrupt(vcpu);
> }
>
On Sun, 30 Jun 2024 21:04:18 +0800, Zeng Guang <guang.zeng@intel.com> wrote: > On 6/29/2024 4:18 AM, Jacob Pan wrote: > > From: Zeng Guang <guang.zeng@intel.com> > > > > If the "NMI exiting" VM-execution control is 1, the value of the 16-bit > > NMI source vector is saved in the exit-qualification field in the VMCS > > when VM exits occur on CPUs that support NMI source. > > > > KVM that is aware of NMI-source reporting will push the bitmask of NMI > > source vectors as the exceptoin event data field on the stack for then > > entry of FRED exception. Subsequently, the host NMI exception handler > > is invoked which will process NMI source information in the event data. > > This operation is independent of vCPU FRED enabling status. > > > > Signed-off-by: Zeng Guang <guang.zeng@intel.com> > > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com> > > --- > > arch/x86/entry/entry_64_fred.S | 2 +- > > arch/x86/kvm/vmx/vmx.c | 11 ++++++++--- > > 2 files changed, 9 insertions(+), 4 deletions(-) > > > > diff --git a/arch/x86/entry/entry_64_fred.S > > b/arch/x86/entry/entry_64_fred.S index a02bc6f3d2e6..0d934a3fcaf8 100644 > > --- a/arch/x86/entry/entry_64_fred.S > > +++ b/arch/x86/entry/entry_64_fred.S > > @@ -92,7 +92,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 IRQ/NMI */ > > push %rdi /* fred_ss handed in by the > > caller */ push %rbp > > pushf > Move this part to previous patch as it changes the common FRED api and > prepares for nmi handling in case of nmi > source enabled in this patch. You are right, will do. > [...] Thanks, Jacob
On 6/28/2024 1:18 PM, Jacob Pan wrote:
> From: Zeng Guang <guang.zeng@intel.com>
>
> If the "NMI exiting" VM-execution control is 1, the value of the 16-bit NMI
> source vector is saved in the exit-qualification field in the VMCS when VM
> exits occur on CPUs that support NMI source.
>
> KVM that is aware of NMI-source reporting will push the bitmask of NMI source
> vectors as the exceptoin event data field on the stack for then entry of FRED
> exception. Subsequently, the host NMI exception handler is invoked which
> will process NMI source information in the event data. This operation is
> independent of vCPU FRED enabling status.
>
> Signed-off-by: Zeng Guang <guang.zeng@intel.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> ---
> arch/x86/entry/entry_64_fred.S | 2 +-
> arch/x86/kvm/vmx/vmx.c | 11 ++++++++---
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64_fred.S b/arch/x86/entry/entry_64_fred.S
> index a02bc6f3d2e6..0d934a3fcaf8 100644
> --- a/arch/x86/entry/entry_64_fred.S
> +++ b/arch/x86/entry/entry_64_fred.S
> @@ -92,7 +92,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 IRQ/NMI */
> push %rdi /* fred_ss handed in by the caller */
> push %rbp
> pushf
This belongs to the previous patch, it is part of the host changes.
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 4e7b36081b76..6719c598fa5f 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7331,10 +7331,15 @@ static noinstr void vmx_vcpu_enter_exit(struct kvm_vcpu *vcpu,
> if ((u16)vmx->exit_reason.basic == EXIT_REASON_EXCEPTION_NMI &&
> is_nmi(vmx_get_intr_info(vcpu))) {
> kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
> - if (cpu_feature_enabled(X86_FEATURE_FRED))
> - fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR, 0);
> - else
> + if (cpu_feature_enabled(X86_FEATURE_FRED)) {
> + unsigned long edata = 0;
> +
> + if (cpu_feature_enabled(X86_FEATURE_NMI_SOURCE))
> + edata = vmx_get_exit_qual(vcpu);
> + fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR, edata);
Simply do fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR,
vmx_get_exit_qual(vcpu))?
> + } else {
> vmx_do_nmi_irqoff();
> + }
> kvm_after_interrupt(vcpu);
> }
>
On Fri, 28 Jun 2024 21:07:04 -0700, Xin Li <xin@zytor.com> wrote:
> On 6/28/2024 1:18 PM, Jacob Pan wrote:
> > From: Zeng Guang <guang.zeng@intel.com>
> >
> > If the "NMI exiting" VM-execution control is 1, the value of the 16-bit
> > NMI source vector is saved in the exit-qualification field in the VMCS
> > when VM exits occur on CPUs that support NMI source.
> >
> > KVM that is aware of NMI-source reporting will push the bitmask of NMI
> > source vectors as the exceptoin event data field on the stack for then
> > entry of FRED exception. Subsequently, the host NMI exception handler
> > is invoked which will process NMI source information in the event data.
> > This operation is independent of vCPU FRED enabling status.
> >
> > Signed-off-by: Zeng Guang <guang.zeng@intel.com>
> > Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > ---
> > arch/x86/entry/entry_64_fred.S | 2 +-
> > arch/x86/kvm/vmx/vmx.c | 11 ++++++++---
> > 2 files changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/x86/entry/entry_64_fred.S
> > b/arch/x86/entry/entry_64_fred.S index a02bc6f3d2e6..0d934a3fcaf8 100644
> > --- a/arch/x86/entry/entry_64_fred.S
> > +++ b/arch/x86/entry/entry_64_fred.S
> > @@ -92,7 +92,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 IRQ/NMI */
> > push %rdi /* fred_ss handed in by the
> > caller */ push %rbp
> > pushf
>
> This belongs to the previous patch, it is part of the host changes.
right, will do.
>
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 4e7b36081b76..6719c598fa5f 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -7331,10 +7331,15 @@ static noinstr void vmx_vcpu_enter_exit(struct
> > kvm_vcpu *vcpu, if ((u16)vmx->exit_reason.basic ==
> > EXIT_REASON_EXCEPTION_NMI && is_nmi(vmx_get_intr_info(vcpu))) {
> > kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
> > - if (cpu_feature_enabled(X86_FEATURE_FRED))
> > - fred_entry_from_kvm(EVENT_TYPE_NMI,
> > NMI_VECTOR, 0);
> > - else
> > + if (cpu_feature_enabled(X86_FEATURE_FRED)) {
> > + unsigned long edata = 0;
> > +
> > + if
> > (cpu_feature_enabled(X86_FEATURE_NMI_SOURCE))
> > + edata = vmx_get_exit_qual(vcpu);
> > + fred_entry_from_kvm(EVENT_TYPE_NMI,
> > NMI_VECTOR, edata);
>
> Simply do fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR,
> vmx_get_exit_qual(vcpu))?
I don't have strong preference but having a local variable improves
readability IMHO.
> > + } else {
> > vmx_do_nmi_irqoff();
> > + }
> > kvm_after_interrupt(vcpu);
> > }
> >
>
Thanks,
Jacob
On 7/1/2024 8:45 AM, Jacob Pan wrote:
>
> On Fri, 28 Jun 2024 21:07:04 -0700, Xin Li <xin@zytor.com> wrote:
>
>> On 6/28/2024 1:18 PM, Jacob Pan wrote:
>>> From: Zeng Guang <guang.zeng@intel.com>
>>>
>>> If the "NMI exiting" VM-execution control is 1, the value of the 16-bit
>>> NMI source vector is saved in the exit-qualification field in the VMCS
>>> when VM exits occur on CPUs that support NMI source.
>>>
>>> KVM that is aware of NMI-source reporting will push the bitmask of NMI
>>> source vectors as the exceptoin event data field on the stack for then
>>> entry of FRED exception. Subsequently, the host NMI exception handler
>>> is invoked which will process NMI source information in the event data.
>>> This operation is independent of vCPU FRED enabling status.
>>>
>>> Signed-off-by: Zeng Guang <guang.zeng@intel.com>
>>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
>>> ---
>>> arch/x86/entry/entry_64_fred.S | 2 +-
>>> arch/x86/kvm/vmx/vmx.c | 11 ++++++++---
>>> 2 files changed, 9 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/x86/entry/entry_64_fred.S
>>> b/arch/x86/entry/entry_64_fred.S index a02bc6f3d2e6..0d934a3fcaf8 100644
>>> --- a/arch/x86/entry/entry_64_fred.S
>>> +++ b/arch/x86/entry/entry_64_fred.S
>>> @@ -92,7 +92,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 IRQ/NMI */
>>> push %rdi /* fred_ss handed in by the
>>> caller */ push %rbp
>>> pushf
>>
>> This belongs to the previous patch, it is part of the host changes.
> right, will do.
>
>>
>>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>>> index 4e7b36081b76..6719c598fa5f 100644
>>> --- a/arch/x86/kvm/vmx/vmx.c
>>> +++ b/arch/x86/kvm/vmx/vmx.c
>>> @@ -7331,10 +7331,15 @@ static noinstr void vmx_vcpu_enter_exit(struct
>>> kvm_vcpu *vcpu, if ((u16)vmx->exit_reason.basic ==
>>> EXIT_REASON_EXCEPTION_NMI && is_nmi(vmx_get_intr_info(vcpu))) {
>>> kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
>>> - if (cpu_feature_enabled(X86_FEATURE_FRED))
>>> - fred_entry_from_kvm(EVENT_TYPE_NMI,
>>> NMI_VECTOR, 0);
>>> - else
>>> + if (cpu_feature_enabled(X86_FEATURE_FRED)) {
>>> + unsigned long edata = 0;
>>> +
>>> + if
>>> (cpu_feature_enabled(X86_FEATURE_NMI_SOURCE))
>>> + edata = vmx_get_exit_qual(vcpu);
>>> + fred_entry_from_kvm(EVENT_TYPE_NMI,
>>> NMI_VECTOR, edata);
>>
>> Simply do fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR,
>> vmx_get_exit_qual(vcpu))?
> I don't have strong preference but having a local variable improves
> readability IMHO.
My point was, do we actually need this check:
(cpu_feature_enabled(X86_FEATURE_NMI_SOURCE)?
I don't have a strong preference either.
>
>>> + } else {
>>> vmx_do_nmi_irqoff();
>>> + }
>>> kvm_after_interrupt(vcpu);
>>> }
>>>
>>
>
>
> Thanks,
>
> Jacob
>
On Mon, 1 Jul 2024 10:03:58 -0700, Xin Li <xin@zytor.com> wrote:
> On 7/1/2024 8:45 AM, Jacob Pan wrote:
> >
> > On Fri, 28 Jun 2024 21:07:04 -0700, Xin Li <xin@zytor.com> wrote:
> >
> >> On 6/28/2024 1:18 PM, Jacob Pan wrote:
> >>> From: Zeng Guang <guang.zeng@intel.com>
> >>>
> >>> If the "NMI exiting" VM-execution control is 1, the value of the
> >>> 16-bit NMI source vector is saved in the exit-qualification field in
> >>> the VMCS when VM exits occur on CPUs that support NMI source.
> >>>
> >>> KVM that is aware of NMI-source reporting will push the bitmask of NMI
> >>> source vectors as the exceptoin event data field on the stack for then
> >>> entry of FRED exception. Subsequently, the host NMI exception handler
> >>> is invoked which will process NMI source information in the event
> >>> data. This operation is independent of vCPU FRED enabling status.
> >>>
> >>> Signed-off-by: Zeng Guang <guang.zeng@intel.com>
> >>> Signed-off-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> >>> ---
> >>> arch/x86/entry/entry_64_fred.S | 2 +-
> >>> arch/x86/kvm/vmx/vmx.c | 11 ++++++++---
> >>> 2 files changed, 9 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/x86/entry/entry_64_fred.S
> >>> b/arch/x86/entry/entry_64_fred.S index a02bc6f3d2e6..0d934a3fcaf8
> >>> 100644 --- a/arch/x86/entry/entry_64_fred.S
> >>> +++ b/arch/x86/entry/entry_64_fred.S
> >>> @@ -92,7 +92,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 IRQ/NMI */
> >>> push %rdi /* fred_ss handed in by
> >>> the caller */ push %rbp
> >>> pushf
> >>
> >> This belongs to the previous patch, it is part of the host changes.
> > right, will do.
> >
> >>
> >>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> >>> index 4e7b36081b76..6719c598fa5f 100644
> >>> --- a/arch/x86/kvm/vmx/vmx.c
> >>> +++ b/arch/x86/kvm/vmx/vmx.c
> >>> @@ -7331,10 +7331,15 @@ static noinstr void vmx_vcpu_enter_exit(struct
> >>> kvm_vcpu *vcpu, if ((u16)vmx->exit_reason.basic ==
> >>> EXIT_REASON_EXCEPTION_NMI && is_nmi(vmx_get_intr_info(vcpu))) {
> >>> kvm_before_interrupt(vcpu, KVM_HANDLING_NMI);
> >>> - if (cpu_feature_enabled(X86_FEATURE_FRED))
> >>> - fred_entry_from_kvm(EVENT_TYPE_NMI,
> >>> NMI_VECTOR, 0);
> >>> - else
> >>> + if (cpu_feature_enabled(X86_FEATURE_FRED)) {
> >>> + unsigned long edata = 0;
> >>> +
> >>> + if
> >>> (cpu_feature_enabled(X86_FEATURE_NMI_SOURCE))
> >>> + edata = vmx_get_exit_qual(vcpu);
> >>> + fred_entry_from_kvm(EVENT_TYPE_NMI,
> >>> NMI_VECTOR, edata);
> >>
> >> Simply do fred_entry_from_kvm(EVENT_TYPE_NMI, NMI_VECTOR,
> >> vmx_get_exit_qual(vcpu))?
> > I don't have strong preference but having a local variable improves
> > readability IMHO.
>
> My point was, do we actually need this check:
> (cpu_feature_enabled(X86_FEATURE_NMI_SOURCE)?
I also pondered about the value of this CPUID bit if it is consistently
linked with the FRED bit. But since the architecture provided this
additional enumeration, as noted in Chapter 9 of the FRED specification,
which states:
"Processors that support FRED *may* also support a related feature called
NMI-source reporting".
The use of "may" suggests that there are scenarios where FRED might exist
without NMI-source reporting. To ensure future compatibility, I believe
it is still valid to maintain this check.
Thanks,
Jacob
© 2016 - 2025 Red Hat, Inc.