Re: [PATCH] xen/x86/svm: Apply the virtual-8086 IOPL check to INT n only

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 22 Oct 2026, Andrew Cooper wrote:
> On 22/10/2026 2:55 pm, Ross Lagerwall wrote:
> > On 10/20/26 11:41 PM, Stefano Stabellini wrote:
> >> In virtual-8086 mode with IOPL < 3, INT n raises #GP(0).  INT1, INT3
> >> and INTO are not affected. They are delivered normally.
> >>
> >> svm_emul_swint_injection() applies the check to all of them, so an ICEBP
> >> raises #GP instead of #DB.  It also reports the IDT entry in the #GP
> >> error code instead of 0.
> >>
> >> Limit the check to INT n and use error code 0.
> >>
> >> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> >> ---
> >>   xen/arch/x86/hvm/svm/svm.c | 5 ++++-
> >>   1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> >> index 71e48351f2..df74531d1a 100644
> >> --- a/xen/arch/x86/hvm/svm/svm.c
> >> +++ b/xen/arch/x86/hvm/svm/svm.c
> >> @@ -1153,9 +1153,12 @@ static void svm_emul_swint_injection(struct
> >> x86_event *event)
> >>        * 32bit OS.  Someone with many TUITs can see about reading the
> >> TSS
> >>        * Software Interrupt Redirection bitmap.
> >>        */
> >> -    if ( (regs->eflags & X86_EFLAGS_VM) &&
> >> +    if ( type == X86_ET_SW_INT && (regs->eflags & X86_EFLAGS_VM) &&
> >>            MASK_EXTR(regs->eflags, X86_EFLAGS_IOPL) != 3 )
> >> +    {
> >> +        ec = 0;
> >>           goto raise_exception;
> >> +    }
> >>         /*
> >>        * Read all 8/16 bytes so the idtr limit check is applied
> >> properly to
> >
> > I think this is correct. The check could perhaps move earlier to avoid
> > needing
> > to reset ec to 0.
> >
> > I do wonder though if there are other cases in the function where ec
> > needs to
> > be reset to 0? The pseudocode in the APM seems to have at least one
> > other case.
> 
> I confirmed, and indeed it is #GP[0] not #GP[sel].  Also that
> ICEBP/INT3/INTO are not subject IOPL like this.
> 
> I agree with Ross that this wants moving earlier; between the CR0.PE
> check and deciding idte_size.  The comment also wants adjusting, so I
> think it wants to look like this:
> 
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 71e48351f22c..c83b28d29727 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1140,6 +1140,17 @@ static void svm_emul_swint_injection(struct
> x86_event *event)
>      if ( !(vmcb_get_cr0(vmcb) & X86_CR0_PE) )
>          goto raise_exception; /* TODO: support real-mode injection? */
>  
> +    /*
> +     * In vm86 mode, INT $N is subject to IOPL.
> +     *
> +     * TODO: It is also subject to CR4.VME=1, which would require
> reading the
> +     * TSS Software Interrupt Redirection bitmap.
> +     */
> +    if ( type == X86_ET_SW_INT &&
> +         regs->eflags & X86_EFLAGS_VM &&
> +         MASK_EXTR(regs->eflags, X86_EFLAGS_IOPL) != 3 )
> +        goto raise_exception;
> +
>      idte_size   = lm ? 16 : 8;
>      idte_offset = trap * idte_size;
>  
> @@ -1147,16 +1158,6 @@ static void svm_emul_swint_injection(struct
> x86_event *event)
>      ec = (trap << 3) | X86_XEC_IDT |
>           (type == X86_ET_PRIV_SW_EXC ? X86_XEC_EXT : 0);
>  
> -    /*
> -     * TODO: This does not cover the v8086 mode with CR4.VME case
> -     * correctly, but falls on the safe side from the point of view of a
> -     * 32bit OS.  Someone with many TUITs can see about reading the TSS
> -     * Software Interrupt Redirection bitmap.
> -     */
> -    if ( (regs->eflags & X86_EFLAGS_VM) &&
> -         MASK_EXTR(regs->eflags, X86_EFLAGS_IOPL) != 3 )
> -        goto raise_exception;
> -
>      /*
>       * Read all 8/16 bytes so the idtr limit check is applied properly to
>       * this entry, even though we don't look at all the words read.
> 
> 
> I'm happy to adjust this on commit if you're happy?

Yes please, thank you!