Re: [PATCH v3 2/2] x86/svm: Use the virtual NMI when available

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/02/2026 1:10 pm, Teddy Astie wrote:
> Le 16/02/2026 à 12:47, Andrew Cooper a écrit :
>> On 16/02/2026 10:46 am, Teddy Astie wrote:
>>> Le 16/02/2026 à 11:16, Andrew Cooper a écrit :
>>>> On 16/02/2026 10:07 am, Teddy Astie wrote:
>>>>> Le 15/02/2026 à 19:24, Abdelkareem Abdelsaamad a écrit :
>>>>>> With the Virtual NMI (vNMI), the pending NMI is simply stuffed into the 
>>>>>> VMCB
>>>>>> and handed off to the hardware. There is no need for the artificial 
>>>>>> tracking
>>>>>> of the NMI handling completion with the IRET instruction interception.
>>>>>>
>>>>>> Adjust the svm_inject_nmi to rather inject the NMIs using the vNMI 
>>>>>> Hardware
>>>>>> accelerated feature when the AMD platform support the vNMI.
>>>>>>
>>>>>> Adjust the svm_get_interrupt_shadow to check if the vNMI is currently 
>>>>>> blocked
>>>>>> by servicing another in-progress NMI.
>>>>>>
>>>>>> Signed-off-by: Abdelkareem Abdelsaamad 
>>>>>> <abdelkareem.abdelsaamad@xxxxxxxxxx>
>>>>>> ---
>>>>>>     xen/arch/x86/hvm/svm/intr.c | 9 +++++++++
>>>>>>     xen/arch/x86/hvm/svm/svm.c  | 5 ++++-
>>>>>>     xen/arch/x86/hvm/svm/vmcb.c | 2 ++
>>>>>>     3 files changed, 15 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/xen/arch/x86/hvm/svm/intr.c b/xen/arch/x86/hvm/svm/intr.c
>>>>>> index 6453a46b85..3e8959f155 100644
>>>>>> --- a/xen/arch/x86/hvm/svm/intr.c
>>>>>> +++ b/xen/arch/x86/hvm/svm/intr.c
>>>>>> @@ -33,6 +33,15 @@ static void svm_inject_nmi(struct vcpu *v)
>>>>>>         u32 general1_intercepts = vmcb_get_general1_intercepts(vmcb);
>>>>>>         intinfo_t event;
>>>>>>     
>>>>>> +    if ( vmcb->_vintr.fields.vnmi_enable )
>>>>>> +    {
>>>>>> +       if ( !vmcb->_vintr.fields.vnmi_pending &&
>>>>>> +            !vmcb->_vintr.fields.vnmi_blocking )
>>>>>> +           vmcb->_vintr.fields.vnmi_pending = 1;
>>>>>> +
>>>>>> +        return;
>>>>>> +    }
>>>>>> +
>>>>> I think you need to update the clearbit for tpr (related to vintr) for
>>>>> the hardware to know that you modified the vnmi_pending bit.
>>>> What makes you think this?  The APM states otherwise.
>>>>
>>> The APM state doesn't appears to state regarding this;
>> The APM does state what is part of the TPR cleanbit, and vNMI is not
>> amongst these.
>>
> APM doesn't explicitly state that; though KVM assumes that it is and 
> sets the cleanbits [1]. I think we want to have some clarifications from 
> AMD on what's actually required here.
>
> [1] 
> https://github.com/torvalds/linux/blob/0f2acd3148e0ef42bdacbd477f90e8533f96b2ac/arch/x86/kvm/svm/svm.c#L3707-L3708

I can see at least 2 bugs in that function.  AMD tentatively say that
vNMI is not subject to a cleanbit, but I'm waiting on some other
clarifications.

~Andrew