| Message ID | 20250307180337.14811-1-philmd@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | hw/vfio: Build various objects once | expand |
Hello, On 3/7/25 19:03, Philippe Mathieu-Daudé wrote: > By doing the following changes: > - Clean some headers up > - Replace compile-time CONFIG_KVM check by kvm_enabled() > - Replace compile-time CONFIG_IOMMUFD check by iommufd_builtin() > we can build less vfio objects. > > Philippe Mathieu-Daudé (14): > hw/vfio/common: Include missing 'system/tcg.h' header > hw/vfio/spapr: Do not include <linux/kvm.h> > hw/vfio: Compile some common objects once > hw/vfio: Compile more objects once > hw/vfio: Compile iommufd.c once > system: Declare qemu_[min/max]rampagesize() in 'system/hostmem.h' > hw/vfio: Compile display.c once > system/kvm: Expose kvm_irqchip_[add,remove]_change_notifier() > hw/vfio/pci: Convert CONFIG_KVM check to runtime one > system/iommufd: Introduce iommufd_builtin() helper > hw/vfio/pci: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() > hw/vfio/ap: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() > hw/vfio/ccw: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() > hw/vfio/platform: Check CONFIG_IOMMUFD at runtime using > iommufd_builtin > > docs/devel/vfio-iommufd.rst | 2 +- > include/exec/ram_addr.h | 3 -- > include/system/hostmem.h | 3 ++ > include/system/iommufd.h | 8 +++++ > include/system/kvm.h | 8 ++--- > target/s390x/kvm/kvm_s390x.h | 2 +- > accel/stubs/kvm-stub.c | 12 ++++++++ > hw/ppc/spapr_caps.c | 1 + > hw/s390x/s390-virtio-ccw.c | 1 + > hw/vfio/ap.c | 27 ++++++++--------- > hw/vfio/ccw.c | 27 ++++++++--------- > hw/vfio/common.c | 1 + > hw/vfio/iommufd.c | 1 - > hw/vfio/migration.c | 1 - > hw/vfio/pci.c | 57 +++++++++++++++++------------------- > hw/vfio/platform.c | 25 ++++++++-------- > hw/vfio/spapr.c | 4 +-- > hw/vfio/meson.build | 33 ++++++++++++--------- > 18 files changed, 117 insertions(+), 99 deletions(-) > Patches 1-9 look ok and should be considered for the next PR if maintainers ack patch 6 and 8. Some comments, vfio-amd-xgbe and vfio-calxeda-xgmac should be treated like vfio-platform, and since vfio-platform was designed for aarch64, these devices should not be available on arm, ppc, ppc64, riscv*, loongarch. That said, vfio-platform and devices being deprecated in the QEMU 10.0 cycle, we could just wait for the removal in QEMU 10.2. How could we (simply) remove CONFIG_VFIO_IGD in hw/vfio/pci-quirks.c ? and compile this file only once. The vfio-pci devices are available in nearly all targets when it only makes sense to have them in i386, x86_64, aarch64, ppc64, where they are supported, and also possibly in ppc (tcg) and arm (tcg) for historical reasons and just because they happen to work. ppc (tcg) doesn't support MSIs with vfio-pci devices so I don't think we care much. Patches 10-14 are wrong because they remove the "iommufd" property of the "vfio-*" devices. We can't take these. Thanks, C.
On Sat, 8 Mar 2025, Cédric Le Goater wrote: > Hello, > > On 3/7/25 19:03, Philippe Mathieu-Daudé wrote: >> By doing the following changes: >> - Clean some headers up >> - Replace compile-time CONFIG_KVM check by kvm_enabled() >> - Replace compile-time CONFIG_IOMMUFD check by iommufd_builtin() >> we can build less vfio objects. >> >> Philippe Mathieu-Daudé (14): >> hw/vfio/common: Include missing 'system/tcg.h' header >> hw/vfio/spapr: Do not include <linux/kvm.h> >> hw/vfio: Compile some common objects once >> hw/vfio: Compile more objects once >> hw/vfio: Compile iommufd.c once >> system: Declare qemu_[min/max]rampagesize() in 'system/hostmem.h' >> hw/vfio: Compile display.c once >> system/kvm: Expose kvm_irqchip_[add,remove]_change_notifier() >> hw/vfio/pci: Convert CONFIG_KVM check to runtime one >> system/iommufd: Introduce iommufd_builtin() helper >> hw/vfio/pci: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() >> hw/vfio/ap: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() >> hw/vfio/ccw: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() >> hw/vfio/platform: Check CONFIG_IOMMUFD at runtime using >> iommufd_builtin >> >> docs/devel/vfio-iommufd.rst | 2 +- >> include/exec/ram_addr.h | 3 -- >> include/system/hostmem.h | 3 ++ >> include/system/iommufd.h | 8 +++++ >> include/system/kvm.h | 8 ++--- >> target/s390x/kvm/kvm_s390x.h | 2 +- >> accel/stubs/kvm-stub.c | 12 ++++++++ >> hw/ppc/spapr_caps.c | 1 + >> hw/s390x/s390-virtio-ccw.c | 1 + >> hw/vfio/ap.c | 27 ++++++++--------- >> hw/vfio/ccw.c | 27 ++++++++--------- >> hw/vfio/common.c | 1 + >> hw/vfio/iommufd.c | 1 - >> hw/vfio/migration.c | 1 - >> hw/vfio/pci.c | 57 +++++++++++++++++------------------- >> hw/vfio/platform.c | 25 ++++++++-------- >> hw/vfio/spapr.c | 4 +-- >> hw/vfio/meson.build | 33 ++++++++++++--------- >> 18 files changed, 117 insertions(+), 99 deletions(-) >> > > Patches 1-9 look ok and should be considered for the next PR if > maintainers ack patch 6 and 8. > > > Some comments, > > vfio-amd-xgbe and vfio-calxeda-xgmac should be treated like > vfio-platform, and since vfio-platform was designed for aarch64, > these devices should not be available on arm, ppc, ppc64, riscv*, > loongarch. That said, vfio-platform and devices being deprecated in > the QEMU 10.0 cycle, we could just wait for the removal in QEMU 10.2. > > How could we (simply) remove CONFIG_VFIO_IGD in hw/vfio/pci-quirks.c ? > and compile this file only once. > > The vfio-pci devices are available in nearly all targets when it > only makes sense to have them in i386, x86_64, aarch64, ppc64, > where they are supported, and also possibly in ppc (tcg) and arm > (tcg) for historical reasons and just because they happen to work. > ppc (tcg) doesn't support MSIs with vfio-pci devices so I don't > think we care much. Maybe it does not support MSI yet but if it can be fixed I might be interested later. But I don't know how that should work and what's needed for it in QEMU. There are ppc machines with PCIe ports (like sam460ex but I did not implement PCIe on it yet) and some GPUs could be used on those so having MSI might help or be needed. If it works on the real machine it could work on QEMU too. Currently people who tried GPU pass through on ppc told that it works but slow. I don't know the why or how to debug that but could it be due to missing MSI support? If so we might be interested to solve that eventually if possible. Regards, BALATON Zoltan
On 8/3/25 18:48, Cédric Le Goater wrote: > Hello, > > On 3/7/25 19:03, Philippe Mathieu-Daudé wrote: >> By doing the following changes: >> - Clean some headers up >> - Replace compile-time CONFIG_KVM check by kvm_enabled() >> - Replace compile-time CONFIG_IOMMUFD check by iommufd_builtin() >> we can build less vfio objects. >> >> Philippe Mathieu-Daudé (14): >> hw/vfio/common: Include missing 'system/tcg.h' header >> hw/vfio/spapr: Do not include <linux/kvm.h> >> hw/vfio: Compile some common objects once >> hw/vfio: Compile more objects once >> hw/vfio: Compile iommufd.c once >> system: Declare qemu_[min/max]rampagesize() in 'system/hostmem.h' >> hw/vfio: Compile display.c once >> system/kvm: Expose kvm_irqchip_[add,remove]_change_notifier() >> hw/vfio/pci: Convert CONFIG_KVM check to runtime one >> system/iommufd: Introduce iommufd_builtin() helper >> hw/vfio/pci: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() >> hw/vfio/ap: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() >> hw/vfio/ccw: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() >> hw/vfio/platform: Check CONFIG_IOMMUFD at runtime using >> iommufd_builtin >> >> docs/devel/vfio-iommufd.rst | 2 +- >> include/exec/ram_addr.h | 3 -- >> include/system/hostmem.h | 3 ++ >> include/system/iommufd.h | 8 +++++ >> include/system/kvm.h | 8 ++--- >> target/s390x/kvm/kvm_s390x.h | 2 +- >> accel/stubs/kvm-stub.c | 12 ++++++++ >> hw/ppc/spapr_caps.c | 1 + >> hw/s390x/s390-virtio-ccw.c | 1 + >> hw/vfio/ap.c | 27 ++++++++--------- >> hw/vfio/ccw.c | 27 ++++++++--------- >> hw/vfio/common.c | 1 + >> hw/vfio/iommufd.c | 1 - >> hw/vfio/migration.c | 1 - >> hw/vfio/pci.c | 57 +++++++++++++++++------------------- >> hw/vfio/platform.c | 25 ++++++++-------- >> hw/vfio/spapr.c | 4 +-- >> hw/vfio/meson.build | 33 ++++++++++++--------- >> 18 files changed, 117 insertions(+), 99 deletions(-) >> > > Patches 1-9 look ok and should be considered for the next PR if > maintainers ack patch 6 and 8. OK. > vfio-amd-xgbe and vfio-calxeda-xgmac should be treated like > vfio-platform, and since vfio-platform was designed for aarch64, > these devices should not be available on arm, ppc, ppc64, riscv*, > loongarch. That said, vfio-platform and devices being deprecated in > the QEMU 10.0 cycle, we could just wait for the removal in QEMU 10.2. > > How could we (simply) remove CONFIG_VFIO_IGD in hw/vfio/pci-quirks.c ? > and compile this file only once. > > The vfio-pci devices are available in nearly all targets when it > only makes sense to have them in i386, x86_64, aarch64, ppc64, > where they are supported, and also possibly in ppc (tcg) and arm > (tcg) for historical reasons and just because they happen to work. > ppc (tcg) doesn't support MSIs with vfio-pci devices so I don't > think we care much. > > Patches 10-14 are wrong because they remove the "iommufd" property of > the "vfio-*" devices. We can't take these. I suppose this is due to the wrong implementation of iommufd_builtin() I mentioned in patch #10, which check instance but not class. Thanks!
By doing the following changes: - Clean some headers up - Replace compile-time CONFIG_KVM check by kvm_enabled() - Replace compile-time CONFIG_IOMMUFD check by iommufd_builtin() we can build less vfio objects. Philippe Mathieu-Daudé (14): hw/vfio/common: Include missing 'system/tcg.h' header hw/vfio/spapr: Do not include <linux/kvm.h> hw/vfio: Compile some common objects once hw/vfio: Compile more objects once hw/vfio: Compile iommufd.c once system: Declare qemu_[min/max]rampagesize() in 'system/hostmem.h' hw/vfio: Compile display.c once system/kvm: Expose kvm_irqchip_[add,remove]_change_notifier() hw/vfio/pci: Convert CONFIG_KVM check to runtime one system/iommufd: Introduce iommufd_builtin() helper hw/vfio/pci: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() hw/vfio/ap: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() hw/vfio/ccw: Check CONFIG_IOMMUFD at runtime using iommufd_builtin() hw/vfio/platform: Check CONFIG_IOMMUFD at runtime using iommufd_builtin docs/devel/vfio-iommufd.rst | 2 +- include/exec/ram_addr.h | 3 -- include/system/hostmem.h | 3 ++ include/system/iommufd.h | 8 +++++ include/system/kvm.h | 8 ++--- target/s390x/kvm/kvm_s390x.h | 2 +- accel/stubs/kvm-stub.c | 12 ++++++++ hw/ppc/spapr_caps.c | 1 + hw/s390x/s390-virtio-ccw.c | 1 + hw/vfio/ap.c | 27 ++++++++--------- hw/vfio/ccw.c | 27 ++++++++--------- hw/vfio/common.c | 1 + hw/vfio/iommufd.c | 1 - hw/vfio/migration.c | 1 - hw/vfio/pci.c | 57 +++++++++++++++++------------------- hw/vfio/platform.c | 25 ++++++++-------- hw/vfio/spapr.c | 4 +-- hw/vfio/meson.build | 33 ++++++++++++--------- 18 files changed, 117 insertions(+), 99 deletions(-)