Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 07/14] arm64: ssbd: Skip apply_ssbd if not using dynamic mitigation
From: Marc Zyngier @ 2018-05-22 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-1-marc.zyngier@arm.com>

In order to avoid checking arm64_ssbd_callback_required on each
kernel entry/exit even if no mitigation is required, let's
add yet another alternative that by default jumps over the mitigation,
and that gets nop'ed out if we're doing dynamic mitigation.

Think of it as a poor man's static key...

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kernel/cpu_errata.c | 14 ++++++++++++++
 arch/arm64/kernel/entry.S      |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index f1d4e75b0ddd..8f686f39b9c1 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -283,6 +283,20 @@ void __init arm64_update_smccc_conduit(struct alt_instr *alt,
 	*updptr = cpu_to_le32(insn);
 }
 
+void __init arm64_enable_wa2_handling(struct alt_instr *alt,
+				      __le32 *origptr, __le32 *updptr,
+				      int nr_inst)
+{
+	BUG_ON(nr_inst != 1);
+	/*
+	 * Only allow mitigation on EL1 entry/exit and guest
+	 * ARCH_WORKAROUND_2 handling if the SSBD state allows it to
+	 * be flipped.
+	 */
+	if (arm64_get_ssbd_state() == ARM64_SSBD_EL1_ENTRY)
+		*updptr = cpu_to_le32(aarch64_insn_gen_nop());
+}
+
 static void do_ssbd(bool state)
 {
 	switch (psci_ops.conduit) {
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 29ad672a6abd..e6f6e2339b22 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -142,6 +142,9 @@ alternative_else_nop_endif
 	// to save/restore them if required.
 	.macro	apply_ssbd, state, targ, tmp1, tmp2
 #ifdef CONFIG_ARM64_SSBD
+alternative_cb	arm64_enable_wa2_handling
+	b	\targ
+alternative_cb_end
 	ldr_this_cpu	\tmp2, arm64_ssbd_callback_required, \tmp1
 	cbz	\tmp2, \targ
 	mov	w0, #ARM_SMCCC_ARCH_WORKAROUND_2
-- 
2.14.2

^ permalink raw reply related

* [PATCH 06/14] arm64: ssbd: Add global mitigation state accessor
From: Marc Zyngier @ 2018-05-22 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-1-marc.zyngier@arm.com>

We're about to need the mitigation state in various parts of the
kernel in order to do the right thing for userspace and guests.

Let's expose an accessor that will let other subsystems know
about the state.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 9bc548e22784..1bacdf57f0af 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -543,6 +543,16 @@ static inline u64 read_zcr_features(void)
 #define ARM64_SSBD_FORCE_ENABLE		2
 #define ARM64_SSBD_MITIGATED		3
 
+static inline int arm64_get_ssbd_state(void)
+{
+#ifdef CONFIG_ARM64_SSBD
+	extern int ssbd_state;
+	return ssbd_state;
+#else
+	return ARM64_SSBD_UNKNOWN;
+#endif
+}
+
 #endif /* __ASSEMBLY__ */
 
 #endif
-- 
2.14.2

^ permalink raw reply related

* [PATCH 05/14] arm64: Add 'ssbd' command-line option
From: Marc Zyngier @ 2018-05-22 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-1-marc.zyngier@arm.com>

On a system where the firmware implements ARCH_WORKAROUND_2,
it may be useful to either permanently enable or disable the
workaround for cases where the user decides that they'd rather
not get a trap overhead, and keep the mitigation permanently
on or off instead of switching it on exception entry/exit.

In any case, default to the mitigation being enabled.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  17 ++++
 arch/arm64/include/asm/cpufeature.h             |   6 ++
 arch/arm64/kernel/cpu_errata.c                  | 102 ++++++++++++++++++++----
 3 files changed, 109 insertions(+), 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f2040d46f095..646e112c6f63 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4092,6 +4092,23 @@
 			expediting.  Set to zero to disable automatic
 			expediting.
 
+	ssbd=		[ARM64,HW]
+			Speculative Store Bypass Disable control
+
+			On CPUs that are vulnerable to the Speculative
+			Store Bypass vulnerability and offer a
+			firmware based mitigation, this parameter
+			indicates how the mitigation should be used:
+
+			force-on:  Unconditionnaly enable mitigation for
+				   for both kernel and userspace
+			force-off: Unconditionnaly disable mitigation for
+				   for both kernel and userspace
+			kernel:    Always enable mitigation in the
+				   kernel, and offer a prctl interface
+				   to allow userspace to register its
+				   interest in being mitigated too.
+
 	stack_guard_gap=	[MM]
 			override the default stack gap protection. The value
 			is in page units and it defines how many pages prior
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 09b0f2a80c8f..9bc548e22784 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -537,6 +537,12 @@ static inline u64 read_zcr_features(void)
 	return zcr;
 }
 
+#define ARM64_SSBD_UNKNOWN		-1
+#define ARM64_SSBD_FORCE_DISABLE	0
+#define ARM64_SSBD_EL1_ENTRY		1
+#define ARM64_SSBD_FORCE_ENABLE		2
+#define ARM64_SSBD_MITIGATED		3
+
 #endif /* __ASSEMBLY__ */
 
 #endif
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 7fd6d5b001f5..f1d4e75b0ddd 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -235,6 +235,38 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
 #ifdef CONFIG_ARM64_SSBD
 DEFINE_PER_CPU_READ_MOSTLY(u64, arm64_ssbd_callback_required);
 
+int ssbd_state __read_mostly = ARM64_SSBD_EL1_ENTRY;
+
+static const struct ssbd_options {
+	const char	*str;
+	int		state;
+} ssbd_options[] = {
+	{ "force-on",	ARM64_SSBD_FORCE_ENABLE, },
+	{ "force-off",	ARM64_SSBD_FORCE_DISABLE, },
+	{ "kernel",	ARM64_SSBD_EL1_ENTRY, },
+};
+
+static int __init ssbd_cfg(char *buf)
+{
+	int i;
+
+	if (!buf || !buf[0])
+		return -EINVAL;
+
+	for (i = 0; i < ARRAY_SIZE(ssbd_options); i++) {
+		int len = strlen(ssbd_options[i].str);
+
+		if (strncmp(buf, ssbd_options[i].str, len))
+			continue;
+
+		ssbd_state = ssbd_options[i].state;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+early_param("ssbd", ssbd_cfg);
+
 void __init arm64_update_smccc_conduit(struct alt_instr *alt,
 				       __le32 *origptr, __le32 *updptr,
 				       int nr_inst)
@@ -272,44 +304,82 @@ static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
 				    int scope)
 {
 	struct arm_smccc_res res;
-	bool supported = true;
+	bool required = true;
+	s32 val;
 
 	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
 
-	if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
+	if (psci_ops.smccc_version == SMCCC_VERSION_1_0) {
+		ssbd_state = ARM64_SSBD_UNKNOWN;
 		return false;
+	}
 
-	/*
-	 * The probe function return value is either negative
-	 * (unsupported or mitigated), positive (unaffected), or zero
-	 * (requires mitigation). We only need to do anything in the
-	 * last case.
-	 */
 	switch (psci_ops.conduit) {
 	case PSCI_CONDUIT_HVC:
 		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 				  ARM_SMCCC_ARCH_WORKAROUND_2, &res);
-		if ((int)res.a0 != 0)
-			supported = false;
 		break;
 
 	case PSCI_CONDUIT_SMC:
 		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
 				  ARM_SMCCC_ARCH_WORKAROUND_2, &res);
-		if ((int)res.a0 != 0)
-			supported = false;
 		break;
 
 	default:
-		supported = false;
+		ssbd_state = ARM64_SSBD_UNKNOWN;
+		return false;
 	}
 
-	if (supported) {
-		__this_cpu_write(arm64_ssbd_callback_required, 1);
+	val = (s32)res.a0;
+
+	switch (val) {
+	case SMCCC_RET_NOT_SUPPORTED:
+		ssbd_state = ARM64_SSBD_UNKNOWN;
+		return false;
+
+	case SMCCC_RET_NOT_REQUIRED:
+		ssbd_state = ARM64_SSBD_MITIGATED;
+		return false;
+
+	case SMCCC_RET_SUCCESS:
+		required = true;
+		break;
+
+	case 1:	/* Mitigation not required on this CPU */
+		required = false;
+		break;
+
+	default:
+		WARN_ON(1);
+		return false;
+	}
+
+	switch (ssbd_state) {
+	case ARM64_SSBD_FORCE_DISABLE:
+		pr_info_once("%s disabled from command-line\n", entry->desc);
+		do_ssbd(false);
+		required = false;
+		break;
+
+	case ARM64_SSBD_EL1_ENTRY:
+		if (required) {
+			__this_cpu_write(arm64_ssbd_callback_required, 1);
+			do_ssbd(true);
+		}
+		break;
+
+	case ARM64_SSBD_FORCE_ENABLE:
+		pr_info_once("%s forced from command-line\n", entry->desc);
 		do_ssbd(true);
+		required = true;
+		break;
+
+	default:
+		WARN_ON(1);
+		break;
 	}
 
-	return supported;
+	return required;
 }
 #endif	/* CONFIG_ARM64_SSBD */
 
-- 
2.14.2

^ permalink raw reply related

* [PATCH 04/14] arm64: Add ARCH_WORKAROUND_2 probing
From: Marc Zyngier @ 2018-05-22 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-1-marc.zyngier@arm.com>

As for Spectre variant-2, we rely on SMCCC 1.1 to provide the
discovery mechanism for detecting the SSBD mitigation.

A new capability is also allocated for that purpose, and a
config option.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/Kconfig               |  9 ++++++
 arch/arm64/include/asm/cpucaps.h |  3 +-
 arch/arm64/kernel/cpu_errata.c   | 69 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index eb2cf4938f6d..b2103b4df467 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -938,6 +938,15 @@ config HARDEN_EL2_VECTORS
 
 	  If unsure, say Y.
 
+config ARM64_SSBD
+	bool "Speculative Store Bypass Disable" if EXPERT
+	default y
+	help
+	  This enables mitigation of the bypassing of previous stores
+	  by speculative loads.
+
+	  If unsure, say Y.
+
 menuconfig ARMV8_DEPRECATED
 	bool "Emulate deprecated/obsolete ARMv8 instructions"
 	depends on COMPAT
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index bc51b72fafd4..5b2facf786ba 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -48,7 +48,8 @@
 #define ARM64_HAS_CACHE_IDC			27
 #define ARM64_HAS_CACHE_DIC			28
 #define ARM64_HW_DBM				29
+#define ARM64_SSBD			30
 
-#define ARM64_NCAPS				30
+#define ARM64_NCAPS				31
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 0288d6cf560e..7fd6d5b001f5 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -250,6 +250,67 @@ void __init arm64_update_smccc_conduit(struct alt_instr *alt,
 
 	*updptr = cpu_to_le32(insn);
 }
+
+static void do_ssbd(bool state)
+{
+	switch (psci_ops.conduit) {
+	case PSCI_CONDUIT_HVC:
+		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
+		break;
+
+	case PSCI_CONDUIT_SMC:
+		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2, state, NULL);
+		break;
+
+	default:
+		WARN_ON_ONCE(1);
+		break;
+	}
+}
+
+static bool has_ssbd_mitigation(const struct arm64_cpu_capabilities *entry,
+				    int scope)
+{
+	struct arm_smccc_res res;
+	bool supported = true;
+
+	WARN_ON(scope != SCOPE_LOCAL_CPU || preemptible());
+
+	if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
+		return false;
+
+	/*
+	 * The probe function return value is either negative
+	 * (unsupported or mitigated), positive (unaffected), or zero
+	 * (requires mitigation). We only need to do anything in the
+	 * last case.
+	 */
+	switch (psci_ops.conduit) {
+	case PSCI_CONDUIT_HVC:
+		arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+				  ARM_SMCCC_ARCH_WORKAROUND_2, &res);
+		if ((int)res.a0 != 0)
+			supported = false;
+		break;
+
+	case PSCI_CONDUIT_SMC:
+		arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+				  ARM_SMCCC_ARCH_WORKAROUND_2, &res);
+		if ((int)res.a0 != 0)
+			supported = false;
+		break;
+
+	default:
+		supported = false;
+	}
+
+	if (supported) {
+		__this_cpu_write(arm64_ssbd_callback_required, 1);
+		do_ssbd(true);
+	}
+
+	return supported;
+}
 #endif	/* CONFIG_ARM64_SSBD */
 
 #define CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max)	\
