| Message ID | 20230728003313.10439-4-takahiro.akashi@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | firmware: scmi: add sanity checks for protocols | expand |
> From: AKASHI Takahiro <takahiro.akashi@linaro.org> > Sent: Friday, July 28, 2023 02:33 > Subject: [RFC 3/3] firmware: scmi: add a sanity check against protocol version > > SCMI_PROTOCOL_VERSION is a mandatory command for all the SCMI protocols. > With this patch, this command is implemented on each protocol. > Then, we can assure that the feature set implemented by SCMI Server > (firmware) is compatible with U-Boot, that is, each protocol's version > must be equal to or higher than the one that U-Boot's corresponding driver > expects. > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- This change enforces some protocol versions that may not be reported by known open source scmi server implementations: scp-firmware, tf-a and op-tee. Latest scp-firwmare/op-tee/tf-a report the following verisons numbers: * base protocol : 0x20000 all * clock protocol: 0x10000 for scp-firwmare / 0x20000 for op-tee and tf-a. * reset-dom: 0x1000 for all * voltage-dom: 0x10000 for scp-firmware / 0x30000 for op-tee / not used in tf-a. * power-dom: 0x20000 for scp-firmware / 0x21000 for tf-a / not used in op-tee. Regarding the SCMI specification, these protocol versions evolve with the specification version: ---------- ---base -powerD --clock -resetD --voltD SCMI v1.0: 0x10000 0x10000 0x10000 ------- ------- SCMI v2.0: 0x20000 0x20000 0x10000 0x10000 ------- SCMI v3.0: 0x20000 0x21000 0x10000 0x20000 0x10000 SCMI v3.1: 0x20000 0x30000 0x30000 0x30000 0x20000 SCMI v3.2: 0x20000 0x30000 0x30000 0x30000 0x20000 This patch enforces the versions defined in SCMI v3.2. Being strict in protocol version is not ideal I think. BR, etienne > drivers/clk/clk_scmi.c | 6 ++++++ > drivers/power/regulator/scmi_regulator.c | 6 ++++++ > drivers/reset/reset-scmi.c | 14 +++++++++++++- > include/scmi_protocols.h | 6 ++++++ > 4 files changed, 31 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c > index 34a49363a51a..3591acb6d3a9 100644 > --- a/drivers/clk/clk_scmi.c > +++ b/drivers/clk/clk_scmi.c > @@ -138,12 +138,18 @@ static int scmi_clk_probe(struct udevice *dev) > { > struct clk *clk; > size_t num_clocks, i; > + u32 version; > int ret; > > ret = devm_scmi_of_get_channel(dev); > if (ret) > return ret; > > + ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_CLOCK, > + &version); > + if (ret || version < SCMI_CLOCK_PROTOCOL_VERSION) > + return -EINVAL; > + > if (!CONFIG_IS_ENABLED(CLK_CCF)) > return 0; > > diff --git a/drivers/power/regulator/scmi_regulator.c b/drivers/power/regulator/scmi_regulator.c > index 08918b20872c..936768d0eeeb 100644 > --- a/drivers/power/regulator/scmi_regulator.c > +++ b/drivers/power/regulator/scmi_regulator.c > @@ -138,12 +138,18 @@ static int scmi_regulator_probe(struct udevice *dev) > .out_msg = (u8 *)&out, > .out_msg_sz = sizeof(out), > }; > + u32 version; > int ret; > > ret = devm_scmi_of_get_channel(dev); > if (ret) > return ret; > > + ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, > + &version); > + if (ret || version < SCMI_VOLTDOM_PROTOCOL_VERSION) > + return -EINVAL; > + > /* Check voltage domain is known from SCMI server */ > in.domain_id = pdata->domain_id; > > diff --git a/drivers/reset/reset-scmi.c b/drivers/reset/reset-scmi.c > index b76711f0a8fb..dbd98dbdbc4f 100644 > --- a/drivers/reset/reset-scmi.c > +++ b/drivers/reset/reset-scmi.c > @@ -73,7 +73,19 @@ static const struct reset_ops scmi_reset_domain_ops = { > > static int scmi_reset_probe(struct udevice *dev) > { > - return devm_scmi_of_get_channel(dev); > + u32 version; > + int ret; > + > + ret = devm_scmi_of_get_channel(dev); > + if (ret) > + return ret; > + > + ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_RESET_DOMAIN, > + &version); > + if (ret || version < SCMI_RESETDOM_PROTOCOL_VERSION) > + return -EINVAL; > + > + return 0; > } > > U_BOOT_DRIVER(scmi_reset_domain) = { > diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h > index 64fd740472b5..6ab16efd49cc 100644 > --- a/include/scmi_protocols.h > +++ b/include/scmi_protocols.h > @@ -398,6 +398,8 @@ int scmi_generic_protocol_version(struct udevice *dev, > * SCMI Clock Protocol > */ > > +#define SCMI_CLOCK_PROTOCOL_VERSION 0x20000 TF-A and OP-TEE scmi servers report version 0x20000 for clock protocol but SCP-firmware reports version 0x10000: https://github.com/ARM-software/SCP-firmware/blob/master/module/scmi_clock/include/internal/scmi_clock.h#L16 > + > enum scmi_clock_message_id { > SCMI_CLOCK_ATTRIBUTES = 0x3, > SCMI_CLOCK_RATE_SET = 0x5, > @@ -509,6 +511,8 @@ struct scmi_clk_rate_set_out { > * SCMI Reset Domain Protocol > */ > > +#define SCMI_RESETDOM_PROTOCOL_VERSION 0x30000 > + > enum scmi_reset_domain_message_id { > SCMI_RESET_DOMAIN_ATTRIBUTES = 0x3, > SCMI_RESET_DOMAIN_RESET = 0x4, > @@ -569,6 +573,8 @@ struct scmi_rd_reset_out { > * SCMI Voltage Domain Protocol > */ > > +#define SCMI_VOLTDOM_PROTOCOL_VERSION 0x20000 > + > enum scmi_voltage_domain_message_id { > SCMI_VOLTAGE_DOMAIN_ATTRIBUTES = 0x3, > SCMI_VOLTAGE_DOMAIN_CONFIG_SET = 0x5, > -- > 2.41.0 > >
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index 34a49363a51a..3591acb6d3a9 100644 --- a/drivers/clk/clk_scmi.c +++ b/drivers/clk/clk_scmi.c @@ -138,12 +138,18 @@ static int scmi_clk_probe(struct udevice *dev) { struct clk *clk; size_t num_clocks, i; + u32 version; int ret; ret = devm_scmi_of_get_channel(dev); if (ret) return ret; + ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_CLOCK, + &version); + if (ret || version < SCMI_CLOCK_PROTOCOL_VERSION) + return -EINVAL; + if (!CONFIG_IS_ENABLED(CLK_CCF)) return 0; diff --git a/drivers/power/regulator/scmi_regulator.c b/drivers/power/regulator/scmi_regulator.c index 08918b20872c..936768d0eeeb 100644 --- a/drivers/power/regulator/scmi_regulator.c +++ b/drivers/power/regulator/scmi_regulator.c @@ -138,12 +138,18 @@ static int scmi_regulator_probe(struct udevice *dev) .out_msg = (u8 *)&out, .out_msg_sz = sizeof(out), }; + u32 version; int ret; ret = devm_scmi_of_get_channel(dev); if (ret) return ret; + ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, + &version); + if (ret || version < SCMI_VOLTDOM_PROTOCOL_VERSION) + return -EINVAL; + /* Check voltage domain is known from SCMI server */ in.domain_id = pdata->domain_id; diff --git a/drivers/reset/reset-scmi.c b/drivers/reset/reset-scmi.c index b76711f0a8fb..dbd98dbdbc4f 100644 --- a/drivers/reset/reset-scmi.c +++ b/drivers/reset/reset-scmi.c @@ -73,7 +73,19 @@ static const struct reset_ops scmi_reset_domain_ops = { static int scmi_reset_probe(struct udevice *dev) { - return devm_scmi_of_get_channel(dev); + u32 version; + int ret; + + ret = devm_scmi_of_get_channel(dev); + if (ret) + return ret; + + ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_RESET_DOMAIN, + &version); + if (ret || version < SCMI_RESETDOM_PROTOCOL_VERSION) + return -EINVAL; + + return 0; } U_BOOT_DRIVER(scmi_reset_domain) = { diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h index 64fd740472b5..6ab16efd49cc 100644 --- a/include/scmi_protocols.h +++ b/include/scmi_protocols.h @@ -398,6 +398,8 @@ int scmi_generic_protocol_version(struct udevice *dev, * SCMI Clock Protocol */ +#define SCMI_CLOCK_PROTOCOL_VERSION 0x20000 + enum scmi_clock_message_id { SCMI_CLOCK_ATTRIBUTES = 0x3, SCMI_CLOCK_RATE_SET = 0x5, @@ -509,6 +511,8 @@ struct scmi_clk_rate_set_out { * SCMI Reset Domain Protocol */ +#define SCMI_RESETDOM_PROTOCOL_VERSION 0x30000 + enum scmi_reset_domain_message_id { SCMI_RESET_DOMAIN_ATTRIBUTES = 0x3, SCMI_RESET_DOMAIN_RESET = 0x4, @@ -569,6 +573,8 @@ struct scmi_rd_reset_out { * SCMI Voltage Domain Protocol */ +#define SCMI_VOLTDOM_PROTOCOL_VERSION 0x20000 + enum scmi_voltage_domain_message_id { SCMI_VOLTAGE_DOMAIN_ATTRIBUTES = 0x3, SCMI_VOLTAGE_DOMAIN_CONFIG_SET = 0x5,
SCMI_PROTOCOL_VERSION is a mandatory command for all the SCMI protocols. With this patch, this command is implemented on each protocol. Then, we can assure that the feature set implemented by SCMI Server (firmware) is compatible with U-Boot, that is, each protocol's version must be equal to or higher than the one that U-Boot's corresponding driver expects. Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- drivers/clk/clk_scmi.c | 6 ++++++ drivers/power/regulator/scmi_regulator.c | 6 ++++++ drivers/reset/reset-scmi.c | 14 +++++++++++++- include/scmi_protocols.h | 6 ++++++ 4 files changed, 31 insertions(+), 1 deletion(-)