diff mbox series

[v6,8/9] iommu/arm-smmu: Get associated RMR info and install bypass SMR

Message ID 20210716083442.1708-9-shameerali.kolothum.thodi@huawei.com (mailing list archive)
State Not Applicable, archived
Headers show
Series ACPI/IORT: Support for IORT RMR node | expand

Commit Message

Shameerali Kolothum Thodi July 16, 2021, 8:34 a.m. UTC
From: Jon Nettleton <jon@solid-run.com>

Check if there is any RMR info associated with the devices behind
the SMMU and if any, install bypass SMRs for them. This is to
keep any ongoing traffic associated with these devices alive
when we enable/reset SMMU during probe().

Signed-off-by: Jon Nettleton <jon@solid-run.com>
Signed-off-by: Steven Price <steven.price@arm.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 drivers/iommu/arm/arm-smmu/arm-smmu.c | 48 +++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

Comments

Steven Price July 16, 2021, 1:52 p.m. UTC | #1
On 16/07/2021 09:34, Shameer Kolothum wrote:
> From: Jon Nettleton <jon@solid-run.com>
> 
> Check if there is any RMR info associated with the devices behind
> the SMMU and if any, install bypass SMRs for them. This is to
> keep any ongoing traffic associated with these devices alive
> when we enable/reset SMMU during probe().
> 
> Signed-off-by: Jon Nettleton <jon@solid-run.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> ---
>  drivers/iommu/arm/arm-smmu/arm-smmu.c | 48 +++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
> 
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index f22dbeb1e510..e9fb3d962a86 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -2063,6 +2063,50 @@ err_reset_platform_ops: __maybe_unused;
>  	return err;
>  }
>  
> +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 reg;
> +
> +	INIT_LIST_HEAD(&rmr_list);
> +	if (iommu_dma_get_rmrs(dev_fwnode(smmu->dev), &rmr_list))
> +		return;
> +
> +	/*
> +	 * 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.
> +	 */
> +	reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sCR0);
> +	reg &= ~ARM_SMMU_sCR0_CLIENTPD;

This looks backwards, the spec states:

  Client Port Disable. The possible values of this bit are:
  0 - Each SMMU client access is subject to SMMU translation, access
      control, and attribute generation.
  1 - Each SMMU client access bypasses SMMU translation, access control,
      and attribute generation.
  This bit resets to 1.

And indeed with the current code if sCR0_USFCFG was set when
sCR0_CLIENTPD is cleared then I get a blank screen until the smmu is reset.

So I believe this should be ORing in the value, i.e.

  reg |= ARM_SMMU_sCR0_CLIENTPD;

And in my testing that works fine even if sCR0_USFCFG is set.

Steve

> +	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;
> +
> +		i = arm_smmu_find_sme(smmu, sid, ~0);
> +		if (i < 0)
> +			continue;
> +		if (smmu->s2crs[i].count == 0) {
> +			smmu->smrs[i].id = sid;
> +			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;
> +
> +		cnt++;
> +	}
> +
> +	dev_notice(smmu->dev, "\tpreserved %d boot mapping%s\n", cnt,
> +		   cnt == 1 ? "" : "s");
> +	iommu_dma_put_rmrs(dev_fwnode(smmu->dev), &rmr_list);
> +}
> +
>  static int arm_smmu_device_probe(struct platform_device *pdev)
>  {
>  	struct resource *res;
> @@ -2189,6 +2233,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
>  	}
>  
>  	platform_set_drvdata(pdev, smmu);
> +
> +	/* Check for RMRs and install bypass SMRs if any */
> +	arm_smmu_rmr_install_bypass_smr(smmu);
> +
>  	arm_smmu_device_reset(smmu);
>  	arm_smmu_test_smr_masks(smmu);
>  
>
Jon Nettleton July 16, 2021, 8:40 p.m. UTC | #2
On Fri, Jul 16, 2021 at 3:52 PM Steven Price <steven.price@arm.com> wrote:
>
> On 16/07/2021 09:34, Shameer Kolothum wrote:
> > From: Jon Nettleton <jon@solid-run.com>
> >
> > Check if there is any RMR info associated with the devices behind
> > the SMMU and if any, install bypass SMRs for them. This is to
> > keep any ongoing traffic associated with these devices alive
> > when we enable/reset SMMU during probe().
> >
> > Signed-off-by: Jon Nettleton <jon@solid-run.com>
> > Signed-off-by: Steven Price <steven.price@arm.com>
> > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> > ---
> >  drivers/iommu/arm/arm-smmu/arm-smmu.c | 48 +++++++++++++++++++++++++++
> >  1 file changed, 48 insertions(+)
> >
> > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > index f22dbeb1e510..e9fb3d962a86 100644
> > --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> > @@ -2063,6 +2063,50 @@ err_reset_platform_ops: __maybe_unused;
> >       return err;
> >  }
> >
> > +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 reg;
> > +
> > +     INIT_LIST_HEAD(&rmr_list);
> > +     if (iommu_dma_get_rmrs(dev_fwnode(smmu->dev), &rmr_list))
> > +             return;
> > +
> > +     /*
> > +      * 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.
> > +      */
> > +     reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sCR0);
> > +     reg &= ~ARM_SMMU_sCR0_CLIENTPD;
>
> This looks backwards, the spec states:
>
>   Client Port Disable. The possible values of this bit are:
>   0 - Each SMMU client access is subject to SMMU translation, access
>       control, and attribute generation.
>   1 - Each SMMU client access bypasses SMMU translation, access control,
>       and attribute generation.
>   This bit resets to 1.
>
> And indeed with the current code if sCR0_USFCFG was set when
> sCR0_CLIENTPD is cleared then I get a blank screen until the smmu is reset.
>
> So I believe this should be ORing in the value, i.e.
>
>   reg |= ARM_SMMU_sCR0_CLIENTPD;
>
> And in my testing that works fine even if sCR0_USFCFG is set.

