Re: [PATCH v3] vfio-pci: Provide reviewers and acceptance criteria for vendor drivers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 14 2022, Alex Williamson <alex.williamson@xxxxxxxxxx> wrote:

> Vendor or device specific extensions for devices exposed to userspace
> through the vfio-pci-core library open both new functionality and new
> risks.  Here we attempt to provided formalized requirements and
> expectations to ensure that future drivers both collaborate in their
> interaction with existing host drivers, as well as receive additional
> reviews from community members with experience in this area.
>
> Cc: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Cc: Yishai Hadas <yishaih@xxxxxxxxxx>
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> Acked-by: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx>
> Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> ---

(...)

> diff --git a/Documentation/driver-api/vfio-pci-vendor-driver-acceptance.rst b/Documentation/driver-api/vfio-pci-vendor-driver-acceptance.rst
> new file mode 100644
> index 000000000000..3a108d748681
> --- /dev/null
> +++ b/Documentation/driver-api/vfio-pci-vendor-driver-acceptance.rst

What about Christoph's request to drop the "vendor" name?
vfio-pci-device-specific-driver-acceptance.rst would match the actual
title of the document, and the only drawback I see is that it is a bit
longer.

> @@ -0,0 +1,35 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Acceptance criteria for vfio-pci device specific driver variants
> +================================================================
> +
> +Overview
> +--------
> +The vfio-pci driver exists as a device agnostic driver using the
> +system IOMMU and relying on the robustness of platform fault
> +handling to provide isolated device access to userspace.  While the
> +vfio-pci driver does include some device specific support, further
> +extensions for yet more advanced device specific features are not
> +sustainable.  The vfio-pci driver has therefore split out
> +vfio-pci-core as a library that may be reused to implement features
> +requiring device specific knowledge, ex. saving and loading device
> +state for the purposes of supporting migration.
> +
> +In support of such features, it's expected that some device specific
> +variants may interact with parent devices (ex. SR-IOV PF in support of
> +a user assigned VF) or other extensions that may not be otherwise
> +accessible via the vfio-pci base driver.  Authors of such drivers
> +should be diligent not to create exploitable interfaces via such
> +interactions or allow unchecked userspace data to have an effect
> +beyond the scope of the assigned device.
> +
> +New driver submissions are therefore requested to have approval via
> +Sign-off/Acked-by/etc for any interactions with parent drivers.

s/Sign-off/Reviewed-by/ ?

I would not generally expect the reviewers listed to sign off on other
people's patches.

> +Additionally, drivers should make an attempt to provide sufficient
> +documentation for reviewers to understand the device specific
> +extensions, for example in the case of migration data, how is the
> +device state composed and consumed, which portions are not otherwise
> +available to the user via vfio-pci, what safeguards exist to validate
> +the data, etc.  To that extent, authors should additionally expect to
> +require reviews from at least one of the listed reviewers, in addition
> +to the overall vfio maintainer.
> diff --git a/Documentation/maintainer/maintainer-entry-profile.rst b/Documentation/maintainer/maintainer-entry-profile.rst
> index 5d5cc3acdf85..8b4971c7e3fa 100644
> --- a/Documentation/maintainer/maintainer-entry-profile.rst
> +++ b/Documentation/maintainer/maintainer-entry-profile.rst
> @@ -103,3 +103,4 @@ to do something different in the near future.
>     ../nvdimm/maintainer-entry-profile
>     ../riscv/patch-acceptance
>     ../driver-api/media/maintainer-entry-profile
> +   ../driver-api/vfio-pci-vendor-driver-acceptance
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4322b5321891..fd17d1891216 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20314,6 +20314,16 @@ F:	drivers/vfio/mdev/
>  F:	include/linux/mdev.h
>  F:	samples/vfio-mdev/
>  
> +VFIO PCI VENDOR DRIVERS

VFIO PCI DEVICE SPECIFIC DRIVERS ?

> +R:	Jason Gunthorpe <jgg@xxxxxxxxxx>
> +R:	Yishai Hadas <yishaih@xxxxxxxxxx>
> +R:	Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx>
> +R:	Kevin Tian <kevin.tian@xxxxxxxxx>
> +L:	kvm@xxxxxxxxxxxxxxx
> +S:	Maintained
> +P:	Documentation/driver-api/vfio-pci-vendor-driver-acceptance.rst
> +F:	drivers/vfio/pci/*/
> +
>  VFIO PLATFORM DRIVER
>  M:	Eric Auger <eric.auger@xxxxxxxxxx>
>  L:	kvm@xxxxxxxxxxxxxxx

Other than that, looks good to me (and thanks to the people volunteering
for review!)




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux