| Message ID | 20250308230917.18907-13-philmd@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hw/vfio: Build various objects once | expand |
On 3/9/25 00:09, Philippe Mathieu-Daudé wrote: > Convert the compile time check on the CONFIG_VFIO_IGD definition > by a runtime one by calling vfio_igd_builtin(), which check > whether VFIO_IGD is built in a qemu-system binary. > > Add stubs to avoid when VFIO_IGD is not built in: I thought we were trying to avoid stubs in QEMU build. Did that change ? Thanks, C. > > /usr/bin/ld: libqemu-x86_64-softmmu.a.p/hw_vfio_pci-quirks.c.o: in function `vfio_bar_quirk_setup': > /usr/bin/ld: ../hw/vfio/pci-quirks.c:1216: undefined reference to `vfio_probe_igd_bar0_quirk' > /usr/bin/ld: ../hw/vfio/pci-quirks.c:1217: undefined reference to `vfio_probe_igd_bar4_quirk' > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > hw/vfio/pci-quirks.h | 6 ++++++ > hw/vfio/igd-stubs.c | 20 ++++++++++++++++++++ > hw/vfio/pci-quirks.c | 9 ++++----- > hw/vfio/meson.build | 3 +++ > 4 files changed, 33 insertions(+), 5 deletions(-) > create mode 100644 hw/vfio/igd-stubs.c > > diff --git a/hw/vfio/pci-quirks.h b/hw/vfio/pci-quirks.h > index fdaa81f00aa..dcdb1962600 100644 > --- a/hw/vfio/pci-quirks.h > +++ b/hw/vfio/pci-quirks.h > @@ -13,6 +13,7 @@ > #define HW_VFIO_VFIO_PCI_QUIRKS_H > > #include "qemu/osdep.h" > +#include "qom/object.h" > #include "exec/memop.h" > > /* > @@ -71,4 +72,9 @@ extern const MemoryRegionOps vfio_generic_mirror_quirk; > > #define TYPE_VFIO_PCI_IGD_LPC_BRIDGE "vfio-pci-igd-lpc-bridge" > > +static inline bool vfio_igd_builtin(void) > +{ > + return type_is_registered(TYPE_VFIO_PCI_IGD_LPC_BRIDGE); > +} > + > #endif /* HW_VFIO_VFIO_PCI_QUIRKS_H */ > diff --git a/hw/vfio/igd-stubs.c b/hw/vfio/igd-stubs.c > new file mode 100644 > index 00000000000..5d4e88aeb1b > --- /dev/null > +++ b/hw/vfio/igd-stubs.c > @@ -0,0 +1,20 @@ > +/* > + * IGD device quirk stubs > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + * > + * Copyright (C) Linaro, Ltd. > + */ > + > +#include "qemu/osdep.h" > +#include "pci.h" > + > +void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr) > +{ > + g_assert_not_reached(); > +} > + > +void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) > +{ > + g_assert_not_reached(); > +} > diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c > index c53591fe2ba..22cb35af8cc 100644 > --- a/hw/vfio/pci-quirks.c > +++ b/hw/vfio/pci-quirks.c > @@ -11,7 +11,6 @@ > */ > > #include "qemu/osdep.h" > -#include CONFIG_DEVICES > #include "exec/memop.h" > #include "qemu/units.h" > #include "qemu/log.h" > @@ -1213,10 +1212,10 @@ void vfio_bar_quirk_setup(VFIOPCIDevice *vdev, int nr) > vfio_probe_nvidia_bar5_quirk(vdev, nr); > vfio_probe_nvidia_bar0_quirk(vdev, nr); > vfio_probe_rtl8168_bar2_quirk(vdev, nr); > -#ifdef CONFIG_VFIO_IGD > - vfio_probe_igd_bar0_quirk(vdev, nr); > - vfio_probe_igd_bar4_quirk(vdev, nr); > -#endif > + if (vfio_igd_builtin()) { > + vfio_probe_igd_bar0_quirk(vdev, nr); > + vfio_probe_igd_bar4_quirk(vdev, nr); > + } > } > > void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr) > diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build > index a8939c83865..6ab711d0539 100644 > --- a/hw/vfio/meson.build > +++ b/hw/vfio/meson.build > @@ -17,6 +17,9 @@ specific_ss.add_all(when: 'CONFIG_VFIO', if_true: vfio_ss) > > system_ss.add(when: 'CONFIG_VFIO_XGMAC', if_true: files('calxeda-xgmac.c')) > system_ss.add(when: 'CONFIG_VFIO_AMD_XGBE', if_true: files('amd-xgbe.c')) > +system_ss.add(when: 'CONFIG_VFIO_IGD', if_false: files( > + 'igd-stubs.c', > +)) > system_ss.add(when: 'CONFIG_VFIO', if_true: files( > 'helpers.c', > 'container-base.c',
On 10/3/25 08:37, Cédric Le Goater wrote: > On 3/9/25 00:09, Philippe Mathieu-Daudé wrote: >> Convert the compile time check on the CONFIG_VFIO_IGD definition >> by a runtime one by calling vfio_igd_builtin(), which check >> whether VFIO_IGD is built in a qemu-system binary. >> >> Add stubs to avoid when VFIO_IGD is not built in: > > I thought we were trying to avoid stubs in QEMU build. Did that change ? Hmm so you want remove the VFIO_IGD Kconfig symbol and have it always builtin with VFIO. It might make sense for quirks, since vfio_realize() already checks for the VFIO_FEATURE_ENABLE_IGD_OPREGION feature. I'll see if there aren't other implications I missed. Thanks, Phil.
On 3/10/25 14:43, Philippe Mathieu-Daudé wrote: > On 10/3/25 08:37, Cédric Le Goater wrote: >> On 3/9/25 00:09, Philippe Mathieu-Daudé wrote: >>> Convert the compile time check on the CONFIG_VFIO_IGD definition >>> by a runtime one by calling vfio_igd_builtin(), which check >>> whether VFIO_IGD is built in a qemu-system binary. >>> >>> Add stubs to avoid when VFIO_IGD is not built in: >> >> I thought we were trying to avoid stubs in QEMU build. Did that change ? > > Hmm so you want remove the VFIO_IGD Kconfig symbol and have it always > builtin with VFIO. It might make sense for quirks, since vfio_realize() > already checks for the VFIO_FEATURE_ENABLE_IGD_OPREGION feature. I have explored this option in the past and it's much more work. Stubs are fine IMO, if we can have them, but I remember someone telling me (you ?) that we were trying to remove them. Thanks, C. > I'll see if there aren't other implications I missed. > > Thanks, > > Phil. >
On 3/10/25 21:43, Philippe Mathieu-Daudé wrote: > On 10/3/25 08:37, Cédric Le Goater wrote: >> On 3/9/25 00:09, Philippe Mathieu-Daudé wrote: >>> Convert the compile time check on the CONFIG_VFIO_IGD definition >>> by a runtime one by calling vfio_igd_builtin(), which check >>> whether VFIO_IGD is built in a qemu-system binary. >>> >>> Add stubs to avoid when VFIO_IGD is not built in: >> >> I thought we were trying to avoid stubs in QEMU build. Did that change ? > > Hmm so you want remove the VFIO_IGD Kconfig symbol and have it always > builtin with VFIO. It might make sense for quirks, since vfio_realize() > already checks for the VFIO_FEATURE_ENABLE_IGD_OPREGION feature. > > I'll see if there aren't other implications I missed. > > Thanks, > > Phil. I personally suggest keeping the VFIO_IGD Kconfig symbol to prevent building IGD-specific code into non x86 target. The change Cedric mentioned in another reply [1] moves VFIO_FEATURE_ENABLE_IGD_OPREGION from vfio_realize() to igd.c. [1] https://lore.kernel.org/qemu-devel/20250306180131.32970-1-tomitamoeko@gmail.com/ Thanks, Moeko
On 10/3/25 14:51, Cédric Le Goater wrote: > On 3/10/25 14:43, Philippe Mathieu-Daudé wrote: >> On 10/3/25 08:37, Cédric Le Goater wrote: >>> On 3/9/25 00:09, Philippe Mathieu-Daudé wrote: >>>> Convert the compile time check on the CONFIG_VFIO_IGD definition >>>> by a runtime one by calling vfio_igd_builtin(), which check >>>> whether VFIO_IGD is built in a qemu-system binary. >>>> >>>> Add stubs to avoid when VFIO_IGD is not built in: >>> >>> I thought we were trying to avoid stubs in QEMU build. Did that change ? >> >> Hmm so you want remove the VFIO_IGD Kconfig symbol and have it always >> builtin with VFIO. It might make sense for quirks, since vfio_realize() >> already checks for the VFIO_FEATURE_ENABLE_IGD_OPREGION feature. > > I have explored this option in the past and it's much more work. > Stubs are fine IMO, if we can have them, but I remember someone > telling me (you ?) that we were trying to remove them. We shouldn't have target-specific stubs. (currently CONFIG_DEVICES::VFIO_IGD is target specific). I don't think we can avoid host-specific stubs. In unified binary, CONFIG_DEVICES disappears, VFIO_IGD will be handled like host configuration. Although if possible I'd rather remove VFIO_IGD, unconditionally including all quirks.
diff --git a/hw/vfio/pci-quirks.h b/hw/vfio/pci-quirks.h index fdaa81f00aa..dcdb1962600 100644 --- a/hw/vfio/pci-quirks.h +++ b/hw/vfio/pci-quirks.h @@ -13,6 +13,7 @@ #define HW_VFIO_VFIO_PCI_QUIRKS_H #include "qemu/osdep.h" +#include "qom/object.h" #include "exec/memop.h" /* @@ -71,4 +72,9 @@ extern const MemoryRegionOps vfio_generic_mirror_quirk; #define TYPE_VFIO_PCI_IGD_LPC_BRIDGE "vfio-pci-igd-lpc-bridge" +static inline bool vfio_igd_builtin(void) +{ + return type_is_registered(TYPE_VFIO_PCI_IGD_LPC_BRIDGE); +} + #endif /* HW_VFIO_VFIO_PCI_QUIRKS_H */ diff --git a/hw/vfio/igd-stubs.c b/hw/vfio/igd-stubs.c new file mode 100644 index 00000000000..5d4e88aeb1b --- /dev/null +++ b/hw/vfio/igd-stubs.c @@ -0,0 +1,20 @@ +/* + * IGD device quirk stubs + * + * SPDX-License-Identifier: GPL-2.0-or-later + * + * Copyright (C) Linaro, Ltd. + */ + +#include "qemu/osdep.h" +#include "pci.h" + +void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr) +{ + g_assert_not_reached(); +} + +void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr) +{ + g_assert_not_reached(); +} diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c index c53591fe2ba..22cb35af8cc 100644 --- a/hw/vfio/pci-quirks.c +++ b/hw/vfio/pci-quirks.c @@ -11,7 +11,6 @@ */ #include "qemu/osdep.h" -#include CONFIG_DEVICES #include "exec/memop.h" #include "qemu/units.h" #include "qemu/log.h" @@ -1213,10 +1212,10 @@ void vfio_bar_quirk_setup(VFIOPCIDevice *vdev, int nr) vfio_probe_nvidia_bar5_quirk(vdev, nr); vfio_probe_nvidia_bar0_quirk(vdev, nr); vfio_probe_rtl8168_bar2_quirk(vdev, nr); -#ifdef CONFIG_VFIO_IGD - vfio_probe_igd_bar0_quirk(vdev, nr); - vfio_probe_igd_bar4_quirk(vdev, nr); -#endif + if (vfio_igd_builtin()) { + vfio_probe_igd_bar0_quirk(vdev, nr); + vfio_probe_igd_bar4_quirk(vdev, nr); + } } void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr) diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build index a8939c83865..6ab711d0539 100644 --- a/hw/vfio/meson.build +++ b/hw/vfio/meson.build @@ -17,6 +17,9 @@ specific_ss.add_all(when: 'CONFIG_VFIO', if_true: vfio_ss) system_ss.add(when: 'CONFIG_VFIO_XGMAC', if_true: files('calxeda-xgmac.c')) system_ss.add(when: 'CONFIG_VFIO_AMD_XGBE', if_true: files('amd-xgbe.c')) +system_ss.add(when: 'CONFIG_VFIO_IGD', if_false: files( + 'igd-stubs.c', +)) system_ss.add(when: 'CONFIG_VFIO', if_true: files( 'helpers.c', 'container-base.c',
Convert the compile time check on the CONFIG_VFIO_IGD definition by a runtime one by calling vfio_igd_builtin(), which check whether VFIO_IGD is built in a qemu-system binary. Add stubs to avoid when VFIO_IGD is not built in: /usr/bin/ld: libqemu-x86_64-softmmu.a.p/hw_vfio_pci-quirks.c.o: in function `vfio_bar_quirk_setup': /usr/bin/ld: ../hw/vfio/pci-quirks.c:1216: undefined reference to `vfio_probe_igd_bar0_quirk' /usr/bin/ld: ../hw/vfio/pci-quirks.c:1217: undefined reference to `vfio_probe_igd_bar4_quirk' Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/vfio/pci-quirks.h | 6 ++++++ hw/vfio/igd-stubs.c | 20 ++++++++++++++++++++ hw/vfio/pci-quirks.c | 9 ++++----- hw/vfio/meson.build | 3 +++ 4 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 hw/vfio/igd-stubs.c