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

Ross Lagerwall posted 1 patch 1 week, 6 days ago
xen/arch/x86/hvm/svm/nestedsvm.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH v2] nestedsvm: Fix multi-byte IO port intercept check
Posted by Ross Lagerwall 1 week, 6 days ago
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. Fix this and at the same time, make things safer by handling
mapping failures as intercepted. Also rename the 'enabled' variable to
make it clearer what it does.

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

In v2:

* Intercept if mapping fails
* Rename "enabled" variable

 xen/arch/x86/hvm/svm/nestedsvm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index 5adb1bd72c4d..8885916399b4 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -830,7 +830,7 @@ nsvm_vmcb_guest_intercepts_ioio(paddr_t iopm_pa, uint64_t exitinfo1)
     ioio_info_t ioinfo;
     uint16_t port;
     unsigned int size;
-    bool enabled;
+    bool intercepted;
 
     ioinfo.bytes = exitinfo1;
     port = ioinfo.fields.port;
@@ -851,8 +851,8 @@ 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 )
+        intercepted = !io_bitmap || test_bit(port, io_bitmap);
+        if ( intercepted || !--size )
             break;
         if ( unlikely(++port == 8 * PAGE_SIZE) )
         {
@@ -863,7 +863,7 @@ nsvm_vmcb_guest_intercepts_ioio(paddr_t iopm_pa, uint64_t exitinfo1)
     }
     hvm_unmap_guest_frame(io_bitmap, 0);
 
-    if ( !enabled )
+    if ( !intercepted )
         return NESTEDHVM_VMEXIT_HOST;
 
     return NESTEDHVM_VMEXIT_INJECT;
-- 
2.53.0
Re: [PATCH v2] nestedsvm: Fix multi-byte IO port intercept check
Posted by Andrew Cooper 1 week, 6 days ago
On 11/09/2026 2:23 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. Fix this and at the same time, make things safer by handling
> mapping failures as intercepted. Also rename the 'enabled' variable to
> make it clearer what it does.
>
> 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>

The reason I asked about 0xcf9 is because it's the one-byte PCH reset
register, which hides fully inside the normal 4 bytes at 0xcf8 for CFG
accesses.

Despite outward appearances, the x86 IO space is not one uniform space. 
It's 3 orthogonal space (1,2 and 4-byte accesses) where the target
device gets to choose whether muti-byte values alias in the spaces, or
are entirely disjoint.

For the HyperV root partition specifically, I'd expect 0xcf9 to be
unintercepted so as not to interfere with the final steps of
shutdown/reboot from the main kernel.

The patch looks ok, but I think the second paragraph needs a bit more
explaining.  Are we saying that because 0xcf9 was permitted, we allowed
4b@0xcf8 to be permitted, changing L1's CFG index rather than changing
L2's CFG index, causing the subsequent 0xcfc access to produce garbage?

~Andrew

Re: [PATCH v2] nestedsvm: Fix multi-byte IO port intercept check
Posted by Ross Lagerwall 1 week, 6 days ago
On 9/11/26 2:36 PM, Andrew Cooper wrote:
> On 11/09/2026 2:23 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. Fix this and at the same time, make things safer by handling
>> mapping failures as intercepted. Also rename the 'enabled' variable to
>> make it clearer what it does.
>>
>> 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>
> 
> The reason I asked about 0xcf9 is because it's the one-byte PCH reset
> register, which hides fully inside the normal 4 bytes at 0xcf8 for CFG
> accesses.
> 
> Despite outward appearances, the x86 IO space is not one uniform space.
> It's 3 orthogonal space (1,2 and 4-byte accesses) where the target
> device gets to choose whether muti-byte values alias in the spaces, or
> are entirely disjoint.
> 
> For the HyperV root partition specifically, I'd expect 0xcf9 to be
> unintercepted so as not to interfere with the final steps of
> shutdown/reboot from the main kernel.
> 
> The patch looks ok, but I think the second paragraph needs a bit more
> explaining.  Are we saying that because 0xcf9 was permitted, we allowed
> 4b@0xcf8 to be permitted, changing L1's CFG index rather than changing
> L2's CFG index, causing the subsequent 0xcfc access to produce garbage?
> 

Yes. The incorrect trace shows:

L2 write 4b@0xcf8, permitted so it updates the L1 CFG index
L2 read  2b@0xcfc, VMEXITs to L1
L1 write 4b@0xcf8, updates the L1 CFG index to the wrong value (since it didn't
see the first L2 write)
L1 read  2b@0xcf8, reads the incorrect value from the device model in L0

How about this for the second paragraph?

This affects Hyper-V which for the root partition intercepts 0xcf8 and
0xcfc-0xcff. L2 issues 4 byte CFG index writes to 0xcf8 which are incorrectly
permitted and update the L1 CFG index. The subsequent L2 CFG data read at 0xcfc
is intercepted by L1, then resubmitted as a CFG index write followed by CFG
data read to L0. Since L1 doesn't see the CFG index write by L2, it uses the
wrong index and gets garbage back, ultimately leading to a failure to boot
since it cannot access its NVMe disk.

Ross
Re: [PATCH v2] nestedsvm: Fix multi-byte IO port intercept check
Posted by Andrew Cooper 1 week, 6 days ago
On 11/09/2026 3:50 pm, Ross Lagerwall wrote:
> On 9/11/26 2:36 PM, Andrew Cooper wrote:
>> On 11/09/2026 2:23 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. Fix this and at the same time, make things safer by handling
>>> mapping failures as intercepted. Also rename the 'enabled' variable to
>>> make it clearer what it does.
>>>
>>> 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>
>>
>> The reason I asked about 0xcf9 is because it's the one-byte PCH reset
>> register, which hides fully inside the normal 4 bytes at 0xcf8 for CFG
>> accesses.
>>
>> Despite outward appearances, the x86 IO space is not one uniform space.
>> It's 3 orthogonal space (1,2 and 4-byte accesses) where the target
>> device gets to choose whether muti-byte values alias in the spaces, or
>> are entirely disjoint.
>>
>> For the HyperV root partition specifically, I'd expect 0xcf9 to be
>> unintercepted so as not to interfere with the final steps of
>> shutdown/reboot from the main kernel.
>>
>> The patch looks ok, but I think the second paragraph needs a bit more
>> explaining.  Are we saying that because 0xcf9 was permitted, we allowed
>> 4b@0xcf8 to be permitted, changing L1's CFG index rather than changing
>> L2's CFG index, causing the subsequent 0xcfc access to produce garbage?
>>
>
> Yes. The incorrect trace shows:
>
> L2 write 4b@0xcf8, permitted so it updates the L1 CFG index
> L2 read  2b@0xcfc, VMEXITs to L1
> L1 write 4b@0xcf8, updates the L1 CFG index to the wrong value (since
> it didn't
> see the first L2 write)
> L1 read  2b@0xcf8, reads the incorrect value from the device model in L0
>
> How about this for the second paragraph?
>
> This affects Hyper-V which for the root partition intercepts 0xcf8 and
> 0xcfc-0xcff. L2 issues 4 byte CFG index writes to 0xcf8 which are
> incorrectly
> permitted and update the L1 CFG index. The subsequent L2 CFG data read
> at 0xcfc
> is intercepted by L1, then resubmitted as a CFG index write followed
> by CFG
> data read to L0. Since L1 doesn't see the CFG index write by L2, it
> uses the
> wrong index and gets garbage back, ultimately leading to a failure to
> boot
> since it cannot access its NVMe disk.

Yeah, much better.  I'll swap this in on commit.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>