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@quicinc.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 56 +++++++++++++++++++++-
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 6 ++-
drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++
4 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 7f52ac67495f..4a38cae29be2 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -14,6 +14,12 @@
#define QCOM_DUMMY_VAL -1
+struct actlr_config {
+ 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);
@@ -205,10 +211,40 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu)
return true;
}
+static void arm_smmu_set_actlr(struct device *dev, struct arm_smmu_device *smmu, int cbndx,
+ const struct actlr_config *actlrcfg, size_t actlrcfg_size)
+{
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+ struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
+ struct arm_smmu_smr *smr;
+ int i;
+ int j;
+ u16 id;
+ u16 mask;
+ int idx;
+
+ for (i = 0; i < actlrcfg_size; ++i) {
+ id = (actlrcfg + i)->sid;
+ mask = (actlrcfg + i)->mask;
+
+ for_each_cfg_sme(cfg, fwspec, j, idx) {
+ smr = &smmu->smrs[idx];
+ if (smr_is_subset(*smr, id, mask))
+ arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR,
+ (actlrcfg + i)->actlr);
+ }
+ }
+}
+
static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
{
struct adreno_smmu_priv *priv;
+ struct arm_smmu_device *smmu = smmu_domain->smmu;
+ struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
+ const struct actlr_config *actlrcfg;
+ size_t actlrcfg_size;
+ int cbndx = smmu_domain->cfg.cbndx;
smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
@@ -238,6 +274,12 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
priv->set_stall = qcom_adreno_smmu_set_stall;
priv->resume_translation = qcom_adreno_smmu_resume_translation;
+ if (qsmmu->data->actlrcfg_gfx) {
+ actlrcfg = qsmmu->data->actlrcfg_gfx;
+ actlrcfg_size = qsmmu->data->actlrcfg_gfx_size;
+ arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
+ }
+
return 0;
}
@@ -263,6 +305,18 @@ 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;
+ size_t actlrcfg_size;
+ int cbndx = smmu_domain->cfg.cbndx;
+
+ if (qsmmu->data->actlrcfg) {
+ actlrcfg = qsmmu->data->actlrcfg;
+ actlrcfg_size = qsmmu->data->actlrcfg_size;
+ arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
+ }
+
smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
return 0;
@@ -464,7 +518,7 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
return ERR_PTR(-ENOMEM);
qsmmu->smmu.impl = impl;
- qsmmu->cfg = data->cfg;
+ qsmmu->data = data;
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..138fc57f7b0d 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
@@ -8,7 +8,7 @@
struct qcom_smmu {
struct arm_smmu_device smmu;
- const struct qcom_smmu_config *cfg;
+ const struct qcom_smmu_match_data *data;
bool bypass_quirk;
u8 bypass_cbndx;
u32 stall_enabled;
@@ -25,6 +25,10 @@ struct qcom_smmu_config {
};
struct qcom_smmu_match_data {
+ const struct actlr_config *actlrcfg;
+ size_t actlrcfg_size;
+ const struct actlr_config *actlrcfg_gfx;
+ size_t actlrcfg_gfx_size;
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 d6d1a2a55cc0..8e4faf015286 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -990,9 +990,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.1Hi Bibek,
kernel test robot noticed the following build errors:
[auto build test ERROR on v6.7-rc3]
[also build test ERROR on linus/master next-20231201]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Bibek-Kumar-Patro/iommu-arm-smmu-add-ACTLR-data-and-support-for-SM8550/20231127-235746
base: v6.7-rc3
patch link: https://lore.kernel.org/r/20231127145412.3981-2-quic_bibekkum%40quicinc.com
patch subject: [PATCH v3 1/4] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20231201/202312011750.Pwca3TWE-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231201/202312011750.Pwca3TWE-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312011750.Pwca3TWE-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c:25:16: error: no member named 'cfg' in 'struct qcom_smmu'
25 | cfg = qsmmu->cfg;
| ~~~~~ ^
1 error generated.
vim +25 drivers/iommu/arm/arm-smmu/arm-smmu-qcom-debug.c
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 12
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 13 void qcom_smmu_tlb_sync_debug(struct arm_smmu_device *smmu)
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 14 {
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 15 int ret;
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 16 u32 tbu_pwr_status, sync_inv_ack, sync_inv_progress;
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 17 struct qcom_smmu *qsmmu = container_of(smmu, struct qcom_smmu, smmu);
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 18 const struct qcom_smmu_config *cfg;
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 19 static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 20 DEFAULT_RATELIMIT_BURST);
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 21
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 22 if (__ratelimit(&rs)) {
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 23 dev_err(smmu->dev, "TLB sync timed out -- SMMU may be deadlocked\n");
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 24
b9b721d117e9d3 Sai Prakash Ranjan 2022-07-08 @25 cfg = qsmmu->cfg;
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Mon, Nov 27, 2023 at 08:24:09PM +0530, Bibek Kumar Patro 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
In the previous discussion with Dmitry you stated that the "prefetch
setting" doesn't directly map to any known values. This commit message
give a clear indication about the meaning of these values.
So, please fix the commit message to properly document the value space -
to avoid confusion when people are searching for the meaning of the
defines...
Please also clarify why there are 4 possible values here, 4 possible
values of the 2 prefetch settings bits in the register, but only 3
defines in the actual patch.
Regards,
Bjorn
> 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@quicinc.com>
>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 56 +++++++++++++++++++++-
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 6 ++-
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++
> 4 files changed, 68 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 7f52ac67495f..4a38cae29be2 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -14,6 +14,12 @@
>
> #define QCOM_DUMMY_VAL -1
>
> +struct actlr_config {
> + 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);
> @@ -205,10 +211,40 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu)
> return true;
> }
>
> +static void arm_smmu_set_actlr(struct device *dev, struct arm_smmu_device *smmu, int cbndx,
> + const struct actlr_config *actlrcfg, size_t actlrcfg_size)
> +{
> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> + struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
> + struct arm_smmu_smr *smr;
> + int i;
> + int j;
> + u16 id;
> + u16 mask;
> + int idx;
> +
> + for (i = 0; i < actlrcfg_size; ++i) {
> + id = (actlrcfg + i)->sid;
> + mask = (actlrcfg + i)->mask;
> +
> + for_each_cfg_sme(cfg, fwspec, j, idx) {
> + smr = &smmu->smrs[idx];
> + if (smr_is_subset(*smr, id, mask))
> + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR,
> + (actlrcfg + i)->actlr);
> + }
> + }
> +}
> +
> static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
> {
> struct adreno_smmu_priv *priv;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
> + const struct actlr_config *actlrcfg;
> + size_t actlrcfg_size;
> + int cbndx = smmu_domain->cfg.cbndx;
>
> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>
> @@ -238,6 +274,12 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
> priv->set_stall = qcom_adreno_smmu_set_stall;
> priv->resume_translation = qcom_adreno_smmu_resume_translation;
>
> + if (qsmmu->data->actlrcfg_gfx) {
> + actlrcfg = qsmmu->data->actlrcfg_gfx;
> + actlrcfg_size = qsmmu->data->actlrcfg_gfx_size;
> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
> + }
> +
> return 0;
> }
>
> @@ -263,6 +305,18 @@ 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;
> + size_t actlrcfg_size;
> + int cbndx = smmu_domain->cfg.cbndx;
> +
> + if (qsmmu->data->actlrcfg) {
> + actlrcfg = qsmmu->data->actlrcfg;
> + actlrcfg_size = qsmmu->data->actlrcfg_size;
> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
> + }
> +
> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>
> return 0;
> @@ -464,7 +518,7 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
> return ERR_PTR(-ENOMEM);
>
> qsmmu->smmu.impl = impl;
> - qsmmu->cfg = data->cfg;
> + qsmmu->data = data;
>
> 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..138fc57f7b0d 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
> @@ -8,7 +8,7 @@
>
> struct qcom_smmu {
> struct arm_smmu_device smmu;
> - const struct qcom_smmu_config *cfg;
> + const struct qcom_smmu_match_data *data;
> bool bypass_quirk;
> u8 bypass_cbndx;
> u32 stall_enabled;
> @@ -25,6 +25,10 @@ struct qcom_smmu_config {
> };
>
> struct qcom_smmu_match_data {
> + const struct actlr_config *actlrcfg;
> + size_t actlrcfg_size;
> + const struct actlr_config *actlrcfg_gfx;
> + size_t actlrcfg_gfx_size;
> 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 d6d1a2a55cc0..8e4faf015286 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -990,9 +990,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
>
>
On 11/28/2023 8:31 AM, Bjorn Andersson wrote:
> On Mon, Nov 27, 2023 at 08:24:09PM +0530, Bibek Kumar Patro 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
>
> In the previous discussion with Dmitry you stated that the "prefetch
> setting" doesn't directly map to any known values. This commit message
> give a clear indication about the meaning of these values.
>
> So, please fix the commit message to properly document the value space -
> to avoid confusion when people are searching for the meaning of the
> defines...
>
Noted, agree on the same. Thanks for pointing this out.
Will fix the description accordingly, avoid mentioning
meaning of these values.
>
> Please also clarify why there are 4 possible values here, 4 possible
> values of the 2 prefetch settings bits in the register, but only 3
> defines in the actual patch.
>
One of the values haven't been yet used in the targets whose list are
posted in this series, hence corresponding define is not mentioned in
the actual patch yet.
Thanks & Regards,
Bibek
> Regards,
> Bjorn
>
>> 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@quicinc.com>
>>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 56 +++++++++++++++++++++-
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 6 ++-
>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
>> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++
>> 4 files changed, 68 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 7f52ac67495f..4a38cae29be2 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -14,6 +14,12 @@
>>
>> #define QCOM_DUMMY_VAL -1
>>
>> +struct actlr_config {
>> + 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);
>> @@ -205,10 +211,40 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu)
>> return true;
>> }
>>
>> +static void arm_smmu_set_actlr(struct device *dev, struct arm_smmu_device *smmu, int cbndx,
>> + const struct actlr_config *actlrcfg, size_t actlrcfg_size)
>> +{
>> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>> + struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
>> + struct arm_smmu_smr *smr;
>> + int i;
>> + int j;
>> + u16 id;
>> + u16 mask;
>> + int idx;
>> +
>> + for (i = 0; i < actlrcfg_size; ++i) {
>> + id = (actlrcfg + i)->sid;
>> + mask = (actlrcfg + i)->mask;
>> +
>> + for_each_cfg_sme(cfg, fwspec, j, idx) {
>> + smr = &smmu->smrs[idx];
>> + if (smr_is_subset(*smr, id, mask))
>> + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR,
>> + (actlrcfg + i)->actlr);
>> + }
>> + }
>> +}
>> +
>> static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
>> {
>> struct adreno_smmu_priv *priv;
>> + struct arm_smmu_device *smmu = smmu_domain->smmu;
>> + struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
>> + const struct actlr_config *actlrcfg;
>> + size_t actlrcfg_size;
>> + int cbndx = smmu_domain->cfg.cbndx;
>>
>> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>>
>> @@ -238,6 +274,12 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>> priv->set_stall = qcom_adreno_smmu_set_stall;
>> priv->resume_translation = qcom_adreno_smmu_resume_translation;
>>
>> + if (qsmmu->data->actlrcfg_gfx) {
>> + actlrcfg = qsmmu->data->actlrcfg_gfx;
>> + actlrcfg_size = qsmmu->data->actlrcfg_gfx_size;
>> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
>> + }
>> +
>> return 0;
>> }
>>
>> @@ -263,6 +305,18 @@ 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;
>> + size_t actlrcfg_size;
>> + int cbndx = smmu_domain->cfg.cbndx;
>> +
>> + if (qsmmu->data->actlrcfg) {
>> + actlrcfg = qsmmu->data->actlrcfg;
>> + actlrcfg_size = qsmmu->data->actlrcfg_size;
>> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
>> + }
>> +
>> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>>
>> return 0;
>> @@ -464,7 +518,7 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
>> return ERR_PTR(-ENOMEM);
>>
>> qsmmu->smmu.impl = impl;
>> - qsmmu->cfg = data->cfg;
>> + qsmmu->data = data;
>>
>> 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..138fc57f7b0d 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> @@ -8,7 +8,7 @@
>>
>> struct qcom_smmu {
>> struct arm_smmu_device smmu;
>> - const struct qcom_smmu_config *cfg;
>> + const struct qcom_smmu_match_data *data;
>> bool bypass_quirk;
>> u8 bypass_cbndx;
>> u32 stall_enabled;
>> @@ -25,6 +25,10 @@ struct qcom_smmu_config {
>> };
>>
>> struct qcom_smmu_match_data {
>> + const struct actlr_config *actlrcfg;
>> + size_t actlrcfg_size;
>> + const struct actlr_config *actlrcfg_gfx;
>> + size_t actlrcfg_gfx_size;
>> 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 d6d1a2a55cc0..8e4faf015286 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> @@ -990,9 +990,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
>>
>>
On Mon, 27 Nov 2023 at 16:54, Bibek Kumar Patro
<quic_bibekkum@quicinc.com> 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@quicinc.com>
>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 56 +++++++++++++++++++++-
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 6 ++-
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++
> 4 files changed, 68 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 7f52ac67495f..4a38cae29be2 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -14,6 +14,12 @@
>
> #define QCOM_DUMMY_VAL -1
>
> +struct actlr_config {
> + 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);
> @@ -205,10 +211,40 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu)
> return true;
> }
>
> +static void arm_smmu_set_actlr(struct device *dev, struct arm_smmu_device *smmu, int cbndx,
> + const struct actlr_config *actlrcfg, size_t actlrcfg_size)
> +{
> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> + struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
> + struct arm_smmu_smr *smr;
> + int i;
> + int j;
> + u16 id;
> + u16 mask;
> + int idx;
> +
> + for (i = 0; i < actlrcfg_size; ++i) {
> + id = (actlrcfg + i)->sid;
> + mask = (actlrcfg + i)->mask;
> +
> + for_each_cfg_sme(cfg, fwspec, j, idx) {
> + smr = &smmu->smrs[idx];
> + if (smr_is_subset(*smr, id, mask))
> + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR,
> + (actlrcfg + i)->actlr);
> + }
> + }
> +}
> +
> static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
> {
> struct adreno_smmu_priv *priv;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
> + const struct actlr_config *actlrcfg;
> + size_t actlrcfg_size;
> + int cbndx = smmu_domain->cfg.cbndx;
>
> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>
> @@ -238,6 +274,12 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
> priv->set_stall = qcom_adreno_smmu_set_stall;
> priv->resume_translation = qcom_adreno_smmu_resume_translation;
>
> + if (qsmmu->data->actlrcfg_gfx) {
> + actlrcfg = qsmmu->data->actlrcfg_gfx;
> + actlrcfg_size = qsmmu->data->actlrcfg_gfx_size;
> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
> + }
> +
> return 0;
> }
>
> @@ -263,6 +305,18 @@ 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;
> + size_t actlrcfg_size;
> + int cbndx = smmu_domain->cfg.cbndx;
> +
> + if (qsmmu->data->actlrcfg) {
> + actlrcfg = qsmmu->data->actlrcfg;
> + actlrcfg_size = qsmmu->data->actlrcfg_size;
> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
> + }
> +
> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>
> return 0;
> @@ -464,7 +518,7 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
> return ERR_PTR(-ENOMEM);
>
> qsmmu->smmu.impl = impl;
> - qsmmu->cfg = data->cfg;
> + qsmmu->data = data;
This should go to a separate commit. It is not related to ACTLR support
>
> 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..138fc57f7b0d 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
> @@ -8,7 +8,7 @@
>
> struct qcom_smmu {
> struct arm_smmu_device smmu;
> - const struct qcom_smmu_config *cfg;
> + const struct qcom_smmu_match_data *data;
> bool bypass_quirk;
> u8 bypass_cbndx;
> u32 stall_enabled;
> @@ -25,6 +25,10 @@ struct qcom_smmu_config {
> };
>
> struct qcom_smmu_match_data {
> + const struct actlr_config *actlrcfg;
> + size_t actlrcfg_size;
> + const struct actlr_config *actlrcfg_gfx;
> + size_t actlrcfg_gfx_size;
> 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 d6d1a2a55cc0..8e4faf015286 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -990,9 +990,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)
A pointer to the struct, please
> +{
> + 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
>
--
With best wishes
Dmitry
On 11/27/2023 9:37 PM, Dmitry Baryshkov wrote:
> On Mon, 27 Nov 2023 at 16:54, Bibek Kumar Patro
> <quic_bibekkum@quicinc.com> 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@quicinc.com>
>>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 56 +++++++++++++++++++++-
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 6 ++-
>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
>> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++
>> 4 files changed, 68 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 7f52ac67495f..4a38cae29be2 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -14,6 +14,12 @@
>>
>> #define QCOM_DUMMY_VAL -1
>>
>> +struct actlr_config {
>> + 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);
>> @@ -205,10 +211,40 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu)
>> return true;
>> }
>>
>> +static void arm_smmu_set_actlr(struct device *dev, struct arm_smmu_device *smmu, int cbndx,
>> + const struct actlr_config *actlrcfg, size_t actlrcfg_size)
>> +{
>> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>> + struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
>> + struct arm_smmu_smr *smr;
>> + int i;
>> + int j;
>> + u16 id;
>> + u16 mask;
>> + int idx;
>> +
>> + for (i = 0; i < actlrcfg_size; ++i) {
>> + id = (actlrcfg + i)->sid;
>> + mask = (actlrcfg + i)->mask;
>> +
>> + for_each_cfg_sme(cfg, fwspec, j, idx) {
>> + smr = &smmu->smrs[idx];
>> + if (smr_is_subset(*smr, id, mask))
>> + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR,
>> + (actlrcfg + i)->actlr);
>> + }
>> + }
>> +}
>> +
>> static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
>> {
>> struct adreno_smmu_priv *priv;
>> + struct arm_smmu_device *smmu = smmu_domain->smmu;
>> + struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
>> + const struct actlr_config *actlrcfg;
>> + size_t actlrcfg_size;
>> + int cbndx = smmu_domain->cfg.cbndx;
>>
>> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>>
>> @@ -238,6 +274,12 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>> priv->set_stall = qcom_adreno_smmu_set_stall;
>> priv->resume_translation = qcom_adreno_smmu_resume_translation;
>>
>> + if (qsmmu->data->actlrcfg_gfx) {
>> + actlrcfg = qsmmu->data->actlrcfg_gfx;
>> + actlrcfg_size = qsmmu->data->actlrcfg_gfx_size;
>> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
>> + }
>> +
>> return 0;
>> }
>>
>> @@ -263,6 +305,18 @@ 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;
>> + size_t actlrcfg_size;
>> + int cbndx = smmu_domain->cfg.cbndx;
>> +
>> + if (qsmmu->data->actlrcfg) {
>> + actlrcfg = qsmmu->data->actlrcfg;
>> + actlrcfg_size = qsmmu->data->actlrcfg_size;
>> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
>> + }
>> +
>> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>>
>> return 0;
>> @@ -464,7 +518,7 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
>> return ERR_PTR(-ENOMEM);
>>
>> qsmmu->smmu.impl = impl;
>> - qsmmu->cfg = data->cfg;
>> + qsmmu->data = data;
>
> This should go to a separate commit. It is not related to ACTLR support
qsmmu->data has the actlrcfg/actlrcfg_gfx as well hence clubbed this
change here as rightly suggested by Robin[1] on v2 revision.
Initially planned[2] for separate patch, but later clubbing it with
actlr patch looked like a cleaner approach.
Would it be okay to keep it here? Or separate patch would be better?
[1]:https://lore.kernel.org/all/c75d107a-44cb-4df3-b583-13719df1f8be@arm.com/
[2]:https://lore.kernel.org/all/9b406a7c-57b8-4b5f-8fbc-714560cce8cf@quicinc.com/
>
>>
>> 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..138fc57f7b0d 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> @@ -8,7 +8,7 @@
>>
>> struct qcom_smmu {
>> struct arm_smmu_device smmu;
>> - const struct qcom_smmu_config *cfg;
>> + const struct qcom_smmu_match_data *data;
>> bool bypass_quirk;
>> u8 bypass_cbndx;
>> u32 stall_enabled;
>> @@ -25,6 +25,10 @@ struct qcom_smmu_config {
>> };
>>
>> struct qcom_smmu_match_data {
>> + const struct actlr_config *actlrcfg;
>> + size_t actlrcfg_size;
>> + const struct actlr_config *actlrcfg_gfx;
>> + size_t actlrcfg_gfx_size;
>> 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 d6d1a2a55cc0..8e4faf015286 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> @@ -990,9 +990,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)
>
> A pointer to the struct, please
>
>> +{
>> + 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
>>
>
>
On 11/27/2023 9:37 PM, Dmitry Baryshkov wrote:
> On Mon, 27 Nov 2023 at 16:54, Bibek Kumar Patro
> <quic_bibekkum@quicinc.com> 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@quicinc.com>
>>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 56 +++++++++++++++++++++-
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 6 ++-
>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
>> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++
>> 4 files changed, 68 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 7f52ac67495f..4a38cae29be2 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -14,6 +14,12 @@
>>
>> #define QCOM_DUMMY_VAL -1
>>
>> +struct actlr_config {
>> + 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);
>> @@ -205,10 +211,40 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu)
>> return true;
>> }
>>
>> +static void arm_smmu_set_actlr(struct device *dev, struct arm_smmu_device *smmu, int cbndx,
>> + const struct actlr_config *actlrcfg, size_t actlrcfg_size)
>> +{
>> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>> + struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
>> + struct arm_smmu_smr *smr;
>> + int i;
>> + int j;
>> + u16 id;
>> + u16 mask;
>> + int idx;
>> +
>> + for (i = 0; i < actlrcfg_size; ++i) {
>> + id = (actlrcfg + i)->sid;
>> + mask = (actlrcfg + i)->mask;
>> +
>> + for_each_cfg_sme(cfg, fwspec, j, idx) {
>> + smr = &smmu->smrs[idx];
>> + if (smr_is_subset(*smr, id, mask))
>> + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR,
>> + (actlrcfg + i)->actlr);
>> + }
>> + }
>> +}
>> +
>> static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
>> {
>> struct adreno_smmu_priv *priv;
>> + struct arm_smmu_device *smmu = smmu_domain->smmu;
>> + struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
>> + const struct actlr_config *actlrcfg;
>> + size_t actlrcfg_size;
>> + int cbndx = smmu_domain->cfg.cbndx;
>>
>> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>>
>> @@ -238,6 +274,12 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>> priv->set_stall = qcom_adreno_smmu_set_stall;
>> priv->resume_translation = qcom_adreno_smmu_resume_translation;
>>
>> + if (qsmmu->data->actlrcfg_gfx) {
>> + actlrcfg = qsmmu->data->actlrcfg_gfx;
>> + actlrcfg_size = qsmmu->data->actlrcfg_gfx_size;
>> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
>> + }
>> +
>> return 0;
>> }
>>
>> @@ -263,6 +305,18 @@ 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;
>> + size_t actlrcfg_size;
>> + int cbndx = smmu_domain->cfg.cbndx;
>> +
>> + if (qsmmu->data->actlrcfg) {
>> + actlrcfg = qsmmu->data->actlrcfg;
>> + actlrcfg_size = qsmmu->data->actlrcfg_size;
>> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
>> + }
>> +
>> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>>
>> return 0;
>> @@ -464,7 +518,7 @@ static struct arm_smmu_device *qcom_smmu_create(struct arm_smmu_device *smmu,
>> return ERR_PTR(-ENOMEM);
>>
>> qsmmu->smmu.impl = impl;
>> - qsmmu->cfg = data->cfg;
>> + qsmmu->data = data;
>
> This should go to a separate commit. It is not related to ACTLR support
>
>>
>> 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..138fc57f7b0d 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h
>> @@ -8,7 +8,7 @@
>>
>> struct qcom_smmu {
>> struct arm_smmu_device smmu;
>> - const struct qcom_smmu_config *cfg;
>> + const struct qcom_smmu_match_data *data;
>> bool bypass_quirk;
>> u8 bypass_cbndx;
>> u32 stall_enabled;
>> @@ -25,6 +25,10 @@ struct qcom_smmu_config {
>> };
>>
>> struct qcom_smmu_match_data {
>> + const struct actlr_config *actlrcfg;
>> + size_t actlrcfg_size;
>> + const struct actlr_config *actlrcfg_gfx;
>> + size_t actlrcfg_gfx_size;
>> 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 d6d1a2a55cc0..8e4faf015286 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
>> @@ -990,9 +990,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)
>
> A pointer to the struct, please
Noted, will evaluate address this in next revision.
Thanks & Regards,
Bibek
>
>> +{
>> + 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
>>
>
>
On 27.11.2023 15:54, Bibek Kumar Patro 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@quicinc.com>
>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 56 +++++++++++++++++++++-
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 6 ++-
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++
> 4 files changed, 68 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 7f52ac67495f..4a38cae29be2 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -14,6 +14,12 @@
>
> #define QCOM_DUMMY_VAL -1
>
> +struct actlr_config {
> + 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);
> @@ -205,10 +211,40 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu)
> return true;
> }
>
> +static void arm_smmu_set_actlr(struct device *dev, struct arm_smmu_device *smmu, int cbndx,
> + const struct actlr_config *actlrcfg, size_t actlrcfg_size)
> +{
> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> + struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
> + struct arm_smmu_smr *smr;
> + int i;
> + int j;
> + u16 id;
> + u16 mask;
> + int idx;
> +
> + for (i = 0; i < actlrcfg_size; ++i) {
> + id = (actlrcfg + i)->sid;
> + mask = (actlrcfg + i)->mask;
actrlcfg[i].id?
> +
> + for_each_cfg_sme(cfg, fwspec, j, idx) {
> + smr = &smmu->smrs[idx];
> + if (smr_is_subset(*smr, id, mask))
Any reason for this value to be a pointer?
> + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR,
> + (actlrcfg + i)->actlr);
ditto
> + }
> + }
> +}
> +
> static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
> {
> struct adreno_smmu_priv *priv;
> + struct arm_smmu_device *smmu = smmu_domain->smmu;
> + struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
> + const struct actlr_config *actlrcfg;
> + size_t actlrcfg_size;
> + int cbndx = smmu_domain->cfg.cbndx;
Reverse-Christmas-tree sorting, please
>
> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>
> @@ -238,6 +274,12 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
> priv->set_stall = qcom_adreno_smmu_set_stall;
> priv->resume_translation = qcom_adreno_smmu_resume_translation;
>
> + if (qsmmu->data->actlrcfg_gfx) {
> + actlrcfg = qsmmu->data->actlrcfg_gfx;
> + actlrcfg_size = qsmmu->data->actlrcfg_gfx_size;
These can be passed directly s arm_smmu_set_actrl arguments
> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
> + }
> +
> return 0;
> }
>
> @@ -263,6 +305,18 @@ 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;
> + size_t actlrcfg_size;
> + int cbndx = smmu_domain->cfg.cbndx;
> +
> + if (qsmmu->data->actlrcfg) {
> + actlrcfg = qsmmu->data->actlrcfg;
> + actlrcfg_size = qsmmu->data->actlrcfg_size;
> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
ditto
Konrad
On 11/27/2023 9:03 PM, Konrad Dybcio wrote:
> On 27.11.2023 15:54, Bibek Kumar Patro 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@quicinc.com>
>>
>> ---
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 56 +++++++++++++++++++++-
>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 6 ++-
>> drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +-
>> drivers/iommu/arm/arm-smmu/arm-smmu.h | 5 ++
>> 4 files changed, 68 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index 7f52ac67495f..4a38cae29be2 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -14,6 +14,12 @@
>>
>> #define QCOM_DUMMY_VAL -1
>>
>> +struct actlr_config {
>> + 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);
>> @@ -205,10 +211,40 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu)
>> return true;
>> }
>>
>> +static void arm_smmu_set_actlr(struct device *dev, struct arm_smmu_device *smmu, int cbndx,
>> + const struct actlr_config *actlrcfg, size_t actlrcfg_size)
>> +{
>> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
>> + struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev);
>> + struct arm_smmu_smr *smr;
>> + int i;
>> + int j;
>> + u16 id;
>> + u16 mask;
>> + int idx;
>> +
>> + for (i = 0; i < actlrcfg_size; ++i) {
>> + id = (actlrcfg + i)->sid;
>> + mask = (actlrcfg + i)->mask;
> actrlcfg[i].id?
>
Noted, array indexing instead of incrementing the base address
should also work.
>> +
>> + for_each_cfg_sme(cfg, fwspec, j, idx) {
>> + smr = &smmu->smrs[idx];
>> + if (smr_is_subset(*smr, id, mask))
> Any reason for this value to be a pointer?
>
>> + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR,
>> + (actlrcfg + i)->actlr);
> ditto
>
Noted
>> + }
>> + }
>> +}
>> +
>> static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev)
>> {
>> struct adreno_smmu_priv *priv;
>> + struct arm_smmu_device *smmu = smmu_domain->smmu;
>> + struct qcom_smmu *qsmmu = to_qcom_smmu(smmu);
>> + const struct actlr_config *actlrcfg;
>> + size_t actlrcfg_size;
>> + int cbndx = smmu_domain->cfg.cbndx;
> Reverse-Christmas-tree sorting, please
>
Noted, thanks for pointing this, I will
take care of this in next revision.
>>
>> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true;
>>
>> @@ -238,6 +274,12 @@ static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain,
>> priv->set_stall = qcom_adreno_smmu_set_stall;
>> priv->resume_translation = qcom_adreno_smmu_resume_translation;
>>
>> + if (qsmmu->data->actlrcfg_gfx) {
>> + actlrcfg = qsmmu->data->actlrcfg_gfx;
>> + actlrcfg_size = qsmmu->data->actlrcfg_gfx_size;
> These can be passed directly s arm_smmu_set_actrl arguments
>
Noted, will address in next revision. since there won't be any issue
during time of access, I can pass these values directly.
>> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
>> + }
>> +
>> return 0;
>> }
>>
>> @@ -263,6 +305,18 @@ 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;
>> + size_t actlrcfg_size;
>> + int cbndx = smmu_domain->cfg.cbndx;
>> +
>> + if (qsmmu->data->actlrcfg) {
>> + actlrcfg = qsmmu->data->actlrcfg;
>> + actlrcfg_size = qsmmu->data->actlrcfg_size;
>> + arm_smmu_set_actlr(dev, smmu, cbndx, actlrcfg, actlrcfg_size);
> ditto
>
Noted.
> Konrad
© 2016 - 2025 Red Hat, Inc.