All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never
@ 2015-07-17 17:30 James Morse
  2015-07-17 17:30 ` [PATCH v2 1/6] arm64: kernel: Add cpuid_feature_extract_field() for 4bit sign extension James Morse
                   ` (6 more replies)
  0 siblings, 7 replies; 19+ messages in thread
From: James Morse @ 2015-07-17 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

This series adds support for Privileged Access Never (PAN; part of the ARMv8.1
Extensions). When enabled, this feature causes a permission fault if the kernel
attempts to access memory that is also accessible by userspace - instead the
PAN bit must be cleared when accessing userspace memory. (or use the
ldt*/stt* instructions).

This series detects and enables this feature, and uses alternatives to change
{get,put}_user() et al to clear the PAN bit while they do their work.

Changes since v1:
* Copied cpuid_feature_extract_field() from arch/arm as a new patch, suggested
  by Russell King [1].
* Changed feature-detection patch to use cpuid_feature_extract_field() for sign
  extension, and '>='.
* Moved SCTLR_EL1_* from asm/cputype.h to asm/sysreg.h
* Added PSR_PAN_BIT in uapi/asm/ptrace.h
* Removed the setting of PSTATE_PAN in kernel/process.c

[1] http://www.spinics.net/lists/arm-kernel/msg432518.html


James Morse (6):
  arm64: kernel: Add cpuid_feature_extract_field() for 4bit sign
    extension
  arm64: kernel: preparatory: Move config_sctlr_el1
  arm64: kernel: Add cpufeature 'enable' callback
  arm64: kernel: Add min_register_value and use '>=' for feature
    detection
  arm64: kernel: Add optional CONFIG_ parameter to ALTERNATIVE()
  arm64: kernel: Add support for Privileged Access Never

 arch/arm64/Kconfig                   | 14 +++++++++++++
 arch/arm64/include/asm/alternative.h | 28 ++++++++++++++++++++++---
 arch/arm64/include/asm/cpufeature.h  | 21 ++++++++++++++++---
 arch/arm64/include/asm/cputype.h     |  3 ---
 arch/arm64/include/asm/futex.h       |  8 ++++++++
 arch/arm64/include/asm/processor.h   |  2 ++
 arch/arm64/include/asm/sysreg.h      | 21 +++++++++++++++++++
 arch/arm64/include/asm/uaccess.h     | 11 ++++++++++
 arch/arm64/include/uapi/asm/ptrace.h |  1 +
 arch/arm64/kernel/armv8_deprecated.c | 11 +---------
 arch/arm64/kernel/cpufeature.c       | 40 +++++++++++++++++++++++++++++++++---
 arch/arm64/lib/clear_user.S          |  8 ++++++++
 arch/arm64/lib/copy_from_user.S      |  8 ++++++++
 arch/arm64/lib/copy_in_user.S        |  8 ++++++++
 arch/arm64/lib/copy_to_user.S        |  8 ++++++++
 arch/arm64/mm/fault.c                | 23 +++++++++++++++++++++
 16 files changed, 193 insertions(+), 22 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/6] arm64: kernel: Add cpuid_feature_extract_field() for 4bit sign extension
  2015-07-17 17:30 [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never James Morse
@ 2015-07-17 17:30 ` James Morse
  2015-07-20 11:32   ` Catalin Marinas
  2015-07-17 17:30 ` [PATCH v2 2/6] arm64: kernel: preparatory: Move config_sctlr_el1 James Morse
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: James Morse @ 2015-07-17 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

Copied from arch/arm/include/asm/cputype.h, this function does the
shifting and sign extension necessary when accessing cpu feature fields.

Signed-off-by: James Morse <james.morse@arm.com>
Suggested-by: Russell King <linux@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index c1044218a63a..56c843918504 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -70,6 +70,19 @@ static inline void cpus_set_cap(unsigned int num)
 		__set_bit(num, cpu_hwcaps);
 }
 
+static inline int __attribute_const__ cpuid_feature_extract_field(u64 features,
+								  int field)
+{
+	int feature = (features >> field) & 15;
+
+	/* feature registers are signed values */
+	if (feature > 8)
+		feature -= 16;
+
+	return feature;
+}
+
+
 void check_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
 			    const char *info);
 void check_local_cpu_errata(void);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/6] arm64: kernel: preparatory: Move config_sctlr_el1
  2015-07-17 17:30 [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never James Morse
  2015-07-17 17:30 ` [PATCH v2 1/6] arm64: kernel: Add cpuid_feature_extract_field() for 4bit sign extension James Morse
@ 2015-07-17 17:30 ` James Morse
  2015-07-17 17:30 ` [PATCH v2 3/6] arm64: kernel: Add cpufeature 'enable' callback James Morse
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: James Morse @ 2015-07-17 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

Later patches need config_sctlr_el1 to set/clear bits in the sctlr_el1
register.

This patch moves this function into header a file.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/cputype.h     |  3 ---
 arch/arm64/include/asm/sysreg.h      | 12 ++++++++++++
 arch/arm64/kernel/armv8_deprecated.c | 11 +----------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/include/asm/cputype.h b/arch/arm64/include/asm/cputype.h
index a84ec605bed8..ee6403df9fe4 100644
--- a/arch/arm64/include/asm/cputype.h
+++ b/arch/arm64/include/asm/cputype.h
@@ -81,9 +81,6 @@
 #define ID_AA64MMFR0_BIGEND(mmfr0)	\
 	(((mmfr0) & ID_AA64MMFR0_BIGEND_MASK) >> ID_AA64MMFR0_BIGEND_SHIFT)
 
-#define SCTLR_EL1_CP15BEN	(0x1 << 5)
-#define SCTLR_EL1_SED		(0x1 << 8)
-
 #ifndef __ASSEMBLY__
 
 /*
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 5c89df0acbcb..56391fbae1e1 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -20,6 +20,9 @@
 #ifndef __ASM_SYSREG_H
 #define __ASM_SYSREG_H
 
+#define SCTLR_EL1_CP15BEN	(0x1 << 5)
+#define SCTLR_EL1_SED		(0x1 << 8)
+
 #define sys_reg(op0, op1, crn, crm, op2) \
 	((((op0)-2)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
 
@@ -55,6 +58,15 @@ asm(
 "	.endm\n"
 );
 
+static inline void config_sctlr_el1(u32 clear, u32 set)
+{
+	u32 val;
+
+	asm volatile("mrs %0, sctlr_el1" : "=r" (val));
+	val &= ~clear;
+	val |= set;
+	asm volatile("msr sctlr_el1, %0" : : "r" (val));
+}
 #endif
 
 #endif	/* __ASM_SYSREG_H */
diff --git a/arch/arm64/kernel/armv8_deprecated.c b/arch/arm64/kernel/armv8_deprecated.c
index 7922c2e710ca..78d56bff91fd 100644
--- a/arch/arm64/kernel/armv8_deprecated.c
+++ b/arch/arm64/kernel/armv8_deprecated.c
@@ -16,6 +16,7 @@
 
 #include <asm/insn.h>
 #include <asm/opcodes.h>
+#include <asm/sysreg.h>
 #include <asm/system_misc.h>
 #include <asm/traps.h>
 #include <asm/uaccess.h>
@@ -504,16 +505,6 @@ ret:
 	return 0;
 }
 
-static inline void config_sctlr_el1(u32 clear, u32 set)
-{
-	u32 val;
-
-	asm volatile("mrs %0, sctlr_el1" : "=r" (val));
-	val &= ~clear;
-	val |= set;
-	asm volatile("msr sctlr_el1, %0" : : "r" (val));
-}
-
 static int cp15_barrier_set_hw_mode(bool enable)
 {
 	if (enable)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 3/6] arm64: kernel: Add cpufeature 'enable' callback
  2015-07-17 17:30 [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never James Morse
  2015-07-17 17:30 ` [PATCH v2 1/6] arm64: kernel: Add cpuid_feature_extract_field() for 4bit sign extension James Morse
  2015-07-17 17:30 ` [PATCH v2 2/6] arm64: kernel: preparatory: Move config_sctlr_el1 James Morse
@ 2015-07-17 17:30 ` James Morse
  2015-07-17 17:30 ` [PATCH v2 4/6] arm64: kernel: Add min_register_value and use '>=' for feature detection James Morse
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 19+ messages in thread
From: James Morse @ 2015-07-17 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

This patch adds an 'enable()' callback to cpu capability/feature
detection, allowing features that require some setup or configuration
to get this opportunity once the feature has been detected.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 1 +
 arch/arm64/kernel/cpufeature.c      | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 56c843918504..7456fc44bee1 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -34,6 +34,7 @@ struct arm64_cpu_capabilities {
 	const char *desc;
 	u16 capability;
 	bool (*matches)(const struct arm64_cpu_capabilities *);
+	void (*enable)(void);
 	union {
 		struct {	/* To be used for erratum handling only */
 			u32 midr_model;
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 5ad86ceac010..650ffc28bedc 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -55,6 +55,12 @@ void check_cpu_capabilities(const struct arm64_cpu_capabilities *caps,
 			pr_info("%s %s\n", info, caps[i].desc);
 		cpus_set_cap(caps[i].capability);
 	}
+
+	/* second pass allows enable() to consider interacting capabilities */
+	for (i = 0; caps[i].desc; i++) {
+		if (cpus_have_cap(caps[i].capability) && caps[i].enable)
+			caps[i].enable();
+	}
 }
 
 void check_local_cpu_features(void)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 4/6] arm64: kernel: Add min_register_value and use '>=' for feature detection
  2015-07-17 17:30 [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never James Morse
                   ` (2 preceding siblings ...)
  2015-07-17 17:30 ` [PATCH v2 3/6] arm64: kernel: Add cpufeature 'enable' callback James Morse
@ 2015-07-17 17:30 ` James Morse
  2015-07-20 13:53   ` Catalin Marinas
  2015-07-17 17:30 ` [PATCH v2 5/6] arm64: kernel: Add optional CONFIG_ parameter to ALTERNATIVE() James Morse
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 19+ messages in thread
From: James Morse @ 2015-07-17 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

When a new cpu feature is available, the cpu feature bits will have some
initial value, which is incremented when the feature is updated.
This patch changes 'register_value' to be 'min_register_value', and checks
the feature bits value (interpreted as a signed int) is greater than this
minimum.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/include/asm/cpufeature.h |  4 ++--
 arch/arm64/kernel/cpufeature.c      | 14 +++++++++++---
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 7456fc44bee1..ef38e21ed719 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -42,8 +42,8 @@ struct arm64_cpu_capabilities {
 		};
 
 		struct {	/* Feature register checking */
-			u64 register_mask;
-			u64 register_value;
+			u8 register_shift;
+			u8 min_register_value;
 		};
 	};
 };
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 650ffc28bedc..cc26d6c6520b 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -23,12 +23,20 @@
 #include <asm/cpufeature.h>
 
 static bool
+feature_matches(u64 reg, const struct arm64_cpu_capabilities *entry)
+{
+	int val = cpuid_feature_extract_field(reg, entry->register_shift);
+
+	return (val >= entry->min_register_value);
+}
+
+static bool
 has_id_aa64pfr0_feature(const struct arm64_cpu_capabilities *entry)
 {
 	u64 val;
 
 	val = read_cpuid(id_aa64pfr0_el1);
-	return (val & entry->register_mask) == entry->register_value;
+	return feature_matches(val, entry);
 }
 
 static const struct arm64_cpu_capabilities arm64_features[] = {
@@ -36,8 +44,8 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.desc = "GIC system register CPU interface",
 		.capability = ARM64_HAS_SYSREG_GIC_CPUIF,
 		.matches = has_id_aa64pfr0_feature,
-		.register_mask = (0xf << 24),
-		.register_value = (1 << 24),
+		.register_shift = 24,
+		.min_register_value = 1,
 	},
 	{},
 };
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 5/6] arm64: kernel: Add optional CONFIG_ parameter to ALTERNATIVE()
  2015-07-17 17:30 [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never James Morse
                   ` (3 preceding siblings ...)
  2015-07-17 17:30 ` [PATCH v2 4/6] arm64: kernel: Add min_register_value and use '>=' for feature detection James Morse
@ 2015-07-17 17:30 ` James Morse
  2015-07-17 17:30 ` [PATCH v2 6/6] arm64: kernel: Add support for Privileged Access Never James Morse
  2015-07-20 11:02 ` [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never Vladimir Murzin
  6 siblings, 0 replies; 19+ messages in thread
From: James Morse @ 2015-07-17 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

Some uses of ALTERNATIVE() may depend on a feature that is disabled at
compile time by a Kconfig option. In this case the unused alternative
instructions waste space, and if the original instruction is a nop, it
wastes time and space.

This patch adds an optional 'config' option to ALTERNATIVE() and
alternative_insn that allows the compiler to remove both the original
and alternative instructions if the config option is not defined.

Signed-off-by: James Morse <james.morse@arm.com>
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/alternative.h | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index c385a0c4057f..5598182dea28 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -3,6 +3,7 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/kconfig.h>
 #include <linux/types.h>
 #include <linux/stddef.h>
 #include <linux/stringify.h>
@@ -40,7 +41,8 @@ void free_alternatives_memory(void);
  * be fixed in a binutils release posterior to 2.25.51.0.2 (anything
  * containing commit 4e4d08cf7399b606 or c1baaddf8861).
  */
-#define ALTERNATIVE(oldinstr, newinstr, feature)			\
+#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled)	\
+	".if "__stringify(cfg_enabled)" == 1\n"				\
 	"661:\n\t"							\
 	oldinstr "\n"							\
 	"662:\n"							\
@@ -53,7 +55,11 @@ void free_alternatives_memory(void);
 	"664:\n\t"							\
 	".popsection\n\t"						\
 	".org	. - (664b-663b) + (662b-661b)\n\t"			\
-	".org	. - (662b-661b) + (664b-663b)\n"
+	".org	. - (662b-661b) + (664b-663b)\n"			\
+	".endif\n"
+
+#define _ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg, ...)	\
+	__ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg))
 
 #else
 
@@ -65,7 +71,8 @@ void free_alternatives_memory(void);
 	.byte \alt_len
 .endm
 
-.macro alternative_insn insn1 insn2 cap
+.macro alternative_insn insn1, insn2, cap, enable = 1
+	.if \enable
 661:	\insn1
 662:	.pushsection .altinstructions, "a"
 	altinstruction_entry 661b, 663f, \cap, 662b-661b, 664f-663f
@@ -75,8 +82,23 @@ void free_alternatives_memory(void);
 664:	.popsection
 	.org	. - (664b-663b) + (662b-661b)
 	.org	. - (662b-661b) + (664b-663b)
+	.endif
 .endm
 
+#define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)	\
+	alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
+
+
 #endif  /*  __ASSEMBLY__  */
 
+/*
+ * Usage: asm(ALTERNATIVE(oldinstr, newinstr, feature));
+ *
+ * Usage: asm(ALTERNATIVE(oldinstr, newinstr, feature, CONFIG_FOO));
+ * N.B. If CONFIG_FOO is specified, but not selected, the whole block
+ *      will be omitted, including oldinstr.
+ */
+#define ALTERNATIVE(oldinstr, newinstr, ...)   \
+	_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
+
 #endif /* __ASM_ALTERNATIVE_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 6/6] arm64: kernel: Add support for Privileged Access Never
  2015-07-17 17:30 [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never James Morse
                   ` (4 preceding siblings ...)
  2015-07-17 17:30 ` [PATCH v2 5/6] arm64: kernel: Add optional CONFIG_ parameter to ALTERNATIVE() James Morse
@ 2015-07-17 17:30 ` James Morse
  2015-07-20 14:01   ` Catalin Marinas
  2015-07-21 10:30   ` Suzuki K. Poulose
  2015-07-20 11:02 ` [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never Vladimir Murzin
  6 siblings, 2 replies; 19+ messages in thread
From: James Morse @ 2015-07-17 17:30 UTC (permalink / raw)
  To: linux-arm-kernel

'Privileged Access Never' is a new arm8.1 feature which prevents
privileged code from accessing any virtual address where read or write
access is also permitted at EL0.

This patch enables the PAN feature on all CPUs, and modifies {get,put}_user
helpers temporarily to permit access.

This will catch kernel bugs where user memory is accessed directly.
'Unprivileged loads and stores' using ldtrb et al are unaffected by PAN.

Signed-off-by: James Morse <james.morse@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/Kconfig                   | 14 ++++++++++++++
 arch/arm64/include/asm/cpufeature.h  |  3 ++-
 arch/arm64/include/asm/futex.h       |  8 ++++++++
 arch/arm64/include/asm/processor.h   |  2 ++
 arch/arm64/include/asm/sysreg.h      |  9 +++++++++
 arch/arm64/include/asm/uaccess.h     | 11 +++++++++++
 arch/arm64/include/uapi/asm/ptrace.h |  1 +
 arch/arm64/kernel/cpufeature.c       | 20 ++++++++++++++++++++
 arch/arm64/lib/clear_user.S          |  8 ++++++++
 arch/arm64/lib/copy_from_user.S      |  8 ++++++++
 arch/arm64/lib/copy_in_user.S        |  8 ++++++++
 arch/arm64/lib/copy_to_user.S        |  8 ++++++++
 arch/arm64/mm/fault.c                | 23 +++++++++++++++++++++++
 13 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 318175f62c24..c53a4b1d5968 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -597,6 +597,20 @@ config FORCE_MAX_ZONEORDER
 	default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
 	default "11"
 
+config ARM64_PAN
+	bool "Enable support for Privileged Access Never (PAN)"
+	default y
+	help
+	 Privileged Access Never (PAN; part of the ARMv8.1 Extensions)
+	 prevents the kernel or hypervisor from accessing user-space (EL0)
+	 memory directly.
+
+	 Choosing this option will cause any unprotected (not using
+	 copy_to_user et al) memory access to fail with a permission fault.
+
+	 The feature is detected at runtime, and will remain as a 'nop'
+	 instruction if the cpu does not implement the feature.
+
 menuconfig ARMV8_DEPRECATED
 	bool "Emulate deprecated/obsolete ARMv8 instructions"
 	depends on COMPAT
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index ef38e21ed719..420329a1b98f 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -25,8 +25,9 @@
 #define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE	1
 #define ARM64_WORKAROUND_845719			2
 #define ARM64_HAS_SYSREG_GIC_CPUIF		3
+#define ARM64_HAS_PAN				4
 
-#define ARM64_NCAPS				4
+#define ARM64_NCAPS				5
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
index 74069b3bd919..775e85b9d1f2 100644
--- a/arch/arm64/include/asm/futex.h
+++ b/arch/arm64/include/asm/futex.h
@@ -20,10 +20,16 @@
 
 #include <linux/futex.h>
 #include <linux/uaccess.h>
+
+#include <asm/alternative.h>
+#include <asm/cpufeature.h>
 #include <asm/errno.h>
+#include <asm/sysreg.h>
 
 #define __futex_atomic_op(insn, ret, oldval, uaddr, tmp, oparg)		\
 	asm volatile(							\
+	ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN,		\
+		    CONFIG_ARM64_PAN)					\
 "1:	ldxr	%w1, %2\n"						\
 	insn "\n"							\
 "2:	stlxr	%w3, %w0, %2\n"						\
@@ -39,6 +45,8 @@
 "	.align	3\n"							\
 "	.quad	1b, 4b, 2b, 4b\n"					\
 "	.popsection\n"							\
+	ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN,		\
+		    CONFIG_ARM64_PAN)					\
 	: "=&r" (ret), "=&r" (oldval), "+Q" (*uaddr), "=&r" (tmp)	\
 	: "r" (oparg), "Ir" (-EFAULT)					\
 	: "memory")
diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
index e4c893e54f01..98f32355dc97 100644
--- a/arch/arm64/include/asm/processor.h
+++ b/arch/arm64/include/asm/processor.h
@@ -186,4 +186,6 @@ static inline void spin_lock_prefetch(const void *x)
 
 #endif
 
+void cpu_enable_pan(void);
+
 #endif /* __ASM_PROCESSOR_H */
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 56391fbae1e1..f243bb1adaa5 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -20,12 +20,21 @@
 #ifndef __ASM_SYSREG_H
 #define __ASM_SYSREG_H
 
+#include <asm/opcodes.h>
+
 #define SCTLR_EL1_CP15BEN	(0x1 << 5)
 #define SCTLR_EL1_SED		(0x1 << 8)
 
 #define sys_reg(op0, op1, crn, crm, op2) \
 	((((op0)-2)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
 
+#define REG_PSTATE_PAN_IMM                     sys_reg(2, 0, 4, 0, 4)
+#define PSTATE_PAN                             (1 << 22)
+#define SCTLR_EL1_SPAN                         (1 << 23)
+
+#define SET_PSTATE_PAN(x) __inst_arm(0xd5000000 | REG_PSTATE_PAN_IMM |\
+				     (!!x)<<8 | 0x1f)
+
 #ifdef __ASSEMBLY__
 
 	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 07e1ba449bf1..b2ede967fe7d 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -24,7 +24,10 @@
 #include <linux/string.h>
 #include <linux/thread_info.h>
 
+#include <asm/alternative.h>
+#include <asm/cpufeature.h>
 #include <asm/ptrace.h>
+#include <asm/sysreg.h>
 #include <asm/errno.h>
 #include <asm/memory.h>
 #include <asm/compiler.h>
@@ -131,6 +134,8 @@ static inline void set_fs(mm_segment_t fs)
 do {									\
 	unsigned long __gu_val;						\
 	__chk_user_ptr(ptr);						\
+	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN,	\
+			CONFIG_ARM64_PAN));				\
 	switch (sizeof(*(ptr))) {					\
 	case 1:								\
 		__get_user_asm("ldrb", "%w", __gu_val, (ptr), (err));	\
@@ -148,6 +153,8 @@ do {									\
 		BUILD_BUG();						\
 	}								\
 	(x) = (__force __typeof__(*(ptr)))__gu_val;			\
+	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN,	\
+			CONFIG_ARM64_PAN));				\
 } while (0)
 
 #define __get_user(x, ptr)						\
@@ -194,6 +201,8 @@ do {									\
 do {									\
 	__typeof__(*(ptr)) __pu_val = (x);				\
 	__chk_user_ptr(ptr);						\
+	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN,	\
+			CONFIG_ARM64_PAN));				\
 	switch (sizeof(*(ptr))) {					\
 	case 1:								\
 		__put_user_asm("strb", "%w", __pu_val, (ptr), (err));	\
@@ -210,6 +219,8 @@ do {									\
 	default:							\
 		BUILD_BUG();						\
 	}								\
+	asm(ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN,	\
+			CONFIG_ARM64_PAN));				\
 } while (0)
 
 #define __put_user(x, ptr)						\
diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index 6913643bbe54..208db3df135a 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -44,6 +44,7 @@
 #define PSR_I_BIT	0x00000080
 #define PSR_A_BIT	0x00000100
 #define PSR_D_BIT	0x00000200
+#define PSR_PAN_BIT	0x00400000
 #define PSR_Q_BIT	0x08000000
 #define PSR_V_BIT	0x10000000
 #define PSR_C_BIT	0x20000000
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index cc26d6c6520b..919dd77bbc5c 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -21,6 +21,7 @@
 #include <linux/types.h>
 #include <asm/cpu.h>
 #include <asm/cpufeature.h>
+#include <asm/processor.h>
 
 static bool
 feature_matches(u64 reg, const struct arm64_cpu_capabilities *entry)
@@ -39,6 +40,15 @@ has_id_aa64pfr0_feature(const struct arm64_cpu_capabilities *entry)
 	return feature_matches(val, entry);
 }
 
