* [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
2016-07-12 18:19 ` [PATCH v2] " Nate Watterson
0 siblings, 2 replies; 7+ 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] 7+ messages in thread
* [PATCH] iommu/arm-smmu-v3: limit use of 2-level stream tables
2016-07-11 18:00 [PATCH] iommu/arm-smmu-v3: limit use of 2-level stream tables Nate Watterson
@ 2016-07-12 9:36 ` Robin Murphy
2016-07-12 18:19 ` [PATCH v2] " Nate Watterson
1 sibling, 0 replies; 7+ 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] 7+ messages in thread
* [PATCH v2] iommu/arm-smmu-v3: limit use of 2-level stream tables
2016-07-11 18:00 [PATCH] iommu/arm-smmu-v3: limit use of 2-level stream tables Nate Watterson
2016-07-12 9:36 ` Robin Murphy
@ 2016-07-12 18:19 ` Nate Watterson
2016-07-14 13:31 ` Will Deacon
1 sibling, 1 reply; 7+ messages in thread
From: Nate Watterson @ 2016-07-12 18:19 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 5f6b3bc..f27b8dc 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -2033,17 +2033,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;
@@ -2531,6 +2523,13 @@ 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;
+
/* 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] 7+ messages in thread
* [PATCH v2] iommu/arm-smmu-v3: limit use of 2-level stream tables
2016-07-12 18:19 ` [PATCH v2] " Nate Watterson
@ 2016-07-14 13:31 ` Will Deacon
2016-07-14 17:33 ` nwatters at codeaurora.org
0 siblings, 1 reply; 7+ messages in thread
From: Will Deacon @ 2016-07-14 13:31 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 12, 2016 at 02:19:20PM -0400, 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.
Just to be clear, what exactly are you seeing as being suboptimal here?
Is it the memory wastage from overallocating the L2 table, or something
more?
if it's just the memory allocation, I'd sooner restrict the span field
in the L1 desc.
Will
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] iommu/arm-smmu-v3: limit use of 2-level stream tables
2016-07-14 13:31 ` Will Deacon
@ 2016-07-14 17:33 ` nwatters at codeaurora.org
0 siblings, 0 replies; 7+ messages in thread
From: nwatters at codeaurora.org @ 2016-07-14 17:33 UTC (permalink / raw)
To: linux-arm-kernel
On 2016-07-14 09:31, Will Deacon wrote:
> On Tue, Jul 12, 2016 at 02:19:20PM -0400, 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.
>
> Just to be clear, what exactly are you seeing as being suboptimal here?
> Is it the memory wastage from overallocating the L2 table, or something
> more?
Disregarding the config cache, fetching an STE when 2-level tables are
being used will require the hw to perform more memory accesses than it
would have to with a linear table since the L1 descriptor must also be
fetched. Presumably this is why the spec states, "ARM recommends that
a more efficient linear table is used instead of programming SPLIT >
LOG2SIZE".
My understanding is that the only benefit to using 2-level tables is
that it can save space when stream ids are sparsely distributed. Are
there any other compelling reasons to use them?
>
> if it's just the memory allocation, I'd sooner restrict the span field
> in the L1 desc.
Although I am not especially concerned about the memory allocation, even
if the span was reduced, we would still be wasting a page for the L1
table unless L1 and L2 tables were allocated in a single
dmam_alloc_coherent
call.
>
> Will
Nate
--
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 [flat|nested] 7+ messages in thread
* [PATCH] iommu/arm-smmu-v3: avoid over allocating for l2 stream tables
@ 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; 7+ 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] 7+ messages in thread
* [PATCH] iommu/arm-smmu-v3: limit use of 2-level stream tables
2016-12-20 10:22 [PATCH] iommu/arm-smmu-v3: avoid over allocating for l2 " Will Deacon
@ 2017-01-10 19:47 ` Nate Watterson
2017-01-12 17:20 ` Will Deacon
0 siblings, 1 reply; 7+ 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] 7+ 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; 7+ 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] 7+ messages in thread
end of thread, other threads:[~2017-01-12 17:20 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-11 18:00 [PATCH] iommu/arm-smmu-v3: limit use of 2-level stream tables Nate Watterson
2016-07-12 9:36 ` Robin Murphy
2016-07-12 18:19 ` [PATCH v2] " Nate Watterson
2016-07-14 13:31 ` Will Deacon
2016-07-14 17:33 ` nwatters at codeaurora.org
2016-12-20 10:22 [PATCH] iommu/arm-smmu-v3: avoid over allocating for l2 " 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
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).