| 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: <bfdb1125-3dcc-496e-8380-80ba669dfe20@oss.qualcomm.com>
Date: Thu, 18 Dec 2025 13:02:32 +0100
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Bibek Kumar Patro <bibek.patro@....qualcomm.com>,
Charan Teja Kalla <charan.kalla@....qualcomm.com>,
robin.clark@....qualcomm.com, will@...nel.org, robin.murphy@....com,
joro@...tes.org, dmitry.baryshkov@....qualcomm.com
Cc: iommu@...ts.linux.dev, linux-arm-msm@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iommu/arm-smmu: add actlr settings for mdss and fastrpc
On 11/27/25 12:14 PM, Bibek Kumar Patro wrote:
>
>
> On 11/12/2025 7:32 PM, Konrad Dybcio wrote:
>> On 11/11/25 3:02 PM, Charan Teja Kalla wrote:
>>>
>>>
>>> On 11/5/2025 2:44 PM, Konrad Dybcio wrote:
>>>>> + { .compatible = "qcom,fastrpc-compute-cb",
>>>>> + .data = (const void *) (PREFETCH_DEEP | CPRE | CMTLB) },
>>>> This will be globally applied to all smmu-v2 targets with fastrpc,
>>>> starting from MSM8996 ending at Kaanapali and everything inbetween
>>>>
>>>> Are these settings always valid?
>>> Oops, you are correct...this settings are not always applicable it seems.
>>>
>>> Example: lpass compute and cdsp compute node uses the
>>> "qcom,fastrpc-compute-cb", but it is for the later that ACTLR settings
>>> are valid.
>>>
>>> Then for these cases, should we be extending the actlr match array with
>>> additional compatible string and then add them in all the device nodes
>>> that require actlr setting? example:
>>>
>>> @@ -3119,7 +3119,8 @@ fastrpc {
>>> compute-cb@1 {
>>> - compatible = "qcom,fastrpc-compute-cb";
>>> + compatible = "qcom,fastrpc-compute-cb",
>>> + "qcom,fastrpc-compute-cb-actlr";
>>
>> dt-bindings (and especially compatible strings) must not be altered solely
>> to work around a driver being suboptimal
>>
>> But because you reported that these settings can change both between
>> platforms and instances of the same devices on these platforms, we could
>> possibly reconsider adding ACTRL settings to the consumer device nodes
>> where they stray away from the otherwise seemingly reasonable baseline
>> we have in the driver so far..
>>
>
> During initial discussion of ACTLR design phase, It was concluded that
> prefetch alike QoS settings are not hardware definitions, and device
> tree would not be the right place to store such configurable/tunable
> settings [1]. So it might be contradicting to add it in consumer nodes.
>
> For fastrpc client, there are 2 hiccups:
> 1. For a particular SoC, Different SIDs of compute client are using the
> same compatible "qcom,fastrpc-compute-cb" but can have different
> prefetch settings <refer table below> e.g. CDSP and ADSP have same
> compat string "qcom,fastrpc-compute-cb" with different labels.
>
> e.g.
> fastrpc {
> compatible = "qcom,fastrpc";
> qcom,glink-channels = "fastrpcglink-apps-dsp";
> label = "adsp"; /*---different label: adsp---*/
> compute-cb@3 {
> compatible = "qcom,fastrpc-compute-cb";
> reg = <3>;
> /*---Prefetch Default---*/
> iommus = <&apps_smmu 0x1003 0x80>,
> <&apps_smmu 0x1063 0x0>;
> dma-coherent;
> fastrpc {
> compatible = "qcom,fastrpc";
> qcom,glink-channels = "fastrpcglink-apps-dsp";
> label = "cdsp"; /*---Different Label: cdsp---*/
> qcom,non-secure-domain;
> #address-cells = <1>;
> #size-cells = <0>;
> compute-cb@1 {
> compatible = "qcom,fastrpc-compute-cb";
> reg = <1>;
> /*---Prefetch DEEP----*/
> iommus = <&apps_smmu 0x1961 0x0>,
> <&apps_smmu 0x0c01 0x20>,
> <&apps_smmu 0x19c1 0x10>;
>
> 2. Different SoCs having the same compatible string and same label but
> different prefetch settings.
> e.g:
> in below table sm6550, sm8250 and sm8550
> compat string = "qcom,fastrpc-compute-cb" and label = "cdsp"
> but prefetch settings are different
>
> As per my idea both the above cases could be resolved by compat string addition,
> case 1 : compatible = "qcom,fastrpc-compute-cb-cdsp";
> case 2 : compatible = "qcom,sm8550-fastrpc-compute-cb-cdsp"
>
> This discrpeancy is not applicable for other clients, except fastrpc.
> One way both the above cases could be resolved by additional fastrpc
> compat string with below format:
> compatible = "qcom,<soc_name>-fastrpc-compute-cb-<label_name>"
>
> e.g: "qcom,sm8550-fastrpc-compute-cb-cdsp", "qcom,sm6550-fastrpc-compute-cb-adsp"
>
> This should handle both case 1 & case 2.
>
> But as you mentioned previously this might need change alteration of DT bindings addition which might not be allowed or favoured always.
>
> [1]: https://lore.kernel.org/all/a01e7e60-6ead-4a9e-ba90-22a8a6bbd03f@quicinc.com/
>
>> Or we could introduce some more bespoke matching logic.
>>
>> I would like to know more about the scope of this issue, i.e. the matrix
>> of (soc, device, actlr_val) that needs special handling.
>>
>
> +---------+-----------------------------------------------+----------+
> | SoC | Description | Prefetch |
> +---------+-----------------------------------------------+----------+
> | sc7180 | qcom,fastrpc-compute-cb | label = cdsp | DEEP |
> | | qcom,fastrpc-compute-cb | label = adsp | DEFAULT |
> +---------+-----------------------------------------------+----------+
> | sc7280 | qcom,fastrpc-compute-cb | label = cdsp | DEEP |
> | | qcom,fastrpc-compute-cb | label = adsp | DEFAULT |
> +---------+-----------------------------------------------+----------+
> | sm6115 | qcom,fastrpc-compute-cb | label = cdsp | DEFAULT |
> | | qcom,fastrpc-compute-cb | label = adsp | DEFAULT |
> +---------+-----------------------------------------------+----------+
> | sm6125 | qcom,fastrpc-compute-cb (NA upstream) | |
> +---------+-----------------------------------------------+----------+
> | sm6350 | qcom,fastrpc-compute-cb | label = cdsp | SHALLOW |
> | | qcom,fastrpc-compute-cb | label = adsp | SHALLOW |
> +---------+-----------------------------------------------+----------+
> | sm8250 | qcom,fastrpc-compute-cb | label = cdsp | DEFAULT |
> | | qcom,fastrpc-compute-cb | label = adsp | DEFAULT |
> | | qcom,fastrpc-compute-cb | label = sdsp | DEFAULT |
> +---------+-----------------------------------------------+----------+
> | sm8350 | qcom,fastrpc-compute-cb | label = cdsp | DEEP |
> | | qcom,fastrpc-compute-cb | label = adsp | DEFAULT |
> | | qcom,fastrpc-compute-cb | label = sdsp | DEFAULT |
> +---------+-----------------------------------------------+----------+
> | sm8450 | qcom,fastrpc-compute-cb | label = cdsp | DEEP |
> | | qcom,fastrpc-compute-cb | label = adsp | DEFAULT |
> | | qcom,fastrpc-compute-cb | label = sdsp | DEFAULT |
> +---------+-----------------------------------------------+----------+
> | sm8550 | qcom,fastrpc-compute-cb | label = cdsp | DEEP |
> | | qcom,fastrpc-compute-cb | label = adsp | DEFAULT |
> +---------+-----------------------------------------------+----------+
> | sm8650 | qcom,fastrpc-compute-cb | label = cdsp | DEEP |
> | | qcom,fastrpc-compute-cb | label = adsp | DEFAULT |
> +---------+-----------------------------------------------+----------+
Thanks for the detailed explanation. It feel quite impossible to keep
track of all this in the current model.
On the other hand, the approach where a third OF cell is introduced
isn't my favorite thing in the world, and that seems to be the general
sentiment.
FWIW downstream solves this by carrying the list of <sid mask val> under
the SMMU node. Perhaps that could be reconsidered, either as a storage
for all the values, or as storage for non-default overrides.. Krzysztof,
Bjorn, opinions?
Konrad
Powered by blists - more mailing lists