1. Patchew
  2. QEMU
  3. View series

[PATCH] vfio/pci: Recover sub-page BAR size when base address is not aligned

Zhenzhong Duan posted 1 patch 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250715065952.213057-1-zhenzhong.duan@intel.com
Maintainers: Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>
hw/vfio/pci.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
[PATCH] vfio/pci: Recover sub-page BAR size when base address is not aligned
Posted by Zhenzhong Duan 4 months ago 
Currently region_mr and mmap_mr size are recovered to original size when
base address is updated to non-PAGE_SIZE aligned, but still leave base_mr
with PAGE_SIZE size. This is harmless as base_mr is a only container but
still a bit confusing when executing hmp command mtree.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/pci.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 1093b28df7..0455e6ce30 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1348,9 +1348,7 @@ static void vfio_sub_page_bar_update_mapping(PCIDevice *pdev, int bar)
 
     memory_region_transaction_begin();
 
-    if (vdev->bars[bar].size < size) {
-        memory_region_set_size(base_mr, size);
-    }
+    memory_region_set_size(base_mr, size);
     memory_region_set_size(region_mr, size);
     memory_region_set_size(mmap_mr, size);
     if (size != vdev->bars[bar].size && memory_region_is_mapped(base_mr)) {
-- 
2.47.1
Re: [PATCH] vfio/pci: Recover sub-page BAR size when base address is not aligned
Posted by Alex Williamson 2 months, 1 week ago 
On Tue, 15 Jul 2025 02:59:52 -0400
Zhenzhong Duan <zhenzhong.duan@intel.com> wrote:

> Currently region_mr and mmap_mr size are recovered to original size when
> base address is updated to non-PAGE_SIZE aligned, but still leave base_mr
> with PAGE_SIZE size. This is harmless as base_mr is a only container but
> still a bit confusing when executing hmp command mtree.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  hw/vfio/pci.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 1093b28df7..0455e6ce30 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1348,9 +1348,7 @@ static void vfio_sub_page_bar_update_mapping(PCIDevice *pdev, int bar)
>  
>      memory_region_transaction_begin();
>  
> -    if (vdev->bars[bar].size < size) {
> -        memory_region_set_size(base_mr, size);
> -    }
> +    memory_region_set_size(base_mr, size);
>      memory_region_set_size(region_mr, size);
>      memory_region_set_size(mmap_mr, size);
>      if (size != vdev->bars[bar].size && memory_region_is_mapped(base_mr)) {

Has this been tested with MSI-X relocation to the sub-page BAR?  The
lowest level MR here is meant to fill the gaps not backed by other MRs,
for example between the region MR and the MSI-X emulation MR.  We call
this function based on the region size, not the BAR size.  For example I
think if we had a 4K host, 2K BAR, and >2K MSI-X table, bars[bar].size
might be 8K, with the MSI-X table offset at 4K.  We'd want the
underlying container BAR MR to cover the full 8K, not just the 4K that
we're expanding the region access to.  OTOH, if host page size were
64K, then we would want to expand the base MR to page size.

I think that's the original reason this is conditional, but maybe you
could share the confusing mtree output and provide any comments
relative to the scenario above.  Thanks,

Alex
RE: [PATCH] vfio/pci: Recover sub-page BAR size when base address is not aligned
Posted by Duan, Zhenzhong 2 months, 1 week ago 

>-----Original Message-----
>From: Alex Williamson <alex.williamson@redhat.com>
>Subject: Re: [PATCH] vfio/pci: Recover sub-page BAR size when base address
>is not aligned
>
>On Tue, 15 Jul 2025 02:59:52 -0400
>Zhenzhong Duan <zhenzhong.duan@intel.com> wrote:
>
>> Currently region_mr and mmap_mr size are recovered to original size when
>> base address is updated to non-PAGE_SIZE aligned, but still leave base_mr
>> with PAGE_SIZE size. This is harmless as base_mr is a only container but
>> still a bit confusing when executing hmp command mtree.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  hw/vfio/pci.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 1093b28df7..0455e6ce30 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -1348,9 +1348,7 @@ static void
>vfio_sub_page_bar_update_mapping(PCIDevice *pdev, int bar)
>>
>>      memory_region_transaction_begin();
>>
>> -    if (vdev->bars[bar].size < size) {
>> -        memory_region_set_size(base_mr, size);
>> -    }
>> +    memory_region_set_size(base_mr, size);
>>      memory_region_set_size(region_mr, size);
>>      memory_region_set_size(mmap_mr, size);
>>      if (size != vdev->bars[bar].size &&
>memory_region_is_mapped(base_mr)) {
>
>Has this been tested with MSI-X relocation to the sub-page BAR?  The
>lowest level MR here is meant to fill the gaps not backed by other MRs,
>for example between the region MR and the MSI-X emulation MR.  We call
>this function based on the region size, not the BAR size.  For example I
>think if we had a 4K host, 2K BAR, and >2K MSI-X table, bars[bar].size
>might be 8K, with the MSI-X table offset at 4K.  We'd want the
>underlying container BAR MR to cover the full 8K, not just the 4K that
>we're expanding the region access to.  OTOH, if host page size were
>64K, then we would want to expand the base MR to page size.

Oh, I see, my patch may lead to truncation of relocated MSI-X region if it is relocated to sub-page BAR, that's not correct, thanks Alex!

>
>I think that's the original reason this is conditional, but maybe you
>could share the confusing mtree output and provide any comments
>relative to the scenario above.  Thanks,

Sorry, I don't have a device with sub-page BAR and a guest kernel writing a non-PAGE_SIZE base  addr. I made this patch by code inspecting.

BRs,
Zhenzhong
Re: [PATCH] vfio/pci: Recover sub-page BAR size when base address is not aligned
Posted by Cédric Le Goater 4 months ago 
On 7/15/25 08:59, Zhenzhong Duan wrote:
> Currently region_mr and mmap_mr size are recovered to original size when
> base address is updated to non-PAGE_SIZE aligned, but still leave base_mr
> with PAGE_SIZE size. This is harmless as base_mr is a only container but
> still a bit confusing when executing hmp command mtree.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   hw/vfio/pci.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 1093b28df7..0455e6ce30 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1348,9 +1348,7 @@ static void vfio_sub_page_bar_update_mapping(PCIDevice *pdev, int bar)
>   
>       memory_region_transaction_begin();
>   
> -    if (vdev->bars[bar].size < size) {
> -        memory_region_set_size(base_mr, size);
> -    }
> +    memory_region_set_size(base_mr, size);
>       memory_region_set_size(region_mr, size);
>       memory_region_set_size(mmap_mr, size);
>       if (size != vdev->bars[bar].size && memory_region_is_mapped(base_mr)) {

Looks ok to me. I'll queue it for 10.2 though.

Thanks,

C.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.