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@citrix.com>
---
 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;
 
-- 
2.53.0

On 23/04/2026 5:11 pm, Ross Lagerwall wrote:
> 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@citrix.com>
> ---
>  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;
>  

I agree this is a bug.

It is common for GIF to be clear when executing VMRUN but it is not
required.  The pseudocode says that VMRUN simply sets GIF, and #VMEXIT
simply clears GIF.

But, looking at nestedsvm_vcpu_clgi(), it's not the only bug here. 
nestedsvm_vcpu_{clgi,stgi}() have additional calls to
local_event_delivery_{disable,enable}().

vgif_enable is strictly an optimisation that allows hardware to manage
the vGIF bit, so at an absolute minimum this ought to be:

    if ( ->vgif_enable )
        ->vgif = 0;
    else
        ->ns_gif = 0;
    local_event_delivery_disable();

to have balanced semantics.  Except that messing with event channels
can't be done when hardware is handling the vgif bit.

In fact, I think it's plain buggy to be touching event channels in
response to GIF changes.  In PV guests  vcpu_info->evtchn_upcall_mask is
the singular mask bit, but this is not true in HVM guests.  Even with
direct vector injection, HVM guests are still subject to normal rules
about INTR delivery, and that CLI blocks them.  This should naturally
extend to GIF for nested-SVM too.

As far as testing goes, perhaps we want to start with something even
more simple.  For real self-IPIs, and self-evtchns (for all reasonable
evtchn delivery configurations), check that delivery is blocked by IF=0
|| GIF=0.

~Andrew

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@citrix.com>
> ---
>   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 ?)

(also making me notice that svm_vmexit_do_{stgi,clgi}() seems to lack 
vGIF specific logic)

Teddy


-- 
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech