Skip to contentSkip to conversation

arm/arm64: add arm-smccc

PATCH Superseded Jens Wiklander 3 participants 10 messages Dec 9, 2015 – Jan 4, 2016 Secure monitor call(SMC) calling convention (SMCCC)

v6 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#0 lore raw

PATCH v6 1/1 arm/arm64: add arm-smccc

Adds helpers to do SMC and HVC based on ARM SMC Calling Convention. CONFIG_HAVE_ARM_SMCCC is enabled for architectures that may support the SMC or HVC instruction. It's the responsibility of the caller to know if the SMC instruction is supported by the platform.

This patch doesn't provide an implementation of the declared functions. Later patches will bring in implementations and set CONFIG_HAVE_ARM_SMCCC for ARM and ARM64 respectively.

---

v6:
* Move HAVE_ARM_SMCCC from init/Kconfig

 arch/Kconfig              |  3 ++
 include/linux/arm-smccc.h | 98 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 include/linux/arm-smccc.h
arch/Kconfig +3 -0
index 4e949e5..ce3c0b0 100644
@@ -564,4 +564,7 @@ config OLD_SIGACTION
 config COMPAT_OLD_SIGACTION
 	bool
 
+config HAVE_ARM_SMCCC
+	bool
+
 source "kernel/gcov/Kconfig"
include/linux/arm-smccc.h new file +98 -0

98 lines not on this page · raw message

Signature · 1 line
1.9.1
#1 lore raw

PATCH v6 1/1 arm/arm64: add arm-smccc

Quoted 25 lines · Jens Wiklander

Adds helpers to do SMC and HVC based on ARM SMC Calling Convention. CONFIG_HAVE_ARM_SMCCC is enabled for architectures that may support the SMC or HVC instruction. It's the responsibility of the caller to know if the SMC instruction is supported by the platform.

This patch doesn't provide an implementation of the declared functions. Later patches will bring in implementations and set CONFIG_HAVE_ARM_SMCCC for ARM and ARM64 respectively.

---

v6:
* Move HAVE_ARM_SMCCC from init/Kconfig

 arch/Kconfig              |  3 ++
 include/linux/arm-smccc.h | 98 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 include/linux/arm-smccc.h
arch/Kconfig +0 -0
index 4e949e5..ce3c0b0 100644

arch/Kconfig config OLD_SIGACTION · line 568

Quoted 5 lines
arch/Kconfig +2 -0
@@ -564,4 +564,7 @@ config OLD_SIGACTION
 config COMPAT_OLD_SIGACTION
 	bool
 
+config HAVE_ARM_SMCCC
+	bool

It is ok by me to move it there, probably we do not want it at the end of the "ABI hall of shame" list :)

Or drivers/firmware/Kconfig ?

Strictly speaking, since PSCI uses this by default, you should also enforce an ARM_PSCI_FW dependency on HAVE_ARM_SMCCC.

Quoted 15 lines
arch/Kconfig +0 -0
 source "kernel/gcov/Kconfig"
include/linux/arm-smccc.h new file +8 -0
new file mode 100644
index 0000000..dea68a9
@@ -0,0 +1,98 @@
+/*
+ * Copyright (c) 2015, Linaro Limited
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,

include/linux/arm-smccc.h line 18

Quoted 10 lines
include/linux/arm-smccc.h new file +10 -0
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef __LINUX_ARM_SMCCC_H
+#define __LINUX_ARM_SMCCC_H
+
+#include <linux/types.h>
+#include <linux/linkage.h>

Nit: alphabetical order please.

Quoted 15 lines
include/linux/arm-smccc.h new file +15 -0
+
+/*
+ * This file provides common defines for ARM SMC Calling Convention as
+ * specified in
+ * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
+ */
+
+#define ARM_SMCCC_SMC_32		(0 << 30)
+#define ARM_SMCCC_SMC_64		(1 << 30)
+#define ARM_SMCCC_FAST_CALL		(1 << 31)
+#define ARM_SMCCC_STD_CALL		(0 << 31)
+
+#define ARM_SMCCC_OWNER_MASK		0x3F
+#define ARM_SMCCC_OWNER_SHIFT		24
+