@@ -506,6 +567,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = {
 		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
 		ERRATA_MIDR_RANGE_LIST(arm64_harden_el2_vectors),
 	},
+#endif
+#ifdef CONFIG_ARM64_SSBD
+	{
+		.desc = "Speculative Store Bypass Disable",
+		.capability = ARM64_SSBD,
+		.type = ARM64_CPUCAP_LOCAL_CPU_ERRATUM,
+		.matches = has_ssbd_mitigation,
+	},
 #endif
 	{
 	}
-- 
2.14.2

^ permalink raw reply related

* [PATCH 03/14] arm64: Add per-cpu infrastructure to call ARCH_WORKAROUND_2
From: Marc Zyngier @ 2018-05-22 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-1-marc.zyngier@arm.com>

In a heterogeneous system, we can end up with both affected and
unaffected CPUs. Let's check their status before calling into the
firmware.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kernel/cpu_errata.c |  2 ++
 arch/arm64/kernel/entry.S      | 11 +++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 46b3aafb631a..0288d6cf560e 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -233,6 +233,8 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
 #endif	/* CONFIG_HARDEN_BRANCH_PREDICTOR */
 
 #ifdef CONFIG_ARM64_SSBD
+DEFINE_PER_CPU_READ_MOSTLY(u64, arm64_ssbd_callback_required);
+
 void __init arm64_update_smccc_conduit(struct alt_instr *alt,
 				       __le32 *origptr, __le32 *updptr,
 				       int nr_inst)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index f33e6aed3037..29ad672a6abd 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -140,8 +140,10 @@ alternative_else_nop_endif
 
 	// This macro corrupts x0-x3. It is the caller's duty
 	// to save/restore them if required.
-	.macro	apply_ssbd, state
+	.macro	apply_ssbd, state, targ, tmp1, tmp2
 #ifdef CONFIG_ARM64_SSBD
+	ldr_this_cpu	\tmp2, arm64_ssbd_callback_required, \tmp1
+	cbz	\tmp2, \targ
 	mov	w0, #ARM_SMCCC_ARCH_WORKAROUND_2
 	mov	w1, #\state
 alternative_cb	arm64_update_smccc_conduit
@@ -176,12 +178,13 @@ alternative_cb_end
 	ldr	x19, [tsk, #TSK_TI_FLAGS]	// since we can unmask debug
 	disable_step_tsk x19, x20		// exceptions when scheduling.
 
-	apply_ssbd 1
+	apply_ssbd 1, 1f, x22, x23
 
 #ifdef CONFIG_ARM64_SSBD
 	ldp	x0, x1, [sp, #16 * 0]
 	ldp	x2, x3, [sp, #16 * 1]
 #endif
+1:
 
 	mov	x29, xzr			// fp pointed to user-space
 	.else
@@ -323,8 +326,8 @@ alternative_if ARM64_WORKAROUND_845719
 alternative_else_nop_endif
 #endif
 3:
-	apply_ssbd 0
-
+	apply_ssbd 0, 5f, x0, x1
+5:
 	.endif
 
 	msr	elr_el1, x21			// set up the return data
-- 
2.14.2

^ permalink raw reply related

* [PATCH 02/14] arm64: Call ARCH_WORKAROUND_2 on transitions between EL0 and EL1
From: Marc Zyngier @ 2018-05-22 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-1-marc.zyngier@arm.com>

In order for the kernel to protect itself, let's call the SSBD mitigation
implemented by the higher exception level (either hypervisor or firmware)
on each transition between userspace and kernel.

We must take the PSCI conduit into account in order to target the
right exception level, hence the introduction of a runtime patching
callback.

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm64/kernel/cpu_errata.c | 18 ++++++++++++++++++
 arch/arm64/kernel/entry.S      | 22 ++++++++++++++++++++++
 include/linux/arm-smccc.h      |  5 +++++
 3 files changed, 45 insertions(+)

diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index a900befadfe8..46b3aafb631a 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -232,6 +232,24 @@ enable_smccc_arch_workaround_1(const struct arm64_cpu_capabilities *entry)
 }
 #endif	/* CONFIG_HARDEN_BRANCH_PREDICTOR */
 
+#ifdef CONFIG_ARM64_SSBD
+void __init arm64_update_smccc_conduit(struct alt_instr *alt,
+				       __le32 *origptr, __le32 *updptr,
+				       int nr_inst)
+{
+	u32 insn;
+
+	BUG_ON(nr_inst != 1);
+
+	if (psci_ops.conduit == PSCI_CONDUIT_HVC)
+		insn = aarch64_insn_get_hvc_value();
+	else
+		insn = aarch64_insn_get_smc_value();
+
+	*updptr = cpu_to_le32(insn);
+}
+#endif	/* CONFIG_ARM64_SSBD */
+
 #define CAP_MIDR_RANGE(model, v_min, r_min, v_max, r_max)	\
 	.matches = is_affected_midr_range,			\
 	.midr_range = MIDR_RANGE(model, v_min, r_min, v_max, r_max)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index ec2ee720e33e..f33e6aed3037 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -18,6 +18,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/arm-smccc.h>
 #include <linux/init.h>
 #include <linux/linkage.h>
 
@@ -137,6 +138,18 @@ alternative_else_nop_endif
 	add	\dst, \dst, #(\sym - .entry.tramp.text)
 	.endm
 
+	// This macro corrupts x0-x3. It is the caller's duty
+	// to save/restore them if required.
+	.macro	apply_ssbd, state
+#ifdef CONFIG_ARM64_SSBD
+	mov	w0, #ARM_SMCCC_ARCH_WORKAROUND_2
+	mov	w1, #\state
+alternative_cb	arm64_update_smccc_conduit
+	nop					// Patched to SMC/HVC #0
+alternative_cb_end
+#endif
+	.endm
+
 	.macro	kernel_entry, el, regsize = 64
 	.if	\regsize == 32
 	mov	w0, w0				// zero upper 32 bits of x0
@@ -163,6 +176,13 @@ alternative_else_nop_endif
 	ldr	x19, [tsk, #TSK_TI_FLAGS]	// since we can unmask debug
 	disable_step_tsk x19, x20		// exceptions when scheduling.
 
+	apply_ssbd 1
+
+#ifdef CONFIG_ARM64_SSBD
+	ldp	x0, x1, [sp, #16 * 0]
+	ldp	x2, x3, [sp, #16 * 1]
+#endif
+
 	mov	x29, xzr			// fp pointed to user-space
 	.else
 	add	x21, sp, #S_FRAME_SIZE
@@ -303,6 +323,8 @@ alternative_if ARM64_WORKAROUND_845719
 alternative_else_nop_endif
 #endif
 3:
+	apply_ssbd 0
+
 	.endif
 
 	msr	elr_el1, x21			// set up the return data
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index c89da86de99f..ca1d2cc2cdfa 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -80,6 +80,11 @@
 			   ARM_SMCCC_SMC_32,				\
 			   0, 0x8000)
 
+#define ARM_SMCCC_ARCH_WORKAROUND_2					\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,				\
+			   ARM_SMCCC_SMC_32,				\
+			   0, 0x7fff)
+
 #ifndef __ASSEMBLY__
 
 #include <linux/linkage.h>
-- 
2.14.2

^ permalink raw reply related

* [PATCH 01/14] arm/arm64: smccc: Add SMCCC-specific return codes
From: Marc Zyngier @ 2018-05-22 15:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522150648.28297-1-marc.zyngier@arm.com>

We've so far used the PSCI return codes for SMCCC because they
were extremely similar. But with the new ARM DEN 0070A specification,
"NOT_REQUIRED" (-2) is clashing with PSCI's "PSCI_RET_INVALID_PARAMS".

Let's bite the bullet and add SMCCC specific return codes. Users
can be repainted as and when required.

Acked-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 include/linux/arm-smccc.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index a031897fca76..c89da86de99f 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -291,5 +291,10 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1,
  */
 #define arm_smccc_1_1_hvc(...)	__arm_smccc_1_1(SMCCC_HVC_INST, __VA_ARGS__)
 
+/* Return codes defined in ARM DEN 0070A */
+#define SMCCC_RET_SUCCESS			0
+#define SMCCC_RET_NOT_SUPPORTED			-1
+#define SMCCC_RET_NOT_REQUIRED			-2
+
 #endif /*__ASSEMBLY__*/
 #endif /*__LINUX_ARM_SMCCC_H*/
-- 
2.14.2

^ permalink raw reply related

* [PATCH 00/14] arm64 SSBD (aka Spectre-v4) mitigation
From: Marc Zyngier @ 2018-05-22 15:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi all,

This patch series implements the Linux kernel side of the "Spectre-v4"
(CVE-2018-3639) mitigation known as "Speculative Store Bypass Disable"
(SSBD).

More information can be found at:

  https://bugs.chromium.org/p/project-zero/issues/detail?id=1528
  https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability

For all released Arm Cortex-A CPUs that are affected by this issue, then
the preferred mitigation is simply to set a chicken bit in the firmware
during CPU initialisation and therefore no change to Linux is required.
Other CPUs may require the chicken bit to be toggled dynamically (for
example, when switching between user-mode and kernel-mode) and this is
achieved by calling into EL3 via an SMC which has been published as part
of the latest SMCCC specification:

  https://developer.arm.com/cache-speculation-vulnerability-firmware-specification

as well as an ATF update for the released ARM cores affected by SSDB:

  https://github.com/ARM-software/arm-trusted-firmware/pull/1392

These patches provide the following:

  1. Safe probing of firmware to establish which CPUs in the system
     require calling into EL3 as part of the mitigation.

  2. For CPUs that require it, call into EL3 on exception entry/exit
     from EL0 to apply the SSBD mitigation when running at EL1.

  3. A command-line option to force the SSBD mitigation to be always on,
     always off, or dymamically toggled (default) for CPUs that require
     the EL3 call.

  4. An initial implementation of a prctl() backend for arm64 that allows
     userspace tasks to opt-in to the mitigation explicitly. This is
     intended to match the interface provided by x86, and so we rely on
     their core changes here. There still is an annoying issue with
     multithreaded seccomp tasks that get flagged with the mitigation
     whilst they are running in userspace.

  5. An initial implementation of the call via KVM, which exposes the
     mitigation to the guest via an HVC interface. This isn't yet
     complete and doesn't include save/restore functionality for the
     workaround state.

All comments welcome,

	M.

Marc Zyngier (14):
  arm/arm64: smccc: Add SMCCC-specific return codes
  arm64: Call ARCH_WORKAROUND_2 on transitions between EL0 and EL1
  arm64: Add per-cpu infrastructure to call ARCH_WORKAROUND_2
  arm64: Add ARCH_WORKAROUND_2 probing
  arm64: Add 'ssbd' command-line option
  arm64: ssbd: Add global mitigation state accessor
  arm64: ssbd: Skip apply_ssbd if not using dynamic mitigation
  arm64: ssbd: Disable mitigation on CPU resume if required by user
  arm64: ssbd: Introduce thread flag to control userspace mitigation
  arm64: ssbd: Add prctl interface for per-thread mitigation
  arm64: KVM: Add HYP per-cpu accessors
  arm64: KVM: Add ARCH_WORKAROUND_2 support for guests
  arm64: KVM: Handle guest's ARCH_WORKAROUND_2 requests
  arm64: KVM: Add ARCH_WORKAROUND_2 discovery through
    ARCH_FEATURES_FUNC_ID

 Documentation/admin-guide/kernel-parameters.txt |  17 +++
 arch/arm/include/asm/kvm_host.h                 |  12 ++
 arch/arm/include/asm/kvm_mmu.h                  |   5 +
 arch/arm64/Kconfig                              |   9 ++
 arch/arm64/include/asm/cpucaps.h                |   3 +-
 arch/arm64/include/asm/cpufeature.h             |  22 +++
 arch/arm64/include/asm/kvm_asm.h                |  30 +++-
 arch/arm64/include/asm/kvm_host.h               |  26 ++++
 arch/arm64/include/asm/kvm_mmu.h                |  24 ++++
 arch/arm64/include/asm/thread_info.h            |   1 +
 arch/arm64/kernel/Makefile                      |   1 +
 arch/arm64/kernel/asm-offsets.c                 |   1 +
 arch/arm64/kernel/cpu_errata.c                  | 173 ++++++++++++++++++++++++
 arch/arm64/kernel/entry.S                       |  30 ++++
 arch/arm64/kernel/ssbd.c                        | 107 +++++++++++++++
 arch/arm64/kernel/suspend.c                     |   8 ++
 arch/arm64/kvm/hyp/hyp-entry.S                  |  38 +++++-
 arch/arm64/kvm/hyp/switch.c                     |  42 ++++++
 arch/arm64/kvm/reset.c                          |   4 +
 include/linux/arm-smccc.h                       |  10 ++
 virt/kvm/arm/arm.c                              |   4 +
 virt/kvm/arm/psci.c                             |  18 ++-
 22 files changed, 579 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm64/kernel/ssbd.c

-- 
2.14.2

^ permalink raw reply

* [patch v21 1/4] drivers: jtag: Add JTAG core driver
From: Oleksandr Shamray @ 2018-05-22 15:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAHp75VctG5RBtLF3nT5e_j7_e9O12u=5gerJ6OMxWbABM9ft2w@mail.gmail.com>

Hi Andy. 
Thanks for review.

Please read my answers inline.

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko at gmail.com]
> Sent: 16 ??? 2018 ?. 0:22
> To: Oleksandr Shamray <oleksandrs@mellanox.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Arnd Bergmann 
> <arnd@arndb.de>; Linux Kernel Mailing List 
> <linux-kernel@vger.kernel.org>; linux-arm Mailing List 
> <linux-arm-kernel@lists.infradead.org>; devicetree 
> <devicetree@vger.kernel.org>; openbmc at lists.ozlabs.org; Joel Stanley 
> <joel@jms.id.au>; Ji?? P?rko <jiri@resnulli.us>; Tobias Klauser 
> <tklauser@distanz.ch>; open list:SERIAL DRIVERS <linux- 
> serial at vger.kernel.org>; Vadim Pasternak <vadimp@mellanox.com>; 
> system-sw-low-level <system-sw-low-level@mellanox.com>; Rob Herring 
> <robh+dt@kernel.org>; openocd-devel-owner at lists.sourceforge.net; 
> linux- api at vger.kernel.org; David S. Miller <davem@davemloft.net>; 
> Mauro Carvalho Chehab <mchehab@kernel.org>; Jiri Pirko 
> <jiri@mellanox.com>
> Subject: Re: [patch v21 1/4] drivers: jtag: Add JTAG core driver
> 
> On Tue, May 15, 2018 at 5:21 PM, Oleksandr Shamray 
> <oleksandrs@mellanox.com> wrote:
> > Initial patch for JTAG driver
> > JTAG class driver provide infrastructure to support 
> > hardware/software JTAG platform drivers. It provide user layer API 
> > interface for flashing and debugging external devices which equipped 
> > with JTAG interface using standard transactions.
> >
> > Driver exposes set of IOCTL to user space for:
> > - XFER:
> > - SIR (Scan Instruction Register, IEEE 1149.1 Data Register scan);
> > - SDR (Scan Data Register, IEEE 1149.1 Instruction Register scan);
> > - RUNTEST (Forces the IEEE 1149.1 bus to a run state for a specified
> >   number of clocks).
> > - SIOCFREQ/GIOCFREQ for setting and reading JTAG frequency.
> >
> > Driver core provides set of internal APIs for allocation and
> > registration:
> > - jtag_register;
> > - jtag_unregister;
> > - jtag_alloc;
> > - jtag_free;
> >
> > Platform driver on registration with jtag-core creates the next 
> > entry in dev folder:
> > /dev/jtagX
> 
> >  0xB0   all     RATIO devices           in development:
> >                                         <mailto:vgo@ratio.de>
> >  0xB1   00-1F   PPPoX                   <mailto:mostrows@styx.uwaterloo.ca>
> > +0xB2   00-0f   linux/jtag.h            JTAG driver
> > +
> > +<mailto:oleksandrs@mellanox.com>
> 
> Consider to preserve style (upper vs. lower).

JTAG in code is lower (jtag) cane but in descriptions and notes it  is upper (JTAG).
In all places which do not correspond to this I will fix. 

> 
> > +         This provides basic core functionality support for JTAG class devices.
> > +         Hardware that is equipped with a JTAG microcontroller can be
> > +         supported by using this driver's interfaces.
> > +         This driver exposes a set of IOCTLs to the user space for
> > +         the following commands:
> > +         SDR: (Scan Data Register) Performs an IEEE 1149.1 Data Register scan
> > +         SIR: (Scan Instruction Register) Performs an IEEE 1149.1 Instruction
> > +         Register scan.
> > +         RUNTEST: Forces the IEEE 1149.1 bus to a run state for a specified
> > +         number of clocks or a specified time period.
> 
> Something feels wrong with formatting here.
> 

Will fix

> > +#define MAX_JTAG_NAME_LEN (sizeof("jtag") + 5)
> 
> Interesting definition. Why not to set to 10 explicitly? And why 10?
> (16 sounds better)
> 
5 - is a max len of JTAG device id in device name. I will add define to it.

In general I don't see the case for the system with hundreds JTAG interfaces.
I will limit maximum jtag master to 255 devices and change  id len to 3

#define MAX_JTAG_ID_STR_LEN 5
#define MAX_JTAG_NAME_LEN (sizeof("jtag") + MAX_JTAG_ID_STR_LEN)

> > +struct jtag {
> > +       struct miscdevice miscdev;
> 
> > +       struct device *dev;
> 
> Doesn't miscdev parent contain exactly this one?

Yes. 
Will fix.

> 
> > +       const struct jtag_ops *ops;
> > +       int id;
> > +       bool opened;
> > +       struct mutex open_lock;
> > +       unsigned long priv[0];
> > +};
> 
> > +               err = copy_to_user(u64_to_user_ptr(xfer.tdio),
> > +                                  (void *)(xfer_data), data_size);
> 
> Redundant parens in one case. Check the rest similar places.
> 

Yes.

> > +static int jtag_open(struct inode *inode, struct file *file) {
> 
> > +       struct jtag *jtag = container_of(file->private_data, struct jtag,
> > +                                        miscdev);
> 
> I would don't care about length and put it on one line.
> 

But following to LINUX kernel style, it should be no longer than 80 symbols. 
It will not pass by  ./scripts/checkpatch.pl

Will it be OK to send a patch which failed 80 symbols limitation check?

> > +       if (jtag->opened) {
> > +       jtag->opened = true;
> > +       jtag->opened = false;
> 
> Can it be opened non exclusively several times? If so, this needs to 
> be a ref counter instead.

It can be opened only once.

> 
> > +       if (!ops->idle || !ops->mode_set || !ops->status_get || !ops->xfer)
> > +               return NULL;
> 
> Are all of them mandatory?
> 

Yes, except "mode_set"
Will remove mode_set from check

> > +int jtag_register(struct jtag *jtag)
> 
> Perhaps devm_ variant.

Jtag driver uses miscdevice and related  misc_register and misc_deregister 
calls for creation and destruction. There is no device object prior to 
call to  misc_register, which could be used in devm_jtag_register.

> 
> > +#define jtag_u64_to_ptr(arg) ((void *)(uintptr_t)arg)
> 

Redundant. Removed.

> Where is this used or supposed to be used?
> 
> > +#define JTAG_MAX_XFER_DATA_LEN 65535
> 
> Is this limitation from some spec?
> Otherwise why not to allow 64K?
> 

It not limited by specification.  
But we enforce an upper bound for the length here, to prevent users from draining 
kernel memory with giant buffers.
So it was limited by  size of unsigned short int (65535)

> > +/**
> > + * struct jtag_ops - callbacks for jtag control functions:
> > + *
> > + * @freq_get: get frequency function. Filled by device driver
> > + * @freq_set: set frequency function. Filled by device driver
> > + * @status_get: set status function. Filled by device driver
> > + * @idle: set JTAG to idle state function. Filled by device driver
> > + * @xfer: send JTAG xfer function. Filled by device driver  */
> 
> Perhaps you need to describe which of them are _really_ mandatory and 
> which are optional.
> 

Ok

> --
> With Best Regards,
> Andy Shevchenko

Best Regards,
Oleksandr Shamray

^ permalink raw reply

* [patch v21 2/4] drivers: jtag: Add Aspeed SoC 24xx and 25xx families JTAG master driver
From: Oleksandr Shamray @ 2018-05-22 15:00 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <CAHp75Ve7EYiWBiE73i0CmyJhPMOdSKsGzCr0SUvta3Uya=Ov1Q@mail.gmail.com>

Hi Andy. 
Thanks for review.

Please read my answers inline.

> -----Original Message-----
> From: Andy Shevchenko [mailto:andy.shevchenko at gmail.com]
> Sent: 16 ??? 2018 ?. 0:00
> To: Oleksandr Shamray <oleksandrs@mellanox.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Arnd Bergmann 
> <arnd@arndb.de>; Linux Kernel Mailing List 
> <linux-kernel@vger.kernel.org>; linux-arm Mailing List 
> <linux-arm-kernel@lists.infradead.org>; devicetree 
> <devicetree@vger.kernel.org>; openbmc at lists.ozlabs.org; Joel Stanley 
> <joel@jms.id.au>; Ji?? P?rko <jiri@resnulli.us>; Tobias Klauser 
> <tklauser@distanz.ch>; open list:SERIAL DRIVERS <linux- 
> serial at vger.kernel.org>; Vadim Pasternak <vadimp@mellanox.com>; 
> system-sw-low-level <system-sw-low-level@mellanox.com>; Rob Herring 
> <robh+dt@kernel.org>; openocd-devel-owner at lists.sourceforge.net; 
> linux- api at vger.kernel.org; David S. Miller <davem@davemloft.net>; 
> Mauro Carvalho Chehab <mchehab@kernel.org>; Jiri Pirko 
> <jiri@mellanox.com>
> Subject: Re: [patch v21 2/4] drivers: jtag: Add Aspeed SoC 24xx and 
> 25xx families JTAG master driver
> 
> On Tue, May 15, 2018 at 5:21 PM, Oleksandr Shamray 
> <oleksandrs@mellanox.com> wrote:
> > Driver adds support of Aspeed 2500/2400 series SOC JTAG master
> controller.
> >
> > Driver implements the following jtag ops:
> > - freq_get;
> > - freq_set;
> > - status_get;
> > - idle;
> > - xfer;
> >
> > It has been tested on Mellanox system with BMC equipped with Aspeed
> > 2520 SoC for programming CPLD devices.
> 
> > +#define ASPEED_JTAG_DATA               0x00
> > +#define ASPEED_JTAG_INST               0x04
> > +#define ASPEED_JTAG_CTRL               0x08
> > +#define ASPEED_JTAG_ISR                        0x0C
> > +#define ASPEED_JTAG_SW                 0x10
> > +#define ASPEED_JTAG_TCK                        0x14
> > +#define ASPEED_JTAG_EC                 0x18
> > +
> > +#define ASPEED_JTAG_DATA_MSB           0x01
> > +#define ASPEED_JTAG_DATA_CHUNK_SIZE    0x20
> 
> 
> > +#define ASPEED_JTAG_IOUT_LEN(len)      (ASPEED_JTAG_CTL_ENG_EN |\
> > +                                        ASPEED_JTAG_CTL_ENG_OUT_EN 
> > +|\
> > +
> > +ASPEED_JTAG_CTL_INST_LEN(len))
> 
> Better to read
> 
> #define MY_COOL_CONST_OR_MACRO(xxx) \
>  ...
> 
> > +#define ASPEED_JTAG_DOUT_LEN(len)      (ASPEED_JTAG_CTL_ENG_EN
> |\
> > +                                        ASPEED_JTAG_CTL_ENG_OUT_EN 
> > + |\
> > +
> > +ASPEED_JTAG_CTL_DATA_LEN(len))
> 
> Ditto.

Ok. Changed to:
#define ASPEED_JTAG_IOUT_LEN(len) \
                               (ASPEED_JTAG_CTL_ENG_EN | \
                               ASPEED_JTAG_CTL_ENG_OUT_EN | \
                               ASPEED_JTAG_CTL_INST_LEN(len))

#define ASPEED_JTAG_DOUT_LEN(len) \
                               (ASPEED_JTAG_CTL_ENG_EN | \
                               ASPEED_JTAG_CTL_ENG_OUT_EN | \
                               ASPEED_JTAG_CTL_DATA_LEN(len))


> 
> > +static char *end_status_str[] = {"idle", "ir pause", "drpause"};
> 
> > +static int aspeed_jtag_freq_set(struct jtag *jtag, u32 freq) {
> > +       struct aspeed_jtag *aspeed_jtag = jtag_priv(jtag);
> > +       unsigned long apb_frq;
> > +       u32 tck_val;
> > +       u16 div;
> > +
> > +       apb_frq = clk_get_rate(aspeed_jtag->pclk);
> 
> > +       div = (apb_frq % freq == 0) ? (apb_frq / freq) - 1 : 
> > + (apb_frq / freq);
> 
> Isn't it the same as
> 
> div = (apb_frq - 1) / freq;
> 
> ?
> 

Seems it is same. Thanks.

> > +       tck_val = aspeed_jtag_read(aspeed_jtag, ASPEED_JTAG_TCK);
> > +       aspeed_jtag_write(aspeed_jtag,
> > +                         (tck_val & ASPEED_JTAG_TCK_DIVISOR_MASK) | div,
> > +                         ASPEED_JTAG_TCK);
> > +       return 0;
> > +}
> 
> > +static void aspeed_jtag_sw_delay(struct aspeed_jtag *aspeed_jtag, 
> > +int
> > +cnt) {
> > +       int i;
> > +
> > +       for (i = 0; i < cnt; i++)
> > +               aspeed_jtag_read(aspeed_jtag, ASPEED_JTAG_SW);
> 
> Isn't it readsl() (or how it's called, I don't remember).
> 

No, readsl reads data into buffer. But in this place read used for make 
software delay. 
Aspeed jtag driver supports 2 modes:
1 - hw mode with the hardware controlled JTAG states
and pins
2 - with software controlled pins. 
This part of code used in sw-mode and generates delay for JTAG bit-bang .
I will change it to ndelay().

> > +}
> 
> > +static void aspeed_jtag_wait_instruction_pause(struct aspeed_jtag
> > +*aspeed_jtag) {
> > +       wait_event_interruptible(aspeed_jtag->jtag_wq, aspeed_jtag->flag &
> > +                                ASPEED_JTAG_ISR_INST_PAUSE);
> 
> In such cases I prefer to see a new line with a parameter in full.
> Check all places.
> 
> > +       aspeed_jtag->flag &= ~ASPEED_JTAG_ISR_INST_PAUSE; }
> 
> > +static void aspeed_jtag_sm_cycle(struct aspeed_jtag *aspeed_jtag, 
> > +const
> u8 *tms,
> > +                                int len) {
> > +       int i;
> > +
> > +       for (i = 0; i < len; i++)
> > +               aspeed_jtag_tck_cycle(aspeed_jtag, tms[i], 0); }
> > +
> > +static void aspeed_jtag_run_test_idle_sw(struct aspeed_jtag
> *aspeed_jtag,
> > +                                        struct jtag_run_test_idle
> > +*runtest) {
> > +       static const u8 sm_pause_irpause[] = {1, 1, 1, 1, 0, 1, 0};
> > +       static const u8 sm_pause_drpause[] = {1, 1, 1, 0, 1, 0};
> > +       static const u8 sm_idle_irpause[] = {1, 1, 0, 1, 0};
> > +       static const u8 sm_idle_drpause[] = {1, 0, 1, 0};
> > +       static const u8 sm_pause_idle[] = {1, 1, 0};
> > +       int i;
> > +
> > +       /* SW mode from idle/pause-> to pause/idle */
> > +       if (runtest->reset) {
> > +               for (i = 0; i < ASPEED_JTAG_RESET_CNTR; i++)
> > +                       aspeed_jtag_tck_cycle(aspeed_jtag, 1, 0);
> > +       }
> 
> I would rather split this big switch to a few helper functions per 
> each status of surrounding switch.
> 

Ok.
Will do it.


> > +
> > +       /* Stay on IDLE for at least  TCK cycle */
> > +       for (i = 0; i < runtest->tck; i++)
> > +               aspeed_jtag_tck_cycle(aspeed_jtag, 0, 0); }
> 
> 
> > +/**
> > + * aspeed_jtag_run_test_idle:
> > + * JTAG reset: generates at least 9 TMS high and 1 TMS low to force
> > + * devices into Run-Test/Idle State.
> > + */
> 
> It's rather broken kernel doc.

Deleted.

> 
> > +               aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_CTL_ENG_EN |
> > +                                 ASPEED_JTAG_CTL_ENG_OUT_EN |
> > +                                 ASPEED_JTAG_CTL_FORCE_TMS, 
> > + ASPEED_JTAG_CTRL);
> 
> > +               aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_EC_GO_IDLE,
> > +                                 ASPEED_JTAG_EC);
> 
> > +       aspeed_jtag_write(aspeed_jtag, ASPEED_JTAG_SW_MODE_EN |
> > +                         ASPEED_JTAG_SW_MODE_TDIO, ASPEED_JTAG_SW);
> 
> Here you have permutations of flag some of which are repeatetive in 
> the code. Perhaps make additional definitions instead.
> Check other similar places.
> 

Ok. Will add defined for repeated flags

> 
> > +       char          tdo;
> 
> Indentation.

Ok.

> 
> > +       if (xfer->direction == JTAG_READ_XFER)
> > +               tdi = UINT_MAX;
> > +       else
> > +               tdi = data[index];
> 
> > +                       if (xfer->direction == JTAG_READ_XFER)
> > +                               tdi = UINT_MAX;
> > +                       else
> > +                               tdi = data[index];
> 
> Take your time to think how the above duplication can be avoided.
> 

In both cases data[] is different, so I should check it twice, but I will 
change it to, macro like:

#define ASPEED_JTAG_GET_TDI(direction, data) \
                              (direction == JTAG_READ_XFER) ? UNIT_MAX : data

> > +               }
> > +       }
> > +
> > +       tdo = aspeed_jtag_tck_cycle(aspeed_jtag, 1, tdi &
> ASPEED_JTAG_DATA_MSB);
> > +       data[index] |= tdo << (shift_bits % 
> > +ASPEED_JTAG_DATA_CHUNK_SIZE); }
> 
> 
> > +       if (endstate != JTAG_STATE_IDLE) {
> 
> Why not to use positive check?
> 

Will restructure to have positive check
if (endstate == JTAG_STATE_IDLE) {
...
} else {
...
} 

> > +       int i;
> > +
> > +       for (i = 0; i <= xfer->length / BITS_PER_BYTE; i++) {
> > +               pos += snprintf(&dbg_str[pos], sizeof(dbg_str) - pos,
> > +                               "0x%02x ", xfer_data[i]);
> > +       }
> 
> Oh, NO! Consider reading printk-formats (for %*ph) and other 
> documentation about available APIs.

Ok.

> 
> > +       if (!(aspeed_jtag->mode & JTAG_XFER_HW_MODE)) {
> > +               /* SW mode */
> 
> This is rather too complex to be in one function.
> 

Will split to separate functions.

> > +       } else {
> 
> > +               /* hw mode */
> > +               aspeed_jtag_write(aspeed_jtag, 0, ASPEED_JTAG_SW);
> > +               aspeed_jtag_xfer_hw(aspeed_jtag, xfer, data);
> 
> For symmetry it might be another function.
> 
> > +       }
> 
> > +       dev_dbg(aspeed_jtag->dev, "status %x\n", status);
> 
> Perhaps someone should become familiar with tracepoints?
> 
> > +               dev_err(aspeed_jtag->dev, "irq status:%x\n",
> > +                       status);
> 
> 
> Huh, really?! SPAM.


I will review and delete redundant debug messages.

> 
> (I would drop it completely, though you may use ratelimited variant)
> 
> > +               ret = IRQ_NONE;
> > +       }
> > +       return ret;
> > +}
> 
> > +       clk_prepare_enable(aspeed_jtag->pclk);
> 
> This might fail.

Will add error check

> 
> > +       dev_dbg(&pdev->dev, "IRQ %d.\n", aspeed_jtag->irq);
> 
> Noise even for debug.

Agree.

> 
> > +       err = jtag_register(jtag);
> 
> Perhaps we might have devm_ variant of this. Check how SPI framework 
> deal with a such.
> 

Jtag driver uses miscdevice and related  misc_register and misc_deregister 
calls for creation and destruction. There is no device object prior 
to call to misc_register, which could be used in devm_jtag_register.

> > +static int aspeed_jtag_remove(struct platform_device *pdev) {
> 
> > +       struct jtag *jtag;
> > +
> > +       jtag = platform_get_drvdata(pdev);
> 
> Usually we put this on one line

+

> 
> > +       aspeed_jtag_deinit(pdev, jtag_priv(jtag));
> > +       jtag_unregister(jtag);
> > +       jtag_free(jtag);
> > +       return 0;
> > +}
> 
> 
> --
> With Best Regards,
> Andy Shevchenko

Best Regards,
Oleksandr Shamray

^ permalink raw reply

* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Ulf Hansson @ 2018-05-22 14:47 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <f5589b88-acf8-b491-df79-e359e596c8d0@nvidia.com>

[...]

>>
>> +/**
>> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
>> + * @dev: Device to attach.
>> + * @index: The index of the PM domain.
>> + *
>> + * Parse device's OF node to find a PM domain specifier at the provided @index.
>> + * If such is found, allocates a new device and attaches it to retrieved
>> + * pm_domain ops.
>> + *
>> + * Returns the allocated device if successfully attached PM domain, NULL when
>> + * the device don't need a PM domain or have a single PM domain, else PTR_ERR()
>> + * in case of failures. Note that if a power-domain exists for the device, but
>> + * cannot be found or turned on, then return PTR_ERR(-EPROBE_DEFER) to ensure
>> + * that the device is not probed and to re-try again later.
>> + */
>> +struct device *genpd_dev_pm_attach_by_id(struct device *dev,
>> +                                      unsigned int index)
>> +{
>> +     struct device *genpd_dev;
>> +     int num_domains;
>> +     int ret;
>> +
>> +     if (!dev->of_node)
>> +             return NULL;
>> +
>> +     /* Deal only with devices using multiple PM domains. */
>> +     num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
>> +                                              "#power-domain-cells");
>> +     if (num_domains < 2 || index >= num_domains)
>> +             return NULL;
>> +
>> +     /* Allocate and register device on the genpd bus. */
>> +     genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
>> +     if (!genpd_dev)
>> +             return ERR_PTR(-ENOMEM);
>> +
>> +     dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
>> +     genpd_dev->bus = &genpd_bus_type;
>> +     genpd_dev->release = genpd_release_dev;
>> +
>> +     ret = device_register(genpd_dev);
>> +     if (ret) {
>> +             kfree(genpd_dev);
>> +             return ERR_PTR(ret);
>> +     }
>> +
>> +     /* Try to attach the device to the PM domain at the specified index. */
>> +     ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
>> +     if (ret < 1) {
>> +             device_unregister(genpd_dev);
>> +             return ret ? ERR_PTR(ret) : NULL;
>> +     }
>> +
>> +     pm_runtime_set_active(genpd_dev);
>> +     pm_runtime_enable(genpd_dev);
>> +
>> +     return genpd_dev;
>> +}
>> +EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);
>
> Thanks for sending this. Believe it or not this has still been on my to-do list
> and so we definitely need a solution for Tegra.
>
> Looking at the above it appears that additional power-domains exposed as devices
> to the client device. So I assume that this means that the drivers for devices
> with multiple power-domains will need to call RPM APIs for each of these
> additional power-domains. Is that correct?

They can, but should not!

Instead, the driver shall use device_link_add() and device_link_del(),
dynamically, depending on what PM domain that their original device
needs for the current running use case.

In that way, they keep existing runtime PM deployment, operating on
its original device.

>
> If so, I can see that that would work, but it does not seem to fit the RPM model
> where currently for something like device clocks, the RPM callbacks can handle
> all clocks at once.
>
> I was wondering why we could not add a list of genpd domains to the
> dev_pm_domain struct for the device? For example ...

See above answer, hopefully that explains it.

FYI, that's why I also discovered the bug around using device links
with runtime PM during probe.
https://patchwork.kernel.org/patch/10408825/

>
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index e723b78d8357..a11d6db3c077 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -659,6 +659,7 @@ extern void dev_pm_put_subsys_data(struct device *dev);
>   * subsystem-level and driver-level callbacks.
>   */
>  struct dev_pm_domain {
> +       struct list_head        genpd_list;
>         struct dev_pm_ops       ops;
>         void (*detach)(struct device *dev, bool power_off);
>         int (*activate)(struct device *dev);
> @@ -666,6 +667,11 @@ struct dev_pm_domain {
>         void (*dismiss)(struct device *dev);
>  };
>
> +struct dev_pm_domain_link {
> +       struct generic_pm_domain *genpd;
> +       struct list_head node;
> +};
> +
>  /*
>   * The PM_EVENT_ messages are also used by drivers implementing the legacy
>   * suspend framework, based on the ->suspend() and ->resume() callbacks common
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index e61b5cd79fe2..019593804670 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -51,7 +51,6 @@ struct dev_pm_opp;
>
>  struct generic_pm_domain {
>         struct device dev;
> -       struct dev_pm_domain domain;    /* PM domain operations */
>         struct list_head gpd_list_node; /* Node in the global PM domains list */
>         struct list_head master_links;  /* Links with PM domain as a master */
>         struct list_head slave_links;   /* Links with PM domain as a slave */
> @@ -99,11 +98,6 @@ struct generic_pm_domain {
>
>  };
>
> -static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
> -{
> -       return container_of(pd, struct generic_pm_domain, domain);
> -}
> -
>
> Obviously the above will not compile but the intent would be to allocate a
> dev_pm_domain_link struct per power-domain that the device needs and add
> to the genpd_list for the device. It would be a much bigger change in
> having to iterate through all the power-domains when turning devices on
> and off, however, it would simplify the client driver.
>
> Cheers
> Jon
>
> --
> nvpublic

Kind regards
Uffe

^ permalink raw reply

* [PATCH 9/9] ARM: dts: silk: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:46 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-9-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)

Drop @0?
4m?

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 8/9] ARM: dts: alt: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:45 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-8-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader),256k(system),-(user)

Drop @0?

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 7/9] ARM: dts: gose: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:44 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-7-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)

Drop @0? 4m?

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 6/9] ARM: dts: wheat: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-6-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)

I think the "@0" can be dropped, as it's optional?
4m?

(Gaining more knowledge during reviewing ;-)

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 5/9] ARM: dts: porter: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:34 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-5-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader_prg),4096k(user_prg),-(flash_fs)

4m?

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 3/9] ARM: dts: stout: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:33 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-3-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:512k at 0(loader),256k(uboot),256k(uboot-env),-(flash)
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 2/9] ARM: dts: lager: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:32 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-2-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:256k at 0(loader),4096k(user),-(flash)

I guess s/4096k/4m/ works, too?

> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH 8/9] PM / Domains: Add support for multi PM domains per device to genpd
From: Jon Hunter @ 2018-05-22 14:31 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526639490-12167-9-git-send-email-ulf.hansson@linaro.org>

Hi Ulf,

On 18/05/18 11:31, Ulf Hansson wrote:
> To support devices being partitioned across multiple PM domains, let's
> start by extending genpd to cope with these configurations.
> 
> More precisely, add a new exported function, genpd_dev_pm_attach_by_id(),
> similar to genpd_dev_pm_attach(), but the new function also allows the
> caller to provide an index to what PM domain it wants to attach.
> 
> Furthermore, let genpd register a new virtual struct device via calling
> device_register() and attach it to the corresponding PM domain, which is
> looked up via calling the existing genpd OF functions. Note that the new
> device is needed, because only one PM domain can be attached per device. At
> successful attachment, genpd_dev_pm_attach_by_id() returns the new device,
> allowing the caller to operate on it to deal with power management.
> 
> To deal with detaching of a PM domain for multiple PM domain case, we can
> still re-use the existing genpd_dev_pm_detach() function, although we need
> to extend it to cover cleanup of the earlier registered device, via calling
> device_unregister().
> 
> An important note, genpd_dev_pm_attach_by_id() shall only be called by the
> driver core / PM core, similar to how genpd_dev_pm_attach() is used.
> Following changes deploys this.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>   drivers/base/power/domain.c | 79 +++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/pm_domain.h   |  8 +++++
>   2 files changed, 87 insertions(+)
> 
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index d538640..ffeb6ea 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -2171,6 +2171,15 @@ struct generic_pm_domain *of_genpd_remove_last(struct device_node *np)
>   }
>   EXPORT_SYMBOL_GPL(of_genpd_remove_last);
>   
> +static void genpd_release_dev(struct device *dev)
> +{
> +	kfree(dev);
> +}
> +
> +static struct bus_type genpd_bus_type = {
> +	.name		= "genpd",
> +};
> +
>   /**
>    * genpd_dev_pm_detach - Detach a device from its PM domain.
>    * @dev: Device to detach.
> @@ -2208,6 +2217,10 @@ static void genpd_dev_pm_detach(struct device *dev, bool power_off)
>   
>   	/* Check if PM domain can be powered off after removing this device. */
>   	genpd_queue_power_off_work(pd);
> +
> +	/* Unregister the device if it was created by genpd. */
> +	if (dev->bus == &genpd_bus_type)
> +		device_unregister(dev);
>   }
>   
>   static void genpd_dev_pm_sync(struct device *dev)
> @@ -2298,6 +2311,66 @@ int genpd_dev_pm_attach(struct device *dev)
>   }
>   EXPORT_SYMBOL_GPL(genpd_dev_pm_attach);
>   
> +/**
> + * genpd_dev_pm_attach_by_id() - Attach a device to one of its PM domain.
> + * @dev: Device to attach.
> + * @index: The index of the PM domain.
> + *
> + * Parse device's OF node to find a PM domain specifier at the provided @index.
> + * If such is found, allocates a new device and attaches it to retrieved
> + * pm_domain ops.
> + *
> + * Returns the allocated device if successfully attached PM domain, NULL when
> + * the device don't need a PM domain or have a single PM domain, else PTR_ERR()
> + * in case of failures. Note that if a power-domain exists for the device, but
> + * cannot be found or turned on, then return PTR_ERR(-EPROBE_DEFER) to ensure
> + * that the device is not probed and to re-try again later.
> + */
> +struct device *genpd_dev_pm_attach_by_id(struct device *dev,
> +					 unsigned int index)
> +{
> +	struct device *genpd_dev;
> +	int num_domains;
> +	int ret;
> +
> +	if (!dev->of_node)
> +		return NULL;
> +
> +	/* Deal only with devices using multiple PM domains. */
> +	num_domains = of_count_phandle_with_args(dev->of_node, "power-domains",
> +						 "#power-domain-cells");
> +	if (num_domains < 2 || index >= num_domains)
> +		return NULL;
> +
> +	/* Allocate and register device on the genpd bus. */
> +	genpd_dev = kzalloc(sizeof(*genpd_dev), GFP_KERNEL);
> +	if (!genpd_dev)
> +		return ERR_PTR(-ENOMEM);
> +
> +	dev_set_name(genpd_dev, "genpd:%u:%s", index, dev_name(dev));
> +	genpd_dev->bus = &genpd_bus_type;
> +	genpd_dev->release = genpd_release_dev;
> +
> +	ret = device_register(genpd_dev);
> +	if (ret) {
> +		kfree(genpd_dev);
> +		return ERR_PTR(ret);
> +	}
> +
> +	/* Try to attach the device to the PM domain at the specified index. */
> +	ret = __genpd_dev_pm_attach(genpd_dev, dev->of_node, index);
> +	if (ret < 1) {
> +		device_unregister(genpd_dev);
> +		return ret ? ERR_PTR(ret) : NULL;
> +	}
> +
> +	pm_runtime_set_active(genpd_dev);
> +	pm_runtime_enable(genpd_dev);
> +
> +	return genpd_dev;
> +}
> +EXPORT_SYMBOL_GPL(genpd_dev_pm_attach_by_id);

