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

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

[Ross Lagerwall]

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.

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

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

[Andrew Cooper]

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

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

[Ross Lagerwall]

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