RE: [PATCH v7 00/27] vfio: Adopt iommufd

Duan, Zhenzhong posted 27 patches 1 year ago
Only 0 patches received!
There is a newer version of this series
RE: [PATCH v7 00/27] vfio: Adopt iommufd
Posted by Duan, Zhenzhong 1 year ago

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Wednesday, November 22, 2023 1:23 AM
>Subject: Re: [PATCH v7 00/27] vfio: Adopt iommufd
>
>Hello Zhenzhong
>
>On 11/21/23 09:43, Zhenzhong Duan wrote:
>> Hi,
>>
>> Thanks all for giving guides and comments on previous series, this is
>> the remaining part of the iommufd support.
>>
>> Besides suggested changes in v6, I'd like to highlight two changes
>> for final review:
>> 1. Instantiate can_be_deleted callback to fix race where iommufd object
>>     can be deleted before vfio device
>> 2. After careful re-thinking, I'd like to follow Nicolin's suggestion in v5,
>>     remove is_ioas check which indeed looks heavy just for tracepoint.
>>     In fact we can get corresponding info by looking over trace context.
>>
>> PATCH 1: Introduce iommufd object
>> PATCH 2-9: add IOMMUFD container and cdev support
>> PATCH 10-17: fd passing for cdev and linking to IOMMUFD
>> PATCH 18: make VFIOContainerBase parameter const
>> PATCH 19-21: Compile out for IOMMUFD for arm, s390x and x86
>> PATCH 22-26: vfio device init code cleanup
>> PATCH 27: add iommufd doc
>>
>>
>> We have done wide test with different combinations, e.g:
>> - PCI device were tested
>> - FD passing and hot reset with some trick.
>> - device hotplug test with legacy and iommufd backends
>> - with or without vIOMMU for legacy and iommufd backends
>> - divices linked to different iommufds
>> - VFIO migration with a E800 net card(no dirty sync support) passthrough
>> - platform, ccw and ap were only compile-tested due to environment limit
>> - test mdev pass through with mtty and mix with real device and different BE
>> - test iommufd object hotplug/unplug and mix with vfio device plug/unplug
>>
>> Given some iommufd kernel limitations, the iommufd backend is
>> not yet fully on par with the legacy backend w.r.t. features like:
>> - p2p mappings (you will see related error traces)
>> - dirty page sync
>> - and etc.
>>
>>
>> qemu code:
>https://github.com/yiliu1765/qemu/commits/zhenzhong/iommufd_cdev_v7
>> Based on vfio-next, commit id: c487fb8a50
>
>The series is pushed on top of vfio-next in the vfio-8.2 tree :
>
>   https://github.com/legoater/qemu/commits/vfio-8.2
>
>with a little extra to deal with a PPC build failure.

Thanks Cédric. That's strange I didn't see this failure on my env
which has CONFIG_VFIO_PCI enabled by default for PPC.

BRs.
Zhenzhong
Re: [PATCH v7 00/27] vfio: Adopt iommufd
Posted by Cédric Le Goater 1 year ago
>> The series is pushed on top of vfio-next in the vfio-8.2 tree :
>>
>>    https://github.com/legoater/qemu/commits/vfio-8.2
>>
>> with a little extra to deal with a PPC build failure.
> 
> Thanks Cédric. That's strange I didn't see this failure on my env
> which has CONFIG_VFIO_PCI enabled by default for PPC.


The compile issue shows with --without-default-devices.

VFIO is always selected (see ppc/Kconfig) but VFIO_PCI is not when
--without-default-devices is used. Hence the compile failure because
of the common vfio-pci routines exported in pci.c.

Ideally, we should use an 'imply VFIO' in ppc/Kconfig because VFIO
is not a required subsystem for the pseries machine. If that was
the case, we wouldn't compile the VFIO EEH hooks defined in
hw/ppc/spapr_pci_vfio.c :

   bool spapr_phb_eeh_available(SpaprPhbState *sphb);
   int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
                                     unsigned int addr, int option);
   int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state);
   int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option);
   int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb);
   void spapr_phb_vfio_reset(DeviceState *qdev);

It is not that simple to fix. The simpler approach is to force compile
of VFIO PCI in ppc/Kconfig with a 'select VFIO_PCI'. we should improve
that.

Thanks,

C.



RE: [PATCH v7 00/27] vfio: Adopt iommufd
Posted by Duan, Zhenzhong 1 year ago

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Wednesday, November 22, 2023 4:07 PM
>Subject: Re: [PATCH v7 00/27] vfio: Adopt iommufd
>
>
>>> The series is pushed on top of vfio-next in the vfio-8.2 tree :
>>>
>>>    https://github.com/legoater/qemu/commits/vfio-8.2
>>>
>>> with a little extra to deal with a PPC build failure.
>>
>> Thanks Cédric. That's strange I didn't see this failure on my env
>> which has CONFIG_VFIO_PCI enabled by default for PPC.
>
>
>The compile issue shows with --without-default-devices.
>
>VFIO is always selected (see ppc/Kconfig) but VFIO_PCI is not when
>--without-default-devices is used. Hence the compile failure because
>of the common vfio-pci routines exported in pci.c.

Clear, thanks

>
>Ideally, we should use an 'imply VFIO' in ppc/Kconfig because VFIO
>is not a required subsystem for the pseries machine. If that was
>the case, we wouldn't compile the VFIO EEH hooks defined in
>hw/ppc/spapr_pci_vfio.c :
>
>   bool spapr_phb_eeh_available(SpaprPhbState *sphb);
>   int spapr_phb_vfio_eeh_set_option(SpaprPhbState *sphb,
>                                     unsigned int addr, int option);
>   int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state);
>   int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option);
>   int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb);
>   void spapr_phb_vfio_reset(DeviceState *qdev);

Indeed, I reproduced same after some try out.
Maybe need a stub file for those functions.

>
>It is not that simple to fix. The simpler approach is to force compile
>of VFIO PCI in ppc/Kconfig with a 'select VFIO_PCI'. we should improve
>that.

Yes, not a blocking issue after your fix. I can have a try when I'm idle.

Thanks
Zhenzhong