linux-kernel - Re: [RFC PATCH v3 1/9] vfio/pci: export vfio_pci_device public and add vfio_pci_device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200221061929.GC30338@joy-OptiPlex-7040>
Date:   Fri, 21 Feb 2020 01:19:29 -0500
From:   Yan Zhao <yan.y.zhao@...el.com>
To:     Alex Williamson <alex.williamson@...hat.com>
Cc:     "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "cohuck@...hat.com" <cohuck@...hat.com>,
        "zhenyuw@...ux.intel.com" <zhenyuw@...ux.intel.com>,
        "Wang, Zhi A" <zhi.a.wang@...el.com>,
        "Tian, Kevin" <kevin.tian@...el.com>,
        "He, Shaopeng" <shaopeng.he@...el.com>,
        "Liu, Yi L" <yi.l.liu@...el.com>
Subject: Re: [RFC PATCH v3 1/9] vfio/pci: export vfio_pci_device public and
 add vfio_pci_device_private

On Fri, Feb 21, 2020 at 05:00:11AM +0800, Alex Williamson wrote:
> On Tue, 11 Feb 2020 05:10:38 -0500
> Yan Zhao <yan.y.zhao@...el.com> wrote:
> 
> > (1) make vfio_pci_device public, so it is accessible from external code.
> > (2) add a private struct vfio_pci_device_private, which is only accessible
> > from internal code. It extends struct vfio_pci_device.
> > 
> > Signed-off-by: Yan Zhao <yan.y.zhao@...el.com>
> > ---
> >  drivers/vfio/pci/vfio_pci.c         | 256 +++++++++++++++-------------
> >  drivers/vfio/pci/vfio_pci_config.c  | 186 ++++++++++++--------
> >  drivers/vfio/pci/vfio_pci_igd.c     |  19 ++-
> >  drivers/vfio/pci/vfio_pci_intrs.c   | 186 +++++++++++---------
> >  drivers/vfio/pci/vfio_pci_nvlink2.c |  22 +--
> >  drivers/vfio/pci/vfio_pci_private.h |   7 +-
> >  drivers/vfio/pci/vfio_pci_rdwr.c    |  40 +++--
> >  include/linux/vfio.h                |   5 +
> >  8 files changed, 408 insertions(+), 313 deletions(-)
> 
> [SNIP!]
> 
> > diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> > index e42a711a2800..70a2b8fb6179 100644
> > --- a/include/linux/vfio.h
> > +++ b/include/linux/vfio.h
> > @@ -195,4 +195,9 @@ extern int vfio_virqfd_enable(void *opaque,
> >  			      void *data, struct virqfd **pvirqfd, int fd);
> >  extern void vfio_virqfd_disable(struct virqfd **pvirqfd);
> >  
> > +struct vfio_pci_device {
> > +	struct pci_dev			*pdev;
> > +	int				num_regions;
> > +	int				irq_type;
> > +};
> >  #endif /* VFIO_H */
> 
> Hi Yan,
> 
> Sorry for the delay.  I'm still not very happy with this result, I was
> hoping the changes could be done less intrusively.  Maybe here's
> another suggestion, why can't the vendor driver use a struct
> vfio_pci_device* as an opaque pointer?  If you only want these three
> things initially, I think this whole massive patch can be reduced to:
> 
> struct pci_dev *vfio_pci_pdev(struct vfio_pci_device *vdev)
> {
> 	return vdev->pdev;
> }
> EXPORT_SYMBOL_GPL(vfio_pci_dev);
> 
> int vfio_pci_num_regions(struct vfio_pci_device *vdev)
> {
> 	return vdev->num_regions;
> }
> EXPORT_SYMBOL_GPL(vfio_pci_num_region);
> 
> int vfio_pci_irq_type(struct vfio_pci_device *vdev)
> {
> 	return vdev->irq_type;
> }
> EXPORT_SYMBOL_GPL(vfio_pci_irq_type);
> 
> This is how vfio-pci works with vfio, we don't know a struct
> vfio_device as anything other than an opaque pointer and we have access
> function where we need to see some property of that object.
> 
> Patch 5/9 would become a vfio_pci_set_vendor_regions() interface.
>
you are right!
I'll change it to this way. Thanks.

Yan