[PATCHv3 0/6] arm/arm64: SMCCC conduit cleanup

[PATCHv3 0/6] arm/arm64: SMCCC conduit cleanup

[Mark Rutland]

Currently, the cpu errata code goes digging into PSCI internals to
discover the SMCCC conduit, using the (arguably misnamed) PSCI_CONDUIT_*
definitions. This lack of abstraction is somewhat unfortunate.

Further, the SDEI code has an almost identical set of CONDUIT_*
definitions, and the duplication is rather unfortunate.

Let's unify things behind a common set of SMCCC_CONDUIT_* definitions,
and expose the SMCCCv1.1 conduit via a new helper that hides the PSCI
driver internals.

Since v1 [1]:
* Rebase to the arm64 for-next/core branch, atop of SSBD patches
* Fold in acks
Since v2 [2]:
* Rebase to v5.3-rc3
* Fix up arm spectre-v2 code
* Drop acks where significant changes have been made

Mark.

[1] https://lkml.kernel.org/r/20180503170330.5591-1-mark.rutland@arm.com
[2] https://lkml.kernel.org/r/20180531173223.9668-1-mark.rutland@arm.com

Mark Rutland (6):
  arm/arm64: smccc/psci: add arm_smccc_1_1_get_conduit()
  arm64: errata: use arm_smccc_1_1_get_conduit()
  arm: spectre-v2: use arm_smccc_1_1_get_conduit()
  firmware/psci: use common SMCCC_CONDUIT_*
  firmware: arm_sdei: use common SMCCC_CONDUIT_*
  smccc: make 1.1 macros value-returning

 arch/arm/mm/proc-v7-bugs.c     | 22 ++++++--------
 arch/arm64/kernel/cpu_errata.c | 61 ++++++++++++++++-----------------------
 arch/arm64/kernel/sdei.c       |  3 +-
 arch/arm64/kvm/hyp/switch.c    |  4 +--
 drivers/firmware/arm_sdei.c    | 12 ++++----
 drivers/firmware/psci/psci.c   | 24 ++++++++++------
 include/linux/arm-smccc.h      | 65 ++++++++++++++++++++++++------------------
 include/linux/arm_sdei.h       |  6 ----
 include/linux/psci.h           |  9 ++----
 9 files changed, 99 insertions(+), 107 deletions(-)

-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 1/6] arm/arm64: smccc/psci: add arm_smccc_1_1_get_conduit()

[Mark Rutland]

SMCCC callers are currently amassing a collection of enums for the SMCCC
conduit, and are having to dig into the PSCI driver's internals in order
to figure out what to do.

Let's clean this up, with common SMCCC_CONDUIT_* definitions, and an
arm_smccc_1_1_get_conduit() helper that abstracts the PSCI driver's
internal state.

We can kill off the PSCI_CONDUIT_* definitions once we've migrated users
over to the new interface.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 drivers/firmware/psci/psci.c | 15 +++++++++++++++
 include/linux/arm-smccc.h    | 16 ++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index f82ccd39a913..5f31f1bea1af 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -57,6 +57,21 @@ struct psci_operations psci_ops = {
 	.smccc_version = SMCCC_VERSION_1_0,
 };
 
+enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void)
+{
+	if (psci_ops.smccc_version < SMCCC_VERSION_1_1)
+		return SMCCC_CONDUIT_NONE;
+
+	switch (psci_ops.conduit) {
+	case PSCI_CONDUIT_SMC:
+		return SMCCC_CONDUIT_SMC;
+	case PSCI_CONDUIT_HVC:
+		return SMCCC_CONDUIT_HVC;
+	default:
+		return SMCCC_CONDUIT_NONE;
+	}
+}
+
 typedef unsigned long (psci_fn)(unsigned long, unsigned long,
 				unsigned long, unsigned long);
 static psci_fn *invoke_psci_fn;
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 080012a6f025..df01a8579034 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -80,6 +80,22 @@
 
 #include <linux/linkage.h>
 #include <linux/types.h>
+
+enum arm_smccc_conduit {
+	SMCCC_CONDUIT_NONE,
+	SMCCC_CONDUIT_SMC,
+	SMCCC_CONDUIT_HVC,
+};
+
+/**
+ * arm_smccc_1_1_get_conduit()
+ *
+ * Returns the conduit to be used for SMCCCv1.1 or later.
+ *
+ * When SMCCCv1.1 is not present, returns SMCCC_CONDUIT_NONE.
+ */
+enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void);
+
 /**
  * struct arm_smccc_res - Result from SMC/HVC call
  * @a0-a3 result values from registers 0 to 3
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 2/6] arm64: errata: use arm_smccc_1_1_get_conduit()

[Mark Rutland]

Now that we have arm_smccc_1_1_get_conduit(), we can hide the PSCI
implementation details from the arm64 cpu errata code, so let's do so.

As arm_smccc_1_1_get_conduit() implicitly checks that the SMCCC version
is at least SMCCC_VERSION_1_1, we no longer need to check this
explicitly where switch statements have a default case, e.g. in
has_ssbd_mitigation().

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/kernel/cpu_errata.c | 37 ++++++++++++-------------------------
 1 file changed, 12 insertions(+), 25 deletions(-)

Will, Lorenzo, you both acked v1, but I dropped your acks when rebasing as the
structure of the code changed somewhat. Are you happy to give new acks?

Mark.


diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 1e43ba5c79b7..6ee09bca82f8 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -6,7 +6,6 @@
  */
 
 #include <linux/arm-smccc.h>
-#include <linux/psci.h>
 #include <linux/types.h>
 #include <linux/cpu.h>
 #include <asm/cpu.h>
@@ -166,9 +165,7 @@ static void install_bp_hardening_cb(bp_hardening_cb_t fn,
 }
 #endif	/* CONFIG_KVM_INDIRECT_VECTORS */
 
-#include <uapi/linux/psci.h>
 #include <linux/arm-smccc.h>
