linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: "Tian, Kevin" <kevin.tian@intel.com>
Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>,
	Jason Gunthorpe <jgg@nvidia.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"cohuck@redhat.com" <cohuck@redhat.com>,
	"mgurtovoy@nvidia.com" <mgurtovoy@nvidia.com>,
	"yishaih@nvidia.com" <yishaih@nvidia.com>,
	Linuxarm <linuxarm@huawei.com>,
	liulongfang <liulongfang@huawei.com>,
	"Zengtao (B)" <prime.zeng@hisilicon.com>,
	"Jonathan Cameron" <jonathan.cameron@huawei.com>,
	"Wangzhou (B)" <wangzhou1@hisilicon.com>,
	Xu Zaibo <xuzaibo@huawei.com>
Subject: Re: [PATCH v8 8/9] hisi_acc_vfio_pci: Add support for VFIO live migration
Date: Tue, 8 Mar 2022 12:33:12 -0700	[thread overview]
Message-ID: <20220308123312.1f4ba768.alex.williamson@redhat.com> (raw)
In-Reply-To: <BN9PR11MB5276EBE887402EBE22630BAB8C099@BN9PR11MB5276.namprd11.prod.outlook.com>

On Tue, 8 Mar 2022 08:11:11 +0000
"Tian, Kevin" <kevin.tian@intel.com> wrote:

> > From: Alex Williamson <alex.williamson@redhat.com>
> > Sent: Tuesday, March 8, 2022 3:53 AM  
> > >  
> > > > I think we still require acks from Bjorn and Zaibo for select patches
> > > > in this series.  
> > >
> > > I checked with Ziabo. He moved projects and is no longer looking into  
> > crypto stuff.  
> > > Wangzhou and LiuLongfang now take care of this. Received acks from  
> > Wangzhou  
> > > already and I will request Longfang to provide his. Hope that's ok.  
> > 
> > Maybe a good time to have them update MAINTAINERS as well.  Thanks,
> >   
> 
> I have one question here (similar to what we discussed for mdev before).
> 
> Now we are adding vendor specific drivers under /drivers/vfio. Two drivers
> on radar and more will come. Then what would be the criteria for 
> accepting such a driver? Do we prefer to a model in which the author should
> provide enough background for vfio community to understand how it works 
> or as done here just rely on the PF driver owner to cover device specific
> code?
> 
> If the former we may need document some process for what information
> is necessary and also need secure increased review bandwidth from key
> reviewers in vfio community.
> 
> If the latter then how can we guarantee no corner case overlooked by both
> sides (i.e. how to know the coverage of total reviews)? Another open is who
> from the PF driver sub-system should be considered as the one to give the
> green signal. If the sub-system maintainer trusts the PF driver owner and
> just pulls commits from him then having the r-b from the PF driver owner is
> sufficient. But if the sub-system maintainer wants to review detail change
> in every underlying driver then we probably also want to get the ack from
> the maintainer.
> 
> Overall I didn't mean to slow down the progress of this series. But above
> does be some puzzle occurred in my review. 😊

Hi Kevin,

Good questions, I'd like a better understanding of expectations as
well.  I think the intentions are the same as any other sub-system, the
drivers make use of shared interfaces and extensions and the role of
the sub-system should be to make sure those interfaces are used
correctly and extensions fit well within the overall design.  However,
just as the network maintainer isn't expected to fully understand every
NIC driver, I think/hope we have the same expectations here.  It's
certainly a benefit to the community and perceived trustworthiness if
each driver outlines its operating model and security nuances, but
those are only ever going to be the nuances identified by the people
who have the access and energy to evaluate the device.

It's going to be up to the community to try to determine that any new
drivers are seriously considering security and not opening any new gaps
relative to behavior using the base vfio-pci driver.  For the driver
examples we have, this seems a bit easier than evaluating an entire
mdev device because they're largely providing direct access to the
device rather than trying to multiplex a shared physical device.  We
can therefore focus on incremental functionality, as both drivers have
done, implementing a boilerplate vendor driver, then adding migration
support.  I imagine this won't always be the case though and some
drivers will re-implement much of the core to support further emulation
and shared resources.

So how do we as a community want to handle this?  I wouldn't mind, I'd
actually welcome, some sort of review requirement for new vfio vendor
driver variants.  Is that reasonable?  What would be the criteria?
Approval from the PF driver owner, if different/necessary, and at least
one unaffiliated reviewer (preferably an active vfio reviewer or
existing vfio variant driver owner/contributor)?  Ideas welcome.
Thanks,

