RE: [RFC v2 1/2] vfio/pci: export common symbols in vfio-pci

["Liu, Yi L" <yi.l.liu@intel.com>]

> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Thursday, March 21, 2019 3:22 AM
> To: Liu, Yi L <yi.l.liu@intel.com>
> Subject: Re: [RFC v2 1/2] vfio/pci: export common symbols in vfio-pci
> 
> On Wed, 20 Mar 2019 11:49:37 +0000
> "Liu, Yi L" <yi.l.liu@intel.com> wrote:
> 
> > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > Sent: Wednesday, March 20, 2019 2:14 AM
> > > To: Liu, Yi L <yi.l.liu@intel.com>
> > > Subject: Re: [RFC v2 1/2] vfio/pci: export common symbols in vfio-pci
> > >
> > > On Tue, 12 Mar 2019 16:18:22 +0800
> > > "Liu, Yi L" <yi.l.liu@intel.com> wrote:
> > >
> > > > This patch exports the following symbols from vfio-pci driver
> > > > for vfio-pci alike driver. e.g. vfio-pci-mdev driver
> > > >
> > > > *) vfio_pci_set_vga_decode();
> > > > *) vfio_pci_release();
> > > > *) vfio_pci_open();
> > > > *) vfio_pci_register_dev_region();
> > > > *) vfio_pci_ioctl();
> > > > *) vfio_pci_rw();
> > > > *) vfio_pci_mmap();
> > > > *) vfio_pci_request();
> > > > *) vfio_pci_probe_misc();
> > > > *) vfio_pci_remove_misc();
> > > > *) vfio_err_handlers;
> > > > *) vfio_pci_reflck_attach();
> > > > *) vfio_pci_reflck_put();
> > >
> > > Exporting all these symbols scares me a bit.  They're GPL and we don't
> > > guarantee a kernel ABI, but I don't really want to support arbitrary
> > > use cases either.  What if we re-factored the shared bits into a common
> > > file and just linked them together?  Thanks,
> >
> > Hi, Alex,
> >
> > Before refactor the code, I'd like to check with you on the module
> > parameters for the two modules. The existing vfio-pci driver has
> > some module parameters. e.g. ids, nointxmask, disable_idle_d3.
> > For future usage and maintain, I think it is better to let the two
> > drivers have same parameters. However, I'm not 100% on whether
> > we want to allow user load vfio-pci.ko and vfio-pci-mdev.ko with
> > different parameter value? e.g. load vfio-pci.ko with nointxmask=false
> > while load vfio-pci-mdev.ko with nointxmask=true. How about your
> > opinion on it?
> 
> Hi Yi,
> 
> I agree that it makes sense to retain the module options for the mdev
> wrapped version, but I expect we should also allow dissimilar user
> settings.  If those lived in the common code that gets linked separately
> with each module, that should work fine, I think.  We can worry about
> refactoring for future driver that might not want those options later.

Hi Alex,

I tried to get a common file which includes the definitions of the module
options and the common interfaces and get it linked separately with each
module. It works well when linked separately by config the
CONFIG_VFIO_PCI=m and CONFIG_VFIO_PCI_MDEV=m in kernel
configuration file. CONFIG_VFIO_PCI_MDEV is a new Kconfig macro
for the mdev wrapped version driver. However, if building the vfio-pci
and the mdev wrapped version into kernel image (config the
CONFIG_VFIO_PCI=y and CONFIG_VFIO_PCI_MDEV=y), then the symbols
defined in the common file will be shared thus doesn't allow dissimilar
user settings.

Per my understanding, I think we expect to allow simultaneous usage of
the two drivers. So I think the way above doesn't meet our expectation.
I considered a possible proposal as below. May listen to your opinion
on it before heading to cook. Also, better idea is welcomed. :-)

- get a common file includes interfaces which are common and have
  input parameters to differentiate the calling from vfio-pci and the
  wrapped version. e.g. vfio_pci_rw(). may call it as vfio_pci_common.c.

- get another common file includes the definitions of the module options,
  and the functions which referred the options. Define all of them as static.
  may call it as common.c

- get vfio_pci.c which includes the module_init/exit interfaces and driver
  registration operations of vfio-pci.ko. This file should include the common.c
  above to have same module options with the mdev wrapped version.

- get vfio_pci_mdev.c which includes the module_init/exit interfaces and
  driver registration operations of vfio-pci-mdev.ko. It should also include
  the common.c above to have same module options with vfio-pci.ko.

- Makefile:
vfio-pci-y := vfio_pci.o vfio_pci_common.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
vfio-pci-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
vfio-pci-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o

vfio-pci-mdev-y := vfio_pci_mdev.o vfio_pci_common.o vfio_pci_intrs.o vfio_pci_rdwr.o vfio_pci_config.o
vfio-pci-mdev-$(CONFIG_VFIO_PCI_IGD) += vfio_pci_igd.o
vfio-pci-mdev-$(CONFIG_VFIO_PCI_NVLINK2) += vfio_pci_nvlink2.o

obj-$(CONFIG_VFIO_PCI) += vfio-pci.o
obj-$(CONFIG_VFIO_PCI_MDEV) += vfio-pci-mdev.o

Thanks,
Yi Liu