| Message ID | 20240920155813.3434021-5-quic_bibekkum@quicinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iommu/arm-smmu: introduction of ACTLR implementation for Qualcomm SoCs | expand |
On Fri, Sep 20, 2024 at 09:28:12PM GMT, 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 custom prefetch setting 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 | 26 ++++++++++++++++++++++ > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 1 + > 2 files changed, 27 insertions(+) > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > index 38ac9cab763b..4ac272d05843 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > @@ -252,6 +252,20 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu) > return true; > } > > +static void qcom_smmu_set_actlr_dev(struct device *dev, struct arm_smmu_device *smmu, int cbndx, > + const struct of_device_id *client_match) > +{ > + const struct of_device_id *match = > + of_match_device(client_match, dev); > + > + if (!match) { > + dev_notice(dev, "no ACTLR settings present\n"); dev_dbg() or even dev_vdbg(), please. We do not want to spam people with messages about a perfectly normal behaviour. LGTM otherwise. > + return; > + } > + > + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR, (u64)match->data); > +} > + > static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain, > struct io_pgtable_cfg *pgtbl_cfg, struct device *dev) > { > @@ -316,8 +330,20 @@ 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 of_device_id *client_match; > + int cbndx = smmu_domain->cfg.cbndx; > + > smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; > > + client_match = qsmmu->data->client_match; > + > + if (!client_match) > + return 0; > + > + qcom_smmu_set_actlr_dev(dev, smmu, cbndx, client_match); > + > return 0; > } > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h > index b55cd3e3ae48..8addd453f5f1 100644 > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h > @@ -28,6 +28,7 @@ struct qcom_smmu_match_data { > const struct qcom_smmu_config *cfg; > const struct arm_smmu_impl *impl; > const struct arm_smmu_impl *adreno_impl; > + const struct of_device_id * const client_match; > }; > > irqreturn_t qcom_smmu_context_fault(int irq, void *dev); > -- > 2.34.1 >
On 9/20/2024 9:50 PM, Dmitry Baryshkov wrote: > On Fri, Sep 20, 2024 at 09:28:12PM GMT, 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 custom prefetch setting 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 | 26 ++++++++++++++++++++++ >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 1 + >> 2 files changed, 27 insertions(+) >> >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> index 38ac9cab763b..4ac272d05843 100644 >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >> @@ -252,6 +252,20 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu) >> return true; >> } >> >> +static void qcom_smmu_set_actlr_dev(struct device *dev, struct arm_smmu_device *smmu, int cbndx, >> + const struct of_device_id *client_match) >> +{ >> + const struct of_device_id *match = >> + of_match_device(client_match, dev); >> + >> + if (!match) { >> + dev_notice(dev, "no ACTLR settings present\n"); > > dev_dbg() or even dev_vdbg(), please. We do not want to spam people with > messages about a perfectly normal behaviour. > > LGTM otherwise. Ack, thanks Dmitry for the suggestion. I was actually wondering if to broadcast this message over the logs through dev_notice or give a option to print it in debug level through dev_dbg. But I'm inline with your suggestion now, will switch to dev_dbg for this message. Thanks & regards, Bibek > > >> + return; >> + } >> + >> + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR, (u64)match->data); >> +} >> + >> static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain, >> struct io_pgtable_cfg *pgtbl_cfg, struct device *dev) >> { >> @@ -316,8 +330,20 @@ 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 of_device_id *client_match; >> + int cbndx = smmu_domain->cfg.cbndx; >> + >> smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; >> >> + client_match = qsmmu->data->client_match; >> + >> + if (!client_match) >> + return 0; >> + >> + qcom_smmu_set_actlr_dev(dev, smmu, cbndx, client_match); >> + >> return 0; >> } >> >> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h >> index b55cd3e3ae48..8addd453f5f1 100644 >> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h >> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h >> @@ -28,6 +28,7 @@ struct qcom_smmu_match_data { >> const struct qcom_smmu_config *cfg; >> const struct arm_smmu_impl *impl; >> const struct arm_smmu_impl *adreno_impl; >> + const struct of_device_id * const client_match; >> }; >> >> irqreturn_t qcom_smmu_context_fault(int irq, void *dev); >> -- >> 2.34.1 >> >
Hi Bibek, kernel test robot noticed the following build warnings: [auto build test WARNING on linus/master] [also build test WARNING on joro-iommu/next v6.11 next-20240920] [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-re-enable-context-caching-in-smmu-reset-operation/20240921-000146 base: linus/master patch link: https://lore.kernel.org/r/20240920155813.3434021-5-quic_bibekkum%40quicinc.com patch subject: [PATCH v15 4/5] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings config: arm-qcom_defconfig (https://download.01.org/0day-ci/archive/20240923/202409230343.Q8KnYl2w-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 13.3.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240923/202409230343.Q8KnYl2w-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/202409230343.Q8KnYl2w-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c:14: drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c: In function 'qcom_smmu_set_actlr_dev': >> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c:266:59: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 266 | arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR, (u64)match->data); | ^ drivers/iommu/arm/arm-smmu/arm-smmu.h:533:59: note: in definition of macro 'arm_smmu_cb_write' 533 | arm_smmu_writel((s), ARM_SMMU_CB((s), (n)), (o), (v)) | ^ vim +266 drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c 254 255 static void qcom_smmu_set_actlr_dev(struct device *dev, struct arm_smmu_device *smmu, int cbndx, 256 const struct of_device_id *client_match) 257 { 258 const struct of_device_id *match = 259 of_match_device(client_match, dev); 260 261 if (!match) { 262 dev_notice(dev, "no ACTLR settings present\n"); 263 return; 264 } 265 > 266 arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR, (u64)match->data); 267 } 268
On 9/23/2024 1:06 AM, kernel test robot wrote: > All warnings (new ones prefixed by >>): > > In file included from drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c:14: > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c: In function 'qcom_smmu_set_actlr_dev': >>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c:266:59: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] > 266 | arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR, (u64)match->data); > | ^ > drivers/iommu/arm/arm-smmu/arm-smmu.h:533:59: note: in definition of macro 'arm_smmu_cb_write' > 533 | arm_smmu_writel((s), ARM_SMMU_CB((s), (n)), (o), (v)) > | I'll fix this warning in next revision after other reviews are received. regards, Bibek
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c index 38ac9cab763b..4ac272d05843 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c @@ -252,6 +252,20 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu) return true; } +static void qcom_smmu_set_actlr_dev(struct device *dev, struct arm_smmu_device *smmu, int cbndx, + const struct of_device_id *client_match) +{ + const struct of_device_id *match = + of_match_device(client_match, dev); + + if (!match) { + dev_notice(dev, "no ACTLR settings present\n"); + return; + } + + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR, (u64)match->data); +} + static int qcom_adreno_smmu_init_context(struct arm_smmu_domain *smmu_domain, struct io_pgtable_cfg *pgtbl_cfg, struct device *dev) { @@ -316,8 +330,20 @@ 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 of_device_id *client_match; + int cbndx = smmu_domain->cfg.cbndx; + smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; + client_match = qsmmu->data->client_match; + + if (!client_match) + return 0; + + qcom_smmu_set_actlr_dev(dev, smmu, cbndx, client_match); + return 0; } diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h index b55cd3e3ae48..8addd453f5f1 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h @@ -28,6 +28,7 @@ struct qcom_smmu_match_data { const struct qcom_smmu_config *cfg; const struct arm_smmu_impl *impl; const struct arm_smmu_impl *adreno_impl; + const struct of_device_id * const client_match; }; irqreturn_t qcom_smmu_context_fault(int irq, void *dev);
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 custom prefetch setting 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 | 26 ++++++++++++++++++++++ drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h | 1 + 2 files changed, 27 insertions(+) -- 2.34.1