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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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?

>
> 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);

~Andrew

[1] One of the few things which can't go wrong is gfn being in the
address space but gfn+3 being outside of it.  This is stated to cause a
VMEntry failure.