+static bool __maybe_unused
+has_id_aa64mmfr1_feature(const struct arm64_cpu_capabilities *entry)
+{
+	u64 val;
+
+	val = read_cpuid(id_aa64mmfr1_el1);
+	return feature_matches(val, entry);
+}
+
 static const struct arm64_cpu_capabilities arm64_features[] = {
 	{
 		.desc = "GIC system register CPU interface",
@@ -47,6 +57,16 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.register_shift = 24,
 		.min_register_value = 1,
 	},
+#ifdef CONFIG_ARM64_PAN
+	{
+		.desc = "Privileged Access Never",
+		.capability = ARM64_HAS_PAN,
+		.matches = has_id_aa64mmfr1_feature,
+		.register_shift = 20,
+		.min_register_value = 1,
+		.enable = cpu_enable_pan,
+	},
+#endif /* CONFIG_ARM64_PAN */
 	{},
 };
 
diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
index c17967fdf5f6..a9723c71c52b 100644
--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S
@@ -16,7 +16,11 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 #include <linux/linkage.h>
+
+#include <asm/alternative.h>
 #include <asm/assembler.h>
+#include <asm/cpufeature.h>
+#include <asm/sysreg.h>
 
 	.text
 
@@ -29,6 +33,8 @@
  * Alignment fixed up by hardware.
  */
 ENTRY(__clear_user)
+ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_HAS_PAN, \
+	    CONFIG_ARM64_PAN)
 	mov	x2, x1			// save the size for fixup return
 	subs	x1, x1, #8
 	b.mi	2f
@@ -48,6 +54,8 @@ USER(9f, strh	wzr, [x0], #2	)
 	b.mi	5f
 USER(9f, strb	wzr, [x0]	)
 5:	mov	x0, #0
+ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_HAS_PAN, \
+	    CONFIG_ARM64_PAN)
 	ret
 ENDPROC(__clear_user)
 
diff --git a/arch/arm64/lib/copy_from_user.S b/arch/arm64/lib/copy_from_user.S
index 5e27add9d362..882c1544a73e 100644
--- a/arch/arm64/lib/copy_from_user.S
+++ b/arch/arm64/lib/copy_from_user.S
@@ -15,7 +15,11 @@
  */
 
 #include <linux/linkage.h>
+
+#include <asm/alternative.h>
 #include <asm/assembler.h>
+#include <asm/cpufeature.h>
+#include <asm/sysreg.h>
 
 /*
  * Copy from user space to a kernel buffer (alignment handled by the hardware)
@@ -28,6 +32,8 @@
  *	x0 - bytes not copied
  */
 ENTRY(__copy_from_user)
+ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_HAS_PAN, \
+	    CONFIG_ARM64_PAN)
 	add	x4, x1, x2			// upper user buffer boundary
 	subs	x2, x2, #8
 	b.mi	2f
@@ -51,6 +57,8 @@ USER(9f, ldrh	w3, [x1], #2	)
 USER(9f, ldrb	w3, [x1]	)
 	strb	w3, [x0]
 5:	mov	x0, #0
+ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_HAS_PAN, \
+	    CONFIG_ARM64_PAN)
 	ret
 ENDPROC(__copy_from_user)
 
diff --git a/arch/arm64/lib/copy_in_user.S b/arch/arm64/lib/copy_in_user.S
index 84b6c9bb9b93..97063c4cba75 100644
--- a/arch/arm64/lib/copy_in_user.S
+++ b/arch/arm64/lib/copy_in_user.S
@@ -17,7 +17,11 @@
  */
 
 #include <linux/linkage.h>
+
+#include <asm/alternative.h>
 #include <asm/assembler.h>
+#include <asm/cpufeature.h>
+#include <asm/sysreg.h>
 
 /*
  * Copy from user space to user space (alignment handled by the hardware)
@@ -30,6 +34,8 @@
  *	x0 - bytes not copied
  */
 ENTRY(__copy_in_user)
+ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_HAS_PAN, \
+	    CONFIG_ARM64_PAN)
 	add	x4, x0, x2			// upper user buffer boundary
 	subs	x2, x2, #8
 	b.mi	2f
@@ -53,6 +59,8 @@ USER(9f, strh	w3, [x0], #2	)
 USER(9f, ldrb	w3, [x1]	)
 USER(9f, strb	w3, [x0]	)
 5:	mov	x0, #0
+ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_HAS_PAN, \
+	    CONFIG_ARM64_PAN)
 	ret
 ENDPROC(__copy_in_user)
 
diff --git a/arch/arm64/lib/copy_to_user.S b/arch/arm64/lib/copy_to_user.S
index a0aeeb9b7a28..c782aaf5494d 100644
--- a/arch/arm64/lib/copy_to_user.S
+++ b/arch/arm64/lib/copy_to_user.S
@@ -15,7 +15,11 @@
  */
 
 #include <linux/linkage.h>
+
+#include <asm/alternative.h>
 #include <asm/assembler.h>
+#include <asm/cpufeature.h>
+#include <asm/sysreg.h>
 
 /*
  * Copy to user space from a kernel buffer (alignment handled by the hardware)
@@ -28,6 +32,8 @@
  *	x0 - bytes not copied
  */
 ENTRY(__copy_to_user)
+ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(0)), ARM64_HAS_PAN, \
+	    CONFIG_ARM64_PAN)
 	add	x4, x0, x2			// upper user buffer boundary
 	subs	x2, x2, #8
 	b.mi	2f
@@ -51,6 +57,8 @@ USER(9f, strh	w3, [x0], #2	)
 	ldrb	w3, [x1]
 USER(9f, strb	w3, [x0]	)
 5:	mov	x0, #0
+ALTERNATIVE("nop", __stringify(SET_PSTATE_PAN(1)), ARM64_HAS_PAN, \
+	    CONFIG_ARM64_PAN)
 	ret
 ENDPROC(__copy_to_user)
 
diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
index 94d98cd1aad8..3c10dcf1537b 100644
--- a/arch/arm64/mm/fault.c
+++ b/arch/arm64/mm/fault.c
@@ -30,9 +30,11 @@
 #include <linux/highmem.h>
 #include <linux/perf_event.h>
 
+#include <asm/cpufeature.h>
 #include <asm/exception.h>
 #include <asm/debug-monitors.h>
 #include <asm/esr.h>
+#include <asm/sysreg.h>
 #include <asm/system_misc.h>
 #include <asm/pgtable.h>
 #include <asm/tlbflush.h>
@@ -147,6 +149,13 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re
 		__do_kernel_fault(mm, addr, esr, regs);
 }
 
+static bool pan_enabled(struct pt_regs *regs)
+{
+	if (IS_ENABLED(CONFIG_ARM64_PAN))
+		return ((regs->pstate & PSTATE_PAN) != 0);
+	return false;
+}
+
 #define VM_FAULT_BADMAP		0x010000
 #define VM_FAULT_BADACCESS	0x020000
 
