From patchwork Fri Dec 1 13:58:58 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Cristian Marussi X-Patchwork-Id: 13475873 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 67300C4167B for ; Fri, 1 Dec 2023 13:59:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:Message-ID:Date:Subject:Cc :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=0fhMTbuZygzdZP4Xhl2QDRjgTWQ8uCDRzfbhO9c6XgQ=; b=THnTC0U7eZBjSz ByIjc7JmP5R17kOOG2LQEKKtzT0vqYuee9ekDHD+1lUkKIRwvAsUfQ7ojGj2eJomvvQU9/M8HKK8b hzDjX3jFmu9bDpg9myMFsguGw5kCNtT1HaefnqZpKK1E8mzDLbKQmzQDnXBSgulH9mVpfJdex+QTJ rIS5JdutnMHSsORNpR/LF7D88di3ZU+aJxGanYCTWg9nt6LA+dgWH1U9ss9MV4qyOcbNr4rNlZ7Yv HrQX22kCAsWzYLb4SgfwhqGDkXNMT7SbS3NKbhRM/+nSvBn8kp4vEBissbkAtXBhyIzu9N9TAHoql HyVODwgiRTreSF0mzAZQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r9435-00Dsgg-0f; Fri, 01 Dec 2023 13:59:19 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1r9432-00Dsed-0o for linux-arm-kernel@lists.infradead.org; Fri, 01 Dec 2023 13:59:18 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id A38E41007; Fri, 1 Dec 2023 05:59:57 -0800 (PST) Received: from pluto.fritz.box (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E89923F73F; Fri, 1 Dec 2023 05:59:09 -0800 (PST) From: Cristian Marussi To: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Cc: sudeep.holla@arm.com, vincent.guittot@linaro.org, souvik.chakravarty@arm.com, nicola.mazzucato@arm.com, Cristian Marussi Subject: [PATCH] firmware: arm_scmi: Add protocol versioning checks Date: Fri, 1 Dec 2023 13:58:58 +0000 Message-ID: <20231201135858.2367651-1-cristian.marussi@arm.com> X-Mailer: git-send-email 2.43.0 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231201_055916_404788_2D16686A X-CRM114-Status: GOOD ( 19.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Platform and agent supported protocols versions do not necessarily match. When talking to an older platform SCMI server, supporting only older protocol versions, the kernel SCMI agent will downgrade the version of the used protocol to match the platform one and avoid compatibility issues. In the case, instead, in which the agent happens to communicate with a newer platform server which can support newer protocol versions unknown to the agent, and potentially backward incompatible, the agent currently carries on, silently, in a best-effort approach. Note that the SCMI server, by the specification, has no means to explicitly detect the protocol versions used by the agents, neither it is required to support multiple, older, protocol versions. Add an explicit protocol version check to let the agent detect when this version mismatch happens and warn the user about this condition. Signed-off-by: Cristian Marussi --- Any suggestion for a more meaningful warn message is very much welcome. Based on sudeep/for-next/scmi/updates --- drivers/firmware/arm_scmi/base.c | 6 +++++- drivers/firmware/arm_scmi/clock.c | 6 +++++- drivers/firmware/arm_scmi/driver.c | 11 ++++++++++- drivers/firmware/arm_scmi/perf.c | 6 +++++- drivers/firmware/arm_scmi/power.c | 6 +++++- drivers/firmware/arm_scmi/powercap.c | 6 +++++- drivers/firmware/arm_scmi/protocols.h | 8 +++++++- drivers/firmware/arm_scmi/reset.c | 6 +++++- drivers/firmware/arm_scmi/sensors.c | 6 +++++- drivers/firmware/arm_scmi/system.c | 6 +++++- drivers/firmware/arm_scmi/voltage.c | 6 +++++- 11 files changed, 62 insertions(+), 11 deletions(-) diff --git a/drivers/firmware/arm_scmi/base.c b/drivers/firmware/arm_scmi/base.c index a52f084a6a87..3f5c89ae5af2 100644 --- a/drivers/firmware/arm_scmi/base.c +++ b/drivers/firmware/arm_scmi/base.c @@ -13,6 +13,9 @@ #include "common.h" #include "notify.h" +/* Must be updated only after ALL new features for that version are merged */ +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x20000 + #define SCMI_BASE_NUM_SOURCES 1 #define SCMI_BASE_MAX_CMD_ERR_COUNT 1024 @@ -385,7 +388,7 @@ static int scmi_base_protocol_init(const struct scmi_protocol_handle *ph) rev->major_ver = PROTOCOL_REV_MAJOR(version), rev->minor_ver = PROTOCOL_REV_MINOR(version); - ph->set_priv(ph, rev); + ph->set_priv(ph, rev, version); ret = scmi_base_attributes_get(ph); if (ret) @@ -423,6 +426,7 @@ static const struct scmi_protocol scmi_base = { .instance_init = &scmi_base_protocol_init, .ops = NULL, .events = &base_protocol_events, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, }; DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(base, scmi_base) diff --git a/drivers/firmware/arm_scmi/clock.c b/drivers/firmware/arm_scmi/clock.c index 98511a3aa367..5e213faa5fe1 100644 --- a/drivers/firmware/arm_scmi/clock.c +++ b/drivers/firmware/arm_scmi/clock.c @@ -12,6 +12,9 @@ #include "protocols.h" #include "notify.h" +/* Must be updated only after ALL new features for that version are merged */ +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x20001 + enum scmi_clock_protocol_cmd { CLOCK_ATTRIBUTES = 0x3, CLOCK_DESCRIBE_RATES = 0x4, @@ -961,7 +964,7 @@ static int scmi_clock_protocol_init(const struct scmi_protocol_handle *ph) } cinfo->version = version; - return ph->set_priv(ph, cinfo); + return ph->set_priv(ph, cinfo, version); } static const struct scmi_protocol scmi_clock = { @@ -970,6 +973,7 @@ static const struct scmi_protocol scmi_clock = { .instance_init = &scmi_clock_protocol_init, .ops = &clk_proto_ops, .events = &clk_protocol_events, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, }; DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(clock, scmi_clock) diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 3eb19ed6f148..46320f627066 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -85,6 +85,7 @@ struct scmi_xfers_info { * @gid: A reference for per-protocol devres management. * @users: A refcount to track effective users of this protocol. * @priv: Reference for optional protocol private data. + * @version: Protocol version supported by the platform as detected at runtime. * @ph: An embedded protocol handle that will be passed down to protocol * initialization code to identify this instance. * @@ -97,6 +98,7 @@ struct scmi_protocol_instance { void *gid; refcount_t users; void *priv; + unsigned int version; struct scmi_protocol_handle ph; }; @@ -1392,15 +1394,17 @@ static int version_get(const struct scmi_protocol_handle *ph, u32 *version) * * @ph: A reference to the protocol handle. * @priv: The private data to set. + * @version: The detected protocol version for the core to register. * * Return: 0 on Success */ static int scmi_set_protocol_priv(const struct scmi_protocol_handle *ph, - void *priv) + void *priv, u32 version) { struct scmi_protocol_instance *pi = ph_to_pi(ph); pi->priv = priv; + pi->version = version; return 0; } @@ -1849,6 +1853,11 @@ scmi_alloc_init_protocol_instance(struct scmi_info *info, devres_close_group(handle->dev, pi->gid); dev_dbg(handle->dev, "Initialized protocol: 0x%X\n", pi->proto->id); + if (pi->version > proto->supported_version) + dev_warn(handle->dev, + "Detected UNSUPPORTED version 0x%X for protocol 0x%X. Backward compatibility is NOT assured.\n", + pi->version, pi->proto->id); + return pi; clean: diff --git a/drivers/firmware/arm_scmi/perf.c b/drivers/firmware/arm_scmi/perf.c index 81dd5c5e5533..92fa60127f47 100644 --- a/drivers/firmware/arm_scmi/perf.c +++ b/drivers/firmware/arm_scmi/perf.c @@ -24,6 +24,9 @@ #include "protocols.h" #include "notify.h" +/* Must be updated only after ALL new features for that version are merged */ +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x40000 + #define MAX_OPPS 32 enum scmi_performance_protocol_cmd { @@ -1104,7 +1107,7 @@ static int scmi_perf_protocol_init(const struct scmi_protocol_handle *ph) if (ret) return ret; - return ph->set_priv(ph, pinfo); + return ph->set_priv(ph, pinfo, version); } static const struct scmi_protocol scmi_perf = { @@ -1113,6 +1116,7 @@ static const struct scmi_protocol scmi_perf = { .instance_init = &scmi_perf_protocol_init, .ops = &perf_proto_ops, .events = &perf_protocol_events, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, }; DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(perf, scmi_perf) diff --git a/drivers/firmware/arm_scmi/power.c b/drivers/firmware/arm_scmi/power.c index 077767d6e902..9d0536baeee5 100644 --- a/drivers/firmware/arm_scmi/power.c +++ b/drivers/firmware/arm_scmi/power.c @@ -13,6 +13,9 @@ #include "protocols.h" #include "notify.h" +/* Must be updated only after ALL new features for that version are merged */ +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x30000 + enum scmi_power_protocol_cmd { POWER_DOMAIN_ATTRIBUTES = 0x3, POWER_STATE_SET = 0x4, @@ -328,7 +331,7 @@ static int scmi_power_protocol_init(const struct scmi_protocol_handle *ph) pinfo->version = version; - return ph->set_priv(ph, pinfo); + return ph->set_priv(ph, pinfo, version); } static const struct scmi_protocol scmi_power = { @@ -337,6 +340,7 @@ static const struct scmi_protocol scmi_power = { .instance_init = &scmi_power_protocol_init, .ops = &power_proto_ops, .events = &power_protocol_events, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, }; DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(power, scmi_power) diff --git a/drivers/firmware/arm_scmi/powercap.c b/drivers/firmware/arm_scmi/powercap.c index 62a7780fedbb..bb9b1a95139c 100644 --- a/drivers/firmware/arm_scmi/powercap.c +++ b/drivers/firmware/arm_scmi/powercap.c @@ -17,6 +17,9 @@ #include "protocols.h" #include "notify.h" +/* Must be updated only after ALL new features for that version are merged */ +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x20000 + enum scmi_powercap_protocol_cmd { POWERCAP_DOMAIN_ATTRIBUTES = 0x3, POWERCAP_CAP_GET = 0x4, @@ -975,7 +978,7 @@ scmi_powercap_protocol_init(const struct scmi_protocol_handle *ph) } pinfo->version = version; - return ph->set_priv(ph, pinfo); + return ph->set_priv(ph, pinfo, version); } static const struct scmi_protocol scmi_powercap = { @@ -984,6 +987,7 @@ static const struct scmi_protocol scmi_powercap = { .instance_init = &scmi_powercap_protocol_init, .ops = &powercap_proto_ops, .events = &powercap_protocol_events, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, }; DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(powercap, scmi_powercap) diff --git a/drivers/firmware/arm_scmi/protocols.h b/drivers/firmware/arm_scmi/protocols.h index b3c6314bb4b8..e683c26f24eb 100644 --- a/drivers/firmware/arm_scmi/protocols.h +++ b/drivers/firmware/arm_scmi/protocols.h @@ -174,7 +174,8 @@ struct scmi_protocol_handle { struct device *dev; const struct scmi_xfer_ops *xops; const struct scmi_proto_helpers_ops *hops; - int (*set_priv)(const struct scmi_protocol_handle *ph, void *priv); + int (*set_priv)(const struct scmi_protocol_handle *ph, void *priv, + u32 version); void *(*get_priv)(const struct scmi_protocol_handle *ph); }; @@ -311,6 +312,10 @@ typedef int (*scmi_prot_init_ph_fn_t)(const struct scmi_protocol_handle *); * @ops: Optional reference to the operations provided by the protocol and * exposed in scmi_protocol.h. * @events: An optional reference to the events supported by this protocol. + * @supported_version: The highest version currently supported for this + * protocol by the agent. Each protocol implementation + * in the agent is supposed to downgrade to match the + * protocol version supported by the platform. */ struct scmi_protocol { const u8 id; @@ -319,6 +324,7 @@ struct scmi_protocol { const scmi_prot_init_ph_fn_t instance_deinit; const void *ops; const struct scmi_protocol_events *events; + unsigned int supported_version; }; #define DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(name, proto) \ diff --git a/drivers/firmware/arm_scmi/reset.c b/drivers/firmware/arm_scmi/reset.c index 7217fd7c6afa..a28ebdd53700 100644 --- a/drivers/firmware/arm_scmi/reset.c +++ b/drivers/firmware/arm_scmi/reset.c @@ -13,6 +13,9 @@ #include "protocols.h" #include "notify.h" +/* Must be updated only after ALL new features for that version are merged */ +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x30000 + enum scmi_reset_protocol_cmd { RESET_DOMAIN_ATTRIBUTES = 0x3, RESET = 0x4, @@ -343,7 +346,7 @@ static int scmi_reset_protocol_init(const struct scmi_protocol_handle *ph) } pinfo->version = version; - return ph->set_priv(ph, pinfo); + return ph->set_priv(ph, pinfo, version); } static const struct scmi_protocol scmi_reset = { @@ -352,6 +355,7 @@ static const struct scmi_protocol scmi_reset = { .instance_init = &scmi_reset_protocol_init, .ops = &reset_proto_ops, .events = &reset_protocol_events, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, }; DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(reset, scmi_reset) diff --git a/drivers/firmware/arm_scmi/sensors.c b/drivers/firmware/arm_scmi/sensors.c index 9952a7bc6682..c5220b1d6b09 100644 --- a/drivers/firmware/arm_scmi/sensors.c +++ b/drivers/firmware/arm_scmi/sensors.c @@ -14,6 +14,9 @@ #include "protocols.h" #include "notify.h" +/* Must be updated only after ALL new features for that version are merged */ +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x30000 + #define SCMI_MAX_NUM_SENSOR_AXIS 63 #define SCMIv2_SENSOR_PROTOCOL 0x10000 @@ -1138,7 +1141,7 @@ static int scmi_sensors_protocol_init(const struct scmi_protocol_handle *ph) if (ret) return ret; - return ph->set_priv(ph, sinfo); + return ph->set_priv(ph, sinfo, version); } static const struct scmi_protocol scmi_sensors = { @@ -1147,6 +1150,7 @@ static const struct scmi_protocol scmi_sensors = { .instance_init = &scmi_sensors_protocol_init, .ops = &sensor_proto_ops, .events = &sensor_protocol_events, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, }; DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(sensors, scmi_sensors) diff --git a/drivers/firmware/arm_scmi/system.c b/drivers/firmware/arm_scmi/system.c index 9383d7584539..06fd542cfce1 100644 --- a/drivers/firmware/arm_scmi/system.c +++ b/drivers/firmware/arm_scmi/system.c @@ -13,6 +13,9 @@ #include "protocols.h" #include "notify.h" +/* Must be updated only after ALL new features for that version are merged */ +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x20000 + #define SCMI_SYSTEM_NUM_SOURCES 1 enum scmi_system_protocol_cmd { @@ -144,7 +147,7 @@ static int scmi_system_protocol_init(const struct scmi_protocol_handle *ph) if (PROTOCOL_REV_MAJOR(pinfo->version) >= 0x2) pinfo->graceful_timeout_supported = true; - return ph->set_priv(ph, pinfo); + return ph->set_priv(ph, pinfo, version); } static const struct scmi_protocol scmi_system = { @@ -153,6 +156,7 @@ static const struct scmi_protocol scmi_system = { .instance_init = &scmi_system_protocol_init, .ops = NULL, .events = &system_protocol_events, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, }; DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(system, scmi_system) diff --git a/drivers/firmware/arm_scmi/voltage.c b/drivers/firmware/arm_scmi/voltage.c index 36e2df77738c..cb51c61ba31c 100644 --- a/drivers/firmware/arm_scmi/voltage.c +++ b/drivers/firmware/arm_scmi/voltage.c @@ -10,6 +10,9 @@ #include "protocols.h" +/* Must be updated only after ALL new features for that version are merged */ +#define SCMI_PROTOCOL_SUPPORTED_VERSION 0x20000 + #define VOLTAGE_DOMS_NUM_MASK GENMASK(15, 0) #define REMAINING_LEVELS_MASK GENMASK(31, 16) #define RETURNED_LEVELS_MASK GENMASK(11, 0) @@ -432,7 +435,7 @@ static int scmi_voltage_protocol_init(const struct scmi_protocol_handle *ph) dev_warn(ph->dev, "No Voltage domains found.\n"); } - return ph->set_priv(ph, vinfo); + return ph->set_priv(ph, vinfo, version); } static const struct scmi_protocol scmi_voltage = { @@ -440,6 +443,7 @@ static const struct scmi_protocol scmi_voltage = { .owner = THIS_MODULE, .instance_init = &scmi_voltage_protocol_init, .ops = &voltage_proto_ops, + .supported_version = SCMI_PROTOCOL_SUPPORTED_VERSION, }; DEFINE_SCMI_PROTOCOL_REGISTER_UNREGISTER(voltage, scmi_voltage)