drivers/firmware/smccc/soc_id.c | 81 +++++++++++++++++++++++++++++++++ include/linux/arm-smccc.h | 40 ++++++++++++++++ 2 files changed, 121 insertions(+)
Issue Number 1.6 of the Arm SMC Calling Convention introduces an optional
SOC_ID name string. If implemented, point the 'machine' field of the SoC
Device Attributes at this string so that it will appear under
/sys/bus/soc/devices/soc0/machine.
On Arm SMC compliant SoCs, this will allow things like 'lscpu' to
eventually get a SoC provider model name from there rather than each
tool/utility needing to get a possibly inconsistent, obsolete, or incorrect
model/machine name from its own hardcoded model/machine name table.
Signed-off-by: Paul Benoit <paul@os.amperecomputing.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: linux-arm-kernel@lists.infradead.org
---
v3->v4: Address code review identified issues of v3.
v2->v3: Add conditionalization to exclude SOC_ID Name from 32-bit builds.
v1->v2: Address code review identified issues.
drivers/firmware/smccc/soc_id.c | 81 +++++++++++++++++++++++++++++++++
include/linux/arm-smccc.h | 40 ++++++++++++++++
2 files changed, 121 insertions(+)
diff --git a/drivers/firmware/smccc/soc_id.c b/drivers/firmware/smccc/soc_id.c
index 1990263fbba0..7b0688d78dab 100644
--- a/drivers/firmware/smccc/soc_id.c
+++ b/drivers/firmware/smccc/soc_id.c
@@ -32,6 +32,86 @@
static struct soc_device *soc_dev;
static struct soc_device_attribute *soc_dev_attr;
+#ifdef CONFIG_ARM64
+
+static char __ro_after_init smccc_soc_id_name[136] = "";
+
+static inline void str_fragment_from_reg(char *dst, unsigned long reg)
+{
+ dst[0] = (reg >> 0) & 0xff;
+ dst[1] = (reg >> 8) & 0xff;
+ dst[2] = (reg >> 16) & 0xff;
+ dst[3] = (reg >> 24) & 0xff;
+ dst[4] = (reg >> 32) & 0xff;
+ dst[5] = (reg >> 40) & 0xff;
+ dst[6] = (reg >> 48) & 0xff;
+ dst[7] = (reg >> 56) & 0xff;
+}
+
+static char __init *smccc_soc_name_init(void)
+{
+ struct arm_smccc_1_2_regs args;
+ struct arm_smccc_1_2_regs res;
+ size_t len;
+
+ /*
+ * Issue Number 1.6 of the Arm SMC Calling Convention
+ * specification introduces an optional "name" string
+ * to the ARM_SMCCC_ARCH_SOC_ID function. Fetch it if
+ * available.
+ */
+ args.a0 = ARM_SMCCC_ARCH_SOC_ID;
+ args.a1 = 2; /* SOC_ID name */
+ arm_smccc_1_2_invoke(&args, &res);
+ if ((u32)res.a0 == 0) {
+ const unsigned int regsize = sizeof(res.a1);
+
+ /*
+ * Copy res.a1..res.a17 to the smccc_soc_id_name string
+ * 8 bytes at a time. As per Issue 1.6 of the Arm SMC
+ * Calling Convention, the string will be NUL terminated
+ * and padded, from the end of the string to the end of the
+ * 136 byte buffer, with NULs.
+ */
+ str_fragment_from_reg(smccc_soc_id_name + 0*regsize, res.a1);
+ str_fragment_from_reg(smccc_soc_id_name + 1*regsize, res.a2);
+ str_fragment_from_reg(smccc_soc_id_name + 2*regsize, res.a3);
+ str_fragment_from_reg(smccc_soc_id_name + 3*regsize, res.a4);
+ str_fragment_from_reg(smccc_soc_id_name + 4*regsize, res.a5);
+ str_fragment_from_reg(smccc_soc_id_name + 5*regsize, res.a6);
+ str_fragment_from_reg(smccc_soc_id_name + 6*regsize, res.a7);
+ str_fragment_from_reg(smccc_soc_id_name + 7*regsize, res.a8);
+ str_fragment_from_reg(smccc_soc_id_name + 8*regsize, res.a9);
+ str_fragment_from_reg(smccc_soc_id_name + 9*regsize, res.a10);
+ str_fragment_from_reg(smccc_soc_id_name + 10*regsize, res.a11);
+ str_fragment_from_reg(smccc_soc_id_name + 11*regsize, res.a12);
+ str_fragment_from_reg(smccc_soc_id_name + 12*regsize, res.a13);
+ str_fragment_from_reg(smccc_soc_id_name + 13*regsize, res.a14);
+ str_fragment_from_reg(smccc_soc_id_name + 14*regsize, res.a15);
+ str_fragment_from_reg(smccc_soc_id_name + 15*regsize, res.a16);
+ str_fragment_from_reg(smccc_soc_id_name + 16*regsize, res.a17);
+
+ len = strnlen(smccc_soc_id_name, sizeof(smccc_soc_id_name));
+ if (len) {
+ if (len == sizeof(smccc_soc_id_name))
+ pr_warn(FW_BUG "Ignoring improperly formatted Name\n");
+ else
+ return smccc_soc_id_name;
+ }
+ }
+
+ return NULL;
+}
+
+#else
+
+static char __init *smccc_soc_name_init(void)
+{
+ return NULL;
+}
+
+#endif
+
static int __init smccc_soc_init(void)
{
int soc_id_rev, soc_id_version;
@@ -72,6 +152,7 @@ static int __init smccc_soc_init(void)
soc_dev_attr->soc_id = soc_id_str;
soc_dev_attr->revision = soc_id_rev_str;
soc_dev_attr->family = soc_id_jep106_id_str;
+ soc_dev_attr->machine = smccc_soc_name_init();
soc_dev = soc_device_register(soc_dev_attr);
if (IS_ERR(soc_dev)) {
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 67f6fdf2e7cd..eb7eab04755a 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -639,5 +639,45 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
method; \
})
+#ifdef CONFIG_ARM64
+
+#define __fail_smccc_1_2(___res) \
+ do { \
+ if (___res) \
+ ___res->a0 = SMCCC_RET_NOT_SUPPORTED; \
+ } while (0)
+
+/*
+ * arm_smccc_1_2_invoke() - make an SMCCC v1.2 compliant call
+ *
+ * @args: SMC args are in the a0..a17 fields of the arm_smcc_1_2_regs structure
+ * @res: result values from registers 0 to 17
+ *
+ * This macro will make either an HVC call or an SMC call depending on the
+ * current SMCCC conduit. If no valid conduit is available then -1
+ * (SMCCC_RET_NOT_SUPPORTED) is returned in @res.a0 (if supplied).
+ *
+ * The return value also provides the conduit that was used.
+ */
+#define arm_smccc_1_2_invoke(args, res) ({ \
+ struct arm_smccc_1_2_regs *__args = args; \
+ struct arm_smccc_1_2_regs *__res = res; \
+ int method = arm_smccc_1_1_get_conduit(); \
+ switch (method) { \
+ case SMCCC_CONDUIT_HVC: \
+ arm_smccc_1_2_hvc(__args, __res); \
+ break; \
+ case SMCCC_CONDUIT_SMC: \
+ arm_smccc_1_2_smc(__args, __res); \
+ break; \
+ default: \
+ __fail_smccc_1_2(__res); \
+ method = SMCCC_CONDUIT_NONE; \
+ break; \
+ } \
+ method; \
+ })
+#endif /*CONFIG_ARM64*/
+
#endif /*__ASSEMBLY__*/
#endif /*__LINUX_ARM_SMCCC_H*/
--
2.48.1On Tue, 18 Feb 2025 16:59:32 -0800, Paul Benoit wrote:
> Issue Number 1.6 of the Arm SMC Calling Convention introduces an optional
> SOC_ID name string. If implemented, point the 'machine' field of the SoC
> Device Attributes at this string so that it will appear under
> /sys/bus/soc/devices/soc0/machine.
>
> On Arm SMC compliant SoCs, this will allow things like 'lscpu' to
> eventually get a SoC provider model name from there rather than each
> tool/utility needing to get a possibly inconsistent, obsolete, or incorrect
> model/machine name from its own hardcoded model/machine name table.
>
> [...]
Applied to sudeep.holla/linux (for-linux-next), thanks!
[1/1] firmware: smccc: Support optional Arm SMC SOC_ID name
https://git.kernel.org/sudeep.holla/c/5f9c23abc477
--
Regards,
Sudeep
Hi Paul,
This looks generally good with a couple of minor nits.
On Tue, Feb 18, 2025 at 04:59:32PM -0800, Paul Benoit wrote:
> Issue Number 1.6 of the Arm SMC Calling Convention introduces an optional
> SOC_ID name string. If implemented, point the 'machine' field of the SoC
> Device Attributes at this string so that it will appear under
> /sys/bus/soc/devices/soc0/machine.
>
> On Arm SMC compliant SoCs, this will allow things like 'lscpu' to
> eventually get a SoC provider model name from there rather than each
> tool/utility needing to get a possibly inconsistent, obsolete, or incorrect
> model/machine name from its own hardcoded model/machine name table.
>
> Signed-off-by: Paul Benoit <paul@os.amperecomputing.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: linux-arm-kernel@lists.infradead.org
> ---
>
> v3->v4: Address code review identified issues of v3.
> v2->v3: Add conditionalization to exclude SOC_ID Name from 32-bit builds.
> v1->v2: Address code review identified issues.
>
> drivers/firmware/smccc/soc_id.c | 81 +++++++++++++++++++++++++++++++++
> include/linux/arm-smccc.h | 40 ++++++++++++++++
> 2 files changed, 121 insertions(+)
>
> diff --git a/drivers/firmware/smccc/soc_id.c b/drivers/firmware/smccc/soc_id.c
> index 1990263fbba0..7b0688d78dab 100644
> --- a/drivers/firmware/smccc/soc_id.c
> +++ b/drivers/firmware/smccc/soc_id.c
> @@ -32,6 +32,86 @@
> static struct soc_device *soc_dev;
> static struct soc_device_attribute *soc_dev_attr;
>
> +#ifdef CONFIG_ARM64
> +
> +static char __ro_after_init smccc_soc_id_name[136] = "";
> +
> +static inline void str_fragment_from_reg(char *dst, unsigned long reg)
> +{
> + dst[0] = (reg >> 0) & 0xff;
> + dst[1] = (reg >> 8) & 0xff;
> + dst[2] = (reg >> 16) & 0xff;
> + dst[3] = (reg >> 24) & 0xff;
> + dst[4] = (reg >> 32) & 0xff;
> + dst[5] = (reg >> 40) & 0xff;
> + dst[6] = (reg >> 48) & 0xff;
> + dst[7] = (reg >> 56) & 0xff;
> +}
> +
> +static char __init *smccc_soc_name_init(void)
> +{
> + struct arm_smccc_1_2_regs args;
> + struct arm_smccc_1_2_regs res;
> + size_t len;
> +
> + /*
> + * Issue Number 1.6 of the Arm SMC Calling Convention
> + * specification introduces an optional "name" string
> + * to the ARM_SMCCC_ARCH_SOC_ID function. Fetch it if
> + * available.
> + */
> + args.a0 = ARM_SMCCC_ARCH_SOC_ID;
> + args.a1 = 2; /* SOC_ID name */
> + arm_smccc_1_2_invoke(&args, &res);
> + if ((u32)res.a0 == 0) {
> + const unsigned int regsize = sizeof(res.a1);
> +
> + /*
> + * Copy res.a1..res.a17 to the smccc_soc_id_name string
> + * 8 bytes at a time. As per Issue 1.6 of the Arm SMC
> + * Calling Convention, the string will be NUL terminated
> + * and padded, from the end of the string to the end of the
> + * 136 byte buffer, with NULs.
> + */
> + str_fragment_from_reg(smccc_soc_id_name + 0*regsize, res.a1);
> + str_fragment_from_reg(smccc_soc_id_name + 1*regsize, res.a2);
> + str_fragment_from_reg(smccc_soc_id_name + 2*regsize, res.a3);
> + str_fragment_from_reg(smccc_soc_id_name + 3*regsize, res.a4);
> + str_fragment_from_reg(smccc_soc_id_name + 4*regsize, res.a5);
> + str_fragment_from_reg(smccc_soc_id_name + 5*regsize, res.a6);
> + str_fragment_from_reg(smccc_soc_id_name + 6*regsize, res.a7);
> + str_fragment_from_reg(smccc_soc_id_name + 7*regsize, res.a8);
> + str_fragment_from_reg(smccc_soc_id_name + 8*regsize, res.a9);
> + str_fragment_from_reg(smccc_soc_id_name + 9*regsize, res.a10);
> + str_fragment_from_reg(smccc_soc_id_name + 10*regsize, res.a11);
> + str_fragment_from_reg(smccc_soc_id_name + 11*regsize, res.a12);
> + str_fragment_from_reg(smccc_soc_id_name + 12*regsize, res.a13);
> + str_fragment_from_reg(smccc_soc_id_name + 13*regsize, res.a14);
> + str_fragment_from_reg(smccc_soc_id_name + 14*regsize, res.a15);
> + str_fragment_from_reg(smccc_soc_id_name + 15*regsize, res.a16);
> + str_fragment_from_reg(smccc_soc_id_name + 16*regsize, res.a17);
Please get rid of 'regsize' and use '8' directly. This only exists for
arm64, where the registeres are 8 bytes, and the comment immediately
above refers to "8 bytes" specifically anyway, so 'regsize' only serves
to make this harder to read.
It'd be a bit clearer as:
str_fragment_from_reg(smccc_soc_id_name + 8 * 0, res.a1);
str_fragment_from_reg(smccc_soc_id_name + 8 * 1, res.a2);
...
str_fragment_from_reg(smccc_soc_id_name + 8 * 15, res.a16);
str_fragment_from_reg(smccc_soc_id_name + 8 * 16, res.a17);
Sudeep, are you happy to fix that up when applying?
> +
> + len = strnlen(smccc_soc_id_name, sizeof(smccc_soc_id_name));
> + if (len) {
> + if (len == sizeof(smccc_soc_id_name))
> + pr_warn(FW_BUG "Ignoring improperly formatted Name\n");
It's odd that 'Name' is capitalized here. Not a big deal, but it doesn't
look quite right.
> + else
> + return smccc_soc_id_name;
> + }
> + }
> +
> + return NULL;
> +}
> +
> +#else
> +
> +static char __init *smccc_soc_name_init(void)
> +{
> + return NULL;
> +}
> +
> +#endif
> +
> static int __init smccc_soc_init(void)
> {
> int soc_id_rev, soc_id_version;
> @@ -72,6 +152,7 @@ static int __init smccc_soc_init(void)
> soc_dev_attr->soc_id = soc_id_str;
> soc_dev_attr->revision = soc_id_rev_str;
> soc_dev_attr->family = soc_id_jep106_id_str;
> + soc_dev_attr->machine = smccc_soc_name_init();
>
> soc_dev = soc_device_register(soc_dev_attr);
> if (IS_ERR(soc_dev)) {
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 67f6fdf2e7cd..eb7eab04755a 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -639,5 +639,45 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
> method; \
> })
>
> +#ifdef CONFIG_ARM64
> +
> +#define __fail_smccc_1_2(___res) \
> + do { \
> + if (___res) \
> + ___res->a0 = SMCCC_RET_NOT_SUPPORTED; \
> + } while (0)
> +
> +/*
> + * arm_smccc_1_2_invoke() - make an SMCCC v1.2 compliant call
> + *
> + * @args: SMC args are in the a0..a17 fields of the arm_smcc_1_2_regs structure
> + * @res: result values from registers 0 to 17
> + *
> + * This macro will make either an HVC call or an SMC call depending on the
> + * current SMCCC conduit. If no valid conduit is available then -1
> + * (SMCCC_RET_NOT_SUPPORTED) is returned in @res.a0 (if supplied).
> + *
> + * The return value also provides the conduit that was used.
> + */
> +#define arm_smccc_1_2_invoke(args, res) ({ \
> + struct arm_smccc_1_2_regs *__args = args; \
> + struct arm_smccc_1_2_regs *__res = res; \
> + int method = arm_smccc_1_1_get_conduit(); \
> + switch (method) { \
> + case SMCCC_CONDUIT_HVC: \
> + arm_smccc_1_2_hvc(__args, __res); \
> + break; \
> + case SMCCC_CONDUIT_SMC: \
> + arm_smccc_1_2_smc(__args, __res); \
> + break; \
> + default: \
> + __fail_smccc_1_2(__res); \
> + method = SMCCC_CONDUIT_NONE; \
> + break; \
> + } \
> + method; \
> + })
> +#endif /*CONFIG_ARM64*/
At some point I intend to rework the existing helpers to not return the
conduit, but this is fine for now.
With the fixups above:
Acked-by: Mark Rutland <mark.rutland@arm.com>
Mark.
> +
> #endif /*__ASSEMBLY__*/
> #endif /*__LINUX_ARM_SMCCC_H*/
> --
> 2.48.1
>
On Mon, Mar 03, 2025 at 02:18:05PM +0000, Mark Rutland wrote:
> Hi Paul,
>
> This looks generally good with a couple of minor nits.
>
> On Tue, Feb 18, 2025 at 04:59:32PM -0800, Paul Benoit wrote:
> > Issue Number 1.6 of the Arm SMC Calling Convention introduces an optional
> > SOC_ID name string. If implemented, point the 'machine' field of the SoC
> > Device Attributes at this string so that it will appear under
> > /sys/bus/soc/devices/soc0/machine.
> >
> > On Arm SMC compliant SoCs, this will allow things like 'lscpu' to
> > eventually get a SoC provider model name from there rather than each
> > tool/utility needing to get a possibly inconsistent, obsolete, or incorrect
> > model/machine name from its own hardcoded model/machine name table.
> >
> > Signed-off-by: Paul Benoit <paul@os.amperecomputing.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: linux-arm-kernel@lists.infradead.org
> > ---
[...]
> > +static char __init *smccc_soc_name_init(void)
> > +{
> > + struct arm_smccc_1_2_regs args;
> > + struct arm_smccc_1_2_regs res;
> > + size_t len;
> > +
> > + /*
> > + * Issue Number 1.6 of the Arm SMC Calling Convention
> > + * specification introduces an optional "name" string
> > + * to the ARM_SMCCC_ARCH_SOC_ID function. Fetch it if
> > + * available.
> > + */
> > + args.a0 = ARM_SMCCC_ARCH_SOC_ID;
> > + args.a1 = 2; /* SOC_ID name */
> > + arm_smccc_1_2_invoke(&args, &res);
> > + if ((u32)res.a0 == 0) {
> > + const unsigned int regsize = sizeof(res.a1);
> > +
> > + /*
> > + * Copy res.a1..res.a17 to the smccc_soc_id_name string
> > + * 8 bytes at a time. As per Issue 1.6 of the Arm SMC
> > + * Calling Convention, the string will be NUL terminated
> > + * and padded, from the end of the string to the end of the
> > + * 136 byte buffer, with NULs.
> > + */
> > + str_fragment_from_reg(smccc_soc_id_name + 0*regsize, res.a1);
> > + str_fragment_from_reg(smccc_soc_id_name + 1*regsize, res.a2);
> > + str_fragment_from_reg(smccc_soc_id_name + 2*regsize, res.a3);
> > + str_fragment_from_reg(smccc_soc_id_name + 3*regsize, res.a4);
> > + str_fragment_from_reg(smccc_soc_id_name + 4*regsize, res.a5);
> > + str_fragment_from_reg(smccc_soc_id_name + 5*regsize, res.a6);
> > + str_fragment_from_reg(smccc_soc_id_name + 6*regsize, res.a7);
> > + str_fragment_from_reg(smccc_soc_id_name + 7*regsize, res.a8);
> > + str_fragment_from_reg(smccc_soc_id_name + 8*regsize, res.a9);
> > + str_fragment_from_reg(smccc_soc_id_name + 9*regsize, res.a10);
> > + str_fragment_from_reg(smccc_soc_id_name + 10*regsize, res.a11);
> > + str_fragment_from_reg(smccc_soc_id_name + 11*regsize, res.a12);
> > + str_fragment_from_reg(smccc_soc_id_name + 12*regsize, res.a13);
> > + str_fragment_from_reg(smccc_soc_id_name + 13*regsize, res.a14);
> > + str_fragment_from_reg(smccc_soc_id_name + 14*regsize, res.a15);
> > + str_fragment_from_reg(smccc_soc_id_name + 15*regsize, res.a16);
> > + str_fragment_from_reg(smccc_soc_id_name + 16*regsize, res.a17);
>
> Please get rid of 'regsize' and use '8' directly. This only exists for
> arm64, where the registeres are 8 bytes, and the comment immediately
> above refers to "8 bytes" specifically anyway, so 'regsize' only serves
> to make this harder to read.
>
> It'd be a bit clearer as:
>
> str_fragment_from_reg(smccc_soc_id_name + 8 * 0, res.a1);
> str_fragment_from_reg(smccc_soc_id_name + 8 * 1, res.a2);
> ...
> str_fragment_from_reg(smccc_soc_id_name + 8 * 15, res.a16);
> str_fragment_from_reg(smccc_soc_id_name + 8 * 16, res.a17);
>
> Sudeep, are you happy to fix that up when applying?
>
> > +
> > + len = strnlen(smccc_soc_id_name, sizeof(smccc_soc_id_name));
> > + if (len) {
> > + if (len == sizeof(smccc_soc_id_name))
> > + pr_warn(FW_BUG "Ignoring improperly formatted Name\n");
>
> It's odd that 'Name' is capitalized here. Not a big deal, but it doesn't
> look quite right.
>
I can fix both of these and apply. No need to repost.
--
Regards,
Sudeep
Thank you Sudeep and Mark.
On 3/3/2025 9:45 AM, Sudeep Holla wrote:
> On Mon, Mar 03, 2025 at 02:18:05PM +0000, Mark Rutland wrote:
>> Hi Paul,
>>
>> This looks generally good with a couple of minor nits.
>>
>> On Tue, Feb 18, 2025 at 04:59:32PM -0800, Paul Benoit wrote:
>>> Issue Number 1.6 of the Arm SMC Calling Convention introduces an optional
>>> SOC_ID name string. If implemented, point the 'machine' field of the SoC
>>> Device Attributes at this string so that it will appear under
>>> /sys/bus/soc/devices/soc0/machine.
>>>
>>> On Arm SMC compliant SoCs, this will allow things like 'lscpu' to
>>> eventually get a SoC provider model name from there rather than each
>>> tool/utility needing to get a possibly inconsistent, obsolete, or incorrect
>>> model/machine name from its own hardcoded model/machine name table.
>>>
>>> Signed-off-by: Paul Benoit <paul@os.amperecomputing.com>
>>> Cc: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
>>> Cc: Sudeep Holla <sudeep.holla@arm.com>
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> ---
>
> [...]
>
>>> +static char __init *smccc_soc_name_init(void)
>>> +{
>>> + struct arm_smccc_1_2_regs args;
>>> + struct arm_smccc_1_2_regs res;
>>> + size_t len;
>>> +
>>> + /*
>>> + * Issue Number 1.6 of the Arm SMC Calling Convention
>>> + * specification introduces an optional "name" string
>>> + * to the ARM_SMCCC_ARCH_SOC_ID function. Fetch it if
>>> + * available.
>>> + */
>>> + args.a0 = ARM_SMCCC_ARCH_SOC_ID;
>>> + args.a1 = 2; /* SOC_ID name */
>>> + arm_smccc_1_2_invoke(&args, &res);
>>> + if ((u32)res.a0 == 0) {
>>> + const unsigned int regsize = sizeof(res.a1);
>>> +
>>> + /*
>>> + * Copy res.a1..res.a17 to the smccc_soc_id_name string
>>> + * 8 bytes at a time. As per Issue 1.6 of the Arm SMC
>>> + * Calling Convention, the string will be NUL terminated
>>> + * and padded, from the end of the string to the end of the
>>> + * 136 byte buffer, with NULs.
>>> + */
>>> + str_fragment_from_reg(smccc_soc_id_name + 0*regsize, res.a1);
>>> + str_fragment_from_reg(smccc_soc_id_name + 1*regsize, res.a2);
>>> + str_fragment_from_reg(smccc_soc_id_name + 2*regsize, res.a3);
>>> + str_fragment_from_reg(smccc_soc_id_name + 3*regsize, res.a4);
>>> + str_fragment_from_reg(smccc_soc_id_name + 4*regsize, res.a5);
>>> + str_fragment_from_reg(smccc_soc_id_name + 5*regsize, res.a6);
>>> + str_fragment_from_reg(smccc_soc_id_name + 6*regsize, res.a7);
>>> + str_fragment_from_reg(smccc_soc_id_name + 7*regsize, res.a8);
>>> + str_fragment_from_reg(smccc_soc_id_name + 8*regsize, res.a9);
>>> + str_fragment_from_reg(smccc_soc_id_name + 9*regsize, res.a10);
>>> + str_fragment_from_reg(smccc_soc_id_name + 10*regsize, res.a11);
>>> + str_fragment_from_reg(smccc_soc_id_name + 11*regsize, res.a12);
>>> + str_fragment_from_reg(smccc_soc_id_name + 12*regsize, res.a13);
>>> + str_fragment_from_reg(smccc_soc_id_name + 13*regsize, res.a14);
>>> + str_fragment_from_reg(smccc_soc_id_name + 14*regsize, res.a15);
>>> + str_fragment_from_reg(smccc_soc_id_name + 15*regsize, res.a16);
>>> + str_fragment_from_reg(smccc_soc_id_name + 16*regsize, res.a17);
>>
>> Please get rid of 'regsize' and use '8' directly. This only exists for
>> arm64, where the registeres are 8 bytes, and the comment immediately
>> above refers to "8 bytes" specifically anyway, so 'regsize' only serves
>> to make this harder to read.
>>
>> It'd be a bit clearer as:
>>
>> str_fragment_from_reg(smccc_soc_id_name + 8 * 0, res.a1);
>> str_fragment_from_reg(smccc_soc_id_name + 8 * 1, res.a2);
>> ...
>> str_fragment_from_reg(smccc_soc_id_name + 8 * 15, res.a16);
>> str_fragment_from_reg(smccc_soc_id_name + 8 * 16, res.a17);
>>
>> Sudeep, are you happy to fix that up when applying?
>>
>>> +
>>> + len = strnlen(smccc_soc_id_name, sizeof(smccc_soc_id_name));
>>> + if (len) {
>>> + if (len == sizeof(smccc_soc_id_name))
>>> + pr_warn(FW_BUG "Ignoring improperly formatted Name\n");
>>
>> It's odd that 'Name' is capitalized here. Not a big deal, but it doesn't
>> look quite right.
>>
>
> I can fix both of these and apply. No need to repost.
>
© 2016 - 2025 Red Hat, Inc.