[PATCH V1] vfio: fix sub-page bar after cpr

[PATCH V1] vfio: fix sub-page bar after cpr

[Steve Sistare]

Regions for sub-page BARs are normally mapped here, in response to the
guest writing to PCI config space:

  vfio_pci_write_config()
    pci_default_write_config()
      pci_update_mappings()
        memory_region_add_subregion()
    vfio_sub_page_bar_update_mapping()
      ... vfio_dma_map()

However, after CPR, the guest does not reconfigure the device and the
code path above is not taken.  To fix, in vfio_cpr_pci_post_load, call
vfio_sub_page_bar_update_mapping for each sub-page BAR with a valid
address.

Fixes: 7e9f21411302 ("vfio/container: restore DMA vaddr")

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 hw/vfio/pci.h |  1 +
 hw/vfio/cpr.c |  2 ++
 hw/vfio/pci.c | 14 ++++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index 495fae7..cb1310d 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -228,6 +228,7 @@ void vfio_pci_write_config(PCIDevice *pdev,
 uint64_t vfio_vga_read(void *opaque, hwaddr addr, unsigned size);
 void vfio_vga_write(void *opaque, hwaddr addr, uint64_t data, unsigned size);
 
+void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev);
 bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev);
 bool vfio_config_quirk_setup(VFIOPCIDevice *vdev, Error **errp);
 void vfio_vga_quirk_setup(VFIOPCIDevice *vdev);
diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c
index af0f12a..384b56c 100644
--- a/hw/vfio/cpr.c
+++ b/hw/vfio/cpr.c
@@ -116,6 +116,8 @@ static int vfio_cpr_pci_post_load(void *opaque, int version_id)
     PCIDevice *pdev = &vdev->pdev;
     int nr_vectors;
 
+    vfio_sub_page_bar_update_mappings(vdev);
+
     if (msix_enabled(pdev)) {
         vfio_pci_msix_set_notifiers(vdev);
         nr_vectors = vdev->msix->entries;
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 1093b28..9c616bd 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2826,6 +2826,20 @@ static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
     return ret;
 }
 
