From: Baolu Lu baolu.lu@linux.intel.com Sent: Friday, August 4, 2023 9:59 AM
On 2023/8/3 22:31, Yi Liu wrote:
Under nested IOMMU translation, userspace owns the stage-1 translation table (e.g. the stage-1 page table of Intel VT-d or the context table of ARM SMMUv3, and etc.). Stage-1 translation tables are vendor specific, and need to be compatiable with the underlying IOMMU hardware. Hence,
compatible
Yes.
userspace should know the IOMMU hardware capability before creating and configuring the stage-1 translation table to kernel.
This adds IOMMU_GET_HW_INFO ioctl to query the IOMMU hardware information (a.k.a capability) for a given device. The returned data is vendor specific, userspace needs to decode it with the structure mapped by the @out_data_type field.
As only physical devices have IOMMU hardware, so this will return error if the given device is not a physical device.
Co-developed-by: Nicolin Chen nicolinc@nvidia.com Signed-off-by: Nicolin Chen nicolinc@nvidia.com Signed-off-by: Yi Liu yi.l.liu@intel.com
drivers/iommu/iommufd/main.c | 79 ++++++++++++++++++++++++++++++++++++ include/uapi/linux/iommufd.h | 36 ++++++++++++++++ 2 files changed, 115 insertions(+)
diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c index 94c498b8fdf6..a0302bcaa97c 100644 --- a/drivers/iommu/iommufd/main.c +++ b/drivers/iommu/iommufd/main.c @@ -17,6 +17,7 @@ #include <linux/bug.h> #include <uapi/linux/iommufd.h> #include <linux/iommufd.h> +#include "../iommu-priv.h"
#include "io_pagetable.h" #include "iommufd_private.h" @@ -177,6 +178,81 @@ static int iommufd_destroy(struct iommufd_ucmd *ucmd) return 0; }
+static int iommufd_zero_fill_user(void __user *ptr, size_t bytes) +{
- int index = 0;
- for (; index < bytes; index++) {
if (put_user(0, (uint8_t __user *)(ptr + index)))return -EFAULT;- }
- return 0;
+}
+static int iommufd_get_hw_info(struct iommufd_ucmd *ucmd) +{
- u32 hw_info_type = IOMMU_HW_INFO_TYPE_NONE;
- struct iommu_hw_info *cmd = ucmd->cmd;
- unsigned int length = 0, data_len;
- struct iommufd_device *idev;
- const struct iommu_ops *ops;
- void __user *user_ptr;
- void *data = NULL;
- int rc = 0;
- if (cmd->flags || cmd->__reserved || !cmd->data_len)
return -EOPNOTSUPP;- idev = iommufd_get_device(ucmd, cmd->dev_id);
- if (IS_ERR(idev))
return PTR_ERR(idev);- user_ptr = u64_to_user_ptr(cmd->data_ptr);
- ops = dev_iommu_ops(idev->dev);
- if (!ops->hw_info)
goto done;- data = ops->hw_info(idev->dev, &data_len, &hw_info_type);
- if (IS_ERR(data)) {
rc = PTR_ERR(data);goto out_err;Can kfree() handle a ERR_PTR input? I am afraid not,
/**
- kfree - free previously allocated memory
- @object: pointer returned by kmalloc() or kmem_cache_alloc()
- If @object is NULL, no operation is performed.
*/ void kfree(const void *object) { struct folio *folio; struct slab *slab; struct kmem_cache *s;
trace_kfree(_RET_IP_, object); if (unlikely(ZERO_OR_NULL_PTR(object))) return;So, perhaps we should add
data = NULL;
before goto out_err;
?
Yes.
- }
- /* driver has hw_info callback should have a unique hw_info_type */
- if (WARN_ON_ONCE(hw_info_type == IOMMU_HW_INFO_TYPE_NONE)) {
rc = -ENODEV;goto out_err;- }
- length = min(cmd->data_len, data_len);
- if (copy_to_user(user_ptr, data, length)) {
rc = -EFAULT;goto out_err;- }
+done:
- /*
* Zero the trailing bytes if the user buffer is bigger than the* data size kernel actually has.*/- if (length < cmd->data_len) {
rc = iommufd_zero_fill_user(user_ptr + length,cmd->data_len - length);if (rc)goto out_err;- }
- cmd->data_len = length;
- cmd->out_data_type = hw_info_type;
- rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
+out_err:
- kfree(data);
- iommufd_put_object(&idev->obj);
- return rc;
+}
Others look good to me, feel free to add:
Reviewed-by: Lu Baolu baolu.lu@linux.intel.com
after above are addressed.
Regards, Yi Liu