include/linux/arm-smccc.h line 43

Quoted 10 lines
include/linux/arm-smccc.h new file +10 -0
+#define ARM_SMCCC_FUNC_MASK		0xFFFF
+
+#define ARM_SMCCC_IS_FAST_CALL(smc_val)	((smc_val) & ARM_SMCCC_FAST_CALL)
+#define ARM_SMCCC_IS_64(smc_val)	((smc_val) & ARM_SMCCC_SMC_64)
+#define ARM_SMCCC_FUNC_NUM(smc_val)	((smc_val) & ARM_SMCCC_FUNC_MASK)
+#define ARM_SMCCC_OWNER_NUM(smc_val) \
+	(((smc_val) >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK)
+
+#define ARM_SMCCC_CALL_VAL(type, calling_convention, owner, func_num) \
+	((type) | (calling_convention) | \

Nit: if you use a shift macro for some fields it would be clearer if you use for all of them (I am referring to type/calling_convention here), it can be changed later.

Other than that:

Quoted 58 lines
include/linux/arm-smccc.h new file +55 -0
+	(((owner) & ARM_SMCCC_OWNER_MASK) << ARM_SMCCC_OWNER_SHIFT) | \
+	((func_num) & ARM_SMCCC_FUNC_MASK))
+
+#define ARM_SMCCC_OWNER_ARCH		0
+#define ARM_SMCCC_OWNER_CPU		1
+#define ARM_SMCCC_OWNER_SIP		2
+#define ARM_SMCCC_OWNER_OEM		3
+#define ARM_SMCCC_OWNER_STANDARD	4
+#define ARM_SMCCC_OWNER_TRUSTED_APP	48
+#define ARM_SMCCC_OWNER_TRUSTED_APP_END	49
+#define ARM_SMCCC_OWNER_TRUSTED_OS	50
+#define ARM_SMCCC_OWNER_TRUSTED_OS_END	63
+
+/**
+ * struct arm_smccc_res - Result from SMC/HVC call
+ * @a0-a3 result values from registers 0 to 3
+ */
+struct arm_smccc_res {
+	unsigned long a0;
+	unsigned long a1;
+	unsigned long a2;
+	unsigned long a3;
+};
+
+/**
+ * arm_smccc_smc() - make SMC calls
+ * @a0-a7: arguments passed in registers 0 to 7
+ * @res: result values from registers 0 to 3
+ *
+ * This function is used to make SMC calls following SMC Calling Convention.
+ * The content of the supplied param are copied to registers 0 to 7 prior
+ * to the SMC instruction. The return values are updated with the content
+ * from register 0 to 3 on return from the SMC instruction.
+ */
+asmlinkage void arm_smccc_smc(unsigned long a0, unsigned long a1,
+			unsigned long a2, unsigned long a3, unsigned long a4,
+			unsigned long a5, unsigned long a6, unsigned long a7,
+			struct arm_smccc_res *res);
+
+/**
+ * arm_smccc_hvc() - make HVC calls
+ * @a0-a7: arguments passed in registers 0 to 7
+ * @res: result values from registers 0 to 3
+ *
+ * This function is used to make HVC calls following SMC Calling
+ * Convention.  The content of the supplied param are copied to registers 0
+ * to 7 prior to the HVC instruction. The return values are updated with
+ * the content from register 0 to 3 on return from the HVC instruction.
+ */
+asmlinkage void arm_smccc_hvc(unsigned long a0, unsigned long a1,
+			unsigned long a2, unsigned long a3, unsigned long a4,
+			unsigned long a5, unsigned long a6, unsigned long a7,
+			struct arm_smccc_res *res);
+
+#endif /*__LINUX_ARM_SMCCC_H*/
Signature · 1 line
1.9.1
#2 lore raw

PATCH v6 1/1 arm/arm64: add arm-smccc

Quoted 29 lines · Lorenzo Pieralisi
Quoted 28 lines · Jens Wiklander

Adds helpers to do SMC and HVC based on ARM SMC Calling Convention. CONFIG_HAVE_ARM_SMCCC is enabled for architectures that may support the SMC or HVC instruction. It's the responsibility of the caller to know if the SMC instruction is supported by the platform.

This patch doesn't provide an implementation of the declared functions. Later patches will bring in implementations and set CONFIG_HAVE_ARM_SMCCC for ARM and ARM64 respectively.

---

v6:
* Move HAVE_ARM_SMCCC from init/Kconfig

 arch/Kconfig              |  3 ++
 include/linux/arm-smccc.h | 98 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 include/linux/arm-smccc.h
arch/Kconfig +0 -0
index 4e949e5..ce3c0b0 100644
@@ -564,4 +564,7 @@ config OLD_SIGACTION
 config COMPAT_OLD_SIGACTION
 	bool
 
Quoted 7 lines

arch/Kconfig config OLD_SIGACTION · line 568

Quoted 2 lines
arch/Kconfig +2 -0
+config HAVE_ARM_SMCCC
+	bool

It is ok by me to move it there, probably we do not want it at the end of the "ABI hall of shame" list :)