+void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev)
+{
+    PCIDevice *pdev = &vdev->pdev;
+    int page_size = qemu_real_host_page_size();
+    int bar;
+
+    for (bar = 0; bar < PCI_ROM_SLOT; bar++) {
+        PCIIORegion *r = &pdev->io_regions[bar];
+        if (r->addr != PCI_BAR_UNMAPPED && r->size > 0 && r->size < page_size) {
+            vfio_sub_page_bar_update_mapping(pdev, bar);
+        }
+    }
+}
+
 static VFIODeviceOps vfio_pci_ops = {
     .vfio_compute_needs_reset = vfio_pci_compute_needs_reset,
     .vfio_hot_reset_multi = vfio_pci_hot_reset_multi,
-- 
1.8.3.1

RE: [PATCH V1] vfio: fix sub-page bar after cpr

[Duan, Zhenzhong]

>-----Original Message-----
>From: Steve Sistare <steven.sistare@oracle.com>
>Subject: [PATCH V1] vfio: fix sub-page bar after cpr
>
>Regions for sub-page BARs are normally mapped here, in response to the
>guest writing to PCI config space:
>
>  vfio_pci_write_config()
>    pci_default_write_config()
>      pci_update_mappings()
>        memory_region_add_subregion()
>    vfio_sub_page_bar_update_mapping()
>      ... vfio_dma_map()
>
>However, after CPR, the guest does not reconfigure the device and the
>code path above is not taken.  To fix, in vfio_cpr_pci_post_load, call
>vfio_sub_page_bar_update_mapping for each sub-page BAR with a valid
>address.
>
>Fixes: 7e9f21411302 ("vfio/container: restore DMA vaddr")
>
>Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Just curious what kind of device have sub-page BARs and what's the size of subpage BAR.
Could you share the output of lspci -s $BDF -v?

Thanks
Zhenzhong

>---
> hw/vfio/pci.h |  1 +
> hw/vfio/cpr.c |  2 ++
> hw/vfio/pci.c | 14 ++++++++++++++
> 3 files changed, 17 insertions(+)
>
>diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
>index 495fae7..cb1310d 100644
>--- a/hw/vfio/pci.h
>+++ b/hw/vfio/pci.h
>@@ -228,6 +228,7 @@ void vfio_pci_write_config(PCIDevice *pdev,
> uint64_t vfio_vga_read(void *opaque, hwaddr addr, unsigned size);
> void vfio_vga_write(void *opaque, hwaddr addr, uint64_t data, unsigned
>size);
>
>+void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev);
> bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev);
> bool vfio_config_quirk_setup(VFIOPCIDevice *vdev, Error **errp);
> void vfio_vga_quirk_setup(VFIOPCIDevice *vdev);
>diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c
>index af0f12a..384b56c 100644
>--- a/hw/vfio/cpr.c
>+++ b/hw/vfio/cpr.c
>@@ -116,6 +116,8 @@ static int vfio_cpr_pci_post_load(void *opaque, int
>version_id)
>     PCIDevice *pdev = &vdev->pdev;
>     int nr_vectors;
>
>+    vfio_sub_page_bar_update_mappings(vdev);
>+
>     if (msix_enabled(pdev)) {
>         vfio_pci_msix_set_notifiers(vdev);
>         nr_vectors = vdev->msix->entries;
>diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>index 1093b28..9c616bd 100644
>--- a/hw/vfio/pci.c
>+++ b/hw/vfio/pci.c
>@@ -2826,6 +2826,20 @@ static int vfio_pci_load_config(VFIODevice
>*vbasedev, QEMUFile *f)
>     return ret;
> }
>
>+void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev)
>+{
>+    PCIDevice *pdev = &vdev->pdev;
>+    int page_size = qemu_real_host_page_size();
>+    int bar;
>+
>+    for (bar = 0; bar < PCI_ROM_SLOT; bar++) {
>+        PCIIORegion *r = &pdev->io_regions[bar];
>+        if (r->addr != PCI_BAR_UNMAPPED && r->size > 0 && r->size <
>page_size) {
>+            vfio_sub_page_bar_update_mapping(pdev, bar);
>+        }
>+    }
>+}
>+
> static VFIODeviceOps vfio_pci_ops = {
>     .vfio_compute_needs_reset = vfio_pci_compute_needs_reset,
>     .vfio_hot_reset_multi = vfio_pci_hot_reset_multi,
>--
>1.8.3.1

Re: [PATCH V1] vfio: fix sub-page bar after cpr

[Steven Sistare]

On 7/15/2025 2:32 AM, Duan, Zhenzhong wrote:
>> -----Original Message-----
>> From: Steve Sistare <steven.sistare@oracle.com>
>> Subject: [PATCH V1] vfio: fix sub-page bar after cpr
>>
>> Regions for sub-page BARs are normally mapped here, in response to the
>> guest writing to PCI config space:
>>
>>   vfio_pci_write_config()
>>     pci_default_write_config()
>>       pci_update_mappings()
>>         memory_region_add_subregion()
>>     vfio_sub_page_bar_update_mapping()
>>       ... vfio_dma_map()
>>
>> However, after CPR, the guest does not reconfigure the device and the
>> code path above is not taken.  To fix, in vfio_cpr_pci_post_load, call
>> vfio_sub_page_bar_update_mapping for each sub-page BAR with a valid
>> address.
>>
>> Fixes: 7e9f21411302 ("vfio/container: restore DMA vaddr")
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> 
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> 
> Just curious what kind of device have sub-page BARs and what's the size of subpage BAR.
> Could you share the output of lspci -s $BDF -v?

I was testing INTx support by directly assigning an x86 USB controller to
the guest, and the BAR was 512B.  Probably not an example we care about,
but it pointed out this bug.

- Steve

>> ---
>> hw/vfio/pci.h |  1 +
>> hw/vfio/cpr.c |  2 ++
>> hw/vfio/pci.c | 14 ++++++++++++++
>> 3 files changed, 17 insertions(+)
>>
>> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
>> index 495fae7..cb1310d 100644
>> --- a/hw/vfio/pci.h
>> +++ b/hw/vfio/pci.h
>> @@ -228,6 +228,7 @@ void vfio_pci_write_config(PCIDevice *pdev,
>> uint64_t vfio_vga_read(void *opaque, hwaddr addr, unsigned size);
>> void vfio_vga_write(void *opaque, hwaddr addr, uint64_t data, unsigned
>> size);
>>
>> +void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev);
>> bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev);
>> bool vfio_config_quirk_setup(VFIOPCIDevice *vdev, Error **errp);
>> void vfio_vga_quirk_setup(VFIOPCIDevice *vdev);
>> diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c
>> index af0f12a..384b56c 100644
>> --- a/hw/vfio/cpr.c
>> +++ b/hw/vfio/cpr.c
>> @@ -116,6 +116,8 @@ static int vfio_cpr_pci_post_load(void *opaque, int
>> version_id)
>>      PCIDevice *pdev = &vdev->pdev;
>>      int nr_vectors;
>>
>> +    vfio_sub_page_bar_update_mappings(vdev);
>> +
>>      if (msix_enabled(pdev)) {
>>          vfio_pci_msix_set_notifiers(vdev);
>>          nr_vectors = vdev->msix->entries;
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 1093b28..9c616bd 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -2826,6 +2826,20 @@ static int vfio_pci_load_config(VFIODevice
>> *vbasedev, QEMUFile *f)
>>      return ret;
>> }
>>
>> +void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev)
>> +{
>> +    PCIDevice *pdev = &vdev->pdev;
>> +    int page_size = qemu_real_host_page_size();
>> +    int bar;
>> +
>> +    for (bar = 0; bar < PCI_ROM_SLOT; bar++) {
>> +        PCIIORegion *r = &pdev->io_regions[bar];
>> +        if (r->addr != PCI_BAR_UNMAPPED && r->size > 0 && r->size <
>> page_size) {
>> +            vfio_sub_page_bar_update_mapping(pdev, bar);
>> +        }
>> +    }
>> +}
>> +
>> static VFIODeviceOps vfio_pci_ops = {
>>      .vfio_compute_needs_reset = vfio_pci_compute_needs_reset,
>>      .vfio_hot_reset_multi = vfio_pci_hot_reset_multi,
>> --
>> 1.8.3.1
> 

Re: [PATCH V1] vfio: fix sub-page bar after cpr

[Cédric Le Goater]

On 7/14/25 21:21, Steve Sistare wrote:
> Regions for sub-page BARs are normally mapped here, in response to the
> guest writing to PCI config space:
> 
>    vfio_pci_write_config()
>      pci_default_write_config()
>        pci_update_mappings()
>          memory_region_add_subregion()
>      vfio_sub_page_bar_update_mapping()
>        ... vfio_dma_map()
> 
> However, after CPR, the guest does not reconfigure the device and the
> code path above is not taken.  To fix, in vfio_cpr_pci_post_load, call
> vfio_sub_page_bar_update_mapping for each sub-page BAR with a valid
> address.
> 
> Fixes: 7e9f21411302 ("vfio/container: restore DMA vaddr")
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   hw/vfio/pci.h |  1 +
>   hw/vfio/cpr.c |  2 ++
>   hw/vfio/pci.c | 14 ++++++++++++++
>   3 files changed, 17 insertions(+)
> 
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 495fae7..cb1310d 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -228,6 +228,7 @@ void vfio_pci_write_config(PCIDevice *pdev,
>   uint64_t vfio_vga_read(void *opaque, hwaddr addr, unsigned size);
>   void vfio_vga_write(void *opaque, hwaddr addr, uint64_t data, unsigned size);
>   
> +void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev);
>   bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev);
>   bool vfio_config_quirk_setup(VFIOPCIDevice *vdev, Error **errp);
>   void vfio_vga_quirk_setup(VFIOPCIDevice *vdev);

