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

[Ross Lagerwall <ross.lagerwall@citrix.com>]

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