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

Jason Gunthorpe
2022-10-10T18:42:11Z

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;
}