RE: [PATCH] vfio/qat: add PCI_IOV dependency

[Zeng, Xin]

On Wednesday, May 29, 2024 10:25 AM, Tian, Kevin <kevin.tian@xxxxxxxxx>
> To: Arnd Bergmann <arnd@xxxxxxxxxx>; Zeng, Xin <xin.zeng@xxxxxxxxx>;
> Cabiddu, Giovanni <giovanni.cabiddu@xxxxxxxxx>; Alex Williamson
> <alex.williamson@xxxxxxxxxx>; Cao, Yahui <yahui.cao@xxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>; Jason Gunthorpe <jgg@xxxxxxxx>;
> Yishai Hadas <yishaih@xxxxxxxxxx>; Shameer Kolothum
> <shameerali.kolothum.thodi@xxxxxxxxxx>; kvm@xxxxxxxxxxxxxxx; qat-
> linux <qat-linux@xxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH] vfio/qat: add PCI_IOV dependency
> 
> > From: Arnd Bergmann <arnd@xxxxxxxxxx>
> > Sent: Tuesday, May 28, 2024 8:05 PM
> >
> > From: Arnd Bergmann <arnd@xxxxxxxx>
> >
> > The newly added driver depends on the crypto driver, but it uses exported
> > symbols that are only available when IOV is also turned on:
> >
> > x86_64-linux-ld: drivers/vfio/pci/qat/main.o: in function
> > `qat_vf_pci_open_device':
> > main.c:(.text+0xd7): undefined reference to `qat_vfmig_open'
> > x86_64-linux-ld: drivers/vfio/pci/qat/main.o: in function
> > `qat_vf_pci_release_dev':
> > main.c:(.text+0x122): undefined reference to `qat_vfmig_cleanup'
> > x86_64-linux-ld: main.c:(.text+0x12d): undefined reference to
> > `qat_vfmig_destroy'
> > x86_64-linux-ld: drivers/vfio/pci/qat/main.o: in function
> > `qat_vf_resume_write':
> > main.c:(.text+0x308): undefined reference to `qat_vfmig_load_setup'
> > x86_64-linux-ld: drivers/vfio/pci/qat/main.o: in function
> > `qat_vf_save_device_data':
> > main.c:(.text+0x64c): undefined reference to `qat_vfmig_save_state'
> > x86_64-linux-ld: main.c:(.text+0x677): undefined reference to
> > `qat_vfmig_save_setup'
> > x86_64-linux-ld: drivers/vfio/pci/qat/main.o: in function
> > `qat_vf_pci_aer_reset_done':
> > main.c:(.text+0x82d): undefined reference to `qat_vfmig_reset'
> > x86_64-linux-ld: drivers/vfio/pci/qat/main.o: in function
> > `qat_vf_pci_close_device':
> > main.c:(.text+0x862): undefined reference to `qat_vfmig_close'
> > x86_64-linux-ld: drivers/vfio/pci/qat/main.o: in function
> > `qat_vf_pci_set_device_state':
> > main.c:(.text+0x9af): undefined reference to `qat_vfmig_suspend'
> > x86_64-linux-ld: main.c:(.text+0xa14): undefined reference to
> > `qat_vfmig_save_state'
> > x86_64-linux-ld: main.c:(.text+0xb37): undefined reference to
> > `qat_vfmig_resume'
> > x86_64-linux-ld: main.c:(.text+0xbc7): undefined reference to
> > `qat_vfmig_load_state'
> 
> at a glance those undefined symbols don't use any symbol under
> IOV. They are just wrappers to certain callbacks registered by
> by respective qat drivers which support migration.
> 
> Probably they'd better be moved out of CONFIG_PCI_IOV in
> "drivers/crypto/intel/qat/qat_common/Makefile" to remove
> this dependency in vfio variant driver.
> 

Thanks, Kevin :-). This dependency is like the relationship between the QAT vfio
variant driver and macro CRYPTO_DEV_QAT_4XXX. The variant driver doesn't
directly reference the symbols exported by module qat_4xxx which is protected
by CRYPTO_DEV_QAT_4XXX, but requires the module qat_4xxx at runtime so far.
Alex suggested to put CRYPTO_DEV_QAT_4XXX as the dependency of this variant
driver.
For CONFIG_PCI_IOV, if it is disabled, this variant driver doesn't serve the user as
well since no VFs will be created by QAT PF driver. To keep the consistency, it might
be right to make it as the dependency of this variant driver as Arnd pointed out.
What do you think?

Thanks,
Xin

> >
> > Add this as a second dependency.
> >
> > Fixes: bb208810b1ab ("vfio/qat: Add vfio_pci driver for Intel QAT SR-IOV VF
> > devices")
> > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> > ---
> >  drivers/vfio/pci/qat/Kconfig | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vfio/pci/qat/Kconfig b/drivers/vfio/pci/qat/Kconfig
> > index bf52cfa4b595..fae9d6cb8ccb 100644
> > --- a/drivers/vfio/pci/qat/Kconfig
> > +++ b/drivers/vfio/pci/qat/Kconfig
> > @@ -1,8 +1,9 @@
> >  # SPDX-License-Identifier: GPL-2.0-only
> >  config QAT_VFIO_PCI
> >  	tristate "VFIO support for QAT VF PCI devices"
> > -	select VFIO_PCI_CORE
> >  	depends on CRYPTO_DEV_QAT_4XXX
> > +	depends on PCI_IOV
> > +	select VFIO_PCI_CORE
> >  	help
> >  	  This provides migration support for Intel(R) QAT Virtual Function
> >  	  using the VFIO framework.
> > --
> > 2.39.2