Or drivers/firmware/Kconfig ?

You tell me, I'm too new here to have a feeling for this.

Quoted 3 lines

Strictly speaking, since PSCI uses this by default, you should also enforce an ARM_PSCI_FW dependency on HAVE_ARM_SMCCC.

ARM_PSCI depends on CPU_V7 and ARM_PSCI_FW doesn't really depend on anything today.

Would it be OK if I changed ARM_PSCI to depend on HAVE_ARM_SMCCC instead of CPU_V7 in the "drivers: psci: replace psci firmware calls" patch? At the same time I would move the "select HAVE_ARM_SMCCC if CPU_V7" line to the "config ARM" block instead in the
"arm: add implementation for arm-smccc" patch.

I'll include this change in the v7 patch set if I don't hear anything.

Quoted 59 lines
Quoted 16 lines
arch/Kconfig +0 -0
 source "kernel/gcov/Kconfig"
include/linux/arm-smccc.h new file +8 -0
new file mode 100644
index 0000000..dea68a9
@@ -0,0 +1,98 @@
+/*
+ * Copyright (c) 2015, Linaro Limited
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,

include/linux/arm-smccc.h line 18

Quoted 10 lines
include/linux/arm-smccc.h new file +10 -0
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef __LINUX_ARM_SMCCC_H
+#define __LINUX_ARM_SMCCC_H
+
+#include <linux/types.h>
+#include <linux/linkage.h>

Nit: alphabetical order please.

Quoted 15 lines
include/linux/arm-smccc.h new file +15 -0
+
+/*
+ * This file provides common defines for ARM SMC Calling Convention as
+ * specified in
+ * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
+ */
+
+#define ARM_SMCCC_SMC_32		(0 << 30)
+#define ARM_SMCCC_SMC_64		(1 << 30)
+#define ARM_SMCCC_FAST_CALL		(1 << 31)
+#define ARM_SMCCC_STD_CALL		(0 << 31)
+
+#define ARM_SMCCC_OWNER_MASK		0x3F
+#define ARM_SMCCC_OWNER_SHIFT		24
+

include/linux/arm-smccc.h line 43

Quoted 10 lines
include/linux/arm-smccc.h new file +10 -0
+#define ARM_SMCCC_FUNC_MASK		0xFFFF
+
+#define ARM_SMCCC_IS_FAST_CALL(smc_val)	((smc_val) & ARM_SMCCC_FAST_CALL)
+#define ARM_SMCCC_IS_64(smc_val)	((smc_val) & ARM_SMCCC_SMC_64)
+#define ARM_SMCCC_FUNC_NUM(smc_val)	((smc_val) & ARM_SMCCC_FUNC_MASK)
+#define ARM_SMCCC_OWNER_NUM(smc_val) \
+	(((smc_val) >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK)
+
+#define ARM_SMCCC_CALL_VAL(type, calling_convention, owner, func_num) \
+	((type) | (calling_convention) | \

Nit: if you use a shift macro for some fields it would be clearer if you use for all of them (I am referring to type/calling_convention here), it can be changed later.

Quoted 3 lines

Other than that:

Thanks for the review.
--
Jens

#3 lore raw

PATCH v6 1/1 arm/arm64: add arm-smccc

Quoted 27 lines · Jens Wiklander
Quoted 9 lines · Lorenzo Pieralisi
Quoted 8 lines · Jens Wiklander
arch/Kconfig +0 -0
index 4e949e5..ce3c0b0 100644
@@ -564,4 +564,7 @@ config OLD_SIGACTION
 config COMPAT_OLD_SIGACTION
 	bool
 
Quoted 7 lines

arch/Kconfig config OLD_SIGACTION · line 568

Quoted 2 lines
arch/Kconfig +2 -0
+config HAVE_ARM_SMCCC
+	bool

It is ok by me to move it there, probably we do not want it at the end of the "ABI hall of shame" list :)

Or drivers/firmware/Kconfig ?

You tell me, I'm too new here to have a feeling for this.

Quoted 3 lines

Strictly speaking, since PSCI uses this by default, you should also enforce an ARM_PSCI_FW dependency on HAVE_ARM_SMCCC.

ARM_PSCI depends on CPU_V7 and ARM_PSCI_FW doesn't really depend on anything today.

Quoted 7 lines

Would it be OK if I changed ARM_PSCI to depend on HAVE_ARM_SMCCC instead of CPU_V7 in the "drivers: psci: replace psci firmware calls" patch? At the same time I would move the "select HAVE_ARM_SMCCC if CPU_V7" line to the "config ARM" block instead in the
"arm: add implementation for arm-smccc" patch.

I'll include this change in the v7 patch set if I don't hear anything.

Okay, I think as we don't have a decision on this, the proximity of Christmas, and the presumable opening of the merge window early in the new year, I can't merge these patches for the upcoming window. I'll discard those which have already been submitted to the patch system.

This also has a knock-on effect on patch 8486/1:

"ARM64: kernel: PSCI: move PSCI idle management code to drivers/firmware"

which is based on top of this series. If someone wants to replace that patch with one which has been tested _without_ this series then I'll look at applying it, provided its only trivially different from the existing one.

Thanks.

Signature · 3 lines
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
#4 lore raw

PATCH v6 1/1 arm/arm64: add arm-smccc

Hi Russell,

Quoted 42 lines · Russell King - ARM Linux
Quoted 27 lines · Jens Wiklander
Quoted 9 lines · Lorenzo Pieralisi
Quoted 8 lines · Jens Wiklander
arch/Kconfig +0 -0
index 4e949e5..ce3c0b0 100644
@@ -564,4 +564,7 @@ config OLD_SIGACTION
 config COMPAT_OLD_SIGACTION
 	bool
 
Quoted 7 lines

arch/Kconfig config OLD_SIGACTION · line 568

Quoted 2 lines
arch/Kconfig +2 -0
+config HAVE_ARM_SMCCC
+	bool

It is ok by me to move it there, probably we do not want it at the end of the "ABI hall of shame" list :)

Or drivers/firmware/Kconfig ?

You tell me, I'm too new here to have a feeling for this.

Quoted 3 lines

Strictly speaking, since PSCI uses this by default, you should also enforce an ARM_PSCI_FW dependency on HAVE_ARM_SMCCC.

ARM_PSCI depends on CPU_V7 and ARM_PSCI_FW doesn't really depend on anything today.

Quoted 7 lines

Would it be OK if I changed ARM_PSCI to depend on HAVE_ARM_SMCCC instead of CPU_V7 in the "drivers: psci: replace psci firmware calls" patch? At the same time I would move the "select HAVE_ARM_SMCCC if CPU_V7" line to the "config ARM" block instead in the
"arm: add implementation for arm-smccc" patch.

I'll include this change in the v7 patch set if I don't hear anything.

Okay, I think as we don't have a decision on this, the proximity of Christmas, and the presumable opening of the merge window early in the new year, I can't merge these patches for the upcoming window. I'll discard those which have already been submitted to the patch system.

Quoted 8 lines

This also has a knock-on effect on patch 8486/1:

