RE: [PATCH v3 01/14] vfio/type1: Refactor vfio_iommu_type1_ioctl()
> From: Alex Williamson
> Sent: Friday, July 3, 2020 5:21 AM
> To: Liu, Yi L
>
> On Wed, 24 Jun 2020 01:55:14 -0700
> Liu Yi L wrote:
>
> > This patch refactors the vfio_iommu_type1_ioctl() to use switch
> > instead of if-else, and each cmd got a helper function.
> >
> > Cc: Kevin Tian
> > CC: Jacob Pan
> > Cc: Alex Williamson
> > Cc: Eric Auger
> > Cc: Jean-Philippe Brucker
> > Cc: Joerg Roedel
> > Cc: Lu Baolu
> > Suggested-by: Christoph Hellwig
> > Signed-off-by: Liu Yi L
> > ---
> > drivers/vfio/vfio_iommu_type1.c | 392
> > ++--
> > 1 file changed, 213 insertions(+), 179 deletions(-)
>
> I can go ahead and grab this one for my v5.9 next branch. Thanks,
thanks, that would be great help. I'll monitor your next branch on github.
Regards,
Yi Liu
> Alex
>
> > diff --git a/drivers/vfio/vfio_iommu_type1.c
> > b/drivers/vfio/vfio_iommu_type1.c index 5e556ac..7accb59 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -2453,6 +2453,23 @@ static int vfio_domains_have_iommu_cache(struct
> vfio_iommu *iommu)
> > return ret;
> > }
> >
> > +static int vfio_iommu_type1_check_extension(struct vfio_iommu *iommu,
> > + unsigned long arg)
> > +{
> > + switch (arg) {
> > + case VFIO_TYPE1_IOMMU:
> > + case VFIO_TYPE1v2_IOMMU:
> > + case VFIO_TYPE1_NESTING_IOMMU:
> > + return 1;
> > + case VFIO_DMA_CC_IOMMU:
> > + if (!iommu)
> > + return 0;
> > + return vfio_domains_have_iommu_cache(iommu);
> > + default:
> > + return 0;
> > + }
> > +}
> > +
> > static int vfio_iommu_iova_add_cap(struct vfio_info_cap *caps,
> > struct vfio_iommu_type1_info_cap_iova_range *cap_iovas,
> > size_t size)
> > @@ -2529,238 +2546,255 @@ static int
> vfio_iommu_migration_build_caps(struct vfio_iommu *iommu,
> > return vfio_info_add_capability(caps, _mig.header,
> > sizeof(cap_mig)); }
> >
> > -static long vfio_iommu_type1_ioctl(void *iommu_data,
> > - unsigned int cmd, unsigned long arg)
> > +static int vfio_iommu_type1_get_info(struct vfio_iommu *iommu,
> > +unsigned long arg)
> > {
> > - struct vfio_iommu *iommu = iommu_data;
> > + struct vfio_iommu_type1_info info;
> > unsigned long minsz;
> > + struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> > + unsigned long capsz;
> > + int ret;
> >
> > - if (cmd == VFIO_CHECK_EXTENSION) {
> > - switch (arg) {
> > - case VFIO_TYPE1_IOMMU:
> > - case VFIO_TYPE1v2_IOMMU:
> > - case VFIO_TYPE1_NESTING_IOMMU:
> > - return 1;
> > - case VFIO_DMA_CC_IOMMU:
> > - if (!iommu)
> > - return 0;
> > - return vfio_domains_have_iommu_cache(iommu);
> > - default:
> > - return 0;
> > - }
> > - } else if (cmd == VFIO_IOMMU_GET_INFO) {
> > - struct vfio_iommu_type1_info info;
> > - struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> > - unsigned long capsz;
> > - int ret;
> > -
> > - minsz = offsetofend(struct vfio_iommu_type1_info,
> iova_pgsizes);
> > + minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes);
> >
> > - /* For backward compatibility, cannot require this */
> > - capsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
> > + /* For backward compatibility, cannot require this */
> > + capsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
> >
> > - if (copy_from_user(, (void __user *)arg, minsz))
> > - return -EFAULT;
> > + if (copy_from_user(, (void __user *)arg, minsz))
> > + return -EFAULT;
> >
> > - if (info.argsz < minsz)
> > - return -EINVAL;
> > + if (info.argsz < minsz)
> > + return -EINVAL;
> >
> > - if (info.argsz >= capsz) {
> > - minsz = capsz;
> > - info.cap_offset = 0; /* output, no-recopy necessary */
> > - }
> > + if (info.argsz >= capsz) {
> > + minsz = capsz;
> > + info.cap_offset = 0; /* output, no-recopy necessary */
> > + }
> >
> > - mutex_lock(>lock);
> > - info.flags = VFIO_IOMMU_INFO_PGSIZES;
> > + mutex_lock(>lock);
> > + info.flags = VFIO_IOMMU_INFO_PGSIZES;
> >
> > - info.iova_pgsizes = iommu->pgsize_bitmap;
> > + info.iova_pgsizes = iommu->pgsize_bitmap;
> >
> > - ret = vfio_iommu_migration_build_caps(iommu, );
> > + ret = vfio_iommu_migration_build_caps(iommu, );
> >
> > - if (!ret)
> > - ret = vfio_iommu_iova_build_caps(iommu, );
> > + if (!ret)
> > + ret = vfio_iommu_iova_build_caps(iommu, );
> >
> > - mutex_unlock(>lock);
>
Re: [PATCH v3 01/14] vfio/type1: Refactor vfio_iommu_type1_ioctl()
On Wed, 24 Jun 2020 01:55:14 -0700
Liu Yi L wrote:
> This patch refactors the vfio_iommu_type1_ioctl() to use switch instead of
> if-else, and each cmd got a helper function.
>
> Cc: Kevin Tian
> CC: Jacob Pan
> Cc: Alex Williamson
> Cc: Eric Auger
> Cc: Jean-Philippe Brucker
> Cc: Joerg Roedel
> Cc: Lu Baolu
> Suggested-by: Christoph Hellwig
> Signed-off-by: Liu Yi L
> ---
> drivers/vfio/vfio_iommu_type1.c | 392
> ++--
> 1 file changed, 213 insertions(+), 179 deletions(-)
I can go ahead and grab this one for my v5.9 next branch. Thanks,
Alex
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 5e556ac..7accb59 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -2453,6 +2453,23 @@ static int vfio_domains_have_iommu_cache(struct
> vfio_iommu *iommu)
> return ret;
> }
>
> +static int vfio_iommu_type1_check_extension(struct vfio_iommu *iommu,
> + unsigned long arg)
> +{
> + switch (arg) {
> + case VFIO_TYPE1_IOMMU:
> + case VFIO_TYPE1v2_IOMMU:
> + case VFIO_TYPE1_NESTING_IOMMU:
> + return 1;
> + case VFIO_DMA_CC_IOMMU:
> + if (!iommu)
> + return 0;
> + return vfio_domains_have_iommu_cache(iommu);
> + default:
> + return 0;
> + }
> +}
> +
> static int vfio_iommu_iova_add_cap(struct vfio_info_cap *caps,
>struct vfio_iommu_type1_info_cap_iova_range *cap_iovas,
>size_t size)
> @@ -2529,238 +2546,255 @@ static int vfio_iommu_migration_build_caps(struct
> vfio_iommu *iommu,
> return vfio_info_add_capability(caps, _mig.header, sizeof(cap_mig));
> }
>
> -static long vfio_iommu_type1_ioctl(void *iommu_data,
> -unsigned int cmd, unsigned long arg)
> +static int vfio_iommu_type1_get_info(struct vfio_iommu *iommu,
> + unsigned long arg)
> {
> - struct vfio_iommu *iommu = iommu_data;
> + struct vfio_iommu_type1_info info;
> unsigned long minsz;
> + struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> + unsigned long capsz;
> + int ret;
>
> - if (cmd == VFIO_CHECK_EXTENSION) {
> - switch (arg) {
> - case VFIO_TYPE1_IOMMU:
> - case VFIO_TYPE1v2_IOMMU:
> - case VFIO_TYPE1_NESTING_IOMMU:
> - return 1;
> - case VFIO_DMA_CC_IOMMU:
> - if (!iommu)
> - return 0;
> - return vfio_domains_have_iommu_cache(iommu);
> - default:
> - return 0;
> - }
> - } else if (cmd == VFIO_IOMMU_GET_INFO) {
> - struct vfio_iommu_type1_info info;
> - struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> - unsigned long capsz;
> - int ret;
> -
> - minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes);
> + minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes);
>
> - /* For backward compatibility, cannot require this */
> - capsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
> + /* For backward compatibility, cannot require this */
> + capsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
>
> - if (copy_from_user(, (void __user *)arg, minsz))
> - return -EFAULT;
> + if (copy_from_user(, (void __user *)arg, minsz))
> + return -EFAULT;
>
> - if (info.argsz < minsz)
> - return -EINVAL;
> + if (info.argsz < minsz)
> + return -EINVAL;
>
> - if (info.argsz >= capsz) {
> - minsz = capsz;
> - info.cap_offset = 0; /* output, no-recopy necessary */
> - }
> + if (info.argsz >= capsz) {
> + minsz = capsz;
> + info.cap_offset = 0; /* output, no-recopy necessary */
> + }
>
> - mutex_lock(>lock);
> - info.flags = VFIO_IOMMU_INFO_PGSIZES;
> + mutex_lock(>lock);
> + info.flags = VFIO_IOMMU_INFO_PGSIZES;
>
> - info.iova_pgsizes = iommu->pgsize_bitmap;
> + info.iova_pgsizes = iommu->pgsize_bitmap;
>
> - ret = vfio_iommu_migration_build_caps(iommu, );
> + ret = vfio_iommu_migration_build_caps(iommu, );
>
> - if (!ret)
> - ret = vfio_iommu_iova_build_caps(iommu, );
> + if (!ret)
> + ret = vfio_iommu_iova_build_caps(iommu, );
>
> - mutex_unlock(>lock);
> + mutex_unlock(>lock);
>
> - if (ret)
> - return ret;
> + if (ret)
> + return ret;
>
> - if (caps.size) {
> - info.flags |= VFIO_IOMMU_INFO_CAPS;
> + if
[PATCH v3 01/14] vfio/type1: Refactor vfio_iommu_type1_ioctl()
This patch refactors the vfio_iommu_type1_ioctl() to use switch instead of
if-else, and each cmd got a helper function.
Cc: Kevin Tian
CC: Jacob Pan
Cc: Alex Williamson
Cc: Eric Auger
Cc: Jean-Philippe Brucker
Cc: Joerg Roedel
Cc: Lu Baolu
Suggested-by: Christoph Hellwig
Signed-off-by: Liu Yi L
---
drivers/vfio/vfio_iommu_type1.c | 392 ++--
1 file changed, 213 insertions(+), 179 deletions(-)
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 5e556ac..7accb59 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -2453,6 +2453,23 @@ static int vfio_domains_have_iommu_cache(struct
vfio_iommu *iommu)
return ret;
}
+static int vfio_iommu_type1_check_extension(struct vfio_iommu *iommu,
+ unsigned long arg)
+{
+ switch (arg) {
+ case VFIO_TYPE1_IOMMU:
+ case VFIO_TYPE1v2_IOMMU:
+ case VFIO_TYPE1_NESTING_IOMMU:
+ return 1;
+ case VFIO_DMA_CC_IOMMU:
+ if (!iommu)
+ return 0;
+ return vfio_domains_have_iommu_cache(iommu);
+ default:
+ return 0;
+ }
+}
+
static int vfio_iommu_iova_add_cap(struct vfio_info_cap *caps,
struct vfio_iommu_type1_info_cap_iova_range *cap_iovas,
size_t size)
@@ -2529,238 +2546,255 @@ static int vfio_iommu_migration_build_caps(struct
vfio_iommu *iommu,
return vfio_info_add_capability(caps, _mig.header, sizeof(cap_mig));
}
-static long vfio_iommu_type1_ioctl(void *iommu_data,
- unsigned int cmd, unsigned long arg)
+static int vfio_iommu_type1_get_info(struct vfio_iommu *iommu,
+unsigned long arg)
{
- struct vfio_iommu *iommu = iommu_data;
+ struct vfio_iommu_type1_info info;
unsigned long minsz;
+ struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
+ unsigned long capsz;
+ int ret;
- if (cmd == VFIO_CHECK_EXTENSION) {
- switch (arg) {
- case VFIO_TYPE1_IOMMU:
- case VFIO_TYPE1v2_IOMMU:
- case VFIO_TYPE1_NESTING_IOMMU:
- return 1;
- case VFIO_DMA_CC_IOMMU:
- if (!iommu)
- return 0;
- return vfio_domains_have_iommu_cache(iommu);
- default:
- return 0;
- }
- } else if (cmd == VFIO_IOMMU_GET_INFO) {
- struct vfio_iommu_type1_info info;
- struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
- unsigned long capsz;
- int ret;
-
- minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes);
+ minsz = offsetofend(struct vfio_iommu_type1_info, iova_pgsizes);
- /* For backward compatibility, cannot require this */
- capsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
+ /* For backward compatibility, cannot require this */
+ capsz = offsetofend(struct vfio_iommu_type1_info, cap_offset);
- if (copy_from_user(, (void __user *)arg, minsz))
- return -EFAULT;
+ if (copy_from_user(, (void __user *)arg, minsz))
+ return -EFAULT;
- if (info.argsz < minsz)
- return -EINVAL;
+ if (info.argsz < minsz)
+ return -EINVAL;
- if (info.argsz >= capsz) {
- minsz = capsz;
- info.cap_offset = 0; /* output, no-recopy necessary */
- }
+ if (info.argsz >= capsz) {
+ minsz = capsz;
+ info.cap_offset = 0; /* output, no-recopy necessary */
+ }
- mutex_lock(>lock);
- info.flags = VFIO_IOMMU_INFO_PGSIZES;
+ mutex_lock(>lock);
+ info.flags = VFIO_IOMMU_INFO_PGSIZES;
- info.iova_pgsizes = iommu->pgsize_bitmap;
+ info.iova_pgsizes = iommu->pgsize_bitmap;
- ret = vfio_iommu_migration_build_caps(iommu, );
+ ret = vfio_iommu_migration_build_caps(iommu, );
- if (!ret)
- ret = vfio_iommu_iova_build_caps(iommu, );
+ if (!ret)
+ ret = vfio_iommu_iova_build_caps(iommu, );
- mutex_unlock(>lock);
+ mutex_unlock(>lock);
- if (ret)
- return ret;
+ if (ret)
+ return ret;
- if (caps.size) {
- info.flags |= VFIO_IOMMU_INFO_CAPS;
+ if (caps.size) {
+ info.flags |= VFIO_IOMMU_INFO_CAPS;
- if (info.argsz < sizeof(info) + caps.size) {
- info.argsz = sizeof(info) + caps.size;
- }
