Re: [PATCH 1/3] vfio/pci: declare generic quirks in a new header file

[Tomita Moeko]

On 12/31/24 23:19, Tomita Moeko wrote:
> Declare generic vfio_generic_{window_address,window_data,mirror}_quirk
> in newly created pci_quirks.h so that they can be used elsewhere, like
> igd.c.
> 
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/pci-quirks.c | 59 ++++--------------------------------
>  hw/vfio/pci-quirks.h | 71 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 77 insertions(+), 53 deletions(-)
>  create mode 100644 hw/vfio/pci-quirks.h
> 
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index c8e60475d5..bb2ce1d904 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -25,6 +25,7 @@
>  #include "hw/nvram/fw_cfg.h"
>  #include "hw/qdev-properties.h"
>  #include "pci.h"
> +#include "pci-quirks.h"
>  #include "trace.h"
>  
>  /*
> @@ -66,40 +67,6 @@ bool vfio_opt_rom_in_denylist(VFIOPCIDevice *vdev)
>   * Device specific region quirks (mostly backdoors to PCI config space)
>   */
>  
> -/*
> - * The generic window quirks operate on an address and data register,
> - * vfio_generic_window_address_quirk handles the address register and
> - * vfio_generic_window_data_quirk handles the data register.  These ops
> - * pass reads and writes through to hardware until a value matching the
> - * stored address match/mask is written.  When this occurs, the data
> - * register access emulated PCI config space for the device rather than
> - * passing through accesses.  This enables devices where PCI config space
> - * is accessible behind a window register to maintain the virtualization
> - * provided through vfio.
> - */
> -typedef struct VFIOConfigWindowMatch {
> -    uint32_t match;
> -    uint32_t mask;
> -} VFIOConfigWindowMatch;
> -
> -typedef struct VFIOConfigWindowQuirk {
> -    struct VFIOPCIDevice *vdev;
> -
> -    uint32_t address_val;
> -
> -    uint32_t address_offset;
> -    uint32_t data_offset;
> -
> -    bool window_enabled;
> -    uint8_t bar;
> -
> -    MemoryRegion *addr_mem;
> -    MemoryRegion *data_mem;
> -
> -    uint32_t nr_matches;
> -    VFIOConfigWindowMatch matches[];
> -} VFIOConfigWindowQuirk;
> -
>  static uint64_t vfio_generic_window_quirk_address_read(void *opaque,
>                                                         hwaddr addr,
>                                                         unsigned size)
> @@ -135,7 +102,7 @@ static void vfio_generic_window_quirk_address_write(void 
> *opaque, hwaddr addr,
>      }
>  }
>  
> -static const MemoryRegionOps vfio_generic_window_address_quirk = {
> +const MemoryRegionOps vfio_generic_window_address_quirk = {
>      .read = vfio_generic_window_quirk_address_read,
>      .write = vfio_generic_window_quirk_address_write,
>      .endianness = DEVICE_LITTLE_ENDIAN,
> @@ -178,26 +145,12 @@ static void vfio_generic_window_quirk_data_write(void 
> *opaque, hwaddr addr,
>                        addr + window->data_offset, data, size);
>  }
>  
> -static const MemoryRegionOps vfio_generic_window_data_quirk = {
> +const MemoryRegionOps vfio_generic_window_data_quirk = {
>      .read = vfio_generic_window_quirk_data_read,
>      .write = vfio_generic_window_quirk_data_write,
>      .endianness = DEVICE_LITTLE_ENDIAN,
>  };
>  
> -/*
> - * The generic mirror quirk handles devices which expose PCI config space
> - * through a region within a BAR.  When enabled, reads and writes are
> - * redirected through to emulated PCI config space.  XXX if PCI config space
> - * used memory regions, this could just be an alias.
> - */
> -typedef struct VFIOConfigMirrorQuirk {
> -    struct VFIOPCIDevice *vdev;
> -    uint32_t offset;
> -    uint8_t bar;
> -    MemoryRegion *mem;
> -    uint8_t data[];
> -} VFIOConfigMirrorQuirk;
> -
>  static uint64_t vfio_generic_quirk_mirror_read(void *opaque,
>                                                 hwaddr addr, unsigned size)
>  {
> @@ -228,7 +181,7 @@ static void vfio_generic_quirk_mirror_write(void *opaque, 
> hwaddr addr,
>                                            addr, data);
>  }
>  
> -static const MemoryRegionOps vfio_generic_mirror_quirk = {
> +const MemoryRegionOps vfio_generic_mirror_quirk = {
>      .read = vfio_generic_quirk_mirror_read,
>      .write = vfio_generic_quirk_mirror_write,
>      .endianness = DEVICE_LITTLE_ENDIAN,
> @@ -1499,7 +1452,7 @@ static void get_nv_gpudirect_clique_id(Object *obj, 
> Visitor *v,
>                                         const char *name, void *opaque,
>                                         Error **errp)
>  {
> -    const Property *prop = opaque;
> +    Property *prop = opaque;

Sorry this was a mistake when I rebasing my changes. I will revert this in v2.

>      uint8_t *ptr = object_field_prop_ptr(obj, prop);
>  
>      visit_type_uint8(v, name, ptr, errp);
> @@ -1509,7 +1462,7 @@ static void set_nv_gpudirect_clique_id(Object *obj, 
> Visitor *v,
>                                         const char *name, void *opaque,
>                                         Error **errp)
>  {
> -    const Property *prop = opaque;
> +    Property *prop = opaque;

ditto

>      uint8_t value, *ptr = object_field_prop_ptr(obj, prop);
>  
>      if (!visit_type_uint8(v, name, &value, errp)) {
> diff --git a/hw/vfio/pci-quirks.h b/hw/vfio/pci-quirks.h
> new file mode 100644
> index 0000000000..c0e96a01cc
> --- /dev/null
> +++ b/hw/vfio/pci-quirks.h
> @@ -0,0 +1,71 @@
> +/*
> + * vfio generic region quirks (mostly backdoors to PCI config space)
> + *
> + * Copyright Red Hat, Inc. 2012-2015
> + *
> + * Authors:
> + *  Alex Williamson <alex.williamson@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + */
> +#ifndef HW_VFIO_VFIO_PCI_QUIRKS_H
> +#define HW_VFIO_VFIO_PCI_QUIRKS_H
> +
> +#include "qemu/osdep.h"
> +#include "exec/memop.h"
> +
> +/*
> + * The generic window quirks operate on an address and data register,
> + * vfio_generic_window_address_quirk handles the address register and
> + * vfio_generic_window_data_quirk handles the data register.  These ops
> + * pass reads and writes through to hardware until a value matching the
> + * stored address match/mask is written.  When this occurs, the data
> + * register access emulated PCI config space for the device rather than
> + * passing through accesses.  This enables devices where PCI config space
> + * is accessible behind a window register to maintain the virtualization
> + * provided through vfio.
> + */
> +typedef struct VFIOConfigWindowMatch {
> +    uint32_t match;
> +    uint32_t mask;
> +} VFIOConfigWindowMatch;
> +
> +typedef struct VFIOConfigWindowQuirk {
> +    struct VFIOPCIDevice *vdev;
> +
> +    uint32_t address_val;
> +
> +    uint32_t address_offset;
> +    uint32_t data_offset;
> +
> +    bool window_enabled;
> +    uint8_t bar;
> +
> +    MemoryRegion *addr_mem;
> +    MemoryRegion *data_mem;
> +
> +    uint32_t nr_matches;
> +    VFIOConfigWindowMatch matches[];
> +} VFIOConfigWindowQuirk;
> +
> +extern const MemoryRegionOps vfio_generic_window_address_quirk;
> +extern const MemoryRegionOps vfio_generic_window_data_quirk;
> +
> +/*
> + * The generic mirror quirk handles devices which expose PCI config space
> + * through a region within a BAR.  When enabled, reads and writes are
> + * redirected through to emulated PCI config space.  XXX if PCI config space
> + * used memory regions, this could just be an alias.
> + */
> +typedef struct VFIOConfigMirrorQuirk {
> +    struct VFIOPCIDevice *vdev;
> +    uint32_t offset;
> +    uint8_t bar;
> +    MemoryRegion *mem;
> +    uint8_t data[];
> +} VFIOConfigMirrorQuirk;
> +
> +extern const MemoryRegionOps vfio_generic_mirror_quirk;
> +
> +#endif /* HW_VFIO_VFIO_PCI_QUIRKS_H */