linux-kernel - Re: [PATCH 4/4] interconnect: qcom: icc-rpm: Remodel how QoS settings are stored

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]

Date: Tue, 18 Jun 2024 16:27:55 +0200
From: Konrad Dybcio <konrad.dybcio@...aro.org>
To: Mike Tipton <quic_mdtipton@...cinc.com>
Cc: Bjorn Andersson <andersson@...nel.org>, Georgi Djakov
 <djakov@...nel.org>, Shawn Guo <shawn.guo@...aro.org>,
 Marijn Suijten <marijn.suijten@...ainline.org>,
 linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org, Stephan Gerhold <stephan@...hold.net>,
 quic_okukatla@...cinc.com
Subject: Re: [PATCH 4/4] interconnect: qcom: icc-rpm: Remodel how QoS settings
 are stored



On 5/8/24 03:45, Mike Tipton wrote:
> Hi Konrad,
> 
> On Tue, Mar 26, 2024 at 08:42:35PM +0100, Konrad Dybcio wrote:
>> Currently, the QoS settings are stored in the node data, even though
>> they're a property of the bus/provider instead. Moreover, they are only
>> needed during the probe step, so they can be easily moved into struct
>> qcom_icc_desc.
> 
> The QoS settings *are* fundamentally a property of the node. The nodes
> are 1:1 with the NOC ports. And the QoS settings tune the priority of
> the data coming out of those ports. So, logically speaking, the QoS data
> does belong in the node structs along with the rest of the data for that
> node and port.
> 
> Only a subset of NOC ports support configurable QoS, but for those ports
> that do it's a property of the port itself. Those settings impact just
> that specific port and nothing else.
> 
> The current method of directly embedding the qcom_icc_qos_data struct
> into qcom_icc_node isn't optimal, since that data is irrelevant for
> ports that don't support it. So, the size could be optimized by
> converting qcom_icc_node::qos into a pointer instead. But I don't think
> we should separate the QoS settings from node struct entirely. It makes
> it very difficult to understand which QoS settings are impacting which
> port.

I think that would make more sense indeed

[...]

>>   
>> +static const struct qcom_icc_qos_data a0noc_qos_data[] = {
>> +	{
>> +		.qos_port = 0,
>> +		.qos_mode = NOC_QOS_MODE_FIXED,
>> +		.areq_prio = 1,
>> +		.prio_level = 1,
>> +		.urg_fwd_en = false,
>> +		.limit_commands = false,
>> +	}, {
> 
> How can I tell that these a0noc_qos_data[0] settings are for the
> mas_pcie_0 port? It's not possible from the code anymore. *We* could
> figure it out internally by looking at the NOC SWI to determine the
> qos_port index. But this should be obvious from the code itself.

Right

Konrad