Re: [PATCH] nestedsvm: Clear GIF when injecting VMEXIT

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 11/05/2026 3:35 pm, Teddy Astie wrote:
> Le 07/05/2026 à 15:03, Ross Lagerwall a écrit :
>> On 4/29/26 5:54 PM, Teddy Astie wrote:
>>> Le 23/04/2026 à 18:13, Ross Lagerwall a écrit :
>>>> If L1 executes VMRUN with the GIF set and it fails consistency checks,
>>>> Xen will inject a VMEXIT and fail the assert checking the GIF is
>>>> cleared.
>>>>
>>>> Instead, clear the GIF when injecting a VMEXIT to match what hardware
>>>> does.
>>>>
>>>> Fixes: 9a779e4fc161 ("Implement SVM specific part for Nested
>>>> Virtualization")
>>>> Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
>>>> ---
>>>>    xen/arch/x86/hvm/svm/nestedsvm.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c
>>>> b/xen/arch/x86/hvm/svm/ nestedsvm.c
>>>> index ef6fa5d23b67..f89b087a1155 100644
>>>> --- a/xen/arch/x86/hvm/svm/nestedsvm.c
>>>> +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
>>>> @@ -733,9 +733,9 @@ nsvm_vcpu_vmexit_inject(struct vcpu *v, struct
>>>> cpu_user_regs *regs,
>>>>        struct vmcb_struct *vmcb = v->arch.hvm.svm.vmcb;
>>>>        if ( vmcb->_vintr.fields.vgif_enable )
>>>> -        ASSERT(vmcb->_vintr.fields.vgif == 0);
>>>> +        vmcb->_vintr.fields.vgif = 0;
>>>>        else
>>>> -        ASSERT(svm->ns_gif == 0);
>>>> +        nestedsvm_vcpu_clgi(v);
>>>>        ns_vmcb = nv->nv_vvmcx;
>>>
>>> Looks good to me, though I think we are here looking to make a "guest
>>> CLGI" (clear GIF), so the vGIF specific logic should be collapsed into
>>> nestedsvm_vcpu_clgi() instead of having it as the non-vgif-support
>>> case.
>>> (as IIUC, vGIF is a hardware accelration for nested GIF handling ?)
>>
>> Not 100% sure I follow your point here but v2 of this series removes
>> nestedsvm_vcpu_clgi() entirely. Does that address your concern?
>>
>>>
>>> (also making me notice that svm_vmexit_do_{stgi,clgi}() seems to lack
>>> vGIF specific logic)
>>>
>>
>> Isn't that by design? If vGIF is enabled the hardware should handle
>> STGI/CLGI without a VMEXIT.
>>
>
> I think at some point in the future, we may need a
> `nestedsvm_vcpu_{stgi,clgi}()` function, or something in that regard;
> e.g for emulating this instruction outside of a SVM intercept path
> (x86 emulator bits ?). As I think there are cases where the
> instruction is not intercepted but still needs to be emulated.
>
> Such function would need to know whether or not we use vGIF.

Yes we will (eventually) want emulation of STGI/CLGI.  It will
specifically not reintroduce nestedsvm_vcpu_{stgi,clgi}().

With introspection, we do get arbitrary instructions running through the
emulator.  This is why we take care to ensure that the emulator can cope
before a feature gets enabled generally.

For normal cases, we either have vGIF active and everything is handled
without VMExit, or we don't have vGIF active and everything hits the
intercept; this is not a split based on hardware support - it includes
both L0 and L1's configuration.

In fact, vGIF is a feature where on older hardware it's actually faster
for L0 to emulate it for L1, because a virtual VMExit/Entry and running
the L1 vmexit handler is much slower than L0 just emulating it and
re-entering L2 as if nothing had happened.  But that's an optimisation
for a later day.

~Andrew