linux-kernel - Re: [PATCH v2 2/3] iommu/arm-smmu: add ACTLR data and support for SM8550

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <61d2885a-50f9-419f-8473-10308baed493@quicinc.com>
Date:   Thu, 16 Nov 2023 11:39:49 +0530
From:   Bibek Kumar Patro <quic_bibekkum@...cinc.com>
To:     Konrad Dybcio <konrad.dybcio@...aro.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC:     <will@...nel.org>, <robin.murphy@....com>, <joro@...tes.org>,
        <a39.skl@...il.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 v2 2/3] iommu/arm-smmu: add ACTLR data and support for
 SM8550



On 11/15/2023 10:12 PM, Konrad Dybcio wrote:
> 
> 
> On 11/15/23 13:49, Bibek Kumar Patro wrote:
>>
>>
>> On 11/15/2023 4:15 PM, Dmitry Baryshkov wrote:
>>> On Wed, 15 Nov 2023 at 11:51, Bibek Kumar Patro
>>> <quic_bibekkum@...cinc.com> wrote:
>>>>
>>>>
>>>>
>>>> On 11/15/2023 3:08 PM, Dmitry Baryshkov wrote:
>>>>> On Wed, 15 Nov 2023 at 11:22, Bibek Kumar Patro
>>>>> <quic_bibekkum@...cinc.com> wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 11/14/2023 7:42 PM, Dmitry Baryshkov wrote:
>>>>>>> On Tue, 14 Nov 2023 at 15:57, 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 | 92 
>>>>>>>> +++++++++++++++++++++-
>>>>>>>>     1 file changed, 88 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 578c662c7c30..0eaf6f2a2e49 100644
>>>>>>>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>>>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>>>>>>>> @@ -25,6 +25,70 @@ struct actlr_data {
>>>>>>>>            u32 actlr;
>>>>>>>>     };
>>>>>>>>
>>>>>>>> +#define PRE_FETCH_1    0
>>>>>>>> +#define PRE_FETCH_2    BIT(8)
>>>>>>>> +#define PRE_FETCH_3    (BIT(9) | BIT(8))
>>>>>>>
>>>>>>> What is the difference between PRE_FETCH_3 and PRE_FETCH_2? And
>>>>>>> PRE_FETCH_1? Are these real numbers that refer to some amount / 
>>>>>>> count
>>>>>>> or just dummy names?
>>>>>>>
>>>>>>
>>>>>> No,these are not real numbers, but prefetch settings for a particular
>>>>>> perfect configuration.
>>>>>
>>>>> Then I'd ask for some better names or descriptions.
>>>>>
>>>>
>>>> Noted, PREFETCH_SETTING_n / PREFETCH_OPTION_n sounds like a better name
>>>> in the following case. Would it be okay to use this name instead?
>>>
>>> Not really.
>>>
>>
>> Any suggestion you have in mind, if not this nomenclature?
> Dmitry's concern seems to be that you provide:
> 
> PRE_FETCH_1 /* prefetcher with settings preset no. 1 */
> PRE_FETCH_2 /* prefetcher with settings preset no. 2 */
> PRE_FETCH_3 /* prefetcher with settings preset no. 3 */
> 
> whereas it would be both useful and interesting to see what these
> settings mean, i.e. what differences there are between all of
> these presets.
> 

Ah, okay got it now from Dimitry and yours' response.
But we exactly won't be able to reveal what each of these settings
mean, as this might risk of revealing IP as ACTLR bits are
implementation defined (except CPRE and CMTLB) which other SoC vendors
might be using it in different manner(or different purpose) in their
downstream implementation.
We can name it like (e.g PREFETCH_DISABLE, PREFETCH_SHALLOW,
PREFETCH_DEEP) to indicate the behaviour, but won't be exactly
name/describe it to explain what it does with a particular setting.

> Konrad