@@ -224,6 +233,13 @@ static int __kprobes do_page_fault(unsigned long addr, unsigned int esr,
 	}
 
 	/*
+	 * PAN bit set implies the fault happened in kernel space, but not
+	 * in the arch's user access functions.
+	 */
+	if (pan_enabled(regs))
+		goto no_context;
+
+	/*
 	 * As per x86, we may deadlock here. However, since the kernel only
 	 * validly references user space from well defined areas of the code,
 	 * we can bug out early if this is from code which shouldn't.
@@ -536,3 +552,10 @@ asmlinkage int __exception do_debug_exception(unsigned long addr,
 
 	return 0;
 }
+
+#ifdef CONFIG_ARM64_PAN
+void cpu_enable_pan(void)
+{
+	config_sctlr_el1(SCTLR_EL1_SPAN, 0);
+}
+#endif /* CONFIG_ARM64_PAN */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never
  2015-07-17 17:30 [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never James Morse
                   ` (5 preceding siblings ...)
  2015-07-17 17:30 ` [PATCH v2 6/6] arm64: kernel: Add support for Privileged Access Never James Morse
@ 2015-07-20 11:02 ` Vladimir Murzin
  2015-07-20 11:17   ` Catalin Marinas
  6 siblings, 1 reply; 19+ messages in thread
From: Vladimir Murzin @ 2015-07-20 11:02 UTC (permalink / raw)
  To: linux-arm-kernel

On 17/07/15 18:30, James Morse wrote:
> This series adds support for Privileged Access Never (PAN; part of the ARMv8.1
> Extensions). When enabled, this feature causes a permission fault if the kernel
> attempts to access memory that is also accessible by userspace - instead the
> PAN bit must be cleared when accessing userspace memory. (or use the
> ldt*/stt* instructions).
> 
> This series detects and enables this feature, and uses alternatives to change
> {get,put}_user() et al to clear the PAN bit while they do their work.
> 
> Changes since v1:
> * Copied cpuid_feature_extract_field() from arch/arm as a new patch, suggested
>   by Russell King [1].
> * Changed feature-detection patch to use cpuid_feature_extract_field() for sign
>   extension, and '>='.
> * Moved SCTLR_EL1_* from asm/cputype.h to asm/sysreg.h
> * Added PSR_PAN_BIT in uapi/asm/ptrace.h
> * Removed the setting of PSTATE_PAN in kernel/process.c
> 
> [1] http://www.spinics.net/lists/arm-kernel/msg432518.html
> 

Hi James,

Looks like it doesn't play nicely with CONFIG_SWP_EMULATION=y. I get
segfault while executing swp and PAN is enabled, though I've been
playing with one of the older version.

Vladimir

> 
> James Morse (6):
>   arm64: kernel: Add cpuid_feature_extract_field() for 4bit sign
>     extension
>   arm64: kernel: preparatory: Move config_sctlr_el1
>   arm64: kernel: Add cpufeature 'enable' callback
>   arm64: kernel: Add min_register_value and use '>=' for feature
>     detection
>   arm64: kernel: Add optional CONFIG_ parameter to ALTERNATIVE()
>   arm64: kernel: Add support for Privileged Access Never
> 
>  arch/arm64/Kconfig                   | 14 +++++++++++++
>  arch/arm64/include/asm/alternative.h | 28 ++++++++++++++++++++++---
>  arch/arm64/include/asm/cpufeature.h  | 21 ++++++++++++++++---
>  arch/arm64/include/asm/cputype.h     |  3 ---
>  arch/arm64/include/asm/futex.h       |  8 ++++++++
>  arch/arm64/include/asm/processor.h   |  2 ++
>  arch/arm64/include/asm/sysreg.h      | 21 +++++++++++++++++++
>  arch/arm64/include/asm/uaccess.h     | 11 ++++++++++
>  arch/arm64/include/uapi/asm/ptrace.h |  1 +
>  arch/arm64/kernel/armv8_deprecated.c | 11 +---------
>  arch/arm64/kernel/cpufeature.c       | 40 +++++++++++++++++++++++++++++++++---
>  arch/arm64/lib/clear_user.S          |  8 ++++++++
>  arch/arm64/lib/copy_from_user.S      |  8 ++++++++
>  arch/arm64/lib/copy_in_user.S        |  8 ++++++++
>  arch/arm64/lib/copy_to_user.S        |  8 ++++++++
>  arch/arm64/mm/fault.c                | 23 +++++++++++++++++++++
>  16 files changed, 193 insertions(+), 22 deletions(-)
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never
  2015-07-20 11:02 ` [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never Vladimir Murzin
@ 2015-07-20 11:17   ` Catalin Marinas
  0 siblings, 0 replies; 19+ messages in thread
From: Catalin Marinas @ 2015-07-20 11:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 20, 2015 at 12:02:06PM +0100, Vladimir Murzin wrote:
> On 17/07/15 18:30, James Morse wrote:
> > This series adds support for Privileged Access Never (PAN; part of the ARMv8.1
> > Extensions). When enabled, this feature causes a permission fault if the kernel
> > attempts to access memory that is also accessible by userspace - instead the
> > PAN bit must be cleared when accessing userspace memory. (or use the
> > ldt*/stt* instructions).
> > 
> > This series detects and enables this feature, and uses alternatives to change
> > {get,put}_user() et al to clear the PAN bit while they do their work.
> > 
> > Changes since v1:
> > * Copied cpuid_feature_extract_field() from arch/arm as a new patch, suggested
> >   by Russell King [1].
> > * Changed feature-detection patch to use cpuid_feature_extract_field() for sign
> >   extension, and '>='.
> > * Moved SCTLR_EL1_* from asm/cputype.h to asm/sysreg.h
> > * Added PSR_PAN_BIT in uapi/asm/ptrace.h
> > * Removed the setting of PSTATE_PAN in kernel/process.c
> > 
> > [1] http://www.spinics.net/lists/arm-kernel/msg432518.html
> 
> Looks like it doesn't play nicely with CONFIG_SWP_EMULATION=y. I get
> segfault while executing swp and PAN is enabled, though I've been
> playing with one of the older version.

Good catch. It looks like __user_swpX_asm in
arch/arm64/kernel/armv8_deprecated.c needs PAN clearing.

-- 
Catalin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/6] arm64: kernel: Add cpuid_feature_extract_field() for 4bit sign extension
  2015-07-17 17:30 ` [PATCH v2 1/6] arm64: kernel: Add cpuid_feature_extract_field() for 4bit sign extension James Morse
@ 2015-07-20 11:32   ` Catalin Marinas
  2015-07-20 13:20     ` Catalin Marinas
  0 siblings, 1 reply; 19+ messages in thread
From: Catalin Marinas @ 2015-07-20 11:32 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 17, 2015 at 06:30:16PM +0100, James Morse wrote:
> Copied from arch/arm/include/asm/cputype.h, this function does the
> shifting and sign extension necessary when accessing cpu feature fields.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Suggested-by: Russell King <linux@arm.linux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/cpufeature.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index c1044218a63a..56c843918504 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -70,6 +70,19 @@ static inline void cpus_set_cap(unsigned int num)
>  		__set_bit(num, cpu_hwcaps);
>  }
>  
> +static inline int __attribute_const__ cpuid_feature_extract_field(u64 features,
> +								  int field)
> +{
> +	int feature = (features >> field) & 15;
> +
> +	/* feature registers are signed values */
> +	if (feature > 8)
> +		feature -= 16;
> +
> +	return feature;
> +}

It may generate better code like, especially when field is a constant:

	return (s64)(feature << (64 - 4 - field)) >> (64 - 4);

(in asm I think we could do it with a single instruction but the above
shouldn't be that bad either).

-- 
Catalin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/6] arm64: kernel: Add cpuid_feature_extract_field() for 4bit sign extension
  2015-07-20 11:32   ` Catalin Marinas
@ 2015-07-20 13:20     ` Catalin Marinas
  0 siblings, 0 replies; 19+ messages in thread
From: Catalin Marinas @ 2015-07-20 13:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 20, 2015 at 12:32:37PM +0100, Catalin Marinas wrote:
> On Fri, Jul 17, 2015 at 06:30:16PM +0100, James Morse wrote:
> > Copied from arch/arm/include/asm/cputype.h, this function does the
> > shifting and sign extension necessary when accessing cpu feature fields.
> > 
> > Signed-off-by: James Morse <james.morse@arm.com>
> > Suggested-by: Russell King <linux@arm.linux.org.uk>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm64/include/asm/cpufeature.h | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> > index c1044218a63a..56c843918504 100644
> > --- a/arch/arm64/include/asm/cpufeature.h
> > +++ b/arch/arm64/include/asm/cpufeature.h
> > @@ -70,6 +70,19 @@ static inline void cpus_set_cap(unsigned int num)
> >  		__set_bit(num, cpu_hwcaps);
> >  }
> >  
> > +static inline int __attribute_const__ cpuid_feature_extract_field(u64 features,
> > +								  int field)
> > +{
> > +	int feature = (features >> field) & 15;
> > +
> > +	/* feature registers are signed values */
> > +	if (feature > 8)
> > +		feature -= 16;
> > +
> > +	return feature;
> > +}
> 
> It may generate better code like, especially when field is a constant:
> 
> 	return (s64)(feature << (64 - 4 - field)) >> (64 - 4);
> 
> (in asm I think we could do it with a single instruction but the above
> shouldn't be that bad either).

gcc managed to impress me ;). It detects the left-right shifts and
inlines "sbfx xn, xn, #field, #4" when the field is a constant.

-- 
Catalin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 4/6] arm64: kernel: Add min_register_value and use '>=' for feature detection
  2015-07-17 17:30 ` [PATCH v2 4/6] arm64: kernel: Add min_register_value and use '>=' for feature detection James Morse
@ 2015-07-20 13:53   ` Catalin Marinas
  0 siblings, 0 replies; 19+ messages in thread
From: Catalin Marinas @ 2015-07-20 13:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 17, 2015 at 06:30:19PM +0100, James Morse wrote:
> When a new cpu feature is available, the cpu feature bits will have some
> initial value, which is incremented when the feature is updated.
> This patch changes 'register_value' to be 'min_register_value', and checks
> the feature bits value (interpreted as a signed int) is greater than this
> minimum.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/include/asm/cpufeature.h |  4 ++--
>  arch/arm64/kernel/cpufeature.c      | 14 +++++++++++---
>  2 files changed, 13 insertions(+), 5 deletions(-)

I think we could move patch 1 into this patch.

> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 7456fc44bee1..ef38e21ed719 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -42,8 +42,8 @@ struct arm64_cpu_capabilities {
>  		};
>  
>  		struct {	/* Feature register checking */
> -			u64 register_mask;
> -			u64 register_value;
> +			u8 register_shift;
> +			u8 min_register_value;

Nitpick: since they are no longer register values/shifts, can we call
them field_value and field_position (or just field_pos)?

>  		};
>  	};
>  };
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 650ffc28bedc..cc26d6c6520b 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -23,12 +23,20 @@
>  #include <asm/cpufeature.h>
>  
>  static bool
> +feature_matches(u64 reg, const struct arm64_cpu_capabilities *entry)
> +{
> +	int val = cpuid_feature_extract_field(reg, entry->register_shift);
> +
> +	return (val >= entry->min_register_value);
> +}

We should rather use an int type for min_register_value (and
register_shift) to avoid confusion with C comparison of signed vs
unsigned.

Nitpick: no brackets needed after return.

-- 
Catalin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 6/6] arm64: kernel: Add support for Privileged Access Never
  2015-07-17 17:30 ` [PATCH v2 6/6] arm64: kernel: Add support for Privileged Access Never James Morse
@ 2015-07-20 14:01   ` Catalin Marinas
  2015-07-21 10:30   ` Suzuki K. Poulose
  1 sibling, 0 replies; 19+ messages in thread
From: Catalin Marinas @ 2015-07-20 14:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 17, 2015 at 06:30:21PM +0100, James Morse wrote:
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 56391fbae1e1..f243bb1adaa5 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -20,12 +20,21 @@
>  #ifndef __ASM_SYSREG_H
>  #define __ASM_SYSREG_H
>  
> +#include <asm/opcodes.h>
> +
>  #define SCTLR_EL1_CP15BEN	(0x1 << 5)
>  #define SCTLR_EL1_SED		(0x1 << 8)
>  
>  #define sys_reg(op0, op1, crn, crm, op2) \
>  	((((op0)-2)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
>  
> +#define REG_PSTATE_PAN_IMM                     sys_reg(2, 0, 4, 0, 4)
> +#define PSTATE_PAN                             (1 << 22)

[...]

> diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
> index 6913643bbe54..208db3df135a 100644
> --- a/arch/arm64/include/uapi/asm/ptrace.h
> +++ b/arch/arm64/include/uapi/asm/ptrace.h
> @@ -44,6 +44,7 @@
>  #define PSR_I_BIT	0x00000080
>  #define PSR_A_BIT	0x00000100
>  #define PSR_D_BIT	0x00000200
> +#define PSR_PAN_BIT	0x00400000

What I meant is to use PSR_PAN_BIT instead of PSTATE_PAN, not just to
define it (i.e. remove the latter).

-- 
Catalin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 6/6] arm64: kernel: Add support for Privileged Access Never
  2015-07-17 17:30 ` [PATCH v2 6/6] arm64: kernel: Add support for Privileged Access Never James Morse
  2015-07-20 14:01   ` Catalin Marinas
@ 2015-07-21 10:30   ` Suzuki K. Poulose
  2015-07-21 11:37     ` Catalin Marinas
  1 sibling, 1 reply; 19+ messages in thread
From: Suzuki K. Poulose @ 2015-07-21 10:30 UTC (permalink / raw)
  To: linux-arm-kernel

On 17/07/15 18:30, James Morse wrote:
> 'Privileged Access Never' is a new arm8.1 feature which prevents
> privileged code from accessing any virtual address where read or write
> access is also permitted at EL0.
>
> This patch enables the PAN feature on all CPUs, and modifies {get,put}_user
> helpers temporarily to permit access.
>
> This will catch kernel bugs where user memory is accessed directly.
> 'Unprivileged loads and stores' using ldtrb et al are unaffected by PAN.
>
> Signed-off-by: James Morse <james.morse@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>   arch/arm64/Kconfig                   | 14 ++++++++++++++
>   arch/arm64/include/asm/cpufeature.h  |  3 ++-
>   arch/arm64/include/asm/futex.h       |  8 ++++++++
>   arch/arm64/include/asm/processor.h   |  2 ++
>   arch/arm64/include/asm/sysreg.h      |  9 +++++++++
>   arch/arm64/include/asm/uaccess.h     | 11 +++++++++++
>   arch/arm64/include/uapi/asm/ptrace.h |  1 +
>   arch/arm64/kernel/cpufeature.c       | 20 ++++++++++++++++++++
>   arch/arm64/lib/clear_user.S          |  8 ++++++++
>   arch/arm64/lib/copy_from_user.S      |  8 ++++++++
>   arch/arm64/lib/copy_in_user.S        |  8 ++++++++
>   arch/arm64/lib/copy_to_user.S        |  8 ++++++++
>   arch/arm64/mm/fault.c                | 23 +++++++++++++++++++++++
>   13 files changed, 122 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 318175f62c24..c53a4b1d5968 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -597,6 +597,20 @@ config FORCE_MAX_ZONEORDER
>          default "14" if (ARM64_64K_PAGES && TRANSPARENT_HUGEPAGE)
>          default "11"
>
> +config ARM64_PAN
> +       bool "Enable support for Privileged Access Never (PAN)"
> +       default y
> +       help
> +        Privileged Access Never (PAN; part of the ARMv8.1 Extensions)
> +        prevents the kernel or hypervisor from accessing user-space (EL0)
> +        memory directly.
> +
> +        Choosing this option will cause any unprotected (not using
> +        copy_to_user et al) memory access to fail with a permission fault.
> +
> +        The feature is detected at runtime, and will remain as a 'nop'
> +        instruction if the cpu does not implement the feature.
> +
>   menuconfig ARMV8_DEPRECATED
>          bool "Emulate deprecated/obsolete ARMv8 instructions"
>          depends on COMPAT
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index ef38e21ed719..420329a1b98f 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -25,8 +25,9 @@
>   #define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE   1
>   #define ARM64_WORKAROUND_845719                        2
>   #define ARM64_HAS_SYSREG_GIC_CPUIF             3
> +#define ARM64_HAS_PAN                          4
>
> -#define ARM64_NCAPS                            4
> +#define ARM64_NCAPS                            5
>
>   #ifndef __ASSEMBLY__
>
> diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> index 74069b3bd919..775e85b9d1f2 100644
> --- a/arch/arm64/include/asm/futex.h
> +++ b/arch/arm64/include/asm/futex.h
> @@ -20,10 +20,16 @@
>
>   #include <linux/futex.h>
>   #include <linux/uaccess.h>
> +
> +#include <asm/alternative.h>
> +#include <asm/cpufeature.h>
>   #include <asm/errno.h>
> +#include <asm/sysreg.h>
>
>   #define __futex_atomic_op(insn, ret, oldval, uaddr, tmp, oparg)                \
>          asm volatile(                                                   \
> +       ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN,            \
> +                   CONFIG_ARM64_PAN)                                   \
>   "1:    ldxr    %w1, %2\n"                                              \
>          insn "\n"                                                       \
>   "2:    stlxr   %w3, %w0, %2\n"                                         \
> @@ -39,6 +45,8 @@
>   "      .align  3\n"                                                    \
>   "      .quad   1b, 4b, 2b, 4b\n"                                       \
>   "      .popsection\n"                                                  \
> +       ALTERNATIVE("nop", SET_PSTATE_PAN(1), ARM64_HAS_PAN,            \
> +                   CONFIG_ARM64_PAN)                                   \
>          : "=&r" (ret), "=&r" (oldval), "+Q" (*uaddr), "=&r" (tmp)       \
>          : "r" (oparg), "Ir" (-EFAULT)                                   \
>          : "memory")
> diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> index e4c893e54f01..98f32355dc97 100644
> --- a/arch/arm64/include/asm/processor.h
> +++ b/arch/arm64/include/asm/processor.h
> @@ -186,4 +186,6 @@ static inline void spin_lock_prefetch(const void *x)
>
>   #endif
>
> +void cpu_enable_pan(void);
> +
>   #endif /* __ASM_PROCESSOR_H */
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 56391fbae1e1..f243bb1adaa5 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -20,12 +20,21 @@
>   #ifndef __ASM_SYSREG_H
>   #define __ASM_SYSREG_H
>
> +#include <asm/opcodes.h>
> +
>   #define SCTLR_EL1_CP15BEN      (0x1 << 5)
>   #define SCTLR_EL1_SED          (0x1 << 8)
>
>   #define sys_reg(op0, op1, crn, crm, op2) \
>          ((((op0)-2)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
>
> +#define REG_PSTATE_PAN_IMM                     sys_reg(2, 0, 4, 0, 4)

I think the above encoding is incorrect (even though, the code works fine).
While setting the PAN with an immediate value, the PAN is treated just like
a Process state field and the encoding becomes:
  Op0=0, Op1=0 ...
The encoding 2,0 ,... works fine in this case due to a bug in the sys_reg()
macro above, where op0 is encoded as (op0 - 2). I took a look at the ARMv8 ARM,
section C5.2.{3, 4, 5, 6} and the system instruction class reserves bits[20-19] for Op0.
I think we should fix that first and use the appropriate encoding mandated by the
architecture to avoid further errors.


> +#define PSTATE_PAN                             (1 << 22)
> +#define SCTLR_EL1_SPAN                         (1 << 23)
> +
> +#define SET_PSTATE_PAN(x) __inst_arm(0xd5000000 | REG_PSTATE_PAN_IMM |\
> +                                    (!!x)<<8 | 0x1f)


Thanks
Suzuki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 6/6] arm64: kernel: Add support for Privileged Access Never
  2015-07-21 10:30   ` Suzuki K. Poulose
@ 2015-07-21 11:37     ` Catalin Marinas
  2015-07-22  9:54       ` [PATCH] arm64: sys_reg() : Fix encoding of system registers Suzuki K. Poulose
  0 siblings, 1 reply; 19+ messages in thread
From: Catalin Marinas @ 2015-07-21 11:37 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Jul 21, 2015 at 11:30:08AM +0100, Suzuki K. Poulose wrote:
> On 17/07/15 18:30, James Morse wrote:
> >diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> >index 56391fbae1e1..f243bb1adaa5 100644
> >--- a/arch/arm64/include/asm/sysreg.h
> >+++ b/arch/arm64/include/asm/sysreg.h
> >@@ -20,12 +20,21 @@
> >  #ifndef __ASM_SYSREG_H
> >  #define __ASM_SYSREG_H
> >
> >+#include <asm/opcodes.h>
> >+
> >  #define SCTLR_EL1_CP15BEN      (0x1 << 5)
> >  #define SCTLR_EL1_SED          (0x1 << 8)
> >
> >  #define sys_reg(op0, op1, crn, crm, op2) \
> >         ((((op0)-2)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
> >
> >+#define REG_PSTATE_PAN_IMM                     sys_reg(2, 0, 4, 0, 4)
> 
> I think the above encoding is incorrect (even though, the code works fine).
> While setting the PAN with an immediate value, the PAN is treated just like
> a Process state field and the encoding becomes:
>  Op0=0, Op1=0 ...
> The encoding 2,0 ,... works fine in this case due to a bug in the sys_reg()
> macro above, where op0 is encoded as (op0 - 2). I took a look at the ARMv8 ARM,
> section C5.2.{3, 4, 5, 6} and the system instruction class reserves bits[20-19] for Op0.
> I think we should fix that first and use the appropriate encoding mandated by the
> architecture to avoid further errors.

The sys_reg() was initially meant for MSR/MRS with a register operand
where op0 is encoded as a single bit (19) and bit 20 is always 1. But
looking at the overall encoding for MSR/MRS in the ARMv8 ARM, I'm happy
to make the sys_reg macro more generic with ((op0 & 3) << 19) together
with changing the 0xd51/0xd53 values in the mrs_s/msr_s macros. Maybe
with a comment as well on the encoding of op0 (I guess you'll post a
patch since you started it ;)).

-- 
Catalin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] arm64: sys_reg() : Fix encoding of system registers
  2015-07-21 11:37     ` Catalin Marinas
@ 2015-07-22  9:54       ` Suzuki K. Poulose
  2015-07-22 10:07         ` Catalin Marinas
  0 siblings, 1 reply; 19+ messages in thread
From: Suzuki K. Poulose @ 2015-07-22  9:54 UTC (permalink / raw)
  To: linux-arm-kernel

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

sys_reg() macro encodes Op0 as (Op0 - 2) and pushes it to Bit 19,
leaving Bit 20 uninitialised. As per ARMv8 ARM, (Ref: ARMv8 ARM,
Section: "System instruction class encoding overview", C5.2,
version:ARM DDI 0487A.f), the instruction encoding reserves
bits [20-19] for Op0. Especially, the sys_reg could give wrong
values for using with MSR(Immediate) form, which is only supported
with Op0 == 0 (e.g, PSTATE.x). Fix this to avoid users getting the
wrong encoding.

Cc: James Morse <james.morse@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
---
 arch/arm64/include/asm/sysreg.h |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 0a579b2..9c5f0f1 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -20,8 +20,18 @@
 #ifndef __ASM_SYSREG_H
 #define __ASM_SYSREG_H
 
+/*
+ * ARMv8 ARM reserves the following encoding for system registers:
+ * (Ref: ARMv8 ARM, Section: "System instruction class encoding overview",
+ *  C5.2, version:ARM DDI 0487A.f)
+ *	[20-19] : Op0
+ *	[18-16] : Op1
+ *	[15-12] : CRn
+ *	[11-8]  : CRm
+ *	[7-5]   : Op2
+ */
 #define sys_reg(op0, op1, crn, crm, op2) \
