| 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: <212c4f7f-4b22-4e21-9848-906bbfd69f0e@quicinc.com>
Date: Mon, 6 Nov 2023 11:42:12 +0530
From: Bibek Kumar Patro <quic_bibekkum@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: <will@...nel.org>, <robin.murphy@....com>, <joro@...tes.org>,
<a39.skl@...il.com>, <konrad.dybcio@...aro.org>,
<quic_saipraka@...cinc.com>, <quic_pkondeti@...cinc.com>,
<quic_molvera@...cinc.com>, <linux-arm-kernel@...ts.infradead.org>,
<iommu@...ts.linux.dev>, <linux-kernel@...r.kernel.org>,
<qipl.kernel.upstream@...cinc.com>
Subject: Re: [PATCH 1/3] iommu/arm-smmu: introduction of ACTLR for custom
prefetcher settings
On 11/4/2023 3:33 AM, Dmitry Baryshkov wrote:
> On Fri, 3 Nov 2023 at 23:53, 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 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@...cinc.com>
>> ---
>> 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.
>> +};
>> +
>> +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
>>
>
>
Powered by blists - more mailing lists