linux-kernel - Re: [RFC 4/7] soc: qcom: Utilize qcom scmi vendor protocol for bus dvfs

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] [day] [month] [year] [list]

Message-ID: <767cfb35-ed52-4d51-b1bb-c69ac5b593b4@linaro.org>
Date: Fri, 12 Jul 2024 14:20:00 +0200
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Sibi Sankar <quic_sibis@...cinc.com>, sudeep.holla@....com,
 cristian.marussi@....com, andersson@...nel.org, jassisinghbrar@...il.com,
 robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org
Cc: linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 devicetree@...r.kernel.org, quic_rgottimu@...cinc.com,
 quic_kshivnan@...cinc.com, conor+dt@...nel.org,
 Amir Vajid <avajid@...cinc.com>
Subject: Re: [RFC 4/7] soc: qcom: Utilize qcom scmi vendor protocol for bus
 dvfs

On 1.07.2024 10:44 AM, Sibi Sankar wrote:
> 
> 
> On 6/19/24 01:07, Konrad Dybcio wrote:
>>
>>
>> On 2/12/24 11:33, Sibi Sankar wrote:
>>
>> [...]
>>
>>
>>>>
>>>>> +            monitor->mon_type = (of_property_read_bool(monitor_np, "qcom,compute-mon")) ? 1 : 0;

BTW: the ternary operator here is unnecessary, but to make it readable,
please make an enum / #define describing the two, as magic values are
discouraged

>>>>> +            monitor->ipm_ceil = (of_property_read_bool(monitor_np, "qcom,compute-mon")) ? 0 : 20000000;

Given that you check the same condition here, an if-else block may be
more readable, perhaps some comment like:

ipm_ceil = 0; /* Always keep a vote, no matter the bus traffic */

>>>>
>>>> What does it even mean for a monitor to be a compute mon?
>>>>
>>>
>>> When a monitor is marked compute-mon it means that the table is
>>> followed religiously irrespective whether the instruction per miss
>>> count threshold (ipm) is exceeded or not. Equivalent to having
>>> a cpufreq map -> l3/DDR bw mapping upstream.

I.. don't really like that this exists as something that requires OS
intervention, but since it does, I suppose it takes a couple lines of
code less than adding OPP entries for each and every PSTATE and NUM_SKUs..

>>
>> I'm sorta puzzled why the OS would even be required to program this, since
>> L3/DDR/CPU frequencies are known by various stages of boot and secure firmware
>> too.
>>
>> What happens if we omit this? Is the default configuration identical to this?
>> Or does it need explicit enabling?
> 
> CPUCP isn't expected to know the various ranges supported by the memory
> buses it can vote on and from a sandboxing perspective one would want to
> control what CPUCP has access to as well. It also can't arrive at the
> exact values just from the OPP tables we pass on as well. So it doesn't
> have any default values to start off with. For all these reasons, they
> need explicit setting up and without it, the algorithm wouldn't function
> as expected.

Ok, I was thinking more of a scenario where XBL/GH would take care of this..

Throwing in my 5 cents, this could perhaps be moved there in future FW
designs (the earlier in the chain the better, especially to keep kicking
out gunyah a viable option), as I don't think Linux is the greatest place
for storing one-shot configuration data, especially for blocks that already
run their own firmware..

I would imagine this could speed up booting as well, if DRAM was appropriately
scaled during the boot splash stage (unless it already is either scaled or
pinned to FMAX)

Konrad