[PATCH v1 6/7] iommu/arm-smmu-v3: Refactor write_ctx_desc
Nicolin Chen
nicolinc at nvidia.com
Thu Jul 27 14:37:45 PDT 2023
On Fri, Jul 28, 2023 at 02:26:22AM +0800, Michael Shavit wrote:
> 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 968559d625c40..57073d278cd7e 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
> @@ -45,10 +45,12 @@ static struct arm_smmu_ctx_desc *
> arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
> {
> int ret;
> + unsigned long flags;
> u32 new_asid;
> struct arm_smmu_ctx_desc *cd;
> struct arm_smmu_device *smmu;
> struct arm_smmu_domain *smmu_domain;
> + struct arm_smmu_master *master;
It seems that the coding style at these struct lines is listing
from shorter to longer, like a Christmas tree? If so, we should
place "master" before "smmu_domain".
> @@ -80,7 +82,11 @@ arm_smmu_share_asid(struct mm_struct *mm, u16 asid)
> * be some overlap between use of both ASIDs, until we invalidate the
> * TLB.
> */
> - arm_smmu_write_ctx_desc(smmu_domain, 0, cd);
> + spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> + list_for_each_entry(master, &smmu_domain->devices, domain_head) {
> + arm_smmu_write_ctx_desc(master, 0, cd);
> + }
+ list_for_each_entry(master, &smmu_domain->devices, domain_head)
+ arm_smmu_write_ctx_desc(master, 0, cd);
> @@ -248,8 +260,10 @@ arm_smmu_mmu_notifier_get(struct arm_smmu_domain *smmu_domain,
> struct mm_struct *mm)
> {
> int ret;
> + unsigned long flags;
> struct arm_smmu_ctx_desc *cd;
> struct arm_smmu_mmu_notifier *smmu_mn;
> + struct arm_smmu_master *master;
For the coding style topic, similarly, "master" before "smmu_mn".
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index af7949b62327b..b211424a85fb2 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -971,14 +971,12 @@ void arm_smmu_tlb_inv_asid(struct arm_smmu_device *smmu, u16 asid)
> arm_smmu_cmdq_issue_cmd_with_sync(smmu, &cmd);
> }
>
> -static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
> +static void arm_smmu_sync_cd(struct arm_smmu_master *master,
> int ssid, bool leaf)
> {
> size_t i;
> - unsigned long flags;
> - struct arm_smmu_master *master;
> struct arm_smmu_cmdq_batch cmds;
> - struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct arm_smmu_device *smmu;
struct arm_smmu_device *smmu = master->smmu;
Then ...
> @@ -987,19 +985,15 @@ static void arm_smmu_sync_cd(struct arm_smmu_domain *smmu_domain,
> },
> };
>
> - if (!smmu_domain->cd_table.installed)
> + if (!master->domain->cd_table.installed)
> return;
>
> + smmu = master->smmu;
... no need of this line.
> @@ -1029,14 +1023,12 @@ static void arm_smmu_write_cd_l1_desc(__le64 *dst,
> WRITE_ONCE(*dst, cpu_to_le64(val));
> }
>
> -static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
> - u32 ssid)
> +static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_master *master, u32 ssid)
> {
> __le64 *l1ptr;
> unsigned int idx;
> struct arm_smmu_l1_ctx_desc *l1_desc;
> - struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_device *smmu = master->smmu;
Then ...
> @@ -1044,19 +1036,19 @@ static __le64 *arm_smmu_get_cd_ptr(struct arm_smmu_domain *smmu_domain,
> idx = ssid >> CTXDESC_SPLIT;
> l1_desc = &cdcfg->l1_desc[idx];
> if (!l1_desc->l2ptr) {
> - if (arm_smmu_alloc_cd_leaf_table(smmu, l1_desc))
> + if (arm_smmu_alloc_cd_leaf_table(master->smmu, l1_desc))
... no need to change this.
> @@ -1101,11 +1094,11 @@ int arm_smmu_write_ctx_desc(struct arm_smmu_domain *smmu_domain, int ssid,
> cdptr[3] = cpu_to_le64(cd->mair);
>
> /*
> - * STE is live, and the SMMU might read dwords of this CD in any
> - * order. Ensure that it observes valid values before reading
> - * V=1.
> + * STE may be live, and the SMMU might read dwords of this CD
> + * in any order. Ensure that it observes valid values before
> + * reading V=1.
This seems to be true only after the following patch? If so, we
should move this part over there too.
Thanks
Nicolin
More information about the linux-arm-kernel
mailing list