Re: [RFC 4/5] vfio: amba: VFIO support for AMBA devices

[Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Tue, 2014-08-26 at 17:39 +0200, Antonios Motakis wrote:
> Hello WIll,
> 
> On Tue, Aug 26, 2014 at 12:50 PM, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org> wrote:
> > Hi Antonios,
> >
> > On Fri, Aug 22, 2014 at 10:01:27AM +0100, Antonios Motakis wrote:
> >> Add support for discovering AMBA devices with VFIO and handle them
> >> similarly to Linux platform devices.
> >
> > [...]
> >
> >> +static struct amba_id pl330_ids[] = {
> >> +     { 0, 0 },
> >> +};
> >> +
> >> +MODULE_DEVICE_TABLE(amba, pl330_ids);
> >> +
> >> +static struct amba_driver vfio_amba_driver = {
> >> +     .probe = vfio_amba_probe,
> >> +     .remove = vfio_amba_remove,
> >> +     .id_table = pl330_ids,
> >> +     .drv = {
> >> +             .name = "vfio-amba",
> >> +             .owner = THIS_MODULE,
> >> +     },
> >> +};
> >
> > I don't understand what you're doing with the IDs here. What's the point in
> > the empty list?
> >
> 
> The empty list means VFIO will not bind to any AMBA devices by
> default. If the user wants to use an AMBA device with VFIO, the idea
> is to use the driver override proposed in [RFC 1/5] driver core: amba:
> add device binding path 'driver_override'.
> 
> > This also raises a larger question about whether or not it's safe to allow
> > device passthrough of arbitrary platform devices with VFIO. In the absence
> > of a bus/device standard like PCI, I really think this should be in opt-in
> > decision, where certain platform drivers can declare that their device can
> > be safely used with passthrough.
> 
> In a sense the driver_override is already an opt-in, although under
> control of the user. I don't know if we want to take the decision for
> the user and whitelist the devices one can safely use, and reject any
> other device one might want to try. I would argue that if a user
> decides to use driver_override, he should be able to do that (and then
> report the consequences to us ;)
> 
> Some of the differences between PLATFORM devices, could be handled at
> the QEMU level. With the addition of the VFIO device node info patches
> we also proposed recently, any VFIO user essentially sees a bunch of
> MMIO regions, IRQs and some device node properties that describe the
> device. So a VFIO user (e.g. QEMU) has access to more or less the same
> stuff a kernel driver does.
> 
> I could see this breaking if a device tries to do memory accesses that
> don't go through the IOMMU (maybe if it is directly connected with
> another device), but in that situation that device is probably
> unusable with VFIO even if we special-case it. Any other ideas?

It would be a flaw in the IOMMU group support of the platform to not
expose lack of isolation between such devices.  The platform IOMMU group
code is where decisions should be made about whether a device or set of
devices is safe to expose through VFIO.  If devices are unsafe they
should not have an IOMMU group.  If there is lack of isolation between
devices, they should be members of the same IOMMU group.  Adjust how
IOMMU grouping works and VFIO will follow.  Thanks,

Alex