Re: [PATCH 1/3] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings
From: Bibek Kumar Patro
Date: Tue Nov 14 2023 - 05:21:01 EST
On 11/6/2023 11:42 AM, Bibek Kumar Patro wrote:
On 11/4/2023 3:33 AM, Dmitry Baryshkov wrote:
On Fri, 3 Nov 2023 at 23:53, Bibek Kumar Patro
<quic_bibekkum@xxxxxxxxxxx> wrote:
Currently in Qualcomm SoCs the default prefetch is set to 1 which
allows
the TLB to fetch just the next page table. MMU-500 features ACTLR
register which is implementation defined and is used for Qualcomm SoCs
to have a prefetch setting of 1/3/7/15 enabling TLB to prefetch
the next set of page tables accordingly allowing for faster
translations.
ACTLR value is unique for each SMR (Stream matching register) and stored
in a pre-populated table. This value is set to the register during
context bank initialisation.
Signed-off-by: Bibek Kumar Patro <quic_bibekkum@xxxxxxxxxxx>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 34 ++++++++++++++++++++++
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 2 ++
drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 ++--
drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++++
4 files changed, 44 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index ae7cae015193..68c1f4908473 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -14,6 +14,17 @@
#define QCOM_DUMMY_VAL -1
+struct actlr_config {
+ const struct actlr_data *adata;
+ u32 size;
This should be size_t.
Also could you please drop the separate struct actlr_config and move
these two fields into struct qcom_smmu_config.
Ack, will address both these inputs in the next patch.
Dimitry, Tried moving both fields to qcom_smmu_config but since
actlr_data need to be a pointer to array and not scalar, size needs
to be calculated dynamically for each SoC data in a loop which is
doable.But readily available implementations like ARRAY_SIZE cannot be
used, so I think this extra struct indirection of actlr_config would be
beneficial.
Some drivers like llcc (drivers/soc/qcom/llcc-qcom.c) is also using
similar implementation, I believe for the same reason.
regards,
Bibek
+};
+
+struct actlr_data {
+ u16 sid;
+ u16 mask;
+ u32 actlr;
+};
+
static struct qcom_smmu *to_qcom_smmu(struct arm_smmu_device *smmu)
{
return container_of(smmu, struct qcom_smmu, smmu);
@@ -270,6 +281,26 @@ static const struct of_device_id
qcom_smmu_client_of_match[] __maybe_unused = {
static int qcom_smmu_init_context(struct arm_smmu_domain *smmu_domain,
struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
{
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
+ const struct actlr_config *actlrcfg;
+ struct arm_smmu_smr *smr = smmu->smrs;
+ int idx = smmu_domain->cfg.cbndx;
+ int i;
+ u16 id;
+ u16 mask;
+
+ if (qsmmu->actlrcfg) {
+ actlrcfg = qsmmu->actlrcfg;
+ for (i = 0; i < actlrcfg->size; ++i) {
+ id = actlrcfg->adata[i].sid;
+ mask = actlrcfg->adata[i].mask;
+ if (!smr_is_subset(*smr, id, mask))
+ arm_smmu_cb_write(smmu, idx,
ARM_SMMU_CB_ACTLR,
+
actlrcfg->adata[i].actlr);
+ }
+ }
Consider extracting this to a separate function. This way you can
reduce 4 indentation levels into a single loop.
Ack, thanks for this sugestion. Will move this entire for loop into a
separate function for simplicity reduced indent levels.
+
smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
return 0;
@@ -459,6 +490,9 @@ static struct arm_smmu_device
*qcom_smmu_create(struct arm_smmu_device *smmu,
qsmmu->smmu.impl = impl;
qsmmu->cfg = data->cfg;
+ if (data->actlrcfg && (data->actlrcfg->size))
+ qsmmu->actlrcfg = data->actlrcfg;
+
return &qsmmu->smmu;
}
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
index 593910567b88..4b6862715070 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
@@ -9,6 +9,7 @@
struct qcom_smmu {
struct arm_smmu_device smmu;
const struct qcom_smmu_config *cfg;
+ const struct actlr_config *actlrcfg;
bool bypass_quirk;
u8 bypass_cbndx;
u32 stall_enabled;
@@ -25,6 +26,7 @@ struct qcom_smmu_config {
};
struct qcom_smmu_match_data {
+ const struct actlr_config *actlrcfg;
const struct qcom_smmu_config *cfg;
const struct arm_smmu_impl *impl;
const struct arm_smmu_impl *adreno_impl;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 4c79ef6f4c75..38ac1cbc799b 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -992,9 +992,10 @@ static int arm_smmu_find_sme(struct
arm_smmu_device *smmu, u16 id, u16 mask)
* expect simply identical entries for this case,
but there's
* no harm in accommodating the generalisation.
*/
- if ((mask & smrs[i].mask) == mask &&
- !((id ^ smrs[i].id) & ~smrs[i].mask))
+
+ if (smr_is_subset(smrs[i], id, mask))
return i;
+
/*
* If the new entry has any other overlap with an
existing one,
* though, then there always exists at least one
stream ID
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index 703fd5817ec1..b1638bbc41d4 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -501,6 +501,11 @@ static inline void arm_smmu_writeq(struct
arm_smmu_device *smmu, int page,
writeq_relaxed(val, arm_smmu_page(smmu, page) +
offset);
}
+static inline bool smr_is_subset(struct arm_smmu_smr smrs, u16 id,
u16 mask)
+{
+ return (mask & smrs.mask) == mask && !((id ^ smrs.id) &
~smrs.mask);
+}
+
#define ARM_SMMU_GR0 0
#define ARM_SMMU_GR1 1
#define ARM_SMMU_CB(s, n) ((s)->numpage + (n))
--
2.17.1