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:
pgp7sVEepcpdB.pgp
Description: PGP signature
