On Wed, 2014-05-28 at 02:44 +0200, Alexander Graf wrote:

On 28.05.14 02:39, Alex Williamson wrote:

quoted

On Wed, 2014-05-28 at 00:49 +0200, Alexander Graf wrote:

quoted

On 27.05.14 20:15, Alex Williamson wrote:

quoted

On Tue, 2014-05-27 at 18:40 +1000, Gavin Shan wrote:

quoted

The patch adds new IOCTL commands for sPAPR VFIO container device
to support EEH functionality for PCI devices, which have been passed
through from host to somebody else via VFIO.

Signed-off-by: Gavin Shan <redacted>
---
   Documentation/vfio.txt              | 92 ++++++++++++++++++++++++++++++++++++-
   drivers/vfio/pci/Makefile           |  1 +
   drivers/vfio/pci/vfio_pci.c         | 20 +++++---
   drivers/vfio/pci/vfio_pci_eeh.c     | 46 +++++++++++++++++++
   drivers/vfio/pci/vfio_pci_private.h |  5 ++
   drivers/vfio/vfio_iommu_spapr_tce.c | 85 ++++++++++++++++++++++++++++++++++
   include/uapi/linux/vfio.h           | 66 ++++++++++++++++++++++++++
   7 files changed, 308 insertions(+), 7 deletions(-)
   create mode 100644 drivers/vfio/pci/vfio_pci_eeh.c

[...]

quoted

quoted

+
+	return ret;
+}
+
   static long tce_iommu_ioctl(void *iommu_data,
   				 unsigned int cmd, unsigned long arg)
   {

@@ -283,6 +363,11 @@ static long tce_iommu_ioctl(void *iommu_data,
   		tce_iommu_disable(container);
   		mutex_unlock(&container->lock);
   		return 0;
+	case VFIO_EEH_PE_SET_OPTION:
+	case VFIO_EEH_PE_GET_STATE:
+	case VFIO_EEH_PE_RESET:
+	case VFIO_EEH_PE_CONFIGURE:
+		return tce_iommu_eeh_ioctl(iommu_data, cmd, arg);

This is where it would have really made sense to have a single
VFIO_EEH_OP ioctl with a data structure passed to indicate the sub-op.
AlexG, are you really attached to splitting these out into separate
ioctls?

I don't see the problem. We need to forward 4 ioctls to a separate piece
of code, so we forward 4 ioctls to a separate piece of code :). Putting
them into one ioctl just moves the switch() into another function.

And uses an extra 3 ioctl numbers and gives us extra things to update if
we ever need to add more ioctls, etc.  ioctl numbers are an address
space, how much address space do we really want to give to EEH?  It's
not a big difference, but I don't think it's completely even either.
Thanks,

Yes, that's the point. I by far prefer to have you push back on anyone 
who introduces useless ioctls rather than have a separate EEH number 
space that people can just throw anything in they like ;).

Well, I appreciate that, but having them as separate ioctls doesn't
really prevent that either.  Any one of these 4 could be set to take a
sub-option to extend and contort the EEH interface.  The only way to
prevent that would be to avoid the argsz+flags hack that make the ioctl
extendable.  Thanks,

Alex