[PATCH] firmware: arm_scmi:: Add opinion for init fastchannel
Cristian Marussi
cristian.marussi at arm.com
Tue Oct 31 01:05:50 PDT 2023
On Tue, Oct 31, 2023 at 09:36:01AM +0800, xinglong.yang wrote:
> Fastchannel may not be supported by the platform. It is not need to
> init the fastchannel if the fastchannel is not supported.
>
Hi,
the commit message is misleading because the patch you provided does not
really does that :D
FCs are indeed not necessarily supported by the platform, and this is
reported for each single domain at discovery time AND we indeed check
already that (dom->perf_fastchannels) BEFORE initiliazing the fastchannel
at all for a specific domain.
What you are doing here, instead, is skipping FC initialization on a
fastchannel by checking the dom->set_perf / dom->set_limit, BUT such flags
are really meant to describe if any set_perf/limit ops can be issued on a
domain at all.
Given that each FC is associated to a specific domain_id AND msg_id it
is indeed plausible that a domain supports FastChannels BUT only for
get_* operations.
In such scenario I would expect, though, the platform to return
NOT_SUPPORTED when required to describe the SET_* FCs.
Having said that, it could be anyway useful to refrain from trying to
describe/init the SET operations on a FC for a domain where the set_*
operations are not supported, because: (1.) avoids unneeded SCMI
exchanges, (2.) avoids to trust the FW reply blindly...
...BUT your patch does NOT really do this, because you are skipping the
fastchannel_init by looking at dom->set_perf and dom->set_limit for BOTH
the set and get commands.
You should just skip the FC init on the SET_ when dom->set_ is set if we
want to do this optimization/hardening.
Am I missing something ?
Thanks,
Cristian
> Signed-off-by: xinglong.yang <xinglong.yang at cixtech.com>
> Change-Id: Id73ab1f37d5a3726243f97beb40c5b2239d65727
> ---
> drivers/firmware/arm_scmi/perf.c | 38 ++++++++++++++++++--------------
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c
> index ecf5c4de851b..26fa71e2aff8 100644
> --- a/drivers/firmware/arm_scmi/perf.c
> +++ b/drivers/firmware/arm_scmi/perf.c
> @@ -526,32 +526,36 @@ static int scmi_perf_level_limits_notify(const struct scmi_protocol_handle *ph,
> }
>
> static void scmi_perf_domain_init_fc(const struct scmi_protocol_handle *ph,
> - u32 domain, struct scmi_fc_info **p_fc)
> + u32 domain, struct perf_dom_info* dom)
> {
> + struct scmi_fc_info **p_fc = &dom->fc_info;
> struct scmi_fc_info *fc;
>
> fc = devm_kcalloc(ph->dev, PERF_FC_MAX, sizeof(*fc), GFP_KERNEL);
> if (!fc)
> return;
>
> - ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL,
> - PERF_LEVEL_SET, 4, domain,
> - &fc[PERF_FC_LEVEL].set_addr,
> - &fc[PERF_FC_LEVEL].set_db);
> + if (dom->set_perf) {
> + ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL,
> + PERF_LEVEL_SET, 4, domain,
> + &fc[PERF_FC_LEVEL].set_addr,
> + &fc[PERF_FC_LEVEL].set_db);
>
> - ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL,
> - PERF_LEVEL_GET, 4, domain,
> - &fc[PERF_FC_LEVEL].get_addr, NULL);
> -
> - ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL,
> - PERF_LIMITS_SET, 8, domain,
> - &fc[PERF_FC_LIMIT].set_addr,
> - &fc[PERF_FC_LIMIT].set_db);
> + ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL,
> + PERF_LEVEL_GET, 4, domain,
> + &fc[PERF_FC_LEVEL].get_addr, NULL);
> + }
>
> - ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL,
> - PERF_LIMITS_GET, 8, domain,
> - &fc[PERF_FC_LIMIT].get_addr, NULL);
> + if (dom->set_limits) {
> + ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL,
> + PERF_LIMITS_SET, 8, domain,
> + &fc[PERF_FC_LIMIT].set_addr,
> + &fc[PERF_FC_LIMIT].set_db);
>
> + ph->hops->fastchannel_init(ph, PERF_DESCRIBE_FASTCHANNEL,
> + PERF_LIMITS_GET, 8, domain,
> + &fc[PERF_FC_LIMIT].get_addr, NULL);
> + }
> *p_fc = fc;
> }
>
> @@ -835,7 +839,7 @@ static int scmi_perf_protocol_init(const struct scmi_protocol_handle *ph)
> scmi_perf_describe_levels_get(ph, domain, dom);
>
> if (dom->perf_fastchannels)
> - scmi_perf_domain_init_fc(ph, domain, &dom->fc_info);
> + scmi_perf_domain_init_fc(ph, domain, dom);
> }
>
> pinfo->version = version;
> --
> 2.42.0
>
More information about the linux-arm-kernel
mailing list