[v2,1/2] arm64: Add initial support for E0PD
diff mbox series

Message ID 20190814183103.33707-2-broonie@kernel.org
State New
Headers show
Series
  • arm64: E0PD support
Related show

Commit Message

Mark Brown Aug. 14, 2019, 6:31 p.m. UTC
Kernel Page Table Isolation (KPTI) is used to mitigate some speculation
based security issues by ensuring that the kernel is not mapped when
userspace is running but this approach is expensive and is incompatible
with SPE.  E0PD, introduced in the ARMv8.5 extensions, provides an
alternative to this which ensures that accesses from userspace to the
kernel's half of the memory map to always fault with constant time,
preventing timing attacks without requiring constant unmapping and
remapping or preventing legitimate accesses.

This initial patch does not yet integrate with KPTI, this will be dealt
with in followup patches.  Ideally we could ensure that by default we
don't use KPTI on CPUs where E0PD is present.

Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
---

Tweak the Kconfig text as suggested by Will.

 arch/arm64/Kconfig                     | 15 ++++++++++++++
 arch/arm64/include/asm/cpucaps.h       |  3 ++-
 arch/arm64/include/asm/pgtable-hwdef.h |  2 ++
 arch/arm64/include/asm/sysreg.h        |  1 +
 arch/arm64/kernel/cpufeature.c         | 27 ++++++++++++++++++++++++++
 5 files changed, 47 insertions(+), 1 deletion(-)

Comments

Mark Rutland Oct. 10, 2019, 4:13 p.m. UTC | #1
Hi Mark,

On Wed, Aug 14, 2019 at 07:31:02PM +0100, Mark Brown wrote:
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 9323bcc40a58..62b01fc35ef6 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -219,6 +219,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = {
>  };
>  
>  static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
> +	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_E0PD_SHIFT, 4, 0),

The presence of this entry means the system-wide value will be exposed
to a KVM guest, but since it's NONSTRICT we won't warn about a
late-onlined CPU that has a mismatch.

So if all the boot-time CPUs have E0PD, we can spawn a VM that starts
using E0PD, but we might (silently) later migrate it to a CPU without
E0PD, breaking the security guarantee.

I think we want this to be STRICT, so that we at least warn in such a
case.

More generally than this patch, I suspect we probably want to abort the
hotplug if we online a CPU that doesn't provide the same gaurantees as
the sys_val for the field.

[...]

> @@ -1559,6 +1573,19 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		.sign = FTR_UNSIGNED,
>  		.min_field_value = 1,
>  	},
> +#endif
> +#ifdef CONFIG_ARM64_E0PD
> +	{
> +		.desc = "E0PD",
> +		.capability = ARM64_HAS_E0PD,
> +		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,

I suspect it would be better to treat this as a system-wide capability,
as with KPTI, which will make it much easier to reason about.

That would rule out having E0PD on a subset of CPUs, with or without
KPTI. With KPTI it's not really necessary, and without KPTI we don't
have a consistent guarantee, so that sounds reasonable to me.

Thanks,
Mark.

> +		.sys_reg = SYS_ID_AA64MMFR2_EL1,
> +		.sign = FTR_UNSIGNED,
> +		.field_pos = ID_AA64MMFR2_E0PD_SHIFT,
> +		.matches = has_cpuid_feature,
> +		.min_field_value = 1,
> +		.cpu_enable = cpu_enable_e0pd,
> +	},
>  #endif
>  	{},
>  };
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Mark Brown Oct. 11, 2019, 11:17 a.m. UTC | #2
On Thu, Oct 10, 2019 at 05:13:17PM +0100, Mark Rutland wrote:

> So if all the boot-time CPUs have E0PD, we can spawn a VM that starts
> using E0PD, but we might (silently) later migrate it to a CPU without
> E0PD, breaking the security guarantee.

> I think we want this to be STRICT, so that we at least warn in such a
> case.

