Re: [PATCH v6 04/33] hw/pci/pci: Factor out pci_bus_map_irqs() from pci_bus_irqs()

[Philippe Mathieu-Daudé]

On 9/1/23 18:23, Bernhard Beschow wrote:
> pci_bus_irqs() coupled together the assignment of pci_set_irq_fn and
> pci_map_irq_fn to a PCI bus. This coupling gets in the way when the
> pci_map_irq_fn is board-specific while the pci_set_irq_fn is device-
> specific.
>
> For example, both of QEMU's PIIX south bridge models have different
> pci_map_irq_fn implementations which are board-specific rather than
> device-specific. These implementations should therefore reside in board
> code. The pci_set_irq_fn's, however, should stay in the device models
> because they access memory internal to the model.
>
> Factoring out pci_bus_map_irqs() from pci_bus_irqs() allows the
> assignments to be decoupled, resolving the problem described above.
>
> Note also how pci_vpb_realize() which gets touched in this commit
> assigns different pci_map_irq_fn's depending on the board.
>
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/pci/pci.h    |  3 ++-
>   hw/i386/pc_q35.c        |  4 ++--
>   hw/isa/piix3.c          |  8 ++++----
>   hw/isa/piix4.c          |  3 ++-
>   hw/pci-host/raven.c     |  3 ++-
>   hw/pci-host/versatile.c |  3 ++-
>   hw/pci/pci.c            | 12 +++++++++---
>   hw/remote/machine.c     |  3 ++-
>   8 files changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 7048a373d1..85ee458cd2 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -282,8 +282,9 @@ PCIBus *pci_root_bus_new(DeviceState *parent, const char 
> *name,
>                            MemoryRegion *address_space_io,
>                            uint8_t devfn_min, const char *typename);
>   void pci_root_bus_cleanup(PCIBus *bus);
> -void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> +void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
>                     void *irq_opaque, int nirq);
> +void pci_bus_map_irqs(PCIBus *bus, pci_map_irq_fn map_irq);

I'm squashing:

-- >8 --
diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index fe1fdfb5f7..46171f22f7 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -667,4 +667,4 @@ void vfu_object_set_bus_irq(PCIBus *pci_bus)

-    pci_bus_irqs(pci_bus, vfu_object_set_irq, vfu_object_map_irq, pci_bus,
-                 max_bdf);
+    pci_bus_irqs(pci_bus, vfu_object_set_irq, , pci_bus, max_bdf);
+    pci_bus_map_irqs(pci_bus, vfu_object_map_irq);
 }
---

to fix:

../hw/remote/vfio-user-obj.c: In function ‘vfu_object_set_bus_irq’:
../hw/remote/vfio-user-obj.c:668:67: error: passing argument 4 of 
‘pci_bus_irqs’ makes integer from pointer without a cast 
[-Werror=int-conversion]
     pci_bus_irqs(pci_bus, vfu_object_set_irq, vfu_object_map_irq, pci_bus,
                                                                   ^~~~~~~
In file included from include/hw/pci/pci_device.h:4,
                 from include/hw/remote/iohub.h:14,
                 from include/hw/remote/machine.h:18,
                 from ../hw/remote/vfio-user-obj.c:43:
include/hw/pci/pci.h:286:41: note: expected ‘int’ but argument is of 
type ‘PCIBus *’ {aka ‘struct PCIBus *’}
                   void *irq_opaque, int nirq);
                                     ~~~~^~~~
../hw/remote/vfio-user-obj.c:668:5: error: too many arguments to 
function ‘pci_bus_irqs’
     pci_bus_irqs(pci_bus, vfu_object_set_irq, vfu_object_map_irq, pci_bus,
     ^~~~~~~~~~~~
In file included from include/hw/pci/pci_device.h:4,
                 from include/hw/remote/iohub.h:14,
                 from include/hw/remote/machine.h:18,
                 from ../hw/remote/vfio-user-obj.c:43:
include/hw/pci/pci.h:285:6: note: declared here
 void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq,
      ^~~~~~~~~~~~