-	((((op0)-2)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
+	((((op0)&3)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
 
 #ifdef __ASSEMBLY__
 
@@ -31,11 +41,11 @@
 	.equ	__reg_num_xzr, 31
 
 	.macro	mrs_s, rt, sreg
-	.inst	0xd5300000|(\sreg)|(__reg_num_\rt)
+	.inst	0xd5200000|(\sreg)|(__reg_num_\rt)
 	.endm
 
 	.macro	msr_s, sreg, rt
-	.inst	0xd5100000|(\sreg)|(__reg_num_\rt)
+	.inst	0xd5000000|(\sreg)|(__reg_num_\rt)
 	.endm
 
 #else
@@ -47,11 +57,11 @@ asm(
 "	.equ	__reg_num_xzr, 31\n"
 "\n"
 "	.macro	mrs_s, rt, sreg\n"
-"	.inst	0xd5300000|(\\sreg)|(__reg_num_\\rt)\n"
+"	.inst	0xd5200000|(\\sreg)|(__reg_num_\\rt)\n"
 "	.endm\n"
 "\n"
 "	.macro	msr_s, sreg, rt\n"
-"	.inst	0xd5100000|(\\sreg)|(__reg_num_\\rt)\n"
+"	.inst	0xd5000000|(\\sreg)|(__reg_num_\\rt)\n"
 "	.endm\n"
 );
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH] arm64: sys_reg() : Fix encoding of system registers
  2015-07-22  9:54       ` [PATCH] arm64: sys_reg() : Fix encoding of system registers Suzuki K. Poulose
@ 2015-07-22 10:07         ` Catalin Marinas
  2015-07-22 10:20           ` Suzuki K. Poulose
  2015-07-22 10:38           ` [PATCH] arm64: Generalise msr_s/mrs_s operations Suzuki K. Poulose
  0 siblings, 2 replies; 19+ messages in thread
From: Catalin Marinas @ 2015-07-22 10:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jul 22, 2015 at 10:54:17AM +0100, Suzuki K. Poulose wrote:
> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
> 
> sys_reg() macro encodes Op0 as (Op0 - 2) and pushes it to Bit 19,
> leaving Bit 20 uninitialised.

A value of 0 doesn't mean uninitialised and as you noticed, it's set by
the value or'ed onto the sys_reg() returned value in the mrs_s etc.
macros.

So I would rather change the subject to "Generalise encoding of system
registers" and drop the uninitialised paragraph above, maybe replace it
with a statement that current sys_reg() was only meant for MSR/MRS
(register) encodings.

Otherwise, the patch itself is fine.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

-- 
Catalin

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] arm64: sys_reg() : Fix encoding of system registers
  2015-07-22 10:07         ` Catalin Marinas
@ 2015-07-22 10:20           ` Suzuki K. Poulose
  2015-07-22 10:38           ` [PATCH] arm64: Generalise msr_s/mrs_s operations Suzuki K. Poulose
  1 sibling, 0 replies; 19+ messages in thread
From: Suzuki K. Poulose @ 2015-07-22 10:20 UTC (permalink / raw)
  To: linux-arm-kernel

On 22/07/15 11:07, Catalin Marinas wrote:
> On Wed, Jul 22, 2015 at 10:54:17AM +0100, Suzuki K. Poulose wrote:
>> From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>
>>
>> sys_reg() macro encodes Op0 as (Op0 - 2) and pushes it to Bit 19,
>> leaving Bit 20 uninitialised.
>
> A value of 0 doesn't mean uninitialised and as you noticed, it's set by
> the value or'ed onto the sys_reg() returned value in the mrs_s etc.
> macros.
>
> So I would rather change the subject to "Generalise encoding of system
> registers" and drop the uninitialised paragraph above, maybe replace it
> with a statement that current sys_reg() was only meant for MSR/MRS
> (register) encodings.

OK. Will send the updated versoin.

>
> Otherwise, the patch itself is fine.
>
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks
Suzuki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH] arm64: Generalise msr_s/mrs_s operations
  2015-07-22 10:07         ` Catalin Marinas
  2015-07-22 10:20           ` Suzuki K. Poulose
@ 2015-07-22 10:38           ` Suzuki K. Poulose
  1 sibling, 0 replies; 19+ messages in thread
From: Suzuki K. Poulose @ 2015-07-22 10:38 UTC (permalink / raw)
  To: linux-arm-kernel

From: "Suzuki K. Poulose" <suzuki.poulose@arm.com>

The system register encoding generated by sys_reg() works only
for MRS/MSR(Register) operations, as we hardcode Bit20 to 1 in
mrs_s/msr_s mask. This makes it unusable for generating instructions
accessing registers with Op0 < 2(e.g, PSTATE.x with Op0=0).

As per ARMv8 ARM, (Ref: ARMv8 ARM, Section: "System instruction class
encoding overview", C5.2, version:ARM DDI 0487A.f), the instruction
encoding reserves bits [20-19] for Op0.

This patch generalises the sys_reg, mrs_s and msr_s macros, so that
we could use them to access any of the supported system register.

Cc: James Morse <james.morse@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Suzuki K. Poulose <suzuki.poulose@arm.com>
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/include/asm/sysreg.h |   20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 0a579b2..9c5f0f1 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -20,8 +20,18 @@
 #ifndef __ASM_SYSREG_H
 #define __ASM_SYSREG_H
 
+/*
+ * ARMv8 ARM reserves the following encoding for system registers:
+ * (Ref: ARMv8 ARM, Section: "System instruction class encoding overview",
+ *  C5.2, version:ARM DDI 0487A.f)
+ *	[20-19] : Op0
+ *	[18-16] : Op1
+ *	[15-12] : CRn
+ *	[11-8]  : CRm
+ *	[7-5]   : Op2
+ */
 #define sys_reg(op0, op1, crn, crm, op2) \
-	((((op0)-2)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
+	((((op0)&3)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
 
 #ifdef __ASSEMBLY__
 
@@ -31,11 +41,11 @@
 	.equ	__reg_num_xzr, 31
 
 	.macro	mrs_s, rt, sreg
-	.inst	0xd5300000|(\sreg)|(__reg_num_\rt)
+	.inst	0xd5200000|(\sreg)|(__reg_num_\rt)
 	.endm
 
 	.macro	msr_s, sreg, rt
-	.inst	0xd5100000|(\sreg)|(__reg_num_\rt)
+	.inst	0xd5000000|(\sreg)|(__reg_num_\rt)
 	.endm
 
 #else
@@ -47,11 +57,11 @@ asm(
 "	.equ	__reg_num_xzr, 31\n"
 "\n"
 "	.macro	mrs_s, rt, sreg\n"
-"	.inst	0xd5300000|(\\sreg)|(__reg_num_\\rt)\n"
+"	.inst	0xd5200000|(\\sreg)|(__reg_num_\\rt)\n"
 "	.endm\n"
 "\n"
 "	.macro	msr_s, sreg, rt\n"
-"	.inst	0xd5100000|(\\sreg)|(__reg_num_\\rt)\n"
+"	.inst	0xd5000000|(\\sreg)|(__reg_num_\\rt)\n"
 "	.endm\n"
 );
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-07-22 10:38 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-17 17:30 [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never James Morse
2015-07-17 17:30 ` [PATCH v2 1/6] arm64: kernel: Add cpuid_feature_extract_field() for 4bit sign extension James Morse
2015-07-20 11:32   ` Catalin Marinas
2015-07-20 13:20     ` Catalin Marinas
2015-07-17 17:30 ` [PATCH v2 2/6] arm64: kernel: preparatory: Move config_sctlr_el1 James Morse
2015-07-17 17:30 ` [PATCH v2 3/6] arm64: kernel: Add cpufeature 'enable' callback James Morse
2015-07-17 17:30 ` [PATCH v2 4/6] arm64: kernel: Add min_register_value and use '>=' for feature detection James Morse
2015-07-20 13:53   ` Catalin Marinas
2015-07-17 17:30 ` [PATCH v2 5/6] arm64: kernel: Add optional CONFIG_ parameter to ALTERNATIVE() James Morse
2015-07-17 17:30 ` [PATCH v2 6/6] arm64: kernel: Add support for Privileged Access Never James Morse
2015-07-20 14:01   ` Catalin Marinas
2015-07-21 10:30   ` Suzuki K. Poulose
2015-07-21 11:37     ` Catalin Marinas
2015-07-22  9:54       ` [PATCH] arm64: sys_reg() : Fix encoding of system registers Suzuki K. Poulose
2015-07-22 10:07         ` Catalin Marinas
2015-07-22 10:20           ` Suzuki K. Poulose
2015-07-22 10:38           ` [PATCH] arm64: Generalise msr_s/mrs_s operations Suzuki K. Poulose
2015-07-20 11:02 ` [PATCH v2 0/6] arm64: kernel: Add support for Privileged Access Never Vladimir Murzin
2015-07-20 11:17   ` Catalin Marinas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.