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