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

["Teddy Astie" <teddy.astie@xxxxxxxxxx>]

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

>>   it's neither a
>> part of the "VMCB Clear Field" layout part nor the the "explicitely not
>> cached" list.
>>
>> So I assume that it may be covered by the TPR clean-bit (which is the
>> same part of vmcb); as it's actually unclear if hardware wants it or not.
>
> This is very different to your original feedback though.
>
> It's fine for review feedback to be of the form "have you investigated
> the clean bits?", but if you're not sure, you must not phrase your
> feedback as an instruction to check the cleanbit.
>

Sorry, I may have been a bit too fast on that.

> ~Andrew
>

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech