linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] iommu/arm-smmu-v3: avoid over allocating for l2 stream tables
@ 2016-12-19 20:26 Nate Watterson
  2016-12-20 10:22 ` Will Deacon
  0 siblings, 1 reply; 6+ messages in thread
From: Nate Watterson @ 2016-12-19 20:26 UTC (permalink / raw)
  To: linux-arm-kernel

Currently, all l2 stream tables are being allocated with space for
(1<<split) stes without regard to the number of sid bits the smmu
physically supports. To avoid allocating memory for inaccessible
stes, this patch limits the span of an l2 table to be no larger
than the sid size of the smmu to which it belongs.

Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
---
 drivers/iommu/arm-smmu-v3.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4d6ec44..5dca671 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1129,6 +1129,7 @@ static void arm_smmu_init_bypass_stes(u64 *strtab, unsigned int nent)
 
 static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
 {
+	u8 span;
 	size_t size;
 	void *strtab;
 	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
@@ -1137,10 +1138,11 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
 	if (desc->l2ptr)
 		return 0;
 
-	size = 1 << (STRTAB_SPLIT + ilog2(STRTAB_STE_DWORDS) + 3);
+	span = (smmu->sid_bits < STRTAB_SPLIT) ? smmu->sid_bits : STRTAB_SPLIT;
+	size = 1 << (span + ilog2(STRTAB_STE_DWORDS) + 3);
 	strtab = &cfg->strtab[(sid >> STRTAB_SPLIT) * STRTAB_L1_DESC_DWORDS];
 
-	desc->span = STRTAB_SPLIT + 1;
+	desc->span = span + 1;
 	desc->l2ptr = dmam_alloc_coherent(smmu->dev, size, &desc->l2ptr_dma,
 					  GFP_KERNEL | __GFP_ZERO);
 	if (!desc->l2ptr) {
@@ -1150,7 +1152,7 @@ static int arm_smmu_init_l2_strtab(struct arm_smmu_device *smmu, u32 sid)
 		return -ENOMEM;
 	}
 
-	arm_smmu_init_bypass_stes(desc->l2ptr, 1 << STRTAB_SPLIT);
+	arm_smmu_init_bypass_stes(desc->l2ptr, 1 << span);
 	arm_smmu_write_strtab_l1_desc(strtab, desc);
 	return 0;
 }
@@ -2001,6 +2003,8 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
 		dev_warn(smmu->dev,
 			 "2-level strtab only covers %u/%u bits of SID\n",
 			 size, smmu->sid_bits);
+	else if (smmu->sid_bits < size)
+		size = smmu->sid_bits;
 
 	l1size = cfg->num_l1_ents * (STRTAB_L1_DESC_DWORDS << 3);
 	strtab = dmam_alloc_coherent(smmu->dev, l1size, &cfg->strtab_dma,
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] iommu/arm-smmu-v3: avoid over allocating for l2 stream tables
  2016-12-19 20:26 [PATCH] iommu/arm-smmu-v3: avoid over allocating for l2 stream tables Nate Watterson
@ 2016-12-20 10:22 ` Will Deacon
  2017-01-10 19:47   ` [PATCH] iommu/arm-smmu-v3: limit use of 2-level " Nate Watterson
  0 siblings, 1 reply; 6+ messages in thread
From: Will Deacon @ 2016-12-20 10:22 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Nate,

On Mon, Dec 19, 2016 at 03:26:40PM -0500, Nate Watterson wrote:
> Currently, all l2 stream tables are being allocated with space for
> (1<<split) stes without regard to the number of sid bits the smmu
> physically supports. To avoid allocating memory for inaccessible
> stes, this patch limits the span of an l2 table to be no larger
> than the sid size of the smmu to which it belongs.
> 
> Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)

I can't help but think you'd be better off using a linear stream table
in this scenario. If we hack the feature check for
ARM_SMMU_FEAT_2_LVL_STRTAB so that it doesn't report support for 2 level
tables if the number of sids is less than that covered by a single l2
entry, would that solve your problem?

Will

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] iommu/arm-smmu-v3: limit use of 2-level stream tables
  2016-12-20 10:22 ` Will Deacon
@ 2017-01-10 19:47   ` Nate Watterson
  2017-01-12 17:20     ` Will Deacon
  0 siblings, 1 reply; 6+ messages in thread
From: Nate Watterson @ 2017-01-10 19:47 UTC (permalink / raw)
  To: linux-arm-kernel

In the current arm-smmu-v3 driver, all smmus that support 2-level
stream tables are being forced to use them. This is suboptimal for
smmus that support fewer stream id bits than would fill in a single
second level table. This patch limits the use of 2-level tables to
smmus that both support the feature and whose first level table can
possibly contain more than a single entry.

Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
---
 drivers/iommu/arm-smmu-v3.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4d6ec44..7d1a7e5 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -1983,17 +1983,9 @@ static int arm_smmu_init_strtab_2lvl(struct arm_smmu_device *smmu)
 	u32 size, l1size;
 	struct arm_smmu_strtab_cfg *cfg = &smmu->strtab_cfg;
 
-	/*
-	 * If we can resolve everything with a single L2 table, then we
-	 * just need a single L1 descriptor. Otherwise, calculate the L1
-	 * size, capped to the SIDSIZE.
-	 */
-	if (smmu->sid_bits < STRTAB_SPLIT) {
-		size = 0;
-	} else {
-		size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
-		size = min(size, smmu->sid_bits - STRTAB_SPLIT);
-	}
+	/* Calculate the L1 size, capped to the SIDSIZE. */
+	size = STRTAB_L1_SZ_SHIFT - (ilog2(STRTAB_L1_DESC_DWORDS) + 3);
+	size = min(size, smmu->sid_bits - STRTAB_SPLIT);
 	cfg->num_l1_ents = 1 << size;
 
 	size += STRTAB_SPLIT;
@@ -2504,6 +2496,13 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 	smmu->ssid_bits = reg >> IDR1_SSID_SHIFT & IDR1_SSID_MASK;
 	smmu->sid_bits = reg >> IDR1_SID_SHIFT & IDR1_SID_MASK;
 
+	/*
+	 * If the SMMU supports fewer bits than would fill a single L2 stream
+	 * table, use a linear table instead.
+	 */
+	if (smmu->sid_bits <= STRTAB_SPLIT)
+		smmu->features &= ~ARM_SMMU_FEAT_2_LVL_STRTAB;
+
 	/* IDR5 */
 	reg = readl_relaxed(smmu->base + ARM_SMMU_IDR5);
 
-- 
Qualcomm Datacenter Technologies, Inc. on behalf of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux
Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] iommu/arm-smmu-v3: limit use of 2-level stream tables
  2017-01-10 19:47   ` [PATCH] iommu/arm-smmu-v3: limit use of 2-level " Nate Watterson
@ 2017-01-12 17:20     ` Will Deacon
  0 siblings, 0 replies; 6+ messages in thread
From: Will Deacon @ 2017-01-12 17:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jan 10, 2017 at 02:47:13PM -0500, Nate Watterson wrote:
> In the current arm-smmu-v3 driver, all smmus that support 2-level
> stream tables are being forced to use them. This is suboptimal for
> smmus that support fewer stream id bits than would fill in a single
> second level table. This patch limits the use of 2-level tables to
> smmus that both support the feature and whose first level table can
> possibly contain more than a single entry.
> 
> Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)

Thanks Nate, I'll queue this for 4.11. Sorry for messing you about before.

Will

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] iommu/arm-smmu-v3: limit use of 2-level stream tables
  2016-07-11 18:00 Nate Watterson
@ 2016-07-12  9:36 ` Robin Murphy
  0 siblings, 0 replies; 6+ messages in thread
From: Robin Murphy @ 2016-07-12  9:36 UTC (permalink / raw)
  To: linux-arm-kernel

On 11/07/16 19:00, Nate Watterson wrote:
> In the current arm-smmu-v3 driver, all smmus that support 2-level
> stream tables are being forced to use them. This is suboptimal for
> smmus that support fewer stream id bits than would fill in a single
> second level table. This patch limits the use of 2-level tables to
> smmus that both support the feature and whose first level table can
> possibly contain more than a single entry.

Makes sense to me, in principle.

> Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
> ---
>  drivers/iommu/arm-smmu-v3.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 5f6b3bc..742254c 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -2531,6 +2531,17 @@ static int arm_smmu_device_probe(struct arm_smmu_device *smmu)
>  	smmu->ssid_bits = reg >> IDR1_SSID_SHIFT & IDR1_SSID_MASK;
>  	smmu->sid_bits = reg >> IDR1_SID_SHIFT & IDR1_SID_MASK;
>  
> +	/*
> +	 * If the SMMU supports fewer bits than would fill a single L2 stream
> +	 * table, use a linear table instead.
> +	 */
> +	if (smmu->sid_bits <= STRTAB_SPLIT &&
> +	    smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
> +		smmu->features &= ~ARM_SMMU_FEAT_2_LVL_STRTAB;
> +		dev_info(smmu->dev, "SIDSIZE (%d) <= STRTAB_SPLIT (%d) : disabling 2-level stream tables\n",
> +			 smmu->sid_bits, STRTAB_SPLIT);

There's no useful reason to squawk about this; it's just noise.

Whatever old version of the spec I have here would appear to agree: "In
all cases, aside from the lookup of the STE itself, the choice of Stream
Table format is irrelevant to any other SMMU operation."

> +	}
> +
>  	/* IDR5 */
>  	reg = readl_relaxed(smmu->base + ARM_SMMU_IDR5);

I think this now leaves some of the logic in arm_smmu_init_strtab_2lvl()
redundant, so it would probably be worth tidying that up at the same time.

Robin.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] iommu/arm-smmu-v3: limit use of 2-level stream tables
@ 2016-07-11 18:00 Nate Watterson
  2016-07-12  9:36 ` Robin Murphy
  0 siblings, 1 reply; 6+ messages in thread
From: Nate Watterson @ 2016-07-11 18:00 UTC (permalink / raw)
  To: linux-arm-kernel

In the current arm-smmu-v3 driver, all smmus that support 2-level
stream tables are being forced to use them. This is suboptimal for
smmus that support fewer stream id bits than would fill in a single
second level table. This patch limits the use of 2-level tables to
smmus that both support the feature and whose first level table can
possibly contain more than a single entry.

Signed-off-by: Nate Watterson <nwatters@codeaurora.org>
---
 drivers/iommu/arm-smmu-v3.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 5f6b3bc..742254c 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2531,6 +2531,17 @@ static int arm_smmu_device_probe(struct arm_smmu_device *smmu)
 	smmu->ssid_bits = reg >> IDR1_SSID_SHIFT & IDR1_SSID_MASK;
 	smmu->sid_bits = reg >> IDR1_SID_SHIFT & IDR1_SID_MASK;
 
+	/*
+	 * If the SMMU supports fewer bits than would fill a single L2 stream
+	 * table, use a linear table instead.
+	 */
+	if (smmu->sid_bits <= STRTAB_SPLIT &&
+	    smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
+		smmu->features &= ~ARM_SMMU_FEAT_2_LVL_STRTAB;
+		dev_info(smmu->dev, "SIDSIZE (%d) <= STRTAB_SPLIT (%d) : disabling 2-level stream tables\n",
+			 smmu->sid_bits, STRTAB_SPLIT);
+	}
+
 	/* IDR5 */
 	reg = readl_relaxed(smmu->base + ARM_SMMU_IDR5);
 
-- 
Qualcomm Technologies, Inc. on behalf
of the Qualcomm Innovation Center, Inc.  The Qualcomm Innovation Center, Inc. 
is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-01-12 17:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-19 20:26 [PATCH] iommu/arm-smmu-v3: avoid over allocating for l2 stream tables Nate Watterson
2016-12-20 10:22 ` Will Deacon
2017-01-10 19:47   ` [PATCH] iommu/arm-smmu-v3: limit use of 2-level " Nate Watterson
2017-01-12 17:20     ` Will Deacon
  -- strict thread matches above, loose matches on Subject: below --
2016-07-11 18:00 Nate Watterson
2016-07-12  9:36 ` Robin Murphy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).