Sorry that is my bad.  It was a hurried and sloppy copy paste on my part.

Thanks for catching it
-Jon

>
> Steve
>
> > +     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;
> > +
> > +             i = arm_smmu_find_sme(smmu, sid, ~0);
> > +             if (i < 0)
> > +                     continue;
> > +             if (smmu->s2crs[i].count == 0) {
> > +                     smmu->smrs[i].id = sid;
> > +                     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;
> > +
> > +             cnt++;
> > +     }
> > +
> > +     dev_notice(smmu->dev, "\tpreserved %d boot mapping%s\n", cnt,
> > +                cnt == 1 ? "" : "s");
> > +     iommu_dma_put_rmrs(dev_fwnode(smmu->dev), &rmr_list);
> > +}
> > +
> >  static int arm_smmu_device_probe(struct platform_device *pdev)
> >  {
> >       struct resource *res;
> > @@ -2189,6 +2233,10 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
> >       }
> >
> >       platform_set_drvdata(pdev, smmu);
> > +
> > +     /* Check for RMRs and install bypass SMRs if any */
> > +     arm_smmu_rmr_install_bypass_smr(smmu);
> > +
> >       arm_smmu_device_reset(smmu);
> >       arm_smmu_test_smr_masks(smmu);
> >
> >
>
diff mbox series

Patch

diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index f22dbeb1e510..e9fb3d962a86 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -2063,6 +2063,50 @@  err_reset_platform_ops: __maybe_unused;
 	return err;
 }
 
+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 reg;
+
+	INIT_LIST_HEAD(&rmr_list);
+	if (iommu_dma_get_rmrs(dev_fwnode(smmu->dev), &rmr_list))
+		return;
+
+	/*
+	 * 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.
+	 */
+	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;
+
+		i = arm_smmu_find_sme(smmu, sid, ~0);
+		if (i < 0)
+			continue;
+		if (smmu->s2crs[i].count == 0) {
+			smmu->smrs[i].id = sid;
+			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;
+
+		cnt++;
+	}
+
+	dev_notice(smmu->dev, "\tpreserved %d boot mapping%s\n", cnt,
+		   cnt == 1 ? "" : "s");
+	iommu_dma_put_rmrs(dev_fwnode(smmu->dev), &rmr_list);
+}
+
 static int arm_smmu_device_probe(struct platform_device *pdev)
 {
 	struct resource *res;
@@ -2189,6 +2233,10 @@  static int arm_smmu_device_probe(struct platform_device *pdev)
 	}
 
 	platform_set_drvdata(pdev, smmu);
+
+	/* Check for RMRs and install bypass SMRs if any */
+	arm_smmu_rmr_install_bypass_smr(smmu);
+
 	arm_smmu_device_reset(smmu);
 	arm_smmu_test_smr_masks(smmu);