Re: [PATCH 2/3] vfio/pci: introduce config_offset field in VFIOConfigMirrorQuirk

[Alex Williamson]

On Tue, 31 Dec 2024 23:19:52 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:

> Device may only expose a specific portion of PCI config space through a
> region in a BAR, such behavior is seen in igd GGC and BDSM mirrors in
> BAR0. To handle these, config_offset is introduced to allow mirroring
> arbitrary region in PCI config space.
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/pci-quirks.c | 5 +++++
>  hw/vfio/pci-quirks.h | 3 ++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index bb2ce1d904..5a0b25a544 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -162,6 +162,7 @@ static uint64_t vfio_generic_quirk_mirror_read(void 
> *opaque,
>      (void)vfio_region_read(&vdev->bars[mirror->bar].region,
>                             addr + mirror->offset, size);
>  
> +    addr += mirror->config_offset;
>      data = vfio_pci_read_config(&vdev->pdev, addr, size);
>      trace_vfio_quirk_generic_mirror_read(vdev->vbasedev.name,
>                                           memory_region_name(mirror->mem),
> @@ -175,6 +176,7 @@ static void vfio_generic_quirk_mirror_write(void *opaque, 
> hwaddr addr,
>      VFIOConfigMirrorQuirk *mirror = opaque;
>      VFIOPCIDevice *vdev = mirror->vdev;
>  
> +    addr += mirror->config_offset;
>      vfio_pci_write_config(&vdev->pdev, addr, data, size);
>      trace_vfio_quirk_generic_mirror_write(vdev->vbasedev.name,
>                                            memory_region_name(mirror->mem),
> @@ -456,6 +458,7 @@ static void vfio_probe_ati_bar2_quirk(VFIOPCIDevice 
> *vdev, int nr)
>      mirror->mem = quirk->mem;
>      mirror->vdev = vdev;
>      mirror->offset = 0x4000;
> +    mirror->config_offset = 0;

These are all allocated with g_malloc0(), we don't need to explicitly
initialize this field in any of these existing uses.  Thanks,

Alex

>      mirror->bar = nr;
>  
>      memory_region_init_io(mirror->mem, OBJECT(vdev),
> @@ -908,6 +911,7 @@ static void vfio_probe_nvidia_bar0_quirk(VFIOPCIDevice 
> *vdev, int nr)
>      mirror->mem = quirk->mem;
>      mirror->vdev = vdev;
>      mirror->offset = 0x88000;
> +    mirror->config_offset = 0;
>      mirror->bar = nr;
>      last = (LastDataSet *)&mirror->data;
>      last->quirk = quirk;
> @@ -929,6 +933,7 @@ static void vfio_probe_nvidia_bar0_quirk(VFIOPCIDevice 
> *vdev, int nr)
>          mirror->mem = quirk->mem;
>          mirror->vdev = vdev;
>          mirror->offset = 0x1800;
> +        mirror->config_offset = 0;
>          mirror->bar = nr;
>          last = (LastDataSet *)&mirror->data;
>          last->quirk = quirk;
> diff --git a/hw/vfio/pci-quirks.h b/hw/vfio/pci-quirks.h
> index c0e96a01cc..d1532e379b 100644
> --- a/hw/vfio/pci-quirks.h
> +++ b/hw/vfio/pci-quirks.h
> @@ -60,7 +60,8 @@ extern const MemoryRegionOps vfio_generic_window_data_quirk;
>   */
>  typedef struct VFIOConfigMirrorQuirk {
>      struct VFIOPCIDevice *vdev;
> -    uint32_t offset;
> +    uint32_t offset; /* Offset in BAR */
> +    uint32_t config_offset; /* Offset in PCI config space */
>      uint8_t bar;
>      MemoryRegion *mem;
>      uint8_t data[];