-#include <linux/psci.h>
 
 static void call_smc_arch_workaround_1(void)
 {
@@ -212,11 +209,8 @@ static int detect_harden_bp_fw(void)
 	struct arm_smccc_res res;
 	u32 midr = read_cpuid_id();
 
-	if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
-		return -1;
-
-	switch (psci_ops.conduit) {
-	case PSCI_CONDUIT_HVC:
+	switch (arm_smccc_1_1_get_conduit()) {
+	case SMCCC_CONDUIT_HVC:
 		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 				  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
 		switch ((int)res.a0) {
@@ -234,7 +228,7 @@ static int detect_harden_bp_fw(void)
 		}
 		break;
 
-	case PSCI_CONDUIT_SMC:
+	case SMCCC_CONDUIT_SMC:
 		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 				  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
 		switch ((int)res.a0) {
@@ -308,11 +302,11 @@ void __init arm64_update_smccc_conduit(struct alt_instr *alt,
 
 	BUG_ON(nr_inst != 1);
 
-	switch (psci_ops.conduit) {
-	case PSCI_CONDUIT_HVC:
+	switch (arm_smccc_1_1_get_conduit()) {
+	case SMCCC_CONDUIT_HVC:
 		insn = aarch64_insn_get_hvc_value();
 		break;
-	case PSCI_CONDUIT_SMC:
+	case SMCCC_CONDUIT_SMC:
 		insn = aarch64_insn_get_smc_value();
 		break;
 	default:
@@ -351,12 +345,12 @@ void arm64_set_ssbd_mitigation(bool state)
 		return;
 	}
 
-	switch (psci_ops.conduit) {
-	case PSCI_CONDUIT_HVC:
+	switch (arm_smccc_1_1_get_conduit()) {
+	case SMCCC_CONDUIT_HVC:
 		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
 		break;
 
-	case PSCI_CONDUIT_SMC:
+	case SMCCC_CONDUIT_SMC:
 		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
 		break;
 
@@ -390,20 +384,13 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
 		goto out_printmsg;
 	}
 
-	if (psci_ops.smccc_version == SMCCC_VERSION_1_0) {
-		ssbd_state = ARM64_SSBD_UNKNOWN;
-		if (!this_cpu_safe)
-			__ssb_safe = false;
-		return false;
-	}
-
-	switch (psci_ops.conduit) {
-	case PSCI_CONDUIT_HVC:
+	switch (arm_smccc_1_1_get_conduit()) {
+	case SMCCC_CONDUIT_HVC:
 		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 				  ARM_SMCCC_ARCH_WORKAROUND_2, &res);
 		break;
 
-	case PSCI_CONDUIT_SMC:
+	case SMCCC_CONDUIT_SMC:
 		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 				  ARM_SMCCC_ARCH_WORKAROUND_2, &res);
 		break;
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 3/6] arm: spectre-v2: use arm_smccc_1_1_get_conduit()

[Mark Rutland]

Now that we have arm_smccc_1_1_get_conduit(), we can hide the PSCI
implementation details from the arm spectre-v2 code, so let's do so.

As arm_smccc_1_1_get_conduit() implicitly checks that the SMCCC version
is at least SMCCC_VERSION_1_1, we no longer need to check this
explicitly where switch statements have a default case.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Russell King <linux@armlinux.org.uk>
---
 arch/arm/mm/proc-v7-bugs.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
index 9a07916af8dd..54d87506d3b5 100644
--- a/arch/arm/mm/proc-v7-bugs.c
+++ b/arch/arm/mm/proc-v7-bugs.c
@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <linux/arm-smccc.h>
 #include <linux/kernel.h>
-#include <linux/psci.h>
 #include <linux/smp.h>
 
 #include <asm/cp15.h>
@@ -75,11 +74,8 @@ static void cpu_v7_spectre_init(void)
 	case ARM_CPU_PART_CORTEX_A72: {
 		struct arm_smccc_res res;
 
-		if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
-			break;
-
-		switch (psci_ops.conduit) {
-		case PSCI_CONDUIT_HVC:
+		switch (arm_smccc_1_1_get_conduit()) {
+		case SMCCC_CONDUIT_HVC:
 			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
 			if ((int)res.a0 != 0)
@@ -90,7 +86,7 @@ static void cpu_v7_spectre_init(void)
 			spectre_v2_method = "hypervisor";
 			break;
 
-		case PSCI_CONDUIT_SMC:
+		case SMCCC_CONDUIT_SMC:
 			arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
 			if ((int)res.a0 != 0)
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 4/6] firmware/psci: use common SMCCC_CONDUIT_*

[Mark Rutland]

Now that we have common SMCCC_CONDUIT_* definitions, migrate the PSCI
code over to them, and kill off the old PSCI_CONDUIT_* definitions.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
---
 drivers/firmware/psci/psci.c | 25 +++++++++----------------
 include/linux/psci.h         |  9 ++-------
 2 files changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index 5f31f1bea1af..b8c07a8f8d5d 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -53,7 +53,7 @@ bool psci_tos_resident_on(int cpu)
 }
 
 struct psci_operations psci_ops = {
-	.conduit = PSCI_CONDUIT_NONE,
+	.conduit = SMCCC_CONDUIT_NONE,
 	.smccc_version = SMCCC_VERSION_1_0,
 };
 
@@ -62,14 +62,7 @@ enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void)
 	if (psci_ops.smccc_version < SMCCC_VERSION_1_1)
 		return SMCCC_CONDUIT_NONE;
 
-	switch (psci_ops.conduit) {
-	case PSCI_CONDUIT_SMC:
-		return SMCCC_CONDUIT_SMC;
-	case PSCI_CONDUIT_HVC:
-		return SMCCC_CONDUIT_HVC;
-	default:
-		return SMCCC_CONDUIT_NONE;
-	}
+	return psci_ops.conduit;
 }
 
 typedef unsigned long (psci_fn)(unsigned long, unsigned long,
@@ -227,13 +220,13 @@ static unsigned long psci_migrate_info_up_cpu(void)
 			      0, 0, 0);
 }
 
-static void set_conduit(enum psci_conduit conduit)
+static void set_conduit(enum arm_smccc_conduit conduit)
 {
 	switch (conduit) {
-	case PSCI_CONDUIT_HVC:
+	case SMCCC_CONDUIT_HVC:
 		invoke_psci_fn = __invoke_psci_fn_hvc;
 		break;
-	case PSCI_CONDUIT_SMC:
+	case SMCCC_CONDUIT_SMC:
 		invoke_psci_fn = __invoke_psci_fn_smc;
 		break;
 	default:
@@ -255,9 +248,9 @@ static int get_set_conduit_method(struct device_node *np)
 	}
 
 	if (!strcmp("hvc", method)) {
-		set_conduit(PSCI_CONDUIT_HVC);
+		set_conduit(SMCCC_CONDUIT_HVC);
 	} else if (!strcmp("smc", method)) {
-		set_conduit(PSCI_CONDUIT_SMC);
+		set_conduit(SMCCC_CONDUIT_SMC);
 	} else {
 		pr_warn("invalid \"method\" property: %s\n", method);
 		return -EINVAL;
@@ -749,9 +742,9 @@ int __init psci_acpi_init(void)
 	pr_info("probing for conduit method from ACPI.\n");
 
 	if (acpi_psci_use_hvc())
-		set_conduit(PSCI_CONDUIT_HVC);
+		set_conduit(SMCCC_CONDUIT_HVC);
 	else
-		set_conduit(PSCI_CONDUIT_SMC);
+		set_conduit(SMCCC_CONDUIT_SMC);
 
 	return psci_probe();
 }
diff --git a/include/linux/psci.h b/include/linux/psci.h
index a8a15613c157..def0b89cc05d 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -7,6 +7,7 @@
 #ifndef __LINUX_PSCI_H
 #define __LINUX_PSCI_H
 
+#include <linux/arm-smccc.h>
 #include <linux/init.h>
 #include <linux/types.h>
 
@@ -18,12 +19,6 @@ bool psci_tos_resident_on(int cpu);
 int psci_cpu_init_idle(unsigned int cpu);
 int psci_cpu_suspend_enter(unsigned long index);
 
-enum psci_conduit {
-	PSCI_CONDUIT_NONE,
-	PSCI_CONDUIT_SMC,
-	PSCI_CONDUIT_HVC,
-};
-
 enum smccc_version {
 	SMCCC_VERSION_1_0,
 	SMCCC_VERSION_1_1,
@@ -38,7 +33,7 @@ struct psci_operations {
 	int (*affinity_info)(unsigned long target_affinity,
 			unsigned long lowest_affinity_level);
 	int (*migrate_info_type)(void);
-	enum psci_conduit conduit;
+	enum arm_smccc_conduit conduit;
 	enum smccc_version smccc_version;
 };
 
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 5/6] firmware: arm_sdei: use common SMCCC_CONDUIT_*

[Mark Rutland]

Now that we have common definitions for SMCCC conduits, move the SDEI
code over to them, and remove the SDEI-specific definitions.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Acked-by: James Morse <james.morse@arm.com>
Acked-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/sdei.c    |  3 ++-
 drivers/firmware/arm_sdei.c | 12 ++++++------
 include/linux/arm_sdei.h    |  6 ------
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
index ea94cf8f9dc6..d6259dac62b6 100644
--- a/arch/arm64/kernel/sdei.c
+++ b/arch/arm64/kernel/sdei.c
@@ -2,6 +2,7 @@
 // Copyright (C) 2017 Arm Ltd.
 #define pr_fmt(fmt) "sdei: " fmt
 
+#include <linux/arm-smccc.h>
 #include <linux/arm_sdei.h>
 #include <linux/hardirq.h>
 #include <linux/irqflags.h>
@@ -161,7 +162,7 @@ unsigned long sdei_arch_get_entry_point(int conduit)
 			return 0;
 	}
 
-	sdei_exit_mode = (conduit == CONDUIT_HVC) ? SDEI_EXIT_HVC : SDEI_EXIT_SMC;
+	sdei_exit_mode = (conduit == SMCCC_CONDUIT_HVC) ? SDEI_EXIT_HVC : SDEI_EXIT_SMC;
 
 #ifdef CONFIG_UNMAP_KERNEL_AT_EL0
 	if (arm64_kernel_unmapped_at_el0()) {
diff --git a/drivers/firmware/arm_sdei.c b/drivers/firmware/arm_sdei.c
index 9cd70d1a5622..a479023fa036 100644
--- a/drivers/firmware/arm_sdei.c
+++ b/drivers/firmware/arm_sdei.c
@@ -967,29 +967,29 @@ static int sdei_get_conduit(struct platform_device *pdev)
 	if (np) {
 		if (of_property_read_string(np, "method", &method)) {
 			pr_warn("missing \"method\" property\n");
-			return CONDUIT_INVALID;
+			return SMCCC_CONDUIT_NONE;
 		}
 
 		if (!strcmp("hvc", method)) {
 			sdei_firmware_call = &sdei_smccc_hvc;
-			return CONDUIT_HVC;
+			return SMCCC_CONDUIT_HVC;
 		} else if (!strcmp("smc", method)) {
 			sdei_firmware_call = &sdei_smccc_smc;
-			return CONDUIT_SMC;
+			return SMCCC_CONDUIT_SMC;
 		}
 
 		pr_warn("invalid \"method\" property: %s\n", method);
 	} else if (IS_ENABLED(CONFIG_ACPI) && !acpi_disabled) {
 		if (acpi_psci_use_hvc()) {
 			sdei_firmware_call = &sdei_smccc_hvc;
-			return CONDUIT_HVC;
+			return SMCCC_CONDUIT_HVC;
 		} else {
 			sdei_firmware_call = &sdei_smccc_smc;
-			return CONDUIT_SMC;
+			return SMCCC_CONDUIT_SMC;
 		}
 	}
 
-	return CONDUIT_INVALID;
+	return SMCCC_CONDUIT_NONE;
 }
 
 static int sdei_probe(struct platform_device *pdev)
diff --git a/include/linux/arm_sdei.h b/include/linux/arm_sdei.h
index 3305ea7f9dc7..0a241c5c911d 100644
--- a/include/linux/arm_sdei.h
+++ b/include/linux/arm_sdei.h
@@ -5,12 +5,6 @@
 
 #include <uapi/linux/arm_sdei.h>
 
-enum sdei_conduit_types {
-	CONDUIT_INVALID = 0,
-	CONDUIT_SMC,
-	CONDUIT_HVC,
-};
-
 #include <acpi/ghes.h>
 
 #ifdef CONFIG_ARM_SDE_INTERFACE
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv3 6/6] smccc: make 1.1 macros value-returning

[Mark Rutland]

The arm_smccc_1_1_{smc,hvc}() macros for inline invocation take a res
pointer as their final argument, matching the out-of-line SMCCC
invocation functions.

However, the inline invocation macros are variadic, so it's easy to mess
up passsing the correct parameters.

Instead, let's make them value-returning, which is less confusing.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mm/proc-v7-bugs.c     | 12 +++++------
 arch/arm64/kernel/cpu_errata.c | 24 ++++++++++-----------
 arch/arm64/kvm/hyp/switch.c    |  4 ++--
 include/linux/arm-smccc.h      | 49 +++++++++++++++++++-----------------------
 4 files changed, 42 insertions(+), 47 deletions(-)

diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
index 54d87506d3b5..6ceee88b1ffc 100644
--- a/arch/arm/mm/proc-v7-bugs.c
+++ b/arch/arm/mm/proc-v7-bugs.c
@@ -28,12 +28,12 @@ static void harden_branch_predictor_iciallu(void)
 
 static void __maybe_unused call_smc_arch_workaround_1(void)
 {
-	arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+	arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1);
 }
 
 static void __maybe_unused call_hvc_arch_workaround_1(void)
 {
-	arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+	arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1);
 }
 
 static void cpu_v7_spectre_init(void)
@@ -76,8 +76,8 @@ static void cpu_v7_spectre_init(void)
 
 		switch (arm_smccc_1_1_get_conduit()) {
 		case SMCCC_CONDUIT_HVC:
-			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
-					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			res = arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+						ARM_SMCCC_ARCH_WORKAROUND_1);
 			if ((int)res.a0 != 0)
 				break;
 			per_cpu(harden_branch_predictor_fn, cpu) =
@@ -87,8 +87,8 @@ static void cpu_v7_spectre_init(void)
 			break;
 
 		case SMCCC_CONDUIT_SMC:
-			arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
-					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			res = arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+						ARM_SMCCC_ARCH_WORKAROUND_1);
 			if ((int)res.a0 != 0)
 				break;
 			per_cpu(harden_branch_predictor_fn, cpu) =
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 6ee09bca82f8..8d4e7d3514b4 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -169,12 +169,12 @@ static void install_bp_hardening_cb(bp_hardening_cb_t fn,
 
 static void call_smc_arch_workaround_1(void)
 {
-	arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+	arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1);
 }
 
 static void call_hvc_arch_workaround_1(void)
 {
-	arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+	arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1);
 }
 
 static void qcom_link_stack_sanitization(void)