"ARM64: kernel: PSCI: move PSCI idle management code to drivers/firmware"

which is based on top of this series. If someone wants to replace that patch with one which has been tested _without_ this series then I'll look at applying it, provided its only trivially different from the existing one.

I guess Jens' patches are not considered material for the upcoming merge window, would you consider applying an updated version of 8486/1 (that does not depend on Jens' series) or it is too late ? That patch has already missed a merge window for patch dependencies, I would avoid another one, I appreciate the timing is not ideal though, please let me know.

Thanks !
Lorenzo

#5 lore raw

PATCH v6 1/1 arm/arm64: add arm-smccc

Quoted 18 lines · Lorenzo Pieralisi

Hi Russell,

Quoted 6 lines · Russell King - ARM Linux

Okay, I think as we don't have a decision on this, the proximity of Christmas, and the presumable opening of the merge window early in the new year, I can't merge these patches for the upcoming window. I'll discard those which have already been submitted to the patch system.

Quoted 8 lines

This also has a knock-on effect on patch 8486/1:

"ARM64: kernel: PSCI: move PSCI idle management code to drivers/firmware"

which is based on top of this series. If someone wants to replace that patch with one which has been tested _without_ this series then I'll look at applying it, provided its only trivially different from the existing one.

Quoted 5 lines

I guess Jens' patches are not considered material for the upcoming merge window, would you consider applying an updated version of 8486/1 (that does not depend on Jens' series) or it is too late ? That patch has already missed a merge window for patch dependencies, I would avoid another one, I appreciate the timing is not ideal though, please let me know.

The reason I said what I did above was to head-off the merge window opening last night, and having a rushed job dealing with problems with patches merged at the last minute.

Consider that the period 24th December to 3rd January does not exist for many as far as working on the kernel is concerned, and linux-next probably wasn't running over that period either.

The situation has changed though; last night, Linus released -rc8, which means we have one week of breathing space before the merge window is likely to open, and we have four days of linux-next trees.

This means that I'm willing to take patches today (and today only), as it will give the remainder of the week (up to Thursday - due to the linux-next timing) to sort out any deficiencies in those patches.

Signature · 3 lines
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
#6 lore raw

PATCH v6 1/1 arm/arm64: add arm-smccc

Quoted 8 lines · Russell King - ARM Linux

The reason I said what I did above was to head-off the merge window opening last night, and having a rushed job dealing with problems with patches merged at the last minute.

Consider that the period 24th December to 3rd January does not exist for many as far as working on the kernel is concerned, and linux-next probably wasn't running over that period either.

Quoted 7 lines

The situation has changed though; last night, Linus released -rc8, which means we have one week of breathing space before the merge window is likely to open, and we have four days of linux-next trees.

This means that I'm willing to take patches today (and today only), as it will give the remainder of the week (up to Thursday - due to the linux-next timing) to sort out any deficiencies in those patches.

That all said, 8486/1 _is_ the patch you've just asked me to merge, which has open questions from Jens that you (Lorenzo) have _not_ responded to.

Jens was asking, in response to your feedback, whether an alternative Kconfig structure would satisfy your feedback... to which he's had no reply from you. So, we're waiting on your response.

Signature · 3 lines
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
#7 lore raw

PATCH v6 1/1 arm/arm64: add arm-smccc

Quoted 17 lines · Russell King - ARM Linux
Quoted 8 lines · Russell King - ARM Linux

The reason I said what I did above was to head-off the merge window opening last night, and having a rushed job dealing with problems with patches merged at the last minute.

Consider that the period 24th December to 3rd January does not exist for many as far as working on the kernel is concerned, and linux-next probably wasn't running over that period either.

Quoted 7 lines

The situation has changed though; last night, Linus released -rc8, which means we have one week of breathing space before the merge window is likely to open, and we have four days of linux-next trees.

This means that I'm willing to take patches today (and today only), as it will give the remainder of the week (up to Thursday - due to the linux-next timing) to sort out any deficiencies in those patches.

Quoted 7 lines

That all said, 8486/1 _is_ the patch you've just asked me to merge, which has open questions from Jens that you (Lorenzo) have _not_ responded to.

