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: <1eee8bae-59f0-4066-9d04-8c3a5f750d3a@linaro.org> Date: Sat, 16 Dec 2023 01:03:07 +0100 From: Konrad Dybcio <konrad.dybcio@...aro.org> To: Robin Murphy <robin.murphy@....com>, Bibek Kumar Patro <quic_bibekkum@...cinc.com>, Dmitry Baryshkov <dmitry.baryshkov@...aro.org> Cc: will@...nel.org, joro@...tes.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 v4 3/5] iommu/arm-smmu: add ACTLR data and support for SM8550 On 15.12.2023 13:54, Robin Murphy wrote: > On 2023-12-15 12:20 pm, Bibek Kumar Patro wrote: >> >> >> On 12/15/2023 4:14 PM, Dmitry Baryshkov wrote: >>> On Fri, 15 Dec 2023 at 12:19, Bibek Kumar Patro >>> <quic_bibekkum@...cinc.com> wrote: >>>> >>>> Add ACTLR data table for SM8550 along with support for >>>> same including SM8550 specific implementation operations. >>>> >>>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@...cinc.com> >>>> --- >>>> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 89 ++++++++++++++++++++++ >>>> 1 file changed, 89 insertions(+) >>>> >>>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> index cb49291f5233..d2006f610243 100644 >>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c >>>> @@ -20,6 +20,85 @@ struct actlr_config { >>>> u32 actlr; >>>> }; >>>> >>>> +/* >>>> + * SMMU-500 TRM defines BIT(0) as CMTLB (Enable context caching in the >>>> + * macro TLB) and BIT(1) as CPRE (Enable context caching in the prefetch >>>> + * buffer). The remaining bits are implementation defined and vary across >>>> + * SoCs. >>>> + */ >>>> + >>>> +#define PREFETCH_DEFAULT 0 >>>> +#define PREFETCH_SHALLOW BIT(8) >>>> +#define PREFETCH_MODERATE BIT(9) >>>> +#define PREFETCH_DEEP (BIT(9) | BIT(8)) >>> >>> I thin the following might be more correct: >>> >>> #include <linux/bitfield.h> >>> >>> #define PREFETCH_MASK GENMASK(9, 8) >>> #define PREFETCH_DEFAULT FIELD_PREP(PREFETCH_MASK, 0) >>> #define PREFETCH_SHALLOW FIELD_PREP(PREFETCH_MASK, 1) >>> #define PREFETCH_MODERATE FIELD_PREP(PREFETCH_MASK, 2) >>> #define PREFETCH_DEEP FIELD_PREP(PREFETCH_MASK, 3) >>> >> >> Ack, thanks for this suggestion. Let me try this out using >> GENMASK. Once tested, will take care of this in next version. > > FWIW the more typical usage would be to just define the named macros for the raw field values, then put the FIELD_PREP() at the point of use. However in this case that's liable to get pretty verbose, so although I'm usually a fan of bitfield.h, the most readable option here might actually be to stick with simpler definitions of "(0 << 8)", "(1 << 8)", etc. However it's not really a big deal either way, and I defer to whatever Dmitry and Konrad prefer, since they're the ones looking after arm-smmu-qcom the most :) My 5 cents would be to just use the "common" style of doing this, so: #define ACTRL_PREFETCH GENMASK(9, 8) #define PREFETCH_DEFAULT 0 #define PREFETCH_SHALLOW 1 #define PREFETCH_MODERATE 2 #define PREFETCH_DEEP 3 and then use | FIELD_PREP(ACTRL_PREFETCH, PREFETCH_x) it can get verbose, but.. arguably that's good, since you really want to make sure the right bits are set here Konrad
Powered by blists - more mailing lists