@@ -211,8 +211,8 @@ static int detect_harden_bp_fw(void)
 
 	switch (arm_smccc_1_1_get_conduit()) {
 	case SMCCC_CONDUIT_HVC:
-		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
-				  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+		res = arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					ARM_SMCCC_ARCH_WORKAROUND_1);
 		switch ((int)res.a0) {
 		case 1:
 			/* Firmware says we're just fine */
@@ -229,8 +229,8 @@ static int detect_harden_bp_fw(void)
 		break;
 
 	case SMCCC_CONDUIT_SMC:
-		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
-				  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+		res = arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					ARM_SMCCC_ARCH_WORKAROUND_1);
 		switch ((int)res.a0) {
 		case 1:
 			/* Firmware says we're just fine */
@@ -347,11 +347,11 @@ void arm64_set_ssbd_mitigation(bool state)
 
 	switch (arm_smccc_1_1_get_conduit()) {
 	case SMCCC_CONDUIT_HVC:
-		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
+		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_2, state);
 		break;
 
 	case SMCCC_CONDUIT_SMC:
-		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
+		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2, state);
 		break;
 
 	default:
@@ -386,13 +386,13 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
 
 	switch (arm_smccc_1_1_get_conduit()) {
 	case SMCCC_CONDUIT_HVC:
-		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
-				  ARM_SMCCC_ARCH_WORKAROUND_2, &res);
+		res = arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					ARM_SMCCC_ARCH_WORKAROUND_2);
 		break;
 
 	case SMCCC_CONDUIT_SMC:
