Use the helper function mm_get_pasid() to get a mm assigned pasid
value. The motivation is to replace mm->pasid with an iommu private
data structure that is introduced in a later patch.
Reviewed-by: Lu Baolu <[email protected]>
Signed-off-by: Tina Zhang <[email protected]>
---
Changes in v4:
- Rebase to v6.6-rc1.
Changes in v2:
- Update commit message.
- Let mm_get_enqcmd_pasid() call mm_get_pasid() to get pasid.
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++++++------
drivers/iommu/iommu-sva.c | 12 ++++++------
include/linux/iommu.h | 10 +++++++++-
3 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 4d83edc2be99..dbb3af323ede 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -215,7 +215,7 @@ static void arm_smmu_mm_arch_invalidate_secondary_tlbs(struct mmu_notifier *mn,
smmu_domain);
}
- arm_smmu_atc_inv_domain(smmu_domain, mm->pasid, start, size);
+ arm_smmu_atc_inv_domain(smmu_domain, mm_get_pasid(mm), start, size);
}
static void arm_smmu_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
@@ -233,10 +233,10 @@ static void arm_smmu_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
* DMA may still be running. Keep the cd valid to avoid C_BAD_CD events,
* but disable translation.
*/
- arm_smmu_write_ctx_desc(smmu_domain, mm->pasid, &quiet_cd);
+ arm_smmu_write_ctx_desc(smmu_domain, mm_get_pasid(mm), &quiet_cd);
arm_smmu_tlb_inv_asid(smmu_domain->smmu, smmu_mn->cd->asid);
- arm_smmu_atc_inv_domain(smmu_domain, mm->pasid, 0, 0);
+ arm_smmu_atc_inv_domain(smmu_domain, mm_get_pasid(mm), 0, 0);
smmu_mn->cleared = true;
mutex_unlock(&sva_lock);
@@ -290,7 +290,7 @@ arm_smmu_mmu_notifier_get(struct arm_smmu_domain *smmu_domain,
goto err_free_cd;
}
- ret = arm_smmu_write_ctx_desc(smmu_domain, mm->pasid, cd);
+ ret = arm_smmu_write_ctx_desc(smmu_domain, mm_get_pasid(mm), cd);
if (ret)
goto err_put_notifier;
@@ -315,7 +315,7 @@ static void arm_smmu_mmu_notifier_put(struct arm_smmu_mmu_notifier *smmu_mn)
return;
list_del(&smmu_mn->list);
- arm_smmu_write_ctx_desc(smmu_domain, mm->pasid, NULL);
+ arm_smmu_write_ctx_desc(smmu_domain, mm_get_pasid(mm), NULL);
/*
* If we went through clear(), we've already invalidated, and no
@@ -323,7 +323,7 @@ static void arm_smmu_mmu_notifier_put(struct arm_smmu_mmu_notifier *smmu_mn)
*/
if (!smmu_mn->cleared) {
arm_smmu_tlb_inv_asid(smmu_domain->smmu, cd->asid);
- arm_smmu_atc_inv_domain(smmu_domain, mm->pasid, 0, 0);
+ arm_smmu_atc_inv_domain(smmu_domain, mm_get_pasid(mm), 0, 0);
}
/* Frees smmu_mn */
diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
index b78671a8a914..0f956ecd0c9b 100644
--- a/drivers/iommu/iommu-sva.c
+++ b/drivers/iommu/iommu-sva.c
@@ -23,7 +23,7 @@ static int iommu_sva_alloc_pasid(struct mm_struct *mm, struct device *dev)
mutex_lock(&iommu_sva_lock);
/* Is a PASID already associated with this mm? */
if (mm_valid_pasid(mm)) {
- if (mm->pasid >= dev->iommu->max_pasids)
+ if (mm_get_pasid(mm) >= dev->iommu->max_pasids)
ret = -EOVERFLOW;
goto out;
}
@@ -73,7 +73,7 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm
mutex_lock(&iommu_sva_lock);
/* Search for an existing domain. */
- domain = iommu_get_domain_for_dev_pasid(dev, mm->pasid,
+ domain = iommu_get_domain_for_dev_pasid(dev, mm_get_pasid(mm),
IOMMU_DOMAIN_SVA);
if (IS_ERR(domain)) {
ret = PTR_ERR(domain);
@@ -92,7 +92,7 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm
goto out_unlock;
}
- ret = iommu_attach_device_pasid(domain, dev, mm->pasid);
+ ret = iommu_attach_device_pasid(domain, dev, mm_get_pasid(mm));
if (ret)
goto out_free_domain;
domain->users = 1;
@@ -124,7 +124,7 @@ EXPORT_SYMBOL_GPL(iommu_sva_bind_device);
void iommu_sva_unbind_device(struct iommu_sva *handle)
{
struct iommu_domain *domain = handle->domain;
- ioasid_t pasid = domain->mm->pasid;
+ ioasid_t pasid = mm_get_pasid(domain->mm);
struct device *dev = handle->dev;
mutex_lock(&iommu_sva_lock);
@@ -141,7 +141,7 @@ u32 iommu_sva_get_pasid(struct iommu_sva *handle)
{
struct iommu_domain *domain = handle->domain;
- return domain->mm->pasid;
+ return mm_get_pasid(domain->mm);
}
EXPORT_SYMBOL_GPL(iommu_sva_get_pasid);
@@ -208,5 +208,5 @@ void mm_pasid_drop(struct mm_struct *mm)
if (likely(!mm_valid_pasid(mm)))
return;
- iommu_free_global_pasid(mm->pasid);
+ iommu_free_global_pasid(mm_get_pasid(mm));
}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index a4eab6697fe1..db2510da28c1 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1189,10 +1189,14 @@ static inline bool mm_valid_pasid(struct mm_struct *mm)
{
return mm->pasid != IOMMU_PASID_INVALID;
}
-static inline u32 mm_get_enqcmd_pasid(struct mm_struct *mm)
+static inline u32 mm_get_pasid(struct mm_struct *mm)
{
return mm->pasid;
}
+static inline u32 mm_get_enqcmd_pasid(struct mm_struct *mm)
+{
+ return mm_get_pasid(mm);
+}
void mm_pasid_drop(struct mm_struct *mm);
struct iommu_sva *iommu_sva_bind_device(struct device *dev,
struct mm_struct *mm);
@@ -1215,6 +1219,10 @@ static inline u32 iommu_sva_get_pasid(struct iommu_sva *handle)
}
static inline void mm_pasid_init(struct mm_struct *mm) {}
static inline bool mm_valid_pasid(struct mm_struct *mm) { return false; }
+static inline u32 mm_get_pasid(struct mm_struct *mm)
+{
+ return IOMMU_PASID_INVALID;
+}
static inline u32 mm_get_enqcmd_pasid(struct mm_struct *mm)
{
return IOMMU_PASID_INVALID;
--
2.34.1
On Tue, Sep 12, 2023 at 08:59:33PM +0800, Tina Zhang wrote:
> Use the helper function mm_get_pasid() to get a mm assigned pasid
> value. The motivation is to replace mm->pasid with an iommu private
> data structure that is introduced in a later patch.
>
> Reviewed-by: Lu Baolu <[email protected]>
> Signed-off-by: Tina Zhang <[email protected]>
> ---
>
> Changes in v4:
> - Rebase to v6.6-rc1.
>
> Changes in v2:
> - Update commit message.
> - Let mm_get_enqcmd_pasid() call mm_get_pasid() to get pasid.
>
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++++++------
> drivers/iommu/iommu-sva.c | 12 ++++++------
> include/linux/iommu.h | 10 +++++++++-
> 3 files changed, 21 insertions(+), 13 deletions(-)
Michael already has a series to remove this mis-use from the ARM
driver like you did for the Intel driver.
I think for the interm it is fine to just use mm_get_enqcmd_pasid()
and Michael can remove it.
It helps point out this is just wrong anyhow and better discourages
anyone else from using it.
> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
> index b78671a8a914..0f956ecd0c9b 100644
> --- a/drivers/iommu/iommu-sva.c
> +++ b/drivers/iommu/iommu-sva.c
> @@ -23,7 +23,7 @@ static int iommu_sva_alloc_pasid(struct mm_struct *mm, struct device *dev)
> mutex_lock(&iommu_sva_lock);
> /* Is a PASID already associated with this mm? */
> if (mm_valid_pasid(mm)) {
> - if (mm->pasid >= dev->iommu->max_pasids)
> + if (mm_get_pasid(mm) >= dev->iommu->max_pasids)
> ret = -EOVERFLOW;
>
> goto out;
> }
>
> @@ -73,7 +73,7 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm
>
> mutex_lock(&iommu_sva_lock);
> /* Search for an existing domain. */
> - domain = iommu_get_domain_for_dev_pasid(dev, mm->pasid,
> + domain = iommu_get_domain_for_dev_pasid(dev, mm_get_pasid(mm),
> IOMMU_DOMAIN_SVA);
> if (IS_ERR(domain)) {
> ret = PTR_ERR(domain);
> @@ -92,7 +92,7 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm
> goto out_unlock;
> }
>
> - ret = iommu_attach_device_pasid(domain, dev, mm->pasid);
> + ret = iommu_attach_device_pasid(domain, dev, mm_get_pasid(mm));
> if (ret)
> goto out_free_domain;
> domain->users = 1;
These ones are all manipulating the enqcmd_pasid really.. If it
wasn't for enqcmd then we'd simply make each device have its own PASID
and then we wouldn't have the issue that a valid pasid is incompatible
with a certain device.
Jason
On 9/22/23 03:02, Jason Gunthorpe wrote:
> On Tue, Sep 12, 2023 at 08:59:33PM +0800, Tina Zhang wrote:
>> Use the helper function mm_get_pasid() to get a mm assigned pasid
>> value. The motivation is to replace mm->pasid with an iommu private
>> data structure that is introduced in a later patch.
>>
>> Reviewed-by: Lu Baolu <[email protected]>
>> Signed-off-by: Tina Zhang <[email protected]>
>> ---
>>
>> Changes in v4:
>> - Rebase to v6.6-rc1.
>>
>> Changes in v2:
>> - Update commit message.
>> - Let mm_get_enqcmd_pasid() call mm_get_pasid() to get pasid.
>>
>> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 12 ++++++------
>> drivers/iommu/iommu-sva.c | 12 ++++++------
>> include/linux/iommu.h | 10 +++++++++-
>> 3 files changed, 21 insertions(+), 13 deletions(-)
>
> Michael already has a series to remove this mis-use from the ARM
> driver like you did for the Intel driver.
>
> I think for the interm it is fine to just use mm_get_enqcmd_pasid()
> and Michael can remove it.
>
> It helps point out this is just wrong anyhow and better discourages
> anyone else from using it.
Sure, I'll update the commit message.
Thanks,
-Tina
>
>> diff --git a/drivers/iommu/iommu-sva.c b/drivers/iommu/iommu-sva.c
>> index b78671a8a914..0f956ecd0c9b 100644
>> --- a/drivers/iommu/iommu-sva.c
>> +++ b/drivers/iommu/iommu-sva.c
>> @@ -23,7 +23,7 @@ static int iommu_sva_alloc_pasid(struct mm_struct *mm, struct device *dev)
>> mutex_lock(&iommu_sva_lock);
>> /* Is a PASID already associated with this mm? */
>> if (mm_valid_pasid(mm)) {
>> - if (mm->pasid >= dev->iommu->max_pasids)
>> + if (mm_get_pasid(mm) >= dev->iommu->max_pasids)
>> ret = -EOVERFLOW;
>>
>> goto out;
>> }
>>
>> @@ -73,7 +73,7 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm
>>
>> mutex_lock(&iommu_sva_lock);
>> /* Search for an existing domain. */
>> - domain = iommu_get_domain_for_dev_pasid(dev, mm->pasid,
>> + domain = iommu_get_domain_for_dev_pasid(dev, mm_get_pasid(mm),
>> IOMMU_DOMAIN_SVA);
>> if (IS_ERR(domain)) {
>> ret = PTR_ERR(domain);
>> @@ -92,7 +92,7 @@ struct iommu_sva *iommu_sva_bind_device(struct device *dev, struct mm_struct *mm
>> goto out_unlock;
>> }
>>
>> - ret = iommu_attach_device_pasid(domain, dev, mm->pasid);
>> + ret = iommu_attach_device_pasid(domain, dev, mm_get_pasid(mm));
>> if (ret)
>> goto out_free_domain;
>> domain->users = 1;
>
> These ones are all manipulating the enqcmd_pasid really.. If it
> wasn't for enqcmd then we'd simply make each device have its own PASID
> and then we wouldn't have the issue that a valid pasid is incompatible
> with a certain device.
>
> Jason