Re: [PATCH v2 2/4] vfio: Move vfio_spapr_iommu_eeh_ioctl into vfio_iommu_spapr_tce.c

[Jason Gunthorpe <jgg@xxxxxxxxxx>]

On Mon, Oct 10, 2022 at 12:09:27AM -0700, Christoph Hellwig wrote:
> On Mon, Oct 03, 2022 at 12:39:31PM -0300, Jason Gunthorpe wrote:
> > So we don't need to worry about the fact that asm/eeh.h doesn't define
> > enough stuff to compile vfio_spapr_iommu_eeh_ioctl() in !CONFIG_EEH. If
> > someday someone changes the kconfig around they can also fix the ifdefs in
> > asm/eeh.h to compile this code too.
> 
> Please just break it up by opencoding the VFIO_CHECK_EXTENSION
> cases and just having a helper for VFIO_EEH_PE_OP.  I could look
> for my patch doing that but again it should be trivial to redo.

You mean to fold the case branches into the existing switch statements
in tce_iommu_ioctl()? Like below with indenting fixed?

diff --git a/drivers/vfio/vfio_iommu_spapr_tce.c b/drivers/vfio/vfio_iommu_spapr_tce.c
index 47a8b138cf7f6d..b6426109db5e0f 100644
--- a/drivers/vfio/vfio_iommu_spapr_tce.c
+++ b/drivers/vfio/vfio_iommu_spapr_tce.c
@@ -773,8 +773,8 @@ static long tce_iommu_create_default_window(struct tce_container *container)
 	return ret;
 }
 
-static long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
-				unsigned int cmd, unsigned long arg)
+static long vfio_spapr_ioctl_eeh_pe_op(struct iommu_group *group,
+				       unsigned long arg)
 {
 	struct eeh_pe *pe;
 	struct vfio_eeh_pe_op op;
@@ -784,14 +784,6 @@ static long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
 	if (!IS_ENABLED(CONFIG_EEH))
 		return -ENOTTY;
 
-	switch (cmd) {
-	case VFIO_CHECK_EXTENSION:
-		if (arg == VFIO_EEH)
-			ret = eeh_enabled() ? 1 : 0;
-		else
-			ret = 0;
-		break;
-	case VFIO_EEH_PE_OP:
 	pe = eeh_iommu_group_to_pe(group);
 	if (!pe)
 		return -ENODEV;
@@ -843,8 +835,6 @@ static long vfio_spapr_iommu_eeh_ioctl(struct iommu_group *group,
 	default:
 		ret = -EINVAL;
 	}
-	}
-
 	return ret;
 }
 
@@ -860,14 +850,14 @@ static long tce_iommu_ioctl(void *iommu_data,
 		switch (arg) {
 		case VFIO_SPAPR_TCE_IOMMU:
 		case VFIO_SPAPR_TCE_v2_IOMMU:
-			ret = 1;
-			break;
+			return 1;
+		case VFIO_EEH:
+			if (IS_ENABLED(CONFIG_EEH) && eeh_enabled)
+				return 1;
+			return 0;
 		default:
-			ret = vfio_spapr_iommu_eeh_ioctl(NULL, cmd, arg);
-			break;
+			return 0;
 		}
-
-		return (ret < 0) ? 0 : ret;
 	}
 
 	/*
@@ -1121,8 +1111,7 @@ static long tce_iommu_ioctl(void *iommu_data,
 
 		ret = 0;
 		list_for_each_entry(tcegrp, &container->group_list, next) {
-			ret = vfio_spapr_iommu_eeh_ioctl(tcegrp->grp,
-					cmd, arg);
+			ret = vfio_spapr_ioctl_eeh_pe_op(tcegrp->grp, arg);
 			if (ret)
 				return ret;
 		}