linux-hardening - Re: [PATCH v3 1/4] interconnect: qcom: icc-rpmh: Add QoS configuration support

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]

Date: Wed, 6 Mar 2024 09:31:21 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Odelu Kukatla <quic_okukatla@...cinc.com>,
 Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konrad.dybcio@...aro.org>, Georgi Djakov <djakov@...nel.org>,
 Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>
Cc: Kees Cook <keescook@...omium.org>, cros-qcom-dts-watchers@...omium.org,
 "Gustavo A . R . Silva" <gustavoars@...nel.org>,
 linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hardening@...r.kernel.org, quic_rlaggysh@...cinc.com,
 quic_mdtipton@...cinc.com
Subject: Re: [PATCH v3 1/4] interconnect: qcom: icc-rpmh: Add QoS
 configuration support

On 06/03/2024 08:30, Odelu Kukatla wrote:
> It adds QoS support for QNOC device and includes support for
> configuring priority, priority forward disable, urgency forwarding.
> This helps in priortizing the traffic originating from different
> interconnect masters at NoC(Network On Chip).
> 
> Signed-off-by: Odelu Kukatla <quic_okukatla@...cinc.com>
> ---
>  drivers/interconnect/qcom/icc-rpmh.c | 105 +++++++++++++++++++++++++++
>  drivers/interconnect/qcom/icc-rpmh.h |  32 ++++++++
>  2 files changed, 137 insertions(+)

>  
> +	if (desc->config) {
> +		struct resource *res;
> +		void __iomem *base;
> +
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +		if (!res) {
> +			dev_warn(dev, "Skipping QoS, failed to get resource\n");

Why is this a warning? If the MMIO is not required, your driver should
not spawn warnings.

> +			goto skip_qos_config;
> +		}
> +
> +		base = devm_ioremap(dev, res->start, resource_size(res));

Combine these two (there's a helper for that).

> +		if (IS_ERR(base)) {
> +			dev_warn(dev, "Skipping QoS, ioremap failed: %ld\n", PTR_ERR(base));
> +			goto skip_qos_config;
> +		};
> +
> +		qp->regmap = devm_regmap_init_mmio(dev, base, desc->config);
> +

Drop blank line.

> +		if (IS_ERR(qp->regmap)) {
> +			dev_warn(dev, "Skipping QoS, regmap failed; %ld\n", PTR_ERR(qp->regmap));
> +			goto skip_qos_config;
> +		}
> +
> +		qp->num_clks = devm_clk_bulk_get_all(qp->dev, &qp->clks);
> +		if (qp->num_clks < 0) {
> +			dev_warn(dev, "Skipping QoS, failed to get clk: %d\n", qp->num_clks);

dev_info(), because current binding sais clocks are not required and you
are not allowed to affect ABI. While ABI here does not look broken,
spawning new warnings to proper users is not welcomed.

Best regards,
Krzysztof