Re: [PULL 00/21] vfio queue

[Eric Auger <eauger@redhat.com>]

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
>>>>>>
>>>>>
>>
>