We should rename all this routines.  For later.

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c
> index af0f12a..384b56c 100644
> --- a/hw/vfio/cpr.c
> +++ b/hw/vfio/cpr.c
> @@ -116,6 +116,8 @@ static int vfio_cpr_pci_post_load(void *opaque, int version_id)
>       PCIDevice *pdev = &vdev->pdev;
>       int nr_vectors;
>   
> +    vfio_sub_page_bar_update_mappings(vdev);
> +
>       if (msix_enabled(pdev)) {
>           vfio_pci_msix_set_notifiers(vdev);
>           nr_vectors = vdev->msix->entries;
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 1093b28..9c616bd 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2826,6 +2826,20 @@ static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
>       return ret;
>   }
>   
> +void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev)
> +{
> +    PCIDevice *pdev = &vdev->pdev;
> +    int page_size = qemu_real_host_page_size();
> +    int bar;
> +
> +    for (bar = 0; bar < PCI_ROM_SLOT; bar++) {
> +        PCIIORegion *r = &pdev->io_regions[bar];
> +        if (r->addr != PCI_BAR_UNMAPPED && r->size > 0 && r->size < page_size) {
> +            vfio_sub_page_bar_update_mapping(pdev, bar);
> +        }
> +    }
> +}
> +
>   static VFIODeviceOps vfio_pci_ops = {
>       .vfio_compute_needs_reset = vfio_pci_compute_needs_reset,
>       .vfio_hot_reset_multi = vfio_pci_hot_reset_multi,

Re: [PATCH V1] vfio: fix sub-page bar after cpr

[Cédric Le Goater]

On 7/14/25 21:21, Steve Sistare wrote:
> Regions for sub-page BARs are normally mapped here, in response to the
> guest writing to PCI config space:
> 
>    vfio_pci_write_config()
>      pci_default_write_config()
>        pci_update_mappings()
>          memory_region_add_subregion()
>      vfio_sub_page_bar_update_mapping()
>        ... vfio_dma_map()
> 
> However, after CPR, the guest does not reconfigure the device and the
> code path above is not taken.  To fix, in vfio_cpr_pci_post_load, call
> vfio_sub_page_bar_update_mapping for each sub-page BAR with a valid
> address.
> 
> Fixes: 7e9f21411302 ("vfio/container: restore DMA vaddr")
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   hw/vfio/pci.h |  1 +
>   hw/vfio/cpr.c |  2 ++
>   hw/vfio/pci.c | 14 ++++++++++++++
>   3 files changed, 17 insertions(+)
> 
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index 495fae7..cb1310d 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -228,6 +228,7 @@ void vfio_pci_write_config(PCIDevice *pdev,
>   uint64_t vfio_vga_read(void *opaque, hwaddr addr, unsigned size);
>   void vfio_vga_write(void *opaque, hwaddr addr, uint64_t data, unsigned size);
>   
> +void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev);
>   bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev);
>   bool vfio_config_quirk_setup(VFIOPCIDevice *vdev, Error **errp);
>   void vfio_vga_quirk_setup(VFIOPCIDevice *vdev);
> diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c
> index af0f12a..384b56c 100644
> --- a/hw/vfio/cpr.c
> +++ b/hw/vfio/cpr.c
> @@ -116,6 +116,8 @@ static int vfio_cpr_pci_post_load(void *opaque, int version_id)
>       PCIDevice *pdev = &vdev->pdev;
>       int nr_vectors;
>   
> +    vfio_sub_page_bar_update_mappings(vdev);
> +
>       if (msix_enabled(pdev)) {
>           vfio_pci_msix_set_notifiers(vdev);
>           nr_vectors = vdev->msix->entries;
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 1093b28..9c616bd 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2826,6 +2826,20 @@ static int vfio_pci_load_config(VFIODevice *vbasedev, QEMUFile *f)
>       return ret;
>   }
>   
> +void vfio_sub_page_bar_update_mappings(VFIOPCIDevice *vdev)
> +{
> +    PCIDevice *pdev = &vdev->pdev;
> +    int page_size = qemu_real_host_page_size();
> +    int bar;
> +
> +    for (bar = 0; bar < PCI_ROM_SLOT; bar++) {
> +        PCIIORegion *r = &pdev->io_regions[bar];
> +        if (r->addr != PCI_BAR_UNMAPPED && r->size > 0 && r->size < page_size) {
> +            vfio_sub_page_bar_update_mapping(pdev, bar);
> +        }
> +    }
> +}
> +
>   static VFIODeviceOps vfio_pci_ops = {
>       .vfio_compute_needs_reset = vfio_pci_compute_needs_reset,
>       .vfio_hot_reset_multi = vfio_pci_hot_reset_multi,


Applied to vfio-next.

Thanks,

C.