> More generally than this patch, I suspect we probably want to abort the
> hotplug if we online a CPU that doesn't provide the same gaurantees as
> the sys_val for the field.

Right, if we make it STRICT we at least avoid that issue with KVM.

> > +#ifdef CONFIG_ARM64_E0PD
> > +	{
> > +		.desc = "E0PD",
> > +		.capability = ARM64_HAS_E0PD,
> > +		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,

> I suspect it would be better to treat this as a system-wide capability,
> as with KPTI, which will make it much easier to reason about.

> That would rule out having E0PD on a subset of CPUs, with or without
> KPTI. With KPTI it's not really necessary, and without KPTI we don't
> have a consistent guarantee, so that sounds reasonable to me.

It does - the main motivation for doing it as a local feature was
to avoid the regression with systems with late CPUs that lack the
capability which Will was concerned about but I'm not sure how
realistic such systems actually are.
Will Deacon Oct. 11, 2019, 11:40 a.m. UTC | #3
On Fri, Oct 11, 2019 at 12:17:15PM +0100, Mark Brown wrote:
> On Thu, Oct 10, 2019 at 05:13:17PM +0100, Mark Rutland wrote:
> 
> > So if all the boot-time CPUs have E0PD, we can spawn a VM that starts
> > using E0PD, but we might (silently) later migrate it to a CPU without
> > E0PD, breaking the security guarantee.
> 
> > I think we want this to be STRICT, so that we at least warn in such a
> > case.
> 
> > More generally than this patch, I suspect we probably want to abort the
> > hotplug if we online a CPU that doesn't provide the same gaurantees as
> > the sys_val for the field.
> 
> Right, if we make it STRICT we at least avoid that issue with KVM.
> 
> > > +#ifdef CONFIG_ARM64_E0PD
> > > +	{
> > > +		.desc = "E0PD",
> > > +		.capability = ARM64_HAS_E0PD,
> > > +		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
> 
> > I suspect it would be better to treat this as a system-wide capability,
> > as with KPTI, which will make it much easier to reason about.
> 
> > That would rule out having E0PD on a subset of CPUs, with or without
> > KPTI. With KPTI it's not really necessary, and without KPTI we don't
> > have a consistent guarantee, so that sounds reasonable to me.
> 
> It does - the main motivation for doing it as a local feature was
> to avoid the regression with systems with late CPUs that lack the
> capability which Will was concerned about but I'm not sure how
> realistic such systems actually are.

I think we need to handle the case where not all CPUs support ExPD :(

It's not like E0PD is something critical like support for a particular page
size, so I think it's a tough sell for SoC integrators to ensure that it's
matched up.

Will
Mark Rutland Oct. 11, 2019, 12:57 p.m. UTC | #4
On Fri, Oct 11, 2019 at 12:40:13PM +0100, Will Deacon wrote:
> On Fri, Oct 11, 2019 at 12:17:15PM +0100, Mark Brown wrote:
> > On Thu, Oct 10, 2019 at 05:13:17PM +0100, Mark Rutland wrote:
> > 
> > > So if all the boot-time CPUs have E0PD, we can spawn a VM that starts
> > > using E0PD, but we might (silently) later migrate it to a CPU without
> > > E0PD, breaking the security guarantee.
> > 
> > > I think we want this to be STRICT, so that we at least warn in such a
> > > case.
> > 
> > > More generally than this patch, I suspect we probably want to abort the
> > > hotplug if we online a CPU that doesn't provide the same gaurantees as
> > > the sys_val for the field.
> > 
> > Right, if we make it STRICT we at least avoid that issue with KVM.
> > 
> > > > +#ifdef CONFIG_ARM64_E0PD
> > > > +	{
> > > > +		.desc = "E0PD",
> > > > +		.capability = ARM64_HAS_E0PD,
> > > > +		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
> > 
> > > I suspect it would be better to treat this as a system-wide capability,
> > > as with KPTI, which will make it much easier to reason about.
> > 
> > > That would rule out having E0PD on a subset of CPUs, with or without
> > > KPTI. With KPTI it's not really necessary, and without KPTI we don't
> > > have a consistent guarantee, so that sounds reasonable to me.
> > 
> > It does - the main motivation for doing it as a local feature was
> > to avoid the regression with systems with late CPUs that lack the
> > capability which Will was concerned about but I'm not sure how
> > realistic such systems actually are.
> 
> I think we need to handle the case where not all CPUs support ExPD :(

I think we'll have to handle systems with mixed support, but:

* We should only make use E0PD the feature when all the boot CPUs have
  it.

* We should reject late-onlining of a CPU without E0PD if we detected it
  at boot time and either used it or expose it to guests.

If people are late-onlining mismatched secondaries they get to pick up
the pieces, as with other features.

Thanks,
Mark.
Catalin Marinas Oct. 11, 2019, 12:58 p.m. UTC | #5
On Fri, Oct 11, 2019 at 12:40:13PM +0100, Will Deacon wrote:
> On Fri, Oct 11, 2019 at 12:17:15PM +0100, Mark Brown wrote:
> > On Thu, Oct 10, 2019 at 05:13:17PM +0100, Mark Rutland wrote:
> > 
> > > So if all the boot-time CPUs have E0PD, we can spawn a VM that starts
> > > using E0PD, but we might (silently) later migrate it to a CPU without
> > > E0PD, breaking the security guarantee.
> > 
> > > I think we want this to be STRICT, so that we at least warn in such a
> > > case.
> > 
> > > More generally than this patch, I suspect we probably want to abort the
> > > hotplug if we online a CPU that doesn't provide the same gaurantees as
> > > the sys_val for the field.
> > 
> > Right, if we make it STRICT we at least avoid that issue with KVM.
> > 
> > > > +#ifdef CONFIG_ARM64_E0PD
> > > > +	{
> > > > +		.desc = "E0PD",
> > > > +		.capability = ARM64_HAS_E0PD,
> > > > +		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
> > 
> > > I suspect it would be better to treat this as a system-wide capability,
> > > as with KPTI, which will make it much easier to reason about.
> > 
> > > That would rule out having E0PD on a subset of CPUs, with or without
> > > KPTI. With KPTI it's not really necessary, and without KPTI we don't
> > > have a consistent guarantee, so that sounds reasonable to me.
> > 
> > It does - the main motivation for doing it as a local feature was
> > to avoid the regression with systems with late CPUs that lack the
> > capability which Will was concerned about but I'm not sure how
> > realistic such systems actually are.
> 
> I think we need to handle the case where not all CPUs support ExPD :(

That's fine but if such CPUs are brought up late and the kernel has
already made the decision to switch to global mappings, we should just
park them.
Mark Brown Oct. 11, 2019, 1:46 p.m. UTC | #6
On Fri, Oct 11, 2019 at 12:40:13PM +0100, Will Deacon wrote:
> On Fri, Oct 11, 2019 at 12:17:15PM +0100, Mark Brown wrote:

> > It does - the main motivation for doing it as a local feature was
> > to avoid the regression with systems with late CPUs that lack the
> > capability which Will was concerned about but I'm not sure how
> > realistic such systems actually are.

> I think we need to handle the case where not all CPUs support ExPD :(

> It's not like E0PD is something critical like support for a particular page
> size, so I think it's a tough sell for SoC integrators to ensure that it's
> matched up.

That's fine, we handle them by falling back to KPTI if we detect
any non-E0PD CPUs.  The only difference in the end result between
doing this by making it a system wide capability and doing it as
a weak local capability is what happens if the first CPU that
appears without E0PD is a late CPU then instead of accepting the
CPU and enabling KPTI we will reject the CPU.

Patch
diff mbox series

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 12de5c6075ec..7bf403405c99 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1392,6 +1392,21 @@  config ARM64_PTR_AUTH
 
 endmenu
 
+menu "ARMv8.5 architectural features"
+
+config ARM64_E0PD
+	bool "Enable support for E0PD"
+	default y
+	help
+	   E0PD (part of the ARMv8.5 extensions) allows us to ensure
+	   that EL0 accesses made via TTBR1 always fault in constant time,
+	   providing benefits to KPTI with lower overhead and without
+	   disrupting legitimate access to kernel memory such as SPE.
+
+	   This option enables E0PD for TTBR1 where available.
+
+endmenu
+
 config ARM64_SVE
 	bool "ARM Scalable Vector Extension support"
 	default y
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index f19fe4b9acc4..f25388981075 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -52,7 +52,8 @@ 
 #define ARM64_HAS_IRQ_PRIO_MASKING		42
 #define ARM64_HAS_DCPODP			43
 #define ARM64_WORKAROUND_1463225		44
+#define ARM64_HAS_E0PD				45
 
-#define ARM64_NCAPS				45
+#define ARM64_NCAPS				46
 
 #endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index db92950bb1a0..1a2708ebcae8 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -292,6 +292,8 @@ 
 #define TCR_HD			(UL(1) << 40)
 #define TCR_NFD0		(UL(1) << 53)
 #define TCR_NFD1		(UL(1) << 54)
+#define TCR_E0PD0		(UL(1) << 55)
+#define TCR_E0PD1		(UL(1) << 56)
 
 /*
  * TTBR.
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 1df45c7ffcf7..37a0926536d3 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -652,6 +652,7 @@ 
 #define ID_AA64MMFR1_VMIDBITS_16	2
 
 /* id_aa64mmfr2 */
+#define ID_AA64MMFR2_E0PD_SHIFT		60
 #define ID_AA64MMFR2_FWB_SHIFT		40
 #define ID_AA64MMFR2_AT_SHIFT		32
 #define ID_AA64MMFR2_LVA_SHIFT		16
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 9323bcc40a58..62b01fc35ef6 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -219,6 +219,7 @@  static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = {
 };
 
 static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = {
+	ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_E0PD_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_FWB_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_AT_SHIFT, 4, 0),
 	ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_LVA_SHIFT, 4, 0),
@@ -1244,6 +1245,19 @@  static void cpu_enable_address_auth(struct arm64_cpu_capabilities const *cap)
 }
 #endif /* CONFIG_ARM64_PTR_AUTH */
 
+#ifdef CONFIG_ARM64_E0PD
+static void cpu_enable_e0pd(struct arm64_cpu_capabilities const *cap)
+{
+	/*
+	 * The cpu_enable() callback gets called even on CPUs that
+	 * don't detect the feature so we need to verify if we can
+	 * enable.
+	 */
+	if (this_cpu_has_cap(ARM64_HAS_E0PD))
+		sysreg_clear_set(tcr_el1, 0, TCR_E0PD1);
+}
+#endif /* CONFIG_ARM64_E0PD */
+
 #ifdef CONFIG_ARM64_PSEUDO_NMI
 static bool enable_pseudo_nmi;
 
@@ -1559,6 +1573,19 @@  static const struct arm64_cpu_capabilities arm64_features[] = {
 		.sign = FTR_UNSIGNED,
 		.min_field_value = 1,
 	},
+#endif
+#ifdef CONFIG_ARM64_E0PD
+	{
+		.desc = "E0PD",
+		.capability = ARM64_HAS_E0PD,
+		.type = ARM64_CPUCAP_WEAK_LOCAL_CPU_FEATURE,
+		.sys_reg = SYS_ID_AA64MMFR2_EL1,
+		.sign = FTR_UNSIGNED,
+		.field_pos = ID_AA64MMFR2_E0PD_SHIFT,
+		.matches = has_cpuid_feature,
+		.min_field_value = 1,
+		.cpu_enable = cpu_enable_e0pd,
+	},
 #endif
 	{},
 };