Re: [PATCH 0/2] VFIO: Accept IOMMU group (PE) ID

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1999 bytes --]

On Fri, Sep 18, 2015 at 09:47:32AM -0600, Alex Williamson wrote:
> On Fri, 2015-09-18 at 16:24 +1000, Gavin Shan wrote:
> > This allows to accept IOMMU group (PE) ID from the parameter from userland
> > when handling EEH operation so that the operation only affects the target
> > IOMMU group (PE). If the IOMMU group (PE) ID in the parameter from userland
> > is invalid, all IOMMU groups (PEs) attached to the specified container are
> > affected as before.
> > 
> > Gavin Shan (2):
> >   drivers/vfio: Support EEH API revision
> >   drivers/vfio: Support IOMMU group for EEH operations
> > 
> >  drivers/vfio/vfio_iommu_spapr_tce.c | 50 ++++++++++++++++++++++++++++++++++---
> >  drivers/vfio/vfio_spapr_eeh.c       | 46 ++++++++++++++++++++++------------
> >  include/linux/vfio.h                | 13 +++++++---
> >  include/uapi/linux/vfio.h           |  6 +++++
> >  4 files changed, 93 insertions(+), 22 deletions(-)
> 
> This interface is terrible.  A function named foo_enabled() should
> return a bool, yes or no, don't try to overload it to also return a
> version.

Sorry, that one's my fault.  I suggested that approach to Gavin
without really thinking it through.


> AFAICT, patch 2/2 breaks current users by changing the offset
> of the union in struct vfio_eeh_pe_err.

Yeah, this one's ugly.  We have to preserve the offset, but that means
putting the group in a very awkward place.  Especially since I'm not
sure if there even are any existing users of the single extant union
branch.

Sigh.

> Also, we generally pass group
> file descriptors rather than a group ID because we can prove the
> ownership of the group through the file descriptor and we don't need to
> worry about races with the group because we can hold a reference to it.
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]