Reviewed-by: Zhen Lei
On 2022/4/4 19:27, John Garry wrote:
> Function iommu_group_store_type() supports changing the default domain
> of an IOMMU group.
>
> Many conditions need to be satisfied and steps taken for this action to be
> successful.
>
> Satisfying these conditions and steps will be required for setting other
> IOMMU group attributes, so factor into a common part and a part specific
> to update the IOMMU group attribute.
>
> No functional change intended.
>
> Some code comments are tidied up also.
>
> Signed-off-by: John Garry
> ---
> drivers/iommu/iommu.c | 96 ---
> 1 file changed, 62 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index f2c45b85b9fc..0dd766030baf 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -3000,21 +3000,57 @@ static int iommu_change_dev_def_domain(struct
> iommu_group *group,
> return ret;
> }
>
> +enum iommu_group_op {
> + CHANGE_GROUP_TYPE,
> +};
> +
> +static int __iommu_group_store_type(const char *buf, struct iommu_group
> *group,
> + struct device *dev)
> +{
> + int type;
> +
> + if (sysfs_streq(buf, "identity"))
> + type = IOMMU_DOMAIN_IDENTITY;
> + else if (sysfs_streq(buf, "DMA"))
> + type = IOMMU_DOMAIN_DMA;
> + else if (sysfs_streq(buf, "DMA-FQ"))
> + type = IOMMU_DOMAIN_DMA_FQ;
> + else if (sysfs_streq(buf, "auto"))
> + type = 0;
> + else
> + return -EINVAL;
> +
> + /*
> + * Check if the only device in the group still has a driver bound or
> + * we're transistioning from DMA -> DMA-FQ
> + */
> + if (device_is_bound(dev) && !(type == IOMMU_DOMAIN_DMA_FQ &&
> + group->default_domain->type == IOMMU_DOMAIN_DMA)) {
> + pr_err_ratelimited("Device is still bound to driver\n");
> + return -EINVAL;
> + }
> +
> + return iommu_change_dev_def_domain(group, dev, type);
> +}
> +
> /*
> * Changing the default domain through sysfs requires the users to unbind the
> * drivers from the devices in the iommu group, except for a DMA -> DMA-FQ
> - * transition. Return failure if this isn't met.
> + * transition. Changing or any other IOMMU group attribute still requires the
> + * user to unbind the drivers from the devices in the iommu group. Return
> + * failure if these conditions are not met.
> *
> * We need to consider the race between this and the device release path.
> * device_lock(dev) is used here to guarantee that the device release path
> * will not be entered at the same time.
> */
> -static ssize_t iommu_group_store_type(struct iommu_group *group,
> - const char *buf, size_t count)
> +static ssize_t iommu_group_store_common(struct iommu_group *group,
> + enum iommu_group_op op,
> + const char *buf, size_t count)
> {
> struct group_device *grp_dev;
> struct device *dev;
> - int ret, req_type;
> + int ret;
>
> if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO))
> return -EACCES;
> @@ -3022,27 +3058,16 @@ static ssize_t iommu_group_store_type(struct
> iommu_group *group,
> if (WARN_ON(!group))
> return -EINVAL;
>
> - if (sysfs_streq(buf, "identity"))
> - req_type = IOMMU_DOMAIN_IDENTITY;
> - else if (sysfs_streq(buf, "DMA"))
> - req_type = IOMMU_DOMAIN_DMA;
> - else if (sysfs_streq(buf, "DMA-FQ"))
> - req_type = IOMMU_DOMAIN_DMA_FQ;
> - else if (sysfs_streq(buf, "auto"))
> - req_type = 0;
> - else
> - return -EINVAL;
> -
> /*
>* Lock/Unlock the group mutex here before device lock to
> - * 1. Make sure that the iommu group has only one device (this is a
> + * 1. Make sure that the IOMMU group has only one device (this is a
>*prerequisite for step 2)
>* 2. Get struct *dev which is needed to lock device
>*/
> mutex_lock(>mutex);
> if (iommu_group_device_count(group) != 1) {
> mutex_unlock(>mutex);
> - pr_err_ratelimited("Cannot change default domain: Group has
> more than one device\n");
> + pr_err_ratelimited("Cannot change IOMMU group default domain
> attribute: Group has more than one device\n");
> return -EINVAL;
> }
>
> @@ -3054,16 +3079,16 @@ static ssize_t iommu_group_store_type(struct
> iommu_group *group,
> /*
>* Don't hold the group mutex because taking group mutex first and then
>* the device lock could potentially cause a deadlock as below. Assume
> - * two threads T1 and T2. T1 is trying to change default domain of an
> - * iommu group and T2 is trying to hot unplug a device or release [1] VF
> - * of a PCIe device which