linux-kernel - Re: [PATCH 02/11] firmware: arm_scmi: hide protocols' private data

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]

Message-ID: <ae5b36f6-68de-d764-636d-ad12d3276302@gmail.com>
Date:   Wed, 14 Oct 2020 12:19:43 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Cristian Marussi <cristian.marussi@....com>,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Cc:     sudeep.holla@....com, lukasz.luba@....com,
        james.quinlan@...adcom.com, Jonathan.Cameron@...wei.com,
        etienne.carriere@...aro.org, thara.gopinath@...aro.org,
        vincent.guittot@...aro.org, souvik.chakravarty@....com
Subject: Re: [PATCH 02/11] firmware: arm_scmi: hide protocols' private data

On 10/14/20 8:05 AM, Cristian Marussi wrote:
> Protocols private data were meant to be used exclusively by protocol code
> but they are currently exposed through the handle, so available also to
> SCMI drivers: move them away from handle into instance specific data and
> provide internal helpers to let protocols implementation set/get their own
> private data from protocol code.
> 
> Signed-off-by: Cristian Marussi <cristian.marussi@....com>

[snip]


> diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c
> index 94bcad9a7d19..4e8dafc36d7e 100644
> --- a/drivers/firmware/arm_scmi/clock.c
> +++ b/drivers/firmware/arm_scmi/clock.c
> @@ -240,7 +240,8 @@ static int scmi_clock_rate_set(const struct scmi_handle *handle, u32 clk_id,
>  	u32 flags = 0;
>  	struct scmi_xfer *t;
>  	struct scmi_clock_set_rate *cfg;
> -	struct clock_info *ci = handle->clk_priv;
> +	struct clock_info *ci =
> +		scmi_get_proto_priv(handle, SCMI_PROTOCOL_CLOCK);

Having to repeat the protocol identifier is a bit error prone, does not
scale really and it will not change throughout the lifecycle of the
protocol handler. This is the least path of resistance though so it
makes sense you went that route for now.

Instead of exposing scmi_handle towards the various operations what you
could do is just pass down an encapsulating object which is comprised of
a scmi_handle reference and the protocol's private structure already
resolved?
-- 
Florian