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

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:

quoted

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