drivers/firmware/smccc/smccc.c | 70 +++++++++++++++++++++++++++++++++ drivers/firmware/smccc/soc_id.c | 1 + include/linux/arm-smccc.h | 10 +++++ 3 files changed, 81 insertions(+)
Issue Number 1.6 of the Arm SMC Calling Convention introduces an
optional SOC_ID name string. If available, 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
---
drivers/firmware/smccc/smccc.c | 70 +++++++++++++++++++++++++++++++++
drivers/firmware/smccc/soc_id.c | 1 +
include/linux/arm-smccc.h | 10 +++++
3 files changed, 81 insertions(+)
diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
index a74600d9f2d7..1c7084b0b8d7 100644
--- a/drivers/firmware/smccc/smccc.c
+++ b/drivers/firmware/smccc/smccc.c
@@ -18,10 +18,12 @@ static enum arm_smccc_conduit smccc_conduit = SMCCC_CONDUIT_NONE;
bool __ro_after_init smccc_trng_available = false;
s32 __ro_after_init smccc_soc_id_version = SMCCC_RET_NOT_SUPPORTED;
s32 __ro_after_init smccc_soc_id_revision = SMCCC_RET_NOT_SUPPORTED;
+char __ro_after_init smccc_soc_id_name[136] = "";
void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
{
struct arm_smccc_res res;
+ struct arm_smccc_1_2_regs regs_1_2;
smccc_version = version;
smccc_conduit = conduit;
@@ -37,6 +39,66 @@ void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
smccc_soc_id_version = (s32)res.a0;
arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_SOC_ID, 1, &res);
smccc_soc_id_revision = (s32)res.a0;
+
+ /* 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.
+ */
+ regs_1_2.a0 = ARM_SMCCC_ARCH_SOC_ID;
+ regs_1_2.a1 = 2; /* SOC_ID name */
+ arm_smccc_1_2_smc(
+ (const struct arm_smccc_1_2_regs *)®s_1_2,
+ (struct arm_smccc_1_2_regs *)®s_1_2);
+
+ if ((u32)regs_1_2.a0 == 0) {
+ unsigned long *destination =
+ (unsigned long *)smccc_soc_id_name;
+
+ /*
+ * Copy regs_1_2.a1..regs_1_2.a17 to the
+ * smccc_soc_id_name string with consideration
+ * to the endianness of the values in a1..a17.
+ * 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.
+ */
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a1);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a2);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a3);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a4);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a5);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a6);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a7);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a8);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a9);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a10);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a11);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a12);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a13);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a14);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a15);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a16);
+ *destination++ =
+ cpu_to_le64p((const __u64 *)®s_1_2.a17);
+ }
}
}
}
@@ -67,6 +129,14 @@ s32 arm_smccc_get_soc_id_revision(void)
}
EXPORT_SYMBOL_GPL(arm_smccc_get_soc_id_revision);
+char *arm_smccc_get_soc_id_name(void)
+{
+ if (strnlen(smccc_soc_id_name, sizeof(smccc_soc_id_name)))
+ return smccc_soc_id_name;
+
+ return NULL;
+}
+
static int __init smccc_devices_init(void)
{
struct platform_device *pdev;
diff --git a/drivers/firmware/smccc/soc_id.c b/drivers/firmware/smccc/soc_id.c
index 1990263fbba0..6f698c703868 100644
--- a/drivers/firmware/smccc/soc_id.c
+++ b/drivers/firmware/smccc/soc_id.c
@@ -72,6 +72,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 = arm_smccc_get_soc_id_name();
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..5935cf636135 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -333,6 +333,16 @@ s32 arm_smccc_get_soc_id_version(void);
*/
s32 arm_smccc_get_soc_id_revision(void);
+/**
+ * arm_smccc_get_soc_id_name()
+ *
+ * Returns the SOC ID name.
+ *
+ * When ARM_SMCCC_ARCH_SOC_ID name is not present, returns NULL.
+ */
+char *arm_smccc_get_soc_id_name(void);
+
+
/**
* struct arm_smccc_res - Result from SMC/HVC call
* @a0-a3 result values from registers 0 to 3
--
2.46.2Hi Paul,
kernel test robot noticed the following build errors:
[auto build test ERROR on soc/for-next]
[also build test ERROR on linus/master v6.12-rc7 next-20241115]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Paul-Benoit/firmware-smccc-Support-optional-Arm-SMC-SOC_ID-name/20241114-113039
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20241114030452.10149-1-paul%40os.amperecomputing.com
patch subject: [PATCH] firmware: smccc: Support optional Arm SMC SOC_ID name
config: arm-randconfig-003-20241117 (https://download.01.org/0day-ci/archive/20241117/202411171059.1mPeDZMS-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241117/202411171059.1mPeDZMS-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411171059.1mPeDZMS-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
drivers/firmware/smccc/smccc.c: In function 'arm_smccc_version_init':
>> drivers/firmware/smccc/smccc.c:26:35: error: storage size of 'regs_1_2' isn't known
26 | struct arm_smccc_1_2_regs regs_1_2;
| ^~~~~~~~
>> drivers/firmware/smccc/smccc.c:50:25: error: implicit declaration of function 'arm_smccc_1_2_smc'; did you mean 'arm_smccc_1_1_smc'? [-Wimplicit-function-declaration]
50 | arm_smccc_1_2_smc(
| ^~~~~~~~~~~~~~~~~
| arm_smccc_1_1_smc
>> drivers/firmware/smccc/smccc.c:26:35: warning: unused variable 'regs_1_2' [-Wunused-variable]
26 | struct arm_smccc_1_2_regs regs_1_2;
| ^~~~~~~~
vim +26 drivers/firmware/smccc/smccc.c
22
23 void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
24 {
25 struct arm_smccc_res res;
> 26 struct arm_smccc_1_2_regs regs_1_2;
27
28 smccc_version = version;
29 smccc_conduit = conduit;
30
31 smccc_trng_available = smccc_probe_trng();
32
33 if ((smccc_version >= ARM_SMCCC_VERSION_1_2) &&
34 (smccc_conduit != SMCCC_CONDUIT_NONE)) {
35 arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
36 ARM_SMCCC_ARCH_SOC_ID, &res);
37 if ((s32)res.a0 >= 0) {
38 arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_SOC_ID, 0, &res);
39 smccc_soc_id_version = (s32)res.a0;
40 arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_SOC_ID, 1, &res);
41 smccc_soc_id_revision = (s32)res.a0;
42
43 /* Issue Number 1.6 of the Arm SMC Calling Convention
44 * specification introduces an optional "name" string
45 * to the ARM_SMCCC_ARCH_SOC_ID function. Fetch it if
46 * available.
47 */
48 regs_1_2.a0 = ARM_SMCCC_ARCH_SOC_ID;
49 regs_1_2.a1 = 2; /* SOC_ID name */
> 50 arm_smccc_1_2_smc(
51 (const struct arm_smccc_1_2_regs *)®s_1_2,
52 (struct arm_smccc_1_2_regs *)®s_1_2);
53
54 if ((u32)regs_1_2.a0 == 0) {
55 unsigned long *destination =
56 (unsigned long *)smccc_soc_id_name;
57
58 /*
59 * Copy regs_1_2.a1..regs_1_2.a17 to the
60 * smccc_soc_id_name string with consideration
61 * to the endianness of the values in a1..a17.
62 * As per Issue 1.6 of the Arm SMC Calling
63 * Convention, the string will be NUL terminated
64 * and padded, from the end of the string to
65 * the end of the 136 byte buffer, with NULs.
66 */
67 *destination++ =
68 cpu_to_le64p((const __u64 *)®s_1_2.a1);
69 *destination++ =
70 cpu_to_le64p((const __u64 *)®s_1_2.a2);
71 *destination++ =
72 cpu_to_le64p((const __u64 *)®s_1_2.a3);
73 *destination++ =
74 cpu_to_le64p((const __u64 *)®s_1_2.a4);
75 *destination++ =
76 cpu_to_le64p((const __u64 *)®s_1_2.a5);
77 *destination++ =
78 cpu_to_le64p((const __u64 *)®s_1_2.a6);
79 *destination++ =
80 cpu_to_le64p((const __u64 *)®s_1_2.a7);
81 *destination++ =
82 cpu_to_le64p((const __u64 *)®s_1_2.a8);
83 *destination++ =
84 cpu_to_le64p((const __u64 *)®s_1_2.a9);
85 *destination++ =
86 cpu_to_le64p((const __u64 *)®s_1_2.a10);
87 *destination++ =
88 cpu_to_le64p((const __u64 *)®s_1_2.a11);
89 *destination++ =
90 cpu_to_le64p((const __u64 *)®s_1_2.a12);
91 *destination++ =
92 cpu_to_le64p((const __u64 *)®s_1_2.a13);
93 *destination++ =
94 cpu_to_le64p((const __u64 *)®s_1_2.a14);
95 *destination++ =
96 cpu_to_le64p((const __u64 *)®s_1_2.a15);
97 *destination++ =
98 cpu_to_le64p((const __u64 *)®s_1_2.a16);
99 *destination++ =
100 cpu_to_le64p((const __u64 *)®s_1_2.a17);
101 }
102 }
103 }
104 }
105
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Paul,
kernel test robot noticed the following build errors:
[auto build test ERROR on soc/for-next]
[also build test ERROR on linus/master v6.12-rc7 next-20241115]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Paul-Benoit/firmware-smccc-Support-optional-Arm-SMC-SOC_ID-name/20241114-113039
base: https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link: https://lore.kernel.org/r/20241114030452.10149-1-paul%40os.amperecomputing.com
patch subject: [PATCH] firmware: smccc: Support optional Arm SMC SOC_ID name
config: arm-randconfig-002-20241117 (https://download.01.org/0day-ci/archive/20241117/202411170820.t1xuMJVL-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 592c0fe55f6d9a811028b5f3507be91458ab2713)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241117/202411170820.t1xuMJVL-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411170820.t1xuMJVL-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/firmware/smccc/smccc.c:26:28: error: variable has incomplete type 'struct arm_smccc_1_2_regs'
26 | struct arm_smccc_1_2_regs regs_1_2;
| ^
drivers/firmware/smccc/smccc.c:26:9: note: forward declaration of 'struct arm_smccc_1_2_regs'
26 | struct arm_smccc_1_2_regs regs_1_2;
| ^
>> drivers/firmware/smccc/smccc.c:50:4: error: call to undeclared function 'arm_smccc_1_2_smc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
50 | arm_smccc_1_2_smc(
| ^
2 errors generated.
vim +26 drivers/firmware/smccc/smccc.c
22
23 void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
24 {
25 struct arm_smccc_res res;
> 26 struct arm_smccc_1_2_regs regs_1_2;
27
28 smccc_version = version;
29 smccc_conduit = conduit;
30
31 smccc_trng_available = smccc_probe_trng();
32
33 if ((smccc_version >= ARM_SMCCC_VERSION_1_2) &&
34 (smccc_conduit != SMCCC_CONDUIT_NONE)) {
35 arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
36 ARM_SMCCC_ARCH_SOC_ID, &res);
37 if ((s32)res.a0 >= 0) {
38 arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_SOC_ID, 0, &res);
39 smccc_soc_id_version = (s32)res.a0;
40 arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_SOC_ID, 1, &res);
41 smccc_soc_id_revision = (s32)res.a0;
42
43 /* Issue Number 1.6 of the Arm SMC Calling Convention
44 * specification introduces an optional "name" string
45 * to the ARM_SMCCC_ARCH_SOC_ID function. Fetch it if
46 * available.
47 */
48 regs_1_2.a0 = ARM_SMCCC_ARCH_SOC_ID;
49 regs_1_2.a1 = 2; /* SOC_ID name */
> 50 arm_smccc_1_2_smc(
51 (const struct arm_smccc_1_2_regs *)®s_1_2,
52 (struct arm_smccc_1_2_regs *)®s_1_2);
53
54 if ((u32)regs_1_2.a0 == 0) {
55 unsigned long *destination =
56 (unsigned long *)smccc_soc_id_name;
57
58 /*
59 * Copy regs_1_2.a1..regs_1_2.a17 to the
60 * smccc_soc_id_name string with consideration
61 * to the endianness of the values in a1..a17.
62 * As per Issue 1.6 of the Arm SMC Calling
63 * Convention, the string will be NUL terminated
64 * and padded, from the end of the string to
65 * the end of the 136 byte buffer, with NULs.
66 */
67 *destination++ =
68 cpu_to_le64p((const __u64 *)®s_1_2.a1);
69 *destination++ =
70 cpu_to_le64p((const __u64 *)®s_1_2.a2);
71 *destination++ =
72 cpu_to_le64p((const __u64 *)®s_1_2.a3);
73 *destination++ =
74 cpu_to_le64p((const __u64 *)®s_1_2.a4);
75 *destination++ =
76 cpu_to_le64p((const __u64 *)®s_1_2.a5);
77 *destination++ =
78 cpu_to_le64p((const __u64 *)®s_1_2.a6);
79 *destination++ =
80 cpu_to_le64p((const __u64 *)®s_1_2.a7);
81 *destination++ =
82 cpu_to_le64p((const __u64 *)®s_1_2.a8);
83 *destination++ =
84 cpu_to_le64p((const __u64 *)®s_1_2.a9);
85 *destination++ =
86 cpu_to_le64p((const __u64 *)®s_1_2.a10);
87 *destination++ =
88 cpu_to_le64p((const __u64 *)®s_1_2.a11);
89 *destination++ =
90 cpu_to_le64p((const __u64 *)®s_1_2.a12);
91 *destination++ =
92 cpu_to_le64p((const __u64 *)®s_1_2.a13);
93 *destination++ =
94 cpu_to_le64p((const __u64 *)®s_1_2.a14);
95 *destination++ =
96 cpu_to_le64p((const __u64 *)®s_1_2.a15);
97 *destination++ =
98 cpu_to_le64p((const __u64 *)®s_1_2.a16);
99 *destination++ =
100 cpu_to_le64p((const __u64 *)®s_1_2.a17);
101 }
102 }
103 }
104 }
105
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Paul,
On Wed, Nov 13, 2024 at 07:04:52PM -0800, Paul Benoit wrote:
> Issue Number 1.6 of the Arm SMC Calling Convention introduces an
> optional SOC_ID name string. If available, 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
> ---
> drivers/firmware/smccc/smccc.c | 70 +++++++++++++++++++++++++++++++++
> drivers/firmware/smccc/soc_id.c | 1 +
> include/linux/arm-smccc.h | 10 +++++
> 3 files changed, 81 insertions(+)
>
> diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
> index a74600d9f2d7..1c7084b0b8d7 100644
> --- a/drivers/firmware/smccc/smccc.c
> +++ b/drivers/firmware/smccc/smccc.c
> @@ -18,10 +18,12 @@ static enum arm_smccc_conduit smccc_conduit = SMCCC_CONDUIT_NONE;
> bool __ro_after_init smccc_trng_available = false;
> s32 __ro_after_init smccc_soc_id_version = SMCCC_RET_NOT_SUPPORTED;
> s32 __ro_after_init smccc_soc_id_revision = SMCCC_RET_NOT_SUPPORTED;
> +char __ro_after_init smccc_soc_id_name[136] = "";
>
> void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
> {
> struct arm_smccc_res res;
> + struct arm_smccc_1_2_regs regs_1_2;
>
> smccc_version = version;
> smccc_conduit = conduit;
> @@ -37,6 +39,66 @@ void __init arm_smccc_version_init(u32 version, enum arm_smccc_conduit conduit)
> smccc_soc_id_version = (s32)res.a0;
> arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_SOC_ID, 1, &res);
> smccc_soc_id_revision = (s32)res.a0;
> +
> + /* 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.
> + */
I think the code for the SOC_ID name should live under soc_id.c, since
it *only* matters to the sysfs interface (and will not be used by other
code in the kernel to identify the SOC). That should be initialised
under smccc_soc_init(), ideally factored into a smccc_soc_name_init()
helper.
Nit: comments should have the leading '/*' on its own line, e.g.
/*
* Multi-line comments should be formatted like this, with a
* leading and trailing line.
*/
> + regs_1_2.a0 = ARM_SMCCC_ARCH_SOC_ID;
> + regs_1_2.a1 = 2; /* SOC_ID name */
> + arm_smccc_1_2_smc(
> + (const struct arm_smccc_1_2_regs *)®s_1_2,
> + (struct arm_smccc_1_2_regs *)®s_1_2);
These casts shouldn't be necessary, and they look suspicious.
Additionally, this should be using whichever conduit SMCCC happens to be
using rather than assuming SMC. As with the rest of this code using
arm_smccc_1_1_invoke(), we should add a arm_smccc_1_2_invoke() wrapper
for that, with this looking like:
arm_smccc_1_2_invoke(®s_1_2, ®s_1_2);
> +
> + if ((u32)regs_1_2.a0 == 0) {
> + unsigned long *destination =
> + (unsigned long *)smccc_soc_id_name;
> +
> + /*
> + * Copy regs_1_2.a1..regs_1_2.a17 to the
> + * smccc_soc_id_name string with consideration
> + * to the endianness of the values in a1..a17.
This indicates that we have to do *something* about endianness, but
doesn't say *what* consideration is necessary, which is rather
unhelpful -- it would be better to describe the format of the registers,
which would indicate what specifically we need to do.
For example:
The string is packed into registers a1 to a17 such that each
register contains 8 successive bytes, and within each register
byte N of the string fragment is encoded into bits [8*N+7:8*N].
> + * 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.
> + */
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a1);
If you used cpu_to_le64(), you wouldn't need a cast, though either way you will
need a cast on the return value since 'destination' is not an array of __le64,
and sparse should complain.
This isn't really an endianness conversion since we're unpacking smaller
elements out of a larger container, so I reckon it would be better to
handle this explicitly, e.g.
void str_fragment_from_reg(char *dst, u64 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;
}
... and then using that:
str_fragment_from_reg(destination + 0, regs_1_2.a1);
str_fragment_from_reg(destination + 8, regs_1_2.a2);
str_fragment_from_reg(destination + 16, regs_1_2.a3);
str_fragment_from_reg(destination + 24, regs_1_2.a4);
...
That way we avoid all the messy casting, and we can more clearly align
this with the way the spec says this is packed.
The generated code looks sane (with GCC 14.2.0, at least):
// little-endian kernel
0000000000000000 <str_fragment_from_reg>:
0: f9000001 str x1, [x0]
4: d65f03c0 ret
// big-endian kernel
0000000000000000 <str_fragment_from_reg>:
0: dac00c21 rev x1, x1
4: f9000001 str x1, [x0]
8: d65f03c0 ret
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a2);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a3);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a4);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a5);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a6);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a7);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a8);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a9);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a10);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a11);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a12);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a13);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a14);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a15);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a16);
> + *destination++ =
> + cpu_to_le64p((const __u64 *)®s_1_2.a17);
We probably want to check that the string is actually NUL terminated
here, and log a FW BUG message if it is not.
> + }
> }
> }
> }
> @@ -67,6 +129,14 @@ s32 arm_smccc_get_soc_id_revision(void)
> }
> EXPORT_SYMBOL_GPL(arm_smccc_get_soc_id_revision);
>
> +char *arm_smccc_get_soc_id_name(void)
> +{
> + if (strnlen(smccc_soc_id_name, sizeof(smccc_soc_id_name)))
> + return smccc_soc_id_name;
> +
> + return NULL;
> +}
As above, I think this can be folded into the probing routine.
> +
> static int __init smccc_devices_init(void)
> {
> struct platform_device *pdev;
> diff --git a/drivers/firmware/smccc/soc_id.c b/drivers/firmware/smccc/soc_id.c
> index 1990263fbba0..6f698c703868 100644
> --- a/drivers/firmware/smccc/soc_id.c
> +++ b/drivers/firmware/smccc/soc_id.c
> @@ -72,6 +72,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 = arm_smccc_get_soc_id_name();
>
> 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..5935cf636135 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -333,6 +333,16 @@ s32 arm_smccc_get_soc_id_version(void);
> */
> s32 arm_smccc_get_soc_id_revision(void);
>
> +/**
> + * arm_smccc_get_soc_id_name()
> + *
> + * Returns the SOC ID name.
> + *
> + * When ARM_SMCCC_ARCH_SOC_ID name is not present, returns NULL.
> + */
> +char *arm_smccc_get_soc_id_name(void);
I don't think this needs to be exposed outside of the SOC ID code.
Thanks,
Mark.
© 2016 - 2024 Red Hat, Inc.