Jens was asking, in response to your feedback, whether an alternative Kconfig structure would satisfy your feedback... to which he's had no reply from you. So, we're waiting on your response.

Sorry for the delay in replying. I re-tested 8486/1 on top of Jens' v7, it is all fine, I commented on the config structure which is a change that does not affect 8486/1 so if it is ok with you I am happy for 8486/1 to be applied on top of Jens' series (I guess he should be sending a v8 today which should be final).

As a heads-up, 8486/1 also depends on 8485/1 that you have already applied.

Thanks,
Lorenzo

#8 lore raw

PATCH v6 1/1 arm/arm64: add arm-smccc

In reply to Jens Wiklander
Quoted 47 lines · Jens Wiklander
Quoted 29 lines · Lorenzo Pieralisi
Quoted 28 lines · Jens Wiklander

Adds helpers to do SMC and HVC based on ARM SMC Calling Convention. CONFIG_HAVE_ARM_SMCCC is enabled for architectures that may support the SMC or HVC instruction. It's the responsibility of the caller to know if the SMC instruction is supported by the platform.

This patch doesn't provide an implementation of the declared functions. Later patches will bring in implementations and set CONFIG_HAVE_ARM_SMCCC for ARM and ARM64 respectively.

---

v6:
* Move HAVE_ARM_SMCCC from init/Kconfig

 arch/Kconfig              |  3 ++
 include/linux/arm-smccc.h | 98 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 include/linux/arm-smccc.h
arch/Kconfig +0 -0
index 4e949e5..ce3c0b0 100644
@@ -564,4 +564,7 @@ config OLD_SIGACTION
 config COMPAT_OLD_SIGACTION
 	bool
 
Quoted 7 lines

arch/Kconfig config OLD_SIGACTION · line 568

Quoted 2 lines
arch/Kconfig +2 -0
+config HAVE_ARM_SMCCC
+	bool

It is ok by me to move it there, probably we do not want it at the end of the "ABI hall of shame" list :)

Or drivers/firmware/Kconfig ?

You tell me, I'm too new here to have a feeling for this.

Quoted 3 lines

Strictly speaking, since PSCI uses this by default, you should also enforce an ARM_PSCI_FW dependency on HAVE_ARM_SMCCC.

ARM_PSCI depends on CPU_V7 and ARM_PSCI_FW doesn't really depend on anything today.

Quoted 7 lines

Would it be OK if I changed ARM_PSCI to depend on HAVE_ARM_SMCCC instead of CPU_V7 in the "drivers: psci: replace psci firmware calls" patch? At the same time I would move the "select HAVE_ARM_SMCCC if CPU_V7" line to the "config ARM" block instead in the
"arm: add implementation for arm-smccc" patch.

I'll include this change in the v7 patch set if I don't hear anything.

Sorry for the delay in getting back to you.

Yes, I still think that HAVE_ARM_SMCCC should not be listed at the end of "ABI hall of shame" in arch/Kconfig and you can move it to drivers/firmware/Kconfig.

Other than that your v7 is fine by me, can you respin quickly and ask Russell to pull today please ?

Thanks,
Lorenzo

Quoted 68 lines
Quoted 60 lines
Quoted 17 lines
arch/Kconfig +0 -0
 source "kernel/gcov/Kconfig"
include/linux/arm-smccc.h new file +8 -0
new file mode 100644
index 0000000..dea68a9
@@ -0,0 +1,98 @@
+/*
+ * Copyright (c) 2015, Linaro Limited
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,

include/linux/arm-smccc.h line 18

Quoted 10 lines
include/linux/arm-smccc.h new file +10 -0
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ */
+#ifndef __LINUX_ARM_SMCCC_H
+#define __LINUX_ARM_SMCCC_H
+
+#include <linux/types.h>
+#include <linux/linkage.h>

Nit: alphabetical order please.