Thanks for sending this. Believe it or not this has still been on my to-do list
and so we definitely need a solution for Tegra.

Looking at the above it appears that additional power-domains exposed as devices
to the client device. So I assume that this means that the drivers for devices
with multiple power-domains will need to call RPM APIs for each of these
additional power-domains. Is that correct?

If so, I can see that that would work, but it does not seem to fit the RPM model
where currently for something like device clocks, the RPM callbacks can handle
all clocks at once.

I was wondering why we could not add a list of genpd domains to the
dev_pm_domain struct for the device? For example ...

diff --git a/include/linux/pm.h b/include/linux/pm.h
index e723b78d8357..a11d6db3c077 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -659,6 +659,7 @@ extern void dev_pm_put_subsys_data(struct device *dev);
  * subsystem-level and driver-level callbacks.
  */
 struct dev_pm_domain {
+       struct list_head        genpd_list;
        struct dev_pm_ops       ops;
        void (*detach)(struct device *dev, bool power_off);
        int (*activate)(struct device *dev);
@@ -666,6 +667,11 @@ struct dev_pm_domain {
        void (*dismiss)(struct device *dev);
 };

+struct dev_pm_domain_link {
+       struct generic_pm_domain *genpd;
+       struct list_head node;
+};
+
 /*
  * The PM_EVENT_ messages are also used by drivers implementing the legacy
  * suspend framework, based on the ->suspend() and ->resume() callbacks common
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index e61b5cd79fe2..019593804670 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -51,7 +51,6 @@ struct dev_pm_opp;

 struct generic_pm_domain {
        struct device dev;
-       struct dev_pm_domain domain;    /* PM domain operations */
        struct list_head gpd_list_node; /* Node in the global PM domains list */
        struct list_head master_links;  /* Links with PM domain as a master */
        struct list_head slave_links;   /* Links with PM domain as a slave */
@@ -99,11 +98,6 @@ struct generic_pm_domain {

 };

-static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
-{
-       return container_of(pd, struct generic_pm_domain, domain);
-}
-

Obviously the above will not compile but the intent would be to allocate a
dev_pm_domain_link struct per power-domain that the device needs and add
to the genpd_list for the device. It would be a much bigger change in
having to iterate through all the power-domains when turning devices on
and off, however, it would simplify the client driver. 

Cheers
Jon

-- 
nvpublic

^ permalink raw reply related

* [PATCH v3] arm64: allwinner: a64: Add Amarula A64-Relic initial support
From: Maxime Ripard @ 2018-05-22 14:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522132228.6564-1-jagan@amarulasolutions.com>

On Tue, May 22, 2018 at 06:52:28PM +0530, Jagan Teki wrote:
> Amarula A64-Relic is Allwinner A64 based IoT device, which support
> - Allwinner A64 Cortex-A53
> - Mali-400MP2 GPU
> - AXP803 PMIC
> - 1GB DDR3 RAM
> - 8GB eMMC
> - AP6330 Wifi/BLE
> - MIPI-DSI
> - CSI: OV5640 sensor
> - USB OTG

You claim that this is doing OTG...

[..]

> +&usb_otg {
> +	dr_mode = "peripheral";
> +	status = "okay";
> +};

... and yet you're setting it as peripheral...

> +&usbphy {
> +	usb0_id_det-gpios = <&pio 7 9 GPIO_ACTIVE_HIGH>; /* PH9 */
> +	usb0_vbus-supply = <&reg_drivevbus>;

While you have an ID pin and a controllable VBUS. Which one is it?

Maxime
-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/f71e03b3/attachment-0001.sig>

^ permalink raw reply

* [RFC PATCH v2] arm64: fault: Don't leak data in ESR context for user fault on kernel VA
From: Peter Maydell @ 2018-05-22 14:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522141146.GF13470@e103592.cambridge.arm.com>

On 22 May 2018 at 15:11, Dave Martin <Dave.Martin@arm.com> wrote:
> On Tue, May 22, 2018 at 02:48:29PM +0100, Peter Maydell wrote:
>> On 22 May 2018 at 14:38, Will Deacon <will.deacon@arm.com> wrote:
>> > Hi Peter,
>> >
>> > Sorry for the delay in getting to this! Comments inline.
>> >
>> > On Thu, Apr 19, 2018 at 04:48:33PM +0100, Peter Maydell wrote:
>>
>> >> +                     /*
>> >> +                      * These bits provide only information about the
>> >> +                      * faulting instruction, which userspace knows already.
>> >> +                      * We explicitly clear bits which are architecturally
>> >> +                      * RES0 in case they are given meanings in future.
>> >> +                      */
>> >> +                     if (esr & ESR_ELx_ISV)
>> >> +                             esr &= ESR_ELx_EC_MASK | ESR_ELx_IL |
>> >> +                                     ESR_ELx_ISV | ESR_ELx_SAS |
>> >> +                                     ESR_ELx_SSE | ESR_ELx_SRT_MASK |
>> >> +                                     ESR_ELx_SF | ESR_ELx_AR | ESR_ELx_CM |
>> >> +                                     ESR_ELx_WNR;
>> >
>> > Reading through the ARM ARM, it seems to say that ISV is always 0 for
>> > faults reported in ESR_EL1, which implies we can drop ISV, SAS, SSE, SRT,
>> > SF and AR from this list and actually drop the conditional altogether.
>>
>> Mmm, I guess so, if we're guaranteed to only be working with ESRs
>> taken to EL1 (or we want to present userspace with an ESR that
>
> There is no direct interface between EL0 and EL2 with the stage2
> translation enabled, so even if this data is available for a fault at
> EL2, we won't be signalling the fault via delivering a signal to EL0.

Right -- and even if we did somehow end up with that, we probably
wouldn't want to leak to userspace that their access was trapped
to EL2 rather than EL1, so we should present the illusion that
it was trapped at EL1 regardless.

thanks
-- PMM

^ permalink raw reply

* [PATCH 4/9] ARM: dts: koelsch: Drop MTD partitioning from DT
From: Geert Uytterhoeven @ 2018-05-22 14:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-4-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Drop the MTD partitioning from DT, since it does not describe HW
> and to give way to a more flexible kernel command line partition
> passing.
>
> To retain the original partitioning, assure you have enabled
> CONFIG_MTD_CMDLINE_PARTS in your kernel config and add the
> following to your kernel command line:
>
>   mtdparts=spi0.0:512k at 0(loader),5632k(user),-(flash)
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH] arm64: Kconfig: Enable LSE atomics by default
From: Marc Zyngier @ 2018-05-22 14:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1526926462-19214-1-git-send-email-will.deacon@arm.com>

On 21/05/18 19:14, Will Deacon wrote:
> Now that we're seeing CPUs shipping with LSE atomics, default them to
> 'on' in Kconfig. CPUs without the instructions will continue to use
> LDXR/STXR-based sequences, but they will be placed out-of-line by the
> compiler.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Marc Zyngier <marc.zyngier@arm.com>

	M.
-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply

* [PATCH 1/9] ARM: shmobile: defconfig: Enable MTD command line partition parsing
From: Geert Uytterhoeven @ 2018-05-22 14:29 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180522120257.13232-1-marek.vasut+renesas@gmail.com>

On Tue, May 22, 2018 at 2:02 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> In preparation for removing MTD partitioning from the DTs and moving
> it over to kernel command line partition parsing, enable the support
> for kernel command line MTD partition parsing.
>
> The argument for not having MTD partitions in the DT is the same as
> for not having hard drive partitions in DT, neither describes the
> hardware itself, so it shouldn't be in the DT. Furthermore, kernel
> command line MTD partition passing allows greater flexibility in
> case someone decided to repartition the flash, which is well in the
> realm of possibility with these systems.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply

* [PATCH v2] ARM64: dts: sun50i: h5: Add spi flash node for OrangePi PC2
From: Maxime Ripard @ 2018-05-22 14:27 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20180521115426.92115-1-manu@freebsd.org>

On Mon, May 21, 2018 at 01:54:26PM +0200, Emmanuel Vadot wrote:
> The OrangePi PC2 have an mx25l1606e spi flash.
> Add a node for it.
> 
> Signed-off-by: Emmanuel Vadot <manu@freebsd.org>

Fixed the prefix and applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180522/3426bebf/attachment-0001.sig>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox