lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAA8EJppypaVCBLV1rGPUgbVdtm9Zz9haJiYd=4ar+-bXLPh7DA@mail.gmail.com> Date: Thu, 21 Dec 2023 02:37:25 +0200 From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org> To: Bibek Kumar Patro <quic_bibekkum@...cinc.com> Cc: will@...nel.org, robin.murphy@....com, joro@...tes.org, konrad.dybcio@...aro.org, jsnitsel@...hat.com, quic_bjorande@...cinc.com, mani@...nel.org, quic_eberman@...cinc.com, robdclark@...omium.org, u.kleine-koenig@...gutronix.de, robh@...nel.org, vladimir.oltean@....com, quic_pkondeti@...cinc.com, quic_molvera@...cinc.com, linux-arm-msm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, iommu@...ts.linux.dev, linux-kernel@...r.kernel.org, qipl.kernel.upstream@...cinc.com Subject: Re: [PATCH v6 3/5] iommu/arm-smmu: introduction of ACTLR for custom prefetcher settings On Thu, 21 Dec 2023 at 02:36, Dmitry Baryshkov <dmitry.baryshkov@...aro.org> wrote: > > On Wed, 20 Dec 2023 at 15:39, Bibek Kumar Patro > <quic_bibekkum@...cinc.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 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@...cinc.com> > > --- > > drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 45 ++++++++++++++++++++++ > > 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, 58 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > > index 20c9836d859b..1cefdd0ca110 100644 > > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c > > @@ -24,6 +24,12 @@ > > #define CPRE (1 << 1) > > #define CMTLB (1 << 0) > > > > +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); > > @@ -215,9 +221,38 @@ static bool qcom_adreno_can_do_ttbr1(struct arm_smmu_device *smmu) > > return true; > > } > > > > +static void qcom_smmu_set_actlr(struct device *dev, struct arm_smmu_device *smmu, int cbndx, > > + const struct actlr_config *actlrcfg) > > +{ > > + struct arm_smmu_master_cfg *cfg = dev_iommu_priv_get(dev); > > + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > > + struct arm_smmu_smr *smr; > > + u16 mask; > > + int idx; > > + u16 id; > > + int i; > > + > > + for (; actlrcfg->sid || actlrcfg->mask || actlrcfg->actlr; actlrcfg++) { > > + id = actlrcfg->sid; > > + mask = actlrcfg->mask; > > + > > + for_each_cfg_sme(cfg, fwspec, i, idx) { > > + smr = &smmu->smrs[idx]; > > + if (smr_is_subset(smr, id, mask)) { > > + arm_smmu_cb_write(smmu, cbndx, ARM_SMMU_CB_ACTLR, > > + actlrcfg->actlr); > > + break; > > + } > > + } > > + } > > +} > > + > > static int qcom_adreno_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); > > + int cbndx = smmu_domain->cfg.cbndx; > > struct adreno_smmu_priv *priv; > > > > smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; > > @@ -248,6 +283,9 @@ 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) > > + qcom_smmu_set_actlr(dev, smmu, cbndx, qsmmu->data->actlrcfg_gfx); > > There was a feedback point against v4 that there can be more than two > (apps + gpu) SMMU devices. No, we can not use additional compat > strings, the SMMU units are compatible with each other. Please add > matching between the smmu and particular actlr table using the IO > address of the SMMU block. > > > + > > return 0; > > } > > > > @@ -274,6 +312,13 @@ 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); > > + int cbndx = smmu_domain->cfg.cbndx; > > + > > + if (qsmmu->data->actlrcfg) > > + qcom_smmu_set_actlr(dev, smmu, cbndx, qsmmu->data->actlrcfg); > > + > > One issue occured to me, while I was reviewing the patchset. The ACTLR > settings are related to the whole SMMU setup, but we are applying them > each time there is an SMMU context init (in other words, one per each > domain). Is that correct? Or it's just that there is no better place > for initialising the global register set? Would it be better to > reprogram the ACTLR registers which are related just to this > particular domain? Ignore this, I went back to qcom_smmu_set_actlr() and understood that I failed to read it properly. > > > smmu_domain->cfg.flush_walk_prefer_tlbiasid = true; > > > > 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 f3b91963e234..cb4cb402c202 100644 > > --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h > > +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.h > > @@ -1,6 +1,6 @@ > > /* SPDX-License-Identifier: GPL-2.0-only */ > > /* > > - * Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved. > > + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved. > > */ > > > > #ifndef _ARM_SMMU_QCOM_H > > @@ -24,7 +24,11 @@ struct qcom_smmu_config { > > const u32 *reg_offset; > > }; > > > > +struct actlr_config; > > + > > struct qcom_smmu_match_data { > > + const struct actlr_config *actlrcfg; > > + const struct actlr_config *actlrcfg_gfx; > > 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..0c7f700b27dd 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..2e4f65412c6b 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 > > > > > -- > With best wishes > Dmitry -- With best wishes Dmitry
Powered by blists - more mailing lists