Quoted 15 lines
include/linux/arm-smccc.h new file +15 -0
+
+/*
+ * This file provides common defines for ARM SMC Calling Convention as
+ * specified in
+ * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
+ */
+
+#define ARM_SMCCC_SMC_32		(0 << 30)
+#define ARM_SMCCC_SMC_64		(1 << 30)
+#define ARM_SMCCC_FAST_CALL		(1 << 31)
+#define ARM_SMCCC_STD_CALL		(0 << 31)
+
+#define ARM_SMCCC_OWNER_MASK		0x3F
+#define ARM_SMCCC_OWNER_SHIFT		24
+

include/linux/arm-smccc.h line 43

Quoted 10 lines
include/linux/arm-smccc.h new file +10 -0
+#define ARM_SMCCC_FUNC_MASK		0xFFFF
+
+#define ARM_SMCCC_IS_FAST_CALL(smc_val)	((smc_val) & ARM_SMCCC_FAST_CALL)
+#define ARM_SMCCC_IS_64(smc_val)	((smc_val) & ARM_SMCCC_SMC_64)
+#define ARM_SMCCC_FUNC_NUM(smc_val)	((smc_val) & ARM_SMCCC_FUNC_MASK)
+#define ARM_SMCCC_OWNER_NUM(smc_val) \
+	(((smc_val) >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK)
+
+#define ARM_SMCCC_CALL_VAL(type, calling_convention, owner, func_num) \
+	((type) | (calling_convention) | \

Nit: if you use a shift macro for some fields it would be clearer if you use for all of them (I am referring to type/calling_convention here), it can be changed later.

Quoted 3 lines

Other than that:

Thanks for the review.
--
Jens

#9 lore raw

PATCH v6 1/1 arm/arm64: add arm-smccc

Quoted 56 lines · Lorenzo Pieralisi
Quoted 47 lines · Jens Wiklander
Quoted 29 lines · Lorenzo Pieralisi
Quoted 28 lines · Jens Wiklander

Adds helpers to do SMC and HVC based on ARM SMC Calling Convention. CONFIG_HAVE_ARM_SMCCC is enabled for architectures that may support the SMC or HVC instruction. It's the responsibility of the caller to know if the SMC instruction is supported by the platform.

This patch doesn't provide an implementation of the declared functions. Later patches will bring in implementations and set CONFIG_HAVE_ARM_SMCCC for ARM and ARM64 respectively.

---

v6:
* Move HAVE_ARM_SMCCC from init/Kconfig

 arch/Kconfig              |  3 ++
 include/linux/arm-smccc.h | 98 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)
 create mode 100644 include/linux/arm-smccc.h
arch/Kconfig +0 -0
index 4e949e5..ce3c0b0 100644
@@ -564,4 +564,7 @@ config OLD_SIGACTION
 config COMPAT_OLD_SIGACTION
 	bool
 
Quoted 7 lines

arch/Kconfig config OLD_SIGACTION · line 568

Quoted 2 lines
arch/Kconfig +2 -0
+config HAVE_ARM_SMCCC
+	bool

It is ok by me to move it there, probably we do not want it at the end of the "ABI hall of shame" list :)

Or drivers/firmware/Kconfig ?

You tell me, I'm too new here to have a feeling for this.

Quoted 3 lines

Strictly speaking, since PSCI uses this by default, you should also enforce an ARM_PSCI_FW dependency on HAVE_ARM_SMCCC.

ARM_PSCI depends on CPU_V7 and ARM_PSCI_FW doesn't really depend on anything today.

Quoted 7 lines

Would it be OK if I changed ARM_PSCI to depend on HAVE_ARM_SMCCC instead of CPU_V7 in the "drivers: psci: replace psci firmware calls" patch? At the same time I would move the "select HAVE_ARM_SMCCC if CPU_V7" line to the "config ARM" block instead in the
"arm: add implementation for arm-smccc" patch.

I'll include this change in the v7 patch set if I don't hear anything.

Quoted 8 lines

Sorry for the delay in getting back to you.

Yes, I still think that HAVE_ARM_SMCCC should not be listed at the end of "ABI hall of shame" in arch/Kconfig and you can move it to drivers/firmware/Kconfig.

Other than that your v7 is fine by me, can you respin quickly and ask Russell to pull today please ?

OK, I'll do that asap.

Thanks,
Jens