Re: [PATCH v9 05/10] uacce: Enable IOMMU_DEV_FEAT_IOPF
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
Date: 2021-01-19 14:55:39
Also in:
linux-acpi, linux-devicetree, linux-iommu
On Fri, 8 Jan 2021 15:52:13 +0100 Jean-Philippe Brucker [off-list ref] wrote:
The IOPF (I/O Page Fault) feature is now enabled independently from the SVA feature, because some IOPF implementations are device-specific and do not require IOMMU support for PCIe PRI or Arm SMMU stall. Enable IOPF unconditionally when enabling SVA for now. In the future, if a device driver implementing a uacce interface doesn't need IOPF support, it will need to tell the uacce module, for example with a new flag. Signed-off-by: Jean-Philippe Brucker <redacted>
Hi Jean-Philippe, A minor suggestion inline but I'm not that bothered so either way looks good to me.
quoted hunk ↗ jump to hunk
--- Cc: Arnd Bergmann <arnd@arndb.de> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Zhangfei Gao <zhangfei.gao@linaro.org> Cc: Zhou Wang <wangzhou1@hisilicon.com> --- drivers/misc/uacce/uacce.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-)diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c index d07af4edfcac..41ef1eb62a14 100644 --- a/drivers/misc/uacce/uacce.c +++ b/drivers/misc/uacce/uacce.c@@ -385,6 +385,24 @@ static void uacce_release(struct device *dev) kfree(uacce); } +static unsigned int uacce_enable_sva(struct device *parent, unsigned int flags) +{ + if (!(flags & UACCE_DEV_SVA)) + return flags; + + flags &= ~UACCE_DEV_SVA; + + if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_IOPF)) + return flags; + + if (iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA)) { + iommu_dev_disable_feature(parent, IOMMU_DEV_FEAT_IOPF); + return flags; + } + + return flags | UACCE_DEV_SVA; +}
I'm a great fan of paired enable / disable functions. Whilst it would be trivial, maybe it is worth introducing uacce_disable_sva()? Also make that do the flags check internally to make it match up with the enable path.
quoted hunk ↗ jump to hunk
+ /** * uacce_alloc() - alloc an accelerator * @parent: pointer of uacce parent device@@ -404,11 +422,7 @@ struct uacce_device *uacce_alloc(struct device *parent, if (!uacce) return ERR_PTR(-ENOMEM); - if (flags & UACCE_DEV_SVA) { - ret = iommu_dev_enable_feature(parent, IOMMU_DEV_FEAT_SVA); - if (ret) - flags &= ~UACCE_DEV_SVA; - } + flags = uacce_enable_sva(parent, flags); uacce->parent = parent; uacce->flags = flags;@@ -432,8 +446,10 @@ struct uacce_device *uacce_alloc(struct device *parent, return uacce; err_with_uacce: - if (flags & UACCE_DEV_SVA) + if (flags & UACCE_DEV_SVA) { iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_SVA); + iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_IOPF); + } kfree(uacce); return ERR_PTR(ret); }@@ -487,8 +503,10 @@ void uacce_remove(struct uacce_device *uacce) mutex_unlock(&uacce->queues_lock); /* disable sva now since no opened queues */ - if (uacce->flags & UACCE_DEV_SVA) + if (uacce->flags & UACCE_DEV_SVA) { iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_SVA); + iommu_dev_disable_feature(uacce->parent, IOMMU_DEV_FEAT_IOPF); + } if (uacce->cdev) cdev_device_del(uacce->cdev, &uacce->dev);
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel