linux-kernel - Re: [PATCH v14 5/6] iommu/arm-smmu: add ACTLR data and support for SC7280

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: <3ae75a75-1717-40b6-9149-bc3673d520d6@quicinc.com>
Date: Mon, 26 Aug 2024 16:33:24 +0530
From: Bibek Kumar Patro <quic_bibekkum@...cinc.com>
To: Will Deacon <will@...nel.org>
CC: <robdclark@...il.com>, <robin.murphy@....com>, <joro@...tes.org>,
        <jgg@...pe.ca>, <jsnitsel@...hat.com>, <robh@...nel.org>,
        <krzysztof.kozlowski@...aro.org>, <quic_c_gdjako@...cinc.com>,
        <dmitry.baryshkov@...aro.org>, <konrad.dybcio@...aro.org>,
        <iommu@...ts.linux.dev>, <linux-arm-msm@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v14 5/6] iommu/arm-smmu: add ACTLR data and support for
 SC7280



On 8/23/2024 9:29 PM, Will Deacon wrote:
> On Fri, Aug 16, 2024 at 11:12:58PM +0530, Bibek Kumar Patro wrote:
>> Add ACTLR data table for SC7280 along with support for
>> same including SC7280 specific implementation operations.
>>
>> Signed-off-by: Bibek Kumar Patro <quic_bibekkum@...cinc.com>
>> ---
>>   drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 58 +++++++++++++++++++++-
>>   1 file changed, 57 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> index dc143b250704..a776c7906c76 100644
>> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
>> @@ -31,6 +31,55 @@
>>   #define PREFETCH_MODERATE	(2 << PREFETCH_SHIFT)
>>   #define PREFETCH_DEEP		(3 << PREFETCH_SHIFT)
>>
>> +static const struct actlr_config sc7280_apps_actlr_cfg[] = {
>> +	{ 0x0800, 0x04e0, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x0900, 0x0402, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x0901, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x0d01, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x1181, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1182, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1183, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1184, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1185, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1186, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1187, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1188, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x1189, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x118b, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x118c, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x118d, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x118e, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x118f, 0x0420, PREFETCH_DEEP | CPRE | CMTLB },
>> +	{ 0x2000, 0x0020, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2040, 0x0000, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2062, 0x0000, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2080, 0x0020, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x20c0, 0x0020, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2100, 0x0020, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2140, 0x0000, PREFETCH_DEFAULT | CMTLB },
>> +	{ 0x2180, 0x0020, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x2181, 0x0004, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x2183, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x2184, 0x0020, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +	{ 0x2187, 0x0000, PREFETCH_SHALLOW | CPRE | CMTLB },
>> +};
>> +
>> +static const struct actlr_config sc7280_gfx_actlr_cfg[] = {
>> +	{ 0x0000, 0x07ff, PREFETCH_DEEP | CPRE | CMTLB },
>> +};
> 
> It's Will "stuck record" Deacon here again to say that I don't think
> this data belongs in the driver.
> 

Hi Will,

It will be difficult to reach a consensus here, with Robin and the DT 
folks okay to keep it in the driver, while you believe it doesn't belong 
there.

Robin, Rob, could you please share your thoughts on concluding the 
placement of this prefetch data?

As discussed earlier [1], the prefetch value for each client doesn’t 
define the hardware topology and is implementation-defined register 
writes used by the software driver.

We're okay with either approach, but these points [2] also raised in the
RFC led us to believe that switching from DT to the driver is the
right approach.


[1]:https://lore.kernel.org/all/2b0d8c5b-7e79-41ff-bc57-003d1b947c16@quicinc.com/
[2]:https://lore.kernel.org/all/a01e7e60-6ead-4a9e-ba90-22a8a6bbd03f@quicinc.com/


> Have a great weekend!
> 
> Will