RE: [PATCH v5 7/8] iommu/arm-smmu: Get associated RMR info and install bypass SMR

[Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>]

> -----Original Message-----
> From: Jon Nettleton [mailto:jon@solid-run.com]
> Sent: 29 June 2021 17:26
> To: Robin Murphy <robin.murphy@arm.com>
> Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> linux-arm-kernel <linux-arm-kernel@lists.infradead.org>; ACPI Devel Maling
> List <linux-acpi@vger.kernel.org>; iommu@lists.linux-foundation.org; Linuxarm
> <linuxarm@huawei.com>; Steven Price <steven.price@arm.com>; Guohanjun
> (Hanjun Guo) <guohanjun@huawei.com>; yangyicong
> <yangyicong@huawei.com>; Sami.Mujawar@arm.com; wanghuiqiang
> <wanghuiqiang@huawei.com>
> Subject: Re: [PATCH v5 7/8] iommu/arm-smmu: Get associated RMR info and
> install bypass SMR
> 

[...]
 
> Shameer,
> 
> Sorry for the delays.  Here is a diff of the changes that should
> address the issues pointed out by Robin,
> I have tested that this works as expected on my HoneyComb LX2160A.

Ok. Thanks for that.

> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index ab7b9db77625..a358bd326d0b 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -2068,29 +2068,21 @@ static void
> arm_smmu_rmr_install_bypass_smr(struct arm_smmu_device *smmu)
>         struct list_head rmr_list;
>         struct iommu_resv_region *e;
>         int i, cnt = 0;
> -       u32 smr;
>         u32 reg;
> 
>         INIT_LIST_HEAD(&rmr_list);
>         if (iommu_dma_get_rmrs(dev_fwnode(smmu->dev), &rmr_list))
>                 return;
> 
> -       reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sCR0);
> +       /* Rather than trying to look at existing mappings that
> +        * are setup by the firmware and then invalidate the ones
> +        * that do no have matching RMR entries, just disable the
> +        * SMMU until it gets enabled again in the reset routine.
> +        */
> 
> -       if ((reg & ARM_SMMU_sCR0_USFCFG) && !(reg &
> ARM_SMMU_sCR0_CLIENTPD)) {
> -               /*
> -                * SMMU is already enabled and disallowing bypass, so
> preserve
> -                * the existing SMRs
> -                */
> -               for (i = 0; i < smmu->num_mapping_groups; i++) {
> -                       smr = arm_smmu_gr0_read(smmu,
> ARM_SMMU_GR0_SMR(i));
> -                       if (!FIELD_GET(ARM_SMMU_SMR_VALID, smr))
> -                               continue;
> -                       smmu->smrs[i].id =
> FIELD_GET(ARM_SMMU_SMR_ID, smr);
> -                       smmu->smrs[i].mask =
> FIELD_GET(ARM_SMMU_SMR_MASK, smr);
> -                       smmu->smrs[i].valid = true;
> -               }
> -       }
> +       reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sCR0);
> +       reg &= ~ARM_SMMU_sCR0_CLIENTPD;
> +       arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sCR0, reg);
> 
>         list_for_each_entry(e, &rmr_list, list) {
>                 u32 sid = e->fw_data.rmr.sid;
> @@ -2100,25 +2092,16 @@ static void
> arm_smmu_rmr_install_bypass_smr(struct arm_smmu_device *smmu)
>                         continue;
>                 if (smmu->s2crs[i].count == 0) {
>                         smmu->smrs[i].id = sid;
> -                       smmu->smrs[i].mask = ~0;
> +                       smmu->smrs[i].mask = 0;
>                         smmu->smrs[i].valid = true;
>                 }
>                 smmu->s2crs[i].count++;
>                 smmu->s2crs[i].type = S2CR_TYPE_BYPASS;
>                 smmu->s2crs[i].privcfg = S2CR_PRIVCFG_DEFAULT;
> -               smmu->s2crs[i].cbndx = 0xff;
> 
>                 cnt++;
>         }
> 
> -       if ((reg & ARM_SMMU_sCR0_USFCFG) && !(reg &
> ARM_SMMU_sCR0_CLIENTPD)) {
> -               /* Remove the valid bit for unused SMRs */
> -               for (i = 0; i < smmu->num_mapping_groups; i++) {
> -                       if (smmu->s2crs[i].count == 0)
> -                               smmu->smrs[i].valid = false;
> -               }
> -       }
> -
>         dev_notice(smmu->dev, "\tpreserved %d boot mapping%s\n", cnt,
>                    cnt == 1 ? "" : "s");
>         iommu_dma_put_rmrs(dev_fwnode(smmu->dev), &rmr_list);
> 
> Please include that in your next patch series.  Let me know if you
> want me to send you the patch direct
> off the list.

No problem, I will take this in next.

Thanks,
Shameer