Messages in this thread | | | Date | Thu, 16 Feb 2023 08:36:12 +0800 | Subject | Re: [PATCH 2/4] iommu: Use group ownership to avoid driver attachment | From | Baolu Lu <> |
| |
On 2/15/23 8:56 PM, Jason Gunthorpe wrote: > On Wed, Feb 15, 2023 at 01:51:14PM +0800, Baolu Lu wrote: >> On 2/13/23 10:19 PM, Jason Gunthorpe wrote: >>> On Mon, Feb 13, 2023 at 03:49:39PM +0800, Lu Baolu wrote: >>>> The iommu_group_store_type() requires the devices in the iommu group are >>>> not bound to any device driver during the whole operation. The existing >>>> code locks the device with device_lock(dev) and use device_is_bound() to >>>> check whether any driver is bound to device. >>>> >>>> In fact, this can be achieved through the DMA ownership helpers. Replace >>>> them with iommu_group_claim/release_dma_owner() helpers. >>>> >>>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com> >>>> --- >>>> drivers/iommu/iommu.c | 27 +++++++++++++-------------- >>>> 1 file changed, 13 insertions(+), 14 deletions(-) >>>> >>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >>>> index 4f71dcd2621b..6547cb38480c 100644 >>>> --- a/drivers/iommu/iommu.c >>>> +++ b/drivers/iommu/iommu.c >>>> @@ -2807,12 +2807,6 @@ static int iommu_change_dev_def_domain(struct iommu_group *group, >>>> mutex_lock(&group->mutex); >>>> - if (group->default_domain != group->domain) { >>>> - dev_err_ratelimited(prev_dev, "Group not assigned to default domain\n"); >>>> - ret = -EBUSY; >>>> - goto out; >>>> - } >>>> - >>>> /* >>>> * iommu group wasn't locked while acquiring device lock in >>>> * iommu_group_store_type(). So, make sure that the device count hasn't >>>> @@ -2971,6 +2965,7 @@ static void iommu_group_unfreeze_dev_ops(struct iommu_group *group) >>>> static ssize_t iommu_group_store_type(struct iommu_group *group, >>>> const char *buf, size_t count) >>>> { >>>> + bool group_owner_claimed = false; >>>> struct group_device *grp_dev; >>>> struct device *dev; >>>> int ret, req_type; >>>> @@ -2992,6 +2987,14 @@ static ssize_t iommu_group_store_type(struct iommu_group *group, >>>> else >>>> return -EINVAL; >>>> + if (req_type != IOMMU_DOMAIN_DMA_FQ || >>>> + group->default_domain->type != IOMMU_DOMAIN_DMA) { >>>> + ret = iommu_group_claim_dma_owner(group, (void *)buf); >>>> + if (ret) >>>> + return ret; >>>> + group_owner_claimed = true; >>>> + } >>> I don't get it, this should be done unconditionally. If we couldn't >>> take ownership then we simply can't progress. >> The existing code allows the user to switch the default domain from >> strict to lazy invalidation mode. The default domain is not changed, >> hence it should be seamless and transparent to the device driver. > So make that a special case, get the group lock check if it is this > case and then just adjust it and exit, otherwise get ownership under > the group lock as discussed.
OK. Will do like this in the next version.
Best regards, baolu
| |