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 >>>>> <[email protected]> >>>>> --- >>>>> 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
