Message-ID: <20200910094903.51deb038@x1.home>
Date:   Thu, 10 Sep 2020 09:49:03 -0600
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Xu Yilun <yilun.xu@...el.com>
Cc:     mdf@...nel.org, kwankhede@...dia.com, linux-fpga@...r.kernel.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org, trix@...hat.com,
        lgoncalv@...hat.com,
        Matthew Gerlach <matthew.gerlach@...ux.intel.com>,
        "Raj, Ashok" <ashok.raj@...el.com>
Subject: Re: [PATCH 3/3] Documentation: fpga: dfl: Add description for VFIO
  Mdev support

On Thu, 10 Sep 2020 16:32:30 +0800
Xu Yilun <yilun.xu@...el.com> wrote:

> Hi Alex:
> 
> Thanks for your quick response and is helpful to me. I did some more
> investigation and some comments inline.
> 
> On Tue, Sep 08, 2020 at 03:10:02PM -0600, Alex Williamson wrote:
> > On Tue,  8 Sep 2020 15:13:32 +0800
> > Xu Yilun <yilun.xu@...el.com> wrote:
> >   
> > > This patch adds description for VFIO Mdev support for dfl devices on
> > > dfl bus.
> > > 
> > > Signed-off-by: Xu Yilun <yilun.xu@...el.com>
> > > Signed-off-by: Matthew Gerlach <matthew.gerlach@...ux.intel.com>
> > > ---
> > >  Documentation/fpga/dfl.rst | 20 ++++++++++++++++++++
> > >  1 file changed, 20 insertions(+)
> > > 
> > > diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
> > > index 0404fe6..f077754 100644
> > > --- a/Documentation/fpga/dfl.rst
> > > +++ b/Documentation/fpga/dfl.rst
> > > @@ -502,6 +502,26 @@ FME Partial Reconfiguration Sub Feature driver (see drivers/fpga/dfl-fme-pr.c)
> > >  could be a reference.
> > >  
> > >  
> > > +VFIO Mdev support for DFL devices
> > > +=================================
> > > +As we introduced a dfl bus for private features, they could be added to dfl bus
> > > +as independent dfl devices. There is a requirement to handle these devices
> > > +either by kernel drivers or by direct access from userspace. Usually we bind
> > > +the kernel drivers to devices which provide board management functions, and
> > > +gives user direct access to devices which cooperate closely with user
> > > +controlled Accelerated Function Unit (AFU). We realize this with a VFIO Mdev
> > > +implementation. When we bind the vfio-mdev-dfl driver to a dfl device, it
> > > +realizes a group of callbacks and registers to the Mdev framework as a
> > > +parent (physical) device. It could then create one (available_instances == 1)
> > > +mdev device.
> > > +Since dfl devices are sub devices of FPGA DFL physical devices (e.g. PCIE
> > > +device), which provide no DMA isolation for each sub device, this may leads to
> > > +DMA isolation problem if a private feature is designed to be capable of DMA.
> > > +The AFU user could potentially access the whole device addressing space and
> > > +impact the private feature. So now the general HW design rule is, no DMA
> > > +capability for private features. It eliminates the DMA isolation problem.  
> > 
> > What's the advantage of entangling mdev/vfio in this approach versus
> > simply exposing the MMIO region of the device via sysfs (similar to a
> > resource file in pci-sysfs)?  This implementation doesn't support
> > interrupts, it doesn't support multiplexing of a device, it doesn't
> > perform any degree of mediation, it seems to simply say "please don't
> > do DMA".  I don't think that's acceptable for an mdev driver.  If you
> > want to play loose with isolation, do it somewhere else.  Thanks,  
> 
> The intention of the patchset is to enable the userspace drivers for dfl
> devices. The dfl devices are actually different IP blocks integrated in
> FPGA to support different board functionalities. They are sub devices of
> the FPGA PCIe device. Their mmio blocks are in PCIE bar regions. And we
> want some of the dfl devices handled by the userspace drivers.
> 
> Some dfl devices are capable of interrupt. I didn't add interrupt code
> in this patch cause now the IRQ capable dfl devices are all handled by
> kernel drivers. But as a generic FPGA platform, IRQ handling for userspace
> drivers should be supported.
> 
> And I can see there are several ways to enable the userspace driver.
> 
> 1. Some specific sysfs like pci do. But seems it is not the common way for
> userspace driver. It does't support interrupt. And potentially users
> operate on the same mmio region together with kernel driver at the same
> time.
> 
> 2. VFIO driver with NOIOMMU enabled. I think it meets our needs. Do you
> think it is good we implement an VFIO driver for dfl devices?
> 
> 3. VFIO mdev. I implemented it because it will not block us from lacking
> of valid iommu group. And since the driver didn't perform any mediation,
> I should give up.
> 
> 4. UIO driver. It should work. I'm wondering if option 2 covers the
> functionalities of UIO and has more enhancement. So option 2 may be
> better?
> 
> Thanks again for your time, and I really appreciate you would give some
> guide on it.


VFIO no-iommu was intended as a transition helper for platforms that do
not support an IOMMU, particularly running within a VM where we use
regular, IOMMU protected devices within the host, but allow no-iommu
within the guest such that the host is still protected from the guest
sandbox.  There should be no new use cases of no-iommu, it's unsafe, it
taints the kernel where it's used (guest in the above intended use
case).  If you intend long term distribution support of a solution,
VFIO no-iommu should not be considered an option.

VFIO mdev requires that the mdev vendor driver mediates access to the
device in order to provide isolation.  In the initial vGPU use cases,
we expect that isolation to be provided via devices specific means, ex.
GPU GART, but we've since included system level components like IOMMU
backing devices and auxiliary domains, the latter to make use of IOMMU
PASID support.

As implemented in this proposal, mdev is being used only to subvert the
IOMMU grouping requirements of VFIO in order to order to expose a
device that is potentially fully capable of DMA to userspace with no
isolation whatsoever.  If not for the IOMMU grouping, this driver could
simply be a VFIO bus driver making use of the vfio-platform interface.
Either way, without isolation, this does not belong in the realm of
VFIO.

Given your architecture, the only potentially valid mdev use case I can
see would be if the mdev vendor driver binds to the PCIe device,
allowing multiplexing of the parent device by carving out fpga
functional blocks from MMIO BAR space, and providing isolation by
enforcing that the parent device never enables bus master, assuming
that would prevent any of the fpga sub-components from performing DMA.

Are there worthwhile use cases of these fpga devices without DMA?

If you need DMA (or the device is potentially capable of DMA and
cannot be audited to prevent it) and cannot provide isolation then
please don't use VFIO or mdev, doing so would violate the notion of
secure userspace device access that we've worked to achieve in this
ecosystem.

If you choose another route, pci-sysfs already provides full BAR access
via the resource files in sysfs, but you could also expose individual
sysfs files with the same capabilities per fpga functional unit to
resolve the conflict between kernel and userspace "ownership".  UIO
might also be a solution.  This proposal to restrict userspace usage to
devices that don't perform DMA is akin to uio_pci_generic, where the
user is not expected to enable bus master, but nothing prevents them
from doing so and as a result it's a gateway for all sorts of
unsupportable drivers.  mdev should not be used to follow that example.
Thanks,

Alex

Powered by blists - more mailing lists