-		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
-				  ARM_SMCCC_ARCH_WORKAROUND_2, &res);
+		res = arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					ARM_SMCCC_ARCH_WORKAROUND_2);
 		break;
 
 	default:
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index adaf266d8de8..ac8351a57c29 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -479,7 +479,7 @@ static void __hyp_text __set_guest_arch_workaround_state(struct kvm_vcpu *vcpu)
 	 */
 	if (__needs_ssbd_off(vcpu) &&
 	    __hyp_this_cpu_read(arm64_ssbd_callback_required))
-		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2, 0, NULL);
+		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2, 0);
 #endif
 }
 
@@ -491,7 +491,7 @@ static void __hyp_text __set_host_arch_workaround_state(struct kvm_vcpu *vcpu)
 	 */
 	if (__needs_ssbd_off(vcpu) &&
 	    __hyp_this_cpu_read(arm64_ssbd_callback_required))
-		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2, 1, NULL);
+		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2, 1);
 #endif
 }
 
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index df01a8579034..1e0bcb292d17 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -177,7 +177,7 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
 
 #endif
 
-#define ___count_args(_0, _1, _2, _3, _4, _5, _6, _7, _8, x, ...) x
+#define ___count_args(_0, _1, _2, _3, _4, _5, _6, _7, x, ...) x
 
 #define __count_args(...)						\
 	___count_args(__VA_ARGS__, 7, 6, 5, 4, 3, 2, 1, 0)
@@ -204,58 +204,54 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
 #define __constraint_read_6	__constraint_read_5, "r" (r6)
 #define __constraint_read_7	__constraint_read_6, "r" (r7)
 
-#define __declare_arg_0(a0, res)					\
-	struct arm_smccc_res   *___res = res;				\
+#define __declare_arg_0(a0)						\
 	register unsigned long r0 asm("r0") = (u32)a0;			\
 	register unsigned long r1 asm("r1");				\
 	register unsigned long r2 asm("r2");				\
 	register unsigned long r3 asm("r3")
 
-#define __declare_arg_1(a0, a1, res)					\
+#define __declare_arg_1(a0, a1)						\
 	typeof(a1) __a1 = a1;						\
-	struct arm_smccc_res   *___res = res;				\
 	register unsigned long r0 asm("r0") = (u32)a0;			\
 	register unsigned long r1 asm("r1") = __a1;			\
 	register unsigned long r2 asm("r2");				\
 	register unsigned long r3 asm("r3")
 
-#define __declare_arg_2(a0, a1, a2, res)				\
+#define __declare_arg_2(a0, a1, a2)					\
 	typeof(a1) __a1 = a1;						\
 	typeof(a2) __a2 = a2;						\
-	struct arm_smccc_res   *___res = res;				\
 	register unsigned long r0 asm("r0") = (u32)a0;			\
 	register unsigned long r1 asm("r1") = __a1;			\
 	register unsigned long r2 asm("r2") = __a2;			\
 	register unsigned long r3 asm("r3")
 
-#define __declare_arg_3(a0, a1, a2, a3, res)				\
+#define __declare_arg_3(a0, a1, a2, a3)					\
 	typeof(a1) __a1 = a1;						\
 	typeof(a2) __a2 = a2;						\
 	typeof(a3) __a3 = a3;						\
-	struct arm_smccc_res   *___res = res;				\
 	register unsigned long r0 asm("r0") = (u32)a0;			\
 	register unsigned long r1 asm("r1") = __a1;			\
 	register unsigned long r2 asm("r2") = __a2;			\
 	register unsigned long r3 asm("r3") = __a3
 
-#define __declare_arg_4(a0, a1, a2, a3, a4, res)			\
+#define __declare_arg_4(a0, a1, a2, a3, a4)				\
 	typeof(a4) __a4 = a4;						\
-	__declare_arg_3(a0, a1, a2, a3, res);				\
+	__declare_arg_3(a0, a1, a2, a3);				\
 	register unsigned long r4 asm("r4") = __a4
 
-#define __declare_arg_5(a0, a1, a2, a3, a4, a5, res)			\
+#define __declare_arg_5(a0, a1, a2, a3, a4, a5)				\
 	typeof(a5) __a5 = a5;						\
-	__declare_arg_4(a0, a1, a2, a3, a4, res);			\
+	__declare_arg_4(a0, a1, a2, a3, a4);				\
 	register unsigned long r5 asm("r5") = __a5
 
-#define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res)		\
+#define __declare_arg_6(a0, a1, a2, a3, a4, a5, a6)			\
 	typeof(a6) __a6 = a6;						\
-	__declare_arg_5(a0, a1, a2, a3, a4, a5, res);			\
+	__declare_arg_5(a0, a1, a2, a3, a4, a5);			\
 	register unsigned long r6 asm("r6") = __a6
 
-#define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7, res)		\
+#define __declare_arg_7(a0, a1, a2, a3, a4, a5, a6, a7)			\
 	typeof(a7) __a7 = a7;						\
-	__declare_arg_6(a0, a1, a2, a3, a4, a5, a6, res);		\
+	__declare_arg_6(a0, a1, a2, a3, a4, a5, a6);			\
 	register unsigned long r7 asm("r7") = __a7
 
 #define ___declare_args(count, ...) __declare_arg_ ## count(__VA_ARGS__)
@@ -273,22 +269,21 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
  * makes it stick.
  */
 #define __arm_smccc_1_1(inst, ...)					\
-	do {								\
+	({								\
 		__declare_args(__count_args(__VA_ARGS__), __VA_ARGS__);	\
 		asm volatile(inst "\n"					\
 			     __constraints(__count_args(__VA_ARGS__)));	\
-		if (___res)						\
-			*___res = (typeof(*___res)){r0, r1, r2, r3};	\
-	} while (0)
+		(struct arm_smccc_res){ r0, r1, r2, r3 };		\
+	})
 
 /*
  * arm_smccc_1_1_smc() - make an SMCCC v1.1 compliant SMC call
  *
- * This is a variadic macro taking one to eight source arguments, and
- * an optional return structure.
+ * This is a variadic macro taking one to eight source arguments.
  *
  * @a0-a7: arguments passed in registers 0 to 7
- * @res: result values from registers 0 to 3
+ *
+ * returns result values from registers 0 to 3
  *
  * This macro is used to make SMC calls following SMC Calling Convention v1.1.
  * The content of the supplied param are copied to registers 0 to 7 prior
@@ -300,11 +295,11 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
 /*
  * arm_smccc_1_1_hvc() - make an SMCCC v1.1 compliant HVC call
  *
- * This is a variadic macro taking one to eight source arguments, and
- * an optional return structure.
+ * This is a variadic macro taking one to eight source arguments.
  *
  * @a0-a7: arguments passed in registers 0 to 7
- * @res: result values from registers 0 to 3
+ *
+ * returns result values from registers 0 to 3
  *
  * This macro is used to make HVC calls following SMC Calling Convention v1.1.
  * The content of the supplied param are copied to registers 0 to 7 prior
-- 
2.11.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 1/6] arm/arm64: smccc/psci: add arm_smccc_1_1_get_conduit()

[Dave Martin]

On Fri, Aug 09, 2019 at 02:22:40PM +0100, Mark Rutland wrote:
> SMCCC callers are currently amassing a collection of enums for the SMCCC
> conduit, and are having to dig into the PSCI driver's internals in order
> to figure out what to do.
> 
> Let's clean this up, with common SMCCC_CONDUIT_* definitions, and an
> arm_smccc_1_1_get_conduit() helper that abstracts the PSCI driver's
> internal state.
> 
> We can kill off the PSCI_CONDUIT_* definitions once we've migrated users
> over to the new interface.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Acked-by: Will Deacon <will.deacon@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  drivers/firmware/psci/psci.c | 15 +++++++++++++++
>  include/linux/arm-smccc.h    | 16 ++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> index f82ccd39a913..5f31f1bea1af 100644
> --- a/drivers/firmware/psci/psci.c
> +++ b/drivers/firmware/psci/psci.c
> @@ -57,6 +57,21 @@ struct psci_operations psci_ops = {
>  	.smccc_version = SMCCC_VERSION_1_0,
>  };
>  
> +enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void)

Do we expect this to be specific to SMCCC v1.1?

> +{
> +	if (psci_ops.smccc_version < SMCCC_VERSION_1_1)
> +		return SMCCC_CONDUIT_NONE;
> +
> +	switch (psci_ops.conduit) {
> +	case PSCI_CONDUIT_SMC:
> +		return SMCCC_CONDUIT_SMC;
> +	case PSCI_CONDUIT_HVC:
> +		return SMCCC_CONDUIT_HVC;
> +	default:
> +		return SMCCC_CONDUIT_NONE;
> +	}
> +}
> +
>  typedef unsigned long (psci_fn)(unsigned long, unsigned long,
>  				unsigned long, unsigned long);
>  static psci_fn *invoke_psci_fn;
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 080012a6f025..df01a8579034 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -80,6 +80,22 @@
>  
>  #include <linux/linkage.h>
>  #include <linux/types.h>
> +
> +enum arm_smccc_conduit {
> +	SMCCC_CONDUIT_NONE,

If this is intended to have the value 0, is it worth making that
explicit?  I can never remember whether enums start at 1 or 0 by
default...

> +	SMCCC_CONDUIT_SMC,
> +	SMCCC_CONDUIT_HVC,
> +};

[...]

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 1/6] arm/arm64: smccc/psci: add arm_smccc_1_1_get_conduit()

[Mark Rutland]

On Mon, Aug 12, 2019 at 04:03:29PM +0100, Dave Martin wrote:
> On Fri, Aug 09, 2019 at 02:22:40PM +0100, Mark Rutland wrote:
> > SMCCC callers are currently amassing a collection of enums for the SMCCC
> > conduit, and are having to dig into the PSCI driver's internals in order
> > to figure out what to do.
> > 
> > Let's clean this up, with common SMCCC_CONDUIT_* definitions, and an
> > arm_smccc_1_1_get_conduit() helper that abstracts the PSCI driver's
> > internal state.
> > 
> > We can kill off the PSCI_CONDUIT_* definitions once we've migrated users
> > over to the new interface.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Acked-by: Will Deacon <will.deacon@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> >  drivers/firmware/psci/psci.c | 15 +++++++++++++++
> >  include/linux/arm-smccc.h    | 16 ++++++++++++++++
> >  2 files changed, 31 insertions(+)
> > 
> > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > index f82ccd39a913..5f31f1bea1af 100644
> > --- a/drivers/firmware/psci/psci.c
> > +++ b/drivers/firmware/psci/psci.c
> > @@ -57,6 +57,21 @@ struct psci_operations psci_ops = {
> >  	.smccc_version = SMCCC_VERSION_1_0,
> >  };
> >  
> > +enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void)
> 
> Do we expect this to be specific to SMCCC v1.1?

I intend it to be 1.1+

> > +{
> > +	if (psci_ops.smccc_version < SMCCC_VERSION_1_1)
> > +		return SMCCC_CONDUIT_NONE;
> > +
> > +	switch (psci_ops.conduit) {
> > +	case PSCI_CONDUIT_SMC:
> > +		return SMCCC_CONDUIT_SMC;
> > +	case PSCI_CONDUIT_HVC:
> > +		return SMCCC_CONDUIT_HVC;
> > +	default:
> > +		return SMCCC_CONDUIT_NONE;
> > +	}
> > +}
> > +
> >  typedef unsigned long (psci_fn)(unsigned long, unsigned long,
> >  				unsigned long, unsigned long);
> >  static psci_fn *invoke_psci_fn;
> > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> > index 080012a6f025..df01a8579034 100644
> > --- a/include/linux/arm-smccc.h
> > +++ b/include/linux/arm-smccc.h
> > @@ -80,6 +80,22 @@
> >  
> >  #include <linux/linkage.h>
> >  #include <linux/types.h>
> > +
> > +enum arm_smccc_conduit {
> > +	SMCCC_CONDUIT_NONE,
> 
> If this is intended to have the value 0, is it worth making that
> explicit?  I can never remember whether enums start at 1 or 0 by
> default...

They start at 0. I intend that checks are done explicitly against an
enum value, so I'm not sure that matters.

Thanks,
Mark.

> 
> > +	SMCCC_CONDUIT_SMC,
> > +	SMCCC_CONDUIT_HVC,
> > +};
> 
> [...]
> 
> Cheers
> ---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 1/6] arm/arm64: smccc/psci: add arm_smccc_1_1_get_conduit()

[Dave Martin]

On Mon, Aug 12, 2019 at 04:06:35PM +0100, Mark Rutland wrote:
> On Mon, Aug 12, 2019 at 04:03:29PM +0100, Dave Martin wrote:
> > On Fri, Aug 09, 2019 at 02:22:40PM +0100, Mark Rutland wrote:
> > > SMCCC callers are currently amassing a collection of enums for the SMCCC
> > > conduit, and are having to dig into the PSCI driver's internals in order
> > > to figure out what to do.
> > > 
> > > Let's clean this up, with common SMCCC_CONDUIT_* definitions, and an
> > > arm_smccc_1_1_get_conduit() helper that abstracts the PSCI driver's
> > > internal state.
> > > 
> > > We can kill off the PSCI_CONDUIT_* definitions once we've migrated users
> > > over to the new interface.
> > > 
> > > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > > Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Acked-by: Will Deacon <will.deacon@arm.com>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > ---
> > >  drivers/firmware/psci/psci.c | 15 +++++++++++++++
> > >  include/linux/arm-smccc.h    | 16 ++++++++++++++++
> > >  2 files changed, 31 insertions(+)
> > > 
> > > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
> > > index f82ccd39a913..5f31f1bea1af 100644
> > > --- a/drivers/firmware/psci/psci.c
> > > +++ b/drivers/firmware/psci/psci.c
> > > @@ -57,6 +57,21 @@ struct psci_operations psci_ops = {
> > >  	.smccc_version = SMCCC_VERSION_1_0,
> > >  };
> > >  
> > > +enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void)
> > 
> > Do we expect this to be specific to SMCCC v1.1?
> 
> I intend it to be 1.1+

It seems overspecific, but I guess we can address this later if it
becomes an issue.  This is an internal API for now (at worst I might
envisage it being EXPORT_SYMBOL_GPL()).

> > > +{
> > > +	if (psci_ops.smccc_version < SMCCC_VERSION_1_1)
> > > +		return SMCCC_CONDUIT_NONE;
> > > +
> > > +	switch (psci_ops.conduit) {
> > > +	case PSCI_CONDUIT_SMC:
> > > +		return SMCCC_CONDUIT_SMC;
> > > +	case PSCI_CONDUIT_HVC:
> > > +		return SMCCC_CONDUIT_HVC;
> > > +	default:
> > > +		return SMCCC_CONDUIT_NONE;
> > > +	}
> > > +}
> > > +
> > >  typedef unsigned long (psci_fn)(unsigned long, unsigned long,
> > >  				unsigned long, unsigned long);
> > >  static psci_fn *invoke_psci_fn;
> > > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> > > index 080012a6f025..df01a8579034 100644
> > > --- a/include/linux/arm-smccc.h
> > > +++ b/include/linux/arm-smccc.h
> > > @@ -80,6 +80,22 @@
> > >  
> > >  #include <linux/linkage.h>
> > >  #include <linux/types.h>
> > > +
> > > +enum arm_smccc_conduit {
> > > +	SMCCC_CONDUIT_NONE,
> > 
> > If this is intended to have the value 0, is it worth making that
> > explicit?  I can never remember whether enums start at 1 or 0 by
> > default...
> 
> They start at 0. I intend that checks are done explicitly against an
> enum value, so I'm not sure that matters.

Not really.

It depends whether code like if (!arm_smccc_1_1_get_conduit()) { ... }
is considered sane or not.

If we don't think people should be doing this, omitting the explicit
value specifier seems fine.

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 1/6] arm/arm64: smccc/psci: add arm_smccc_1_1_get_conduit()

[Mark Rutland]

On Mon, Aug 12, 2019 at 04:10:43PM +0100, Dave Martin wrote:
> On Mon, Aug 12, 2019 at 04:06:35PM +0100, Mark Rutland wrote:
> > On Mon, Aug 12, 2019 at 04:03:29PM +0100, Dave Martin wrote:
> > > On Fri, Aug 09, 2019 at 02:22:40PM +0100, Mark Rutland wrote:
> > > > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> > > > index 080012a6f025..df01a8579034 100644
> > > > --- a/include/linux/arm-smccc.h
> > > > +++ b/include/linux/arm-smccc.h
> > > > @@ -80,6 +80,22 @@
> > > >  
> > > >  #include <linux/linkage.h>
> > > >  #include <linux/types.h>
> > > > +
> > > > +enum arm_smccc_conduit {
> > > > +	SMCCC_CONDUIT_NONE,
> > > 
> > > If this is intended to have the value 0, is it worth making that
> > > explicit?  I can never remember whether enums start at 1 or 0 by
> > > default...
> > 
> > They start at 0. I intend that checks are done explicitly against an
> > enum value, so I'm not sure that matters.
> 
> Not really.
> 
> It depends whether code like if (!arm_smccc_1_1_get_conduit()) { ... }
> is considered sane or not.
> 
> If we don't think people should be doing this, omitting the explicit
> value specifier seems fine.

My expectation was that they'd check explicitly against
SMCCC_CONDUIT_NONE, since all of the existing callers care about the
specific conduit for other reasons (e.g. patching).

I also expect to wrap this in a sbusequent patch that provides helpers:

* arm_smccc_1_1_available()
* arm_smccc_1_1_call(...)

... for the cases where we just want to make a call and don't care about
the specific conduit.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 1/6] arm/arm64: smccc/psci: add arm_smccc_1_1_get_conduit()

[Dave Martin]

On Mon, Aug 12, 2019 at 04:26:49PM +0100, Mark Rutland wrote:
> On Mon, Aug 12, 2019 at 04:10:43PM +0100, Dave Martin wrote:
> > On Mon, Aug 12, 2019 at 04:06:35PM +0100, Mark Rutland wrote:
> > > On Mon, Aug 12, 2019 at 04:03:29PM +0100, Dave Martin wrote:
> > > > On Fri, Aug 09, 2019 at 02:22:40PM +0100, Mark Rutland wrote:
> > > > > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> > > > > index 080012a6f025..df01a8579034 100644
> > > > > --- a/include/linux/arm-smccc.h
> > > > > +++ b/include/linux/arm-smccc.h
> > > > > @@ -80,6 +80,22 @@
> > > > >  
> > > > >  #include <linux/linkage.h>
> > > > >  #include <linux/types.h>
> > > > > +
> > > > > +enum arm_smccc_conduit {
> > > > > +	SMCCC_CONDUIT_NONE,
> > > > 
> > > > If this is intended to have the value 0, is it worth making that
> > > > explicit?  I can never remember whether enums start at 1 or 0 by
> > > > default...
> > > 
> > > They start at 0. I intend that checks are done explicitly against an
> > > enum value, so I'm not sure that matters.
> > 
> > Not really.
> > 
> > It depends whether code like if (!arm_smccc_1_1_get_conduit()) { ... }
> > is considered sane or not.
> > 
> > If we don't think people should be doing this, omitting the explicit
> > value specifier seems fine.
> 
> My expectation was that they'd check explicitly against
> SMCCC_CONDUIT_NONE, since all of the existing callers care about the
> specific conduit for other reasons (e.g. patching).
> 
> I also expect to wrap this in a sbusequent patch that provides helpers:
> 
> * arm_smccc_1_1_available()
> * arm_smccc_1_1_call(...)
> 
> ... for the cases where we just want to make a call and don't care about
> the specific conduit.

Seems reasonable.

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 6/6] smccc: make 1.1 macros value-returning

[Will Deacon]

On Fri, Aug 09, 2019 at 02:22:45PM +0100, Mark Rutland wrote:
> The arm_smccc_1_1_{smc,hvc}() macros for inline invocation take a res
> pointer as their final argument, matching the out-of-line SMCCC
> invocation functions.
> 
> However, the inline invocation macros are variadic, so it's easy to mess
> up passsing the correct parameters.

passing

> Instead, let's make them value-returning, which is less confusing.

I'm not completely sure I agree with you here because, as far as I can
tell, it means that we have a different calling convention for 1.0 (i.e.
arm_smccc_smc()) and 1.1 (i.e. arm_smccc_1_1_smc).

Can we do the same for 1.0 as well or am I missing something?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 6/6] smccc: make 1.1 macros value-returning

[Mark Rutland]

On Thu, Aug 15, 2019 at 05:42:44PM +0100, Will Deacon wrote:
> On Fri, Aug 09, 2019 at 02:22:45PM +0100, Mark Rutland wrote:
> > The arm_smccc_1_1_{smc,hvc}() macros for inline invocation take a res
> > pointer as their final argument, matching the out-of-line SMCCC
> > invocation functions.
> > 
> > However, the inline invocation macros are variadic, so it's easy to mess
> > up passsing the correct parameters.
> 
> passing
> 
> > Instead, let's make them value-returning, which is less confusing.
> 
> I'm not completely sure I agree with you here because, as far as I can
> tell, it means that we have a different calling convention for 1.0 (i.e.
> arm_smccc_smc()) and 1.1 (i.e. arm_smccc_1_1_smc).
> 
> Can we do the same for 1.0 as well or am I missing something?

I will take a look; I think the QC quirk is the only thing that gets in
the way.

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCHv3 3/6] arm: spectre-v2: use arm_smccc_1_1_get_conduit()

[Catalin Marinas]

Hi Russell,

Are you ok with the arm patch below to go in via the arm64 tree? I'd
like patches 1-5 from [1] to go into 5.5 but patches 4, 5 depend on
patch 3 also being queued.

If you'd rather merge it separately, I can provide a stable branch with
patch 1 so that you can merge it and apply patch 3 on top (or any other
option that works for you).

Thanks,

Catalin

[1] http://lkml.kernel.org/r/20190809132245.43505-1-mark.rutland@arm.com

On Fri, Aug 09, 2019 at 02:22:42PM +0100, Mark Rutland wrote:
> Now that we have arm_smccc_1_1_get_conduit(), we can hide the PSCI
> implementation details from the arm spectre-v2 code, so let's do so.
> 
> As arm_smccc_1_1_get_conduit() implicitly checks that the SMCCC version
> is at least SMCCC_VERSION_1_1, we no longer need to check this
> explicitly where switch statements have a default case.
> 
> There should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Russell King <linux@armlinux.org.uk>
> ---
>  arch/arm/mm/proc-v7-bugs.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
> index 9a07916af8dd..54d87506d3b5 100644
> --- a/arch/arm/mm/proc-v7-bugs.c
> +++ b/arch/arm/mm/proc-v7-bugs.c
> @@ -1,7 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include <linux/arm-smccc.h>
>  #include <linux/kernel.h>
> -#include <linux/psci.h>
>  #include <linux/smp.h>
>  
>  #include <asm/cp15.h>
> @@ -75,11 +74,8 @@ static void cpu_v7_spectre_init(void)
>  	case ARM_CPU_PART_CORTEX_A72: {
>  		struct arm_smccc_res res;
>  
> -		if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
> -			break;
> -
> -		switch (psci_ops.conduit) {
> -		case PSCI_CONDUIT_HVC:
> +		switch (arm_smccc_1_1_get_conduit()) {
> +		case SMCCC_CONDUIT_HVC:
>  			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
>  					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
>  			if ((int)res.a0 != 0)
> @@ -90,7 +86,7 @@ static void cpu_v7_spectre_init(void)
>  			spectre_v2_method = "hypervisor";
>  			break;
>  
> -		case PSCI_CONDUIT_SMC:
> +		case SMCCC_CONDUIT_SMC:
>  			arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
>  					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
>  			if ((int)res.a0 != 0)
> -- 
> 2.11.0

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel