Hi Cédric, On 10/6/23 19:08, Cédric Le Goater wrote: > Hello Eric, > > On 10/6/23 14:25, Eric Auger wrote: >> Hi Cédric, >> >> On 10/6/23 13:46, Eric Auger wrote: >>> Hi Cédric, >>> >>> On 10/6/23 13:42, Eric Auger wrote: >>>> Hi Cédric, >>>> >>>> On 10/6/23 12:33, Cédric Le Goater wrote: >>>>> On 10/6/23 08:19, Cédric Le Goater wrote: >>>>>> The following changes since commit >>>>>> 2f3913f4b2ad74baeb5a6f1d36efbd9ecdf1057d: >>>>>> >>>>>> Merge tag 'for_upstream' of >>>>>> https://git.kernel.org/pub/scm/virt/kvm/mst/qemu into staging >>>>>> (2023-10-05 09:01:01 -0400) >>>>>> >>>>>> are available in the Git repository at: >>>>>> >>>>>> https://github.com/legoater/qemu/ tags/pull-vfio-20231006 >>>>>> >>>>>> for you to fetch changes up to >>>>>> 6e86aaef9ac57066aa923211a164df95b7b3cdf7: >>>>>> >>>>>> vfio/common: Move legacy VFIO backend code into separate >>>>>> container.c (2023-10-05 22:04:52 +0200) >>>>>> >>>>>> ---------------------------------------------------------------- >>>>>> vfio queue: >>>>>> >>>>>> * Fix for VFIO display when using Intel vGPUs >>>>>> * Support for dynamic MSI-X >>>>>> * Preliminary work for IOMMUFD support >>>>> >>>>> Stefan, >>>>> >>>>> I just did some tests on z with passthough devices (PCI and AP) and >>>>> the series is not bisectable. QEMU crashes at patch : >>>>> >>>>> "vfio/pci: Introduce vfio_[attach/detach]_device". >>>>> >>>>> Also, with everything applied, the guest fails to start with : >>>>> >>>>> vfio: IRQ 0 not available (number of irqs 0) >>>>> >>>>> So, please hold on and sorry for the noise. I will start digging >>>>> on my side. >>>> I just tested with the head on vfio/pci: Introduce >>>> vfio_[attach/detach]_device, with PCIe assignment on ARM and I fail to >>>> reproduce the crash. >>>> >>>> Do you try hotplug or something simpler? >>> >>> also works for me with hotplug/hotunplug. Please let me know if I can >>> help. >> >> I think this is related to the error handling. >> >> if you hotplug a vfio-device and if this encounters an error, >> vfio_realize fails and you end at the 'error' label where the name of >> the device is freed: g_free(vbasedev->name); >> >> However I see that the vfio_finalize is called (Zhengzhong warned me !!) >> calls vfio_pci_put_device >> which calls g_free(vdev->vbasedev.name) again. >> please try adding >> vdev->vbasedev.name = NULL after freeing the name in vfio_realize error: >> so see if it fixes the crash. >> >> Then wrt irq stuff, I would be tempted to say it sounds unrelated to the >> iommufd prereq series but well. >> >> Please let me know how you want me to fix that mess, sorry. > > So, the issue was a bit complex to dig because it only crashed > with a s390 guest under libvirt. This is my all-in-one combo VM > which has 2 PCI, 2 AP, 1 CCW passthrough devices and the issue > is in AP I think. > > vfio_ap_realize lacks : > > @@ -188,6 +188,7 @@ static void vfio_ap_realize(DeviceState > error_report_err(err); > } > + return; Hum indeed! Thanks for fixing that. > error: > error_prepend(errp, VFIO_MSG_PREFIX, vbasedev->name); > g_free(vbasedev->name); > > > No error were to return and since everything was fine, the error > path generated a lot of mess indeed ! > > If you are ok with the above, I will squash the change in the > related patch and send a v2. Unfortunately this is not sufficient. There is another regression (crash) on a double free of vbasedev->name as I reported before. I was able to hit it on a failing hotplug. How do you want me to send the fix? I can resend the whole series of just fixes on the related patches.
Thanks Eric> Thanks, > > C. > > > > >> >> Eric >> >> >>> >>> Eric >>>> >>>> Thanks >>>> >>>> Eric >>>> >>>> >>>>> >>>>> Thanks, >>>>> >>>>> C. >>>>> >>>>>> ---------------------------------------------------------------- >>>>>> Alex Williamson (1): >>>>>> vfio/display: Fix missing update to set backing fields >>>>>> >>>>>> Eric Auger (7): >>>>>> scripts/update-linux-headers: Add iommufd.h >>>>>> vfio/common: Propagate KVM_SET_DEVICE_ATTR error if any >>>>>> vfio/common: Introduce >>>>>> vfio_container_add|del_section_window() >>>>>> vfio/pci: Introduce vfio_[attach/detach]_device >>>>>> vfio/platform: Use vfio_[attach/detach]_device >>>>>> vfio/ap: Use vfio_[attach/detach]_device >>>>>> vfio/ccw: Use vfio_[attach/detach]_device >>>>>> >>>>>> Jing Liu (4): >>>>>> vfio/pci: detect the support of dynamic MSI-X allocation >>>>>> vfio/pci: enable vector on dynamic MSI-X allocation >>>>>> vfio/pci: use an invalid fd to enable MSI-X >>>>>> vfio/pci: enable MSI-X in interrupt restoring on dynamic >>>>>> allocation >>>>>> >>>>>> Yi Liu (2): >>>>>> vfio/common: Move IOMMU agnostic helpers to a separate file >>>>>> vfio/common: Move legacy VFIO backend code into separate >>>>>> container.c >>>>>> >>>>>> Zhenzhong Duan (7): >>>>>> vfio/pci: rename vfio_put_device to vfio_pci_put_device >>>>>> linux-headers: Add iommufd.h >>>>>> vfio/common: Extract out vfio_kvm_device_[add/del]_fd >>>>>> vfio/common: Move VFIO reset handler registration to a group >>>>>> agnostic function >>>>>> vfio/common: Introduce a per container device list >>>>>> vfio/common: Store the parent container in VFIODevice >>>>>> vfio/common: Introduce a global VFIODevice list >>>>>> >>>>>> hw/vfio/pci.h | 1 + >>>>>> include/hw/vfio/vfio-common.h | 60 +- >>>>>> linux-headers/linux/iommufd.h | 444 +++++++++ >>>>>> hw/vfio/ap.c | 69 +- >>>>>> hw/vfio/ccw.c | 122 +-- >>>>>> hw/vfio/common.c | 1885 >>>>>> +++------------------------------------ >>>>>> hw/vfio/container.c | 1161 ++++++++++++++++++++++++ >>>>>> hw/vfio/display.c | 2 + >>>>>> hw/vfio/helpers.c | 612 +++++++++++++ >>>>>> hw/vfio/pci.c | 194 ++-- >>>>>> hw/vfio/platform.c | 43 +- >>>>>> hw/vfio/meson.build | 2 + >>>>>> hw/vfio/trace-events | 6 +- >>>>>> scripts/update-linux-headers.sh | 3 +- >>>>>> 14 files changed, 2580 insertions(+), 2024 deletions(-) >>>>>> create mode 100644 linux-headers/linux/iommufd.h >>>>>> create mode 100644 hw/vfio/container.c >>>>>> create mode 100644 hw/vfio/helpers.c >>>>>> >>>>> >> >
