Re: [PATCH v1] nestedsvm: Fix multi-byte IO port intercept check

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

On 9/11/26 11:06 AM, Andrew Cooper wrote:
> On 10/09/2026 5:39 pm, Ross Lagerwall wrote:
> > For multi-byte IO port accesses, the APM says that SVM should intercept
> > if any of the corresponding permission bits are set. However, the code
> > has this backwards and only intercepts if all the permission bits are
> > set.
> By any chance is this for the root partition, with 0xcf9 permitted but
> 0xcf8,a,b intercepted?
Yes, for the root partition. It intercepts 0xcf8,c,d,e,f and permits
0xcf9,a,b.

> > This affects Hyper-V since it does not generally set all the permission
> > bits of the multi-byte ports it allows its root partition to access.
> > This results in an L2 root partition that cannot do PCI config space
> > accesses and therefore cannot access its NVMe disk to continue booting.
> >
> > Signed-off-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> > ---
> >   xen/arch/x86/hvm/svm/nestedsvm.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
> > index 5adb1bd72c4d..249fde43b5be 100644
> > --- a/xen/arch/x86/hvm/svm/nestedsvm.c
> > +++ b/xen/arch/x86/hvm/svm/nestedsvm.c
> > @@ -852,7 +852,7 @@ nsvm_vmcb_guest_intercepts_ioio(paddr_t iopm_pa, uint64_t 
> > exitinfo1)
> >       for ( io_bitmap = hvm_map_guest_frame_ro(gfn, 0); ; )
> >       {
> >           enabled = io_bitmap && test_bit(port, io_bitmap);
> > -        if ( !enabled || !--size )
> > +        if ( enabled || !--size )
> >               break;
> >           if ( unlikely(++port == 8 * PAGE_SIZE) )
> >           {
> While this does fix a bug, I think the behaviour is still unsafe.
>
> For starters, 'enabled' is a terrible name and is probably a major
> factor in getting this wrong.  It should be 'intercepted'.
>
> hvm_map_guest_frame_ro() can return NULL for several reasons[1],
> including ballooned out frames/etc.  It is not by accident that a set
> bit means intercept; it's for the same reason that the byte sequence FF
> FF is #UD (with the PUSH that should have been in that position moving
> elsewhere in the opcode table), and that's because ~0 is the return
> value for "nothing here on the memory bus".
>
> Either way, if io_bitmap is NULL, the port should be intercepted rather
> than access being permitted, so the other prior line needs to be of the
> form:
>
>      intercepted = !io_bitmap || test_bit(port, io_bitmap);
OK, I'll send an updated patch.

Ross