linux-kernel - Re: [PATCH V3 1/4] firmware: arm_scmi: Ensure that the message-id supports fastchannel

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: <ZwaJQmO4a7cM7MFY@bogus>
Date: Wed, 9 Oct 2024 14:46:42 +0100
From: Sudeep Holla <sudeep.holla@....com>
To: Sibi Sankar <quic_sibis@...cinc.com>
Cc: <cristian.marussi@....com>, <ulf.hansson@...aro.org>,
	<jassisinghbrar@...il.com>, <linux-kernel@...r.kernel.org>,
	<arm-scmi@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
	<linux-arm-msm@...r.kernel.org>, <johan@...nel.org>,
	<konradybcio@...nel.org>, <linux-pm@...r.kernel.org>,
	<tstrudel@...gle.com>, <rafael@...nel.org>,
	"Johan Hovold" <johan+linaro@...nel.org>
Subject: Re: [PATCH V3 1/4] firmware: arm_scmi: Ensure that the message-id
 supports fastchannel

On Mon, Oct 07, 2024 at 11:36:39AM +0530, Sibi Sankar wrote:
> Currently the perf and powercap protocol relies on the protocol domain
> attributes, which just ensures that one fastchannel per domain, before
> instantiating fastchannels for all possible message-ids. Fix this by
> ensuring that each message-id supports fastchannel before initialization.
>

Looks good to me. With the minor nit below addressed,

Reviewed-by: Sudeep Holla <sudeep.holla@....com>

(assuming you will take this all via pmdomain or qcom soc tree)

> Reported-by: Johan Hovold <johan+linaro@...nel.org>
> Closes: https://lore.kernel.org/lkml/ZoQjAWse2YxwyRJv@hovoldconsulting.com/
> Fixes: 6f9ea4dabd2d ("firmware: arm_scmi: Generalize the fast channel support")
> Signed-off-by: Sibi Sankar <quic_sibis@...cinc.com>
> ---
>  drivers/firmware/arm_scmi/driver.c    | 9 +++++++++
>  drivers/firmware/arm_scmi/protocols.h | 2 ++
>  2 files changed, 11 insertions(+)
>
> diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
> index 88c5c4ff4bb6..80a9a615672a 100644
> --- a/drivers/firmware/arm_scmi/driver.c
> +++ b/drivers/firmware/arm_scmi/driver.c
> @@ -56,6 +56,9 @@ static atomic_t transfer_last_id;
>
>  static struct dentry *scmi_top_dentry;
>
> +static int scmi_protocol_msg_check(const struct scmi_protocol_handle *ph,
> +				   u32 message_id, u32 *attributes);
>

I prefer to just move the function above if possible to avoid this extra
declaration just keep keep it consistent with other such internal/static
function calls within this file. No hard opinion, just preference to avoid
me thinking(or scratching my head) why only this is done different few
months down the line.

--
Regards,
Sudeep