Alex


  reply	other threads:[~2022-03-08 19:33 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-03 23:01 [PATCH v8 0/9] vfio/hisilicon: add ACC live migration driver Shameer Kolothum
2022-03-03 23:01 ` [PATCH v8 1/9] crypto: hisilicon/qm: Move the QM header to include/linux Shameer Kolothum
2022-03-04  9:03   ` Zhou Wang
2022-03-04 11:33     ` Shameerali Kolothum Thodi
2022-03-08  1:07   ` liulongfang
2022-03-08 10:27   ` yekai(A)
2022-03-03 23:01 ` [PATCH v8 2/9] crypto: hisilicon/qm: Move few definitions to common header Shameer Kolothum
2022-03-04  9:06   ` Zhou Wang
2022-03-03 23:01 ` [PATCH v8 3/9] hisi_acc_qm: Move VF PCI device IDs " Shameer Kolothum
2022-03-04  9:34   ` Zhou Wang
2022-03-04 11:35     ` Shameerali Kolothum Thodi
2022-03-07 17:53   ` Alex Williamson
2022-03-10 13:55     ` Shameerali Kolothum Thodi
2022-03-08  1:08   ` liulongfang
2022-03-08 10:28   ` yekai(A)
2022-03-03 23:01 ` [PATCH v8 4/9] hisi_acc_vfio_pci: add new vfio_pci driver for HiSilicon ACC devices Shameer Kolothum
2022-03-03 23:01 ` [PATCH v8 5/9] hisi_acc_vfio_pci: Restrict access to VF dev BAR2 migration region Shameer Kolothum
2022-03-08  1:11   ` liulongfang
2022-03-08  6:23   ` Tian, Kevin
2022-03-08  8:33     ` Shameerali Kolothum Thodi
2022-03-08 10:09       ` Tian, Kevin
2022-03-08 11:02         ` Shameerali Kolothum Thodi
2022-03-03 23:01 ` [PATCH v8 6/9] hisi_acc_vfio_pci: Add helper to retrieve the struct pci_driver Shameer Kolothum
2022-03-04  9:40   ` Zhou Wang
2022-03-04 11:31     ` Shameerali Kolothum Thodi
2022-03-08 10:28   ` yekai(A)
2022-03-08 12:02   ` liulongfang
2022-03-03 23:01 ` [PATCH v8 7/9] crypto: hisilicon/qm: Set the VF QM state register Shameer Kolothum
2022-03-04  9:43   ` Zhou Wang
2022-03-08  6:31   ` Tian, Kevin
2022-03-08  8:46     ` Shameerali Kolothum Thodi
2022-03-08 10:10       ` Tian, Kevin
2022-03-03 23:01 ` [PATCH v8 8/9] hisi_acc_vfio_pci: Add support for VFIO live migration Shameer Kolothum
2022-03-04  8:48   ` Shameerali Kolothum Thodi
2022-03-04 19:44     ` Alex Williamson
2022-03-04 20:36       ` Shameerali Kolothum Thodi
2022-03-04 20:40         ` Alex Williamson
2022-03-04 20:57   ` Jason Gunthorpe
2022-03-07 19:05     ` Alex Williamson
2022-03-07 19:29       ` Shameerali Kolothum Thodi
2022-03-07 19:52         ` Alex Williamson
2022-03-08  8:11           ` Tian, Kevin
2022-03-08 19:33             ` Alex Williamson [this message]
2022-03-09 10:11               ` Tian, Kevin
2022-03-10 20:49                 ` Alex Williamson
2022-03-11  8:52                   ` Cornelia Huck
2022-03-11 13:21                   ` Shameerali Kolothum Thodi
2022-03-14  3:40                   ` Tian, Kevin
2022-03-14 15:03                   ` Jason Gunthorpe
2022-03-08  9:46           ` liulongfang
2022-03-08  7:41   ` Tian, Kevin
2022-03-08  8:52     ` Shameerali Kolothum Thodi
2022-03-08 10:17       ` Tian, Kevin
2022-03-03 23:01 ` [PATCH v8 9/9] hisi_acc_vfio_pci: Use its own PCI reset_done error handler Shameer Kolothum
2022-03-04 20:54   ` Jason Gunthorpe
2022-03-08  1:14   ` liulongfang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220308123312.1f4ba768.alex.williamson@redhat.com \
    --to=alex.williamson@redhat.com \
    --cc=cohuck@redhat.com \
    --cc=jgg@nvidia.com \
    --cc=jonathan.cameron@huawei.com \
    --cc=kevin.tian@intel.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linuxarm@huawei.com \
    --cc=liulongfang@huawei.com \
    --cc=mgurtovoy@nvidia.com \
    --cc=prime.zeng@hisilicon.com \
    --cc=shameerali.kolothum.thodi@huawei.com \
    --cc=wangzhou1@hisilicon.com \
    --cc=xuzaibo@huawei.com \
    --cc=yishaih@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).