diff mbox series

[v2,11/11] arm64: Implement branch predictor hardening for affected Cortex-A CPUs

Message ID 1515157961-20963-12-git-send-email-will.deacon@arm.com
State Superseded
Headers show
Series arm64 kpti hardening and variant 2 workarounds | expand

Commit Message

Will Deacon Jan. 5, 2018, 1:12 p.m. UTC
Cortex-A57, A72, A73 and A75 are susceptible to branch predictor aliasing
and can theoretically be attacked by malicious code.

This patch implements a PSCI-based mitigation for these CPUs when available.
The call into firmware will invalidate the branch predictor state, preventing
any malicious entries from affecting other victim contexts.

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

Signed-off-by: Will Deacon <will.deacon@arm.com>

---
 arch/arm64/kernel/bpi.S        | 24 ++++++++++++++++++++++++
 arch/arm64/kernel/cpu_errata.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

-- 
2.1.4

Comments

James Morse Jan. 5, 2018, 2:46 p.m. UTC | #1
Hi Marc, Will,

(SOB-chain suggests a missing From: tag on this and patch 7)

On 05/01/18 13:12, Will Deacon wrote:
> Cortex-A57, A72, A73 and A75 are susceptible to branch predictor aliasing

> and can theoretically be attacked by malicious code.

> 

> This patch implements a PSCI-based mitigation for these CPUs when available.

> The call into firmware will invalidate the branch predictor state, preventing

> any malicious entries from affecting other victim contexts.

> 

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

> Signed-off-by: Will Deacon <will.deacon@arm.com>


> diff --git a/arch/arm64/kernel/bpi.S b/arch/arm64/kernel/bpi.S

> index 06a931eb2673..2e9146534174 100644

> --- a/arch/arm64/kernel/bpi.S

> +++ b/arch/arm64/kernel/bpi.S

> @@ -53,3 +53,27 @@ ENTRY(__bp_harden_hyp_vecs_start)

>  	vectors __kvm_hyp_vector

>  	.endr

>  ENTRY(__bp_harden_hyp_vecs_end)

> +ENTRY(__psci_hyp_bp_inval_start)


> +	sub	sp, sp, #(8 * 18)


Where does 18 come from? Isn't this storing 9 sets of 16 bytes?


> +	stp	x16, x17, [sp, #(16 * 0)]

> +	stp	x14, x15, [sp, #(16 * 1)]

> +	stp	x12, x13, [sp, #(16 * 2)]

> +	stp	x10, x11, [sp, #(16 * 3)]

> +	stp	x8, x9, [sp, #(16 * 4)]

> +	stp	x6, x7, [sp, #(16 * 5)]

> +	stp	x4, x5, [sp, #(16 * 6)]

> +	stp	x2, x3, [sp, #(16 * 7)]


> +	stp	x0, x1, [sp, #(18 * 8)]


16->18 typo?


> +	mov	x0, #0x84000000

> +	smc	#0

> +	ldp	x16, x17, [sp, #(16 * 0)]

> +	ldp	x14, x15, [sp, #(16 * 1)]

> +	ldp	x12, x13, [sp, #(16 * 2)]

> +	ldp	x10, x11, [sp, #(16 * 3)]

> +	ldp	x8, x9, [sp, #(16 * 4)]

> +	ldp	x6, x7, [sp, #(16 * 5)]

> +	ldp	x4, x5, [sp, #(16 * 6)]

> +	ldp	x2, x3, [sp, #(16 * 7)]


> +	ldp	x0, x1, [sp, #(18 * 8)]

> +	add	sp, sp, #(8 * 18)


(and here?)

> +ENTRY(__psci_hyp_bp_inval_end)



Thanks,

James
Marc Zyngier Jan. 5, 2018, 2:57 p.m. UTC | #2
On 05/01/18 14:46, James Morse wrote:
> Hi Marc, Will,

> 

> (SOB-chain suggests a missing From: tag on this and patch 7)

> 

> On 05/01/18 13:12, Will Deacon wrote:

>> Cortex-A57, A72, A73 and A75 are susceptible to branch predictor aliasing

>> and can theoretically be attacked by malicious code.

>>

>> This patch implements a PSCI-based mitigation for these CPUs when available.

>> The call into firmware will invalidate the branch predictor state, preventing

>> any malicious entries from affecting other victim contexts.

>>

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

>> Signed-off-by: Will Deacon <will.deacon@arm.com>

> 

>> diff --git a/arch/arm64/kernel/bpi.S b/arch/arm64/kernel/bpi.S

>> index 06a931eb2673..2e9146534174 100644

>> --- a/arch/arm64/kernel/bpi.S

>> +++ b/arch/arm64/kernel/bpi.S

>> @@ -53,3 +53,27 @@ ENTRY(__bp_harden_hyp_vecs_start)

>>  	vectors __kvm_hyp_vector

>>  	.endr

>>  ENTRY(__bp_harden_hyp_vecs_end)

>> +ENTRY(__psci_hyp_bp_inval_start)

> 

>> +	sub	sp, sp, #(8 * 18)

> 

> Where does 18 come from? Isn't this storing 9 sets of 16 bytes?


Or 18 registers of 8 bytes each.

> 

>> +	stp	x16, x17, [sp, #(16 * 0)]

>> +	stp	x14, x15, [sp, #(16 * 1)]

>> +	stp	x12, x13, [sp, #(16 * 2)]

>> +	stp	x10, x11, [sp, #(16 * 3)]

>> +	stp	x8, x9, [sp, #(16 * 4)]

>> +	stp	x6, x7, [sp, #(16 * 5)]

>> +	stp	x4, x5, [sp, #(16 * 6)]

>> +	stp	x2, x3, [sp, #(16 * 7)]

> 

>> +	stp	x0, x1, [sp, #(18 * 8)]

> 

> 16->18 typo?


/me bashes head against keyboard, as this is likely to generate less
crap then me trying to write something half correct...

The fun part is that the Seattle box I booted yesterday with that crap
is still happily churning along. I guess I'm corrupting something that
really doesn't matter...

> 

> 

>> +	mov	x0, #0x84000000

>> +	smc	#0

>> +	ldp	x16, x17, [sp, #(16 * 0)]

>> +	ldp	x14, x15, [sp, #(16 * 1)]

>> +	ldp	x12, x13, [sp, #(16 * 2)]

>> +	ldp	x10, x11, [sp, #(16 * 3)]

>> +	ldp	x8, x9, [sp, #(16 * 4)]

>> +	ldp	x6, x7, [sp, #(16 * 5)]

>> +	ldp	x4, x5, [sp, #(16 * 6)]

>> +	ldp	x2, x3, [sp, #(16 * 7)]

> 

>> +	ldp	x0, x1, [sp, #(18 * 8)]

>> +	add	sp, sp, #(8 * 18)

> 

> (and here?)


Yup.

Thanks for pointing that out. I'll fetch a brown paper bag.

	M.
-- 
Jazz is not dead. It just smells funny...
Jayachandran C Jan. 8, 2018, 6:31 a.m. UTC | #3
On Fri, Jan 05, 2018 at 01:12:41PM +0000, Will Deacon wrote:
> Cortex-A57, A72, A73 and A75 are susceptible to branch predictor aliasing

> and can theoretically be attacked by malicious code.

> 

> This patch implements a PSCI-based mitigation for these CPUs when available.

> The call into firmware will invalidate the branch predictor state, preventing

> any malicious entries from affecting other victim contexts.

> 

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

> Signed-off-by: Will Deacon <will.deacon@arm.com>

> ---

>  arch/arm64/kernel/bpi.S        | 24 ++++++++++++++++++++++++

>  arch/arm64/kernel/cpu_errata.c | 42 ++++++++++++++++++++++++++++++++++++++++++

>  2 files changed, 66 insertions(+)

> 

> diff --git a/arch/arm64/kernel/bpi.S b/arch/arm64/kernel/bpi.S

> index 06a931eb2673..2e9146534174 100644

> --- a/arch/arm64/kernel/bpi.S

> +++ b/arch/arm64/kernel/bpi.S

> @@ -53,3 +53,27 @@ ENTRY(__bp_harden_hyp_vecs_start)

>  	vectors __kvm_hyp_vector

>  	.endr

>  ENTRY(__bp_harden_hyp_vecs_end)

> +ENTRY(__psci_hyp_bp_inval_start)

> +	sub	sp, sp, #(8 * 18)

> +	stp	x16, x17, [sp, #(16 * 0)]

> +	stp	x14, x15, [sp, #(16 * 1)]

> +	stp	x12, x13, [sp, #(16 * 2)]

> +	stp	x10, x11, [sp, #(16 * 3)]

> +	stp	x8, x9, [sp, #(16 * 4)]

> +	stp	x6, x7, [sp, #(16 * 5)]

> +	stp	x4, x5, [sp, #(16 * 6)]

> +	stp	x2, x3, [sp, #(16 * 7)]

> +	stp	x0, x1, [sp, #(18 * 8)]

> +	mov	x0, #0x84000000

> +	smc	#0

> +	ldp	x16, x17, [sp, #(16 * 0)]

> +	ldp	x14, x15, [sp, #(16 * 1)]

> +	ldp	x12, x13, [sp, #(16 * 2)]

> +	ldp	x10, x11, [sp, #(16 * 3)]

> +	ldp	x8, x9, [sp, #(16 * 4)]

> +	ldp	x6, x7, [sp, #(16 * 5)]

> +	ldp	x4, x5, [sp, #(16 * 6)]

> +	ldp	x2, x3, [sp, #(16 * 7)]

> +	ldp	x0, x1, [sp, #(18 * 8)]

> +	add	sp, sp, #(8 * 18)

> +ENTRY(__psci_hyp_bp_inval_end)

> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c

> index 16ea5c6f314e..cb0fb3796bb8 100644

> --- a/arch/arm64/kernel/cpu_errata.c

> +++ b/arch/arm64/kernel/cpu_errata.c

> @@ -53,6 +53,8 @@ static int cpu_enable_trap_ctr_access(void *__unused)

>  DEFINE_PER_CPU_READ_MOSTLY(struct bp_hardening_data, bp_hardening_data);

>  

>  #ifdef CONFIG_KVM

> +extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];

> +

>  static void __copy_hyp_vect_bpi(int slot, const char *hyp_vecs_start,

>  				const char *hyp_vecs_end)

>  {

> @@ -94,6 +96,9 @@ static void __install_bp_hardening_cb(bp_hardening_cb_t fn,

>  	spin_unlock(&bp_lock);

>  }

>  #else

> +#define __psci_hyp_bp_inval_start	NULL

> +#define __psci_hyp_bp_inval_end		NULL

> +

>  static void __install_bp_hardening_cb(bp_hardening_cb_t fn,

>  				      const char *hyp_vecs_start,

>  				      const char *hyp_vecs_end)

> @@ -118,6 +123,21 @@ static void  install_bp_hardening_cb(const struct arm64_cpu_capabilities *entry,

>  

>  	__install_bp_hardening_cb(fn, hyp_vecs_start, hyp_vecs_end);

>  }

> +

> +#include <linux/psci.h>

> +

> +static int enable_psci_bp_hardening(void *data)

> +{

> +	const struct arm64_cpu_capabilities *entry = data;

> +

> +	if (psci_ops.get_version)

> +		install_bp_hardening_cb(entry,

> +				       (bp_hardening_cb_t)psci_ops.get_version,

> +				       __psci_hyp_bp_inval_start,

> +				       __psci_hyp_bp_inval_end);

> +

> +	return 0;

> +}

>  #endif	/* CONFIG_HARDEN_BRANCH_PREDICTOR */

>  

>  #define MIDR_RANGE(model, min, max) \

> @@ -261,6 +281,28 @@ const struct arm64_cpu_capabilities arm64_errata[] = {

>  		MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),

>  	},

>  #endif

> +#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR

> +	{

> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

> +		MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),

> +		.enable = enable_psci_bp_hardening,

> +	},

> +	{

> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

> +		MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),

> +		.enable = enable_psci_bp_hardening,

> +	},

> +	{

> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

> +		MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),

> +		.enable = enable_psci_bp_hardening,

> +	},

> +	{

> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

> +		MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),

> +		.enable = enable_psci_bp_hardening,

> +	},

> +#endif

>  	{

>  	}

>  };


On ThunderX2, we would like to do something similar, but it is important to
find out if the current firmware has support for BTB invalidate in the
PSCI version call. Otherwise, we will end up doing version calls that do
nothing but return version (and waste cycles).

I will follow up with ThunderX2 patches, but it would be good to have a
standard way of figuring out if the firmware has BTB invalidate support.

JC.
Will Deacon Jan. 8, 2018, 4:46 p.m. UTC | #4
On Sun, Jan 07, 2018 at 10:53:36PM -0800, Jayachandran C wrote:
> Use PSCI based mitigation for speculative execution attacks targeting

> the branch predictor. The approach is similar to the one used for

> Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

> test if the firmware supports the capability.

> 

> If the secure firmware has been updated with the mitigation code to

> invalidate the branch target buffer, we use the PSCI version call to

> invoke it.

> 

> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

> ---

>  arch/arm64/kernel/cpu_errata.c | 38 ++++++++++++++++++++++++++++++++++++++

>  1 file changed, 38 insertions(+)

> 

> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c

> index cb0fb37..abceb5d 100644

> --- a/arch/arm64/kernel/cpu_errata.c

> +++ b/arch/arm64/kernel/cpu_errata.c

> @@ -124,6 +124,7 @@ static void  install_bp_hardening_cb(const struct arm64_cpu_capabilities *entry,

>  	__install_bp_hardening_cb(fn, hyp_vecs_start, hyp_vecs_end);

>  }

>  

> +#include <linux/arm-smccc.h>

>  #include <linux/psci.h>

>  

>  static int enable_psci_bp_hardening(void *data)

> @@ -138,6 +139,33 @@ static int enable_psci_bp_hardening(void *data)

>  

>  	return 0;

>  }

> +

> +#define CAVIUM_TX2_SIP_SMC_CALL		0xC200FF00

> +#define CAVIUM_TX2_BTB_HARDEN_CAP	0xB0A0

> +

> +static int enable_tx2_psci_bp_hardening(void *data)

> +{

> +	const struct arm64_cpu_capabilities *entry = data;

> +	struct arm_smccc_res res;

> +

> +	if (!entry->matches(entry, SCOPE_LOCAL_CPU))

> +		return;

> +

> +	arm_smccc_smc(CAVIUM_TX2_SIP_SMC_CALL, CAVIUM_TX2_BTB_HARDEN_CAP, 0, 0, 0, 0, 0, 0, &res);


One thing to be aware of here is that if somebody configures qemu to emulate
a TX2, this may actually disappear into EL3 and not return. You're better
off sticking with PSCI GET_VERSION in terms of portability, but it's your
call -- I'd expect you to deal with any breakage reports on the list due
to the SMC above. Fair?

> +	if (res.a0 != 0) {

> +		pr_warn("Error: CONFIG_HARDEN_BRANCH_PREDICTOR enabled, but firmware does not support it\n");

> +		return 0;

> +	}


Please don't print this here; see below.

> +	if (res.a1 == 1 && psci_ops.get_version) {

> +		pr_info("CPU%d: Branch predictor hardening enabled\n", smp_processor_id());


If you want to print a message, please put it in the capability structure
.desc field.

Will
Jayachandran C Jan. 8, 2018, 5:19 p.m. UTC | #5
On Mon, Jan 08, 2018 at 04:46:52PM +0000, Will Deacon wrote:
> On Sun, Jan 07, 2018 at 10:53:36PM -0800, Jayachandran C wrote:

> > Use PSCI based mitigation for speculative execution attacks targeting

> > the branch predictor. The approach is similar to the one used for

> > Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

> > test if the firmware supports the capability.

> > 

> > If the secure firmware has been updated with the mitigation code to

> > invalidate the branch target buffer, we use the PSCI version call to

> > invoke it.

> > 

> > Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

> > ---

> >  arch/arm64/kernel/cpu_errata.c | 38 ++++++++++++++++++++++++++++++++++++++

> >  1 file changed, 38 insertions(+)

> > 

> > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c

> > index cb0fb37..abceb5d 100644

> > --- a/arch/arm64/kernel/cpu_errata.c

> > +++ b/arch/arm64/kernel/cpu_errata.c

> > @@ -124,6 +124,7 @@ static void  install_bp_hardening_cb(const struct arm64_cpu_capabilities *entry,

> >  	__install_bp_hardening_cb(fn, hyp_vecs_start, hyp_vecs_end);

> >  }

> >  

> > +#include <linux/arm-smccc.h>

> >  #include <linux/psci.h>

> >  

> >  static int enable_psci_bp_hardening(void *data)

> > @@ -138,6 +139,33 @@ static int enable_psci_bp_hardening(void *data)

> >  

> >  	return 0;

> >  }

> > +

> > +#define CAVIUM_TX2_SIP_SMC_CALL		0xC200FF00

> > +#define CAVIUM_TX2_BTB_HARDEN_CAP	0xB0A0

> > +

> > +static int enable_tx2_psci_bp_hardening(void *data)

> > +{

> > +	const struct arm64_cpu_capabilities *entry = data;

> > +	struct arm_smccc_res res;

> > +

> > +	if (!entry->matches(entry, SCOPE_LOCAL_CPU))

> > +		return;

> > +

> > +	arm_smccc_smc(CAVIUM_TX2_SIP_SMC_CALL, CAVIUM_TX2_BTB_HARDEN_CAP, 0, 0, 0, 0, 0, 0, &res);

> 

> One thing to be aware of here is that if somebody configures qemu to emulate

> a TX2, this may actually disappear into EL3 and not return. You're better

> off sticking with PSCI GET_VERSION in terms of portability, but it's your

> call -- I'd expect you to deal with any breakage reports on the list due

> to the SMC above. Fair?


I don't like having a custom SMC here either. But Overloading PSCI get version
is the problem as I wrote earlier - there is no way to check if the firmware
implements BTB hardening with overloading. There is a good chance that users
with old firmware will just fail without any warning.

Is there a reason for overloading PSCI get version? Allocating a new standard
SMC number would make checking for existance and usage much simpler.

I did not quite understand the possible issue in qemu, unimplemented SMC calls
are expected to return an error code. What am I missing here?

> 

> > +	if (res.a0 != 0) {

> > +		pr_warn("Error: CONFIG_HARDEN_BRANCH_PREDICTOR enabled, but firmware does not support it\n");

> > +		return 0;

> > +	}

> 

> Please don't print this here; see below.

> 

> > +	if (res.a1 == 1 && psci_ops.get_version) {

> > +		pr_info("CPU%d: Branch predictor hardening enabled\n", smp_processor_id());

> 

> If you want to print a message, please put it in the capability structure

> .desc field.


Thanks, will fix this in the next rev.

JC.
Will Deacon Jan. 8, 2018, 5:23 p.m. UTC | #6
On Mon, Jan 08, 2018 at 09:19:43AM -0800, Jayachandran C wrote:
> On Mon, Jan 08, 2018 at 04:46:52PM +0000, Will Deacon wrote:

> > On Sun, Jan 07, 2018 at 10:53:36PM -0800, Jayachandran C wrote:

> > > Use PSCI based mitigation for speculative execution attacks targeting

> > > the branch predictor. The approach is similar to the one used for

> > > Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

> > > test if the firmware supports the capability.

> > > 

> > > If the secure firmware has been updated with the mitigation code to

> > > invalidate the branch target buffer, we use the PSCI version call to

> > > invoke it.

> > > 

> > > Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

> > > ---

> > >  arch/arm64/kernel/cpu_errata.c | 38 ++++++++++++++++++++++++++++++++++++++

> > >  1 file changed, 38 insertions(+)

> > > 

> > > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c

> > > index cb0fb37..abceb5d 100644

> > > --- a/arch/arm64/kernel/cpu_errata.c

> > > +++ b/arch/arm64/kernel/cpu_errata.c

> > > @@ -124,6 +124,7 @@ static void  install_bp_hardening_cb(const struct arm64_cpu_capabilities *entry,

> > >  	__install_bp_hardening_cb(fn, hyp_vecs_start, hyp_vecs_end);

> > >  }

> > >  

> > > +#include <linux/arm-smccc.h>

> > >  #include <linux/psci.h>

> > >  

> > >  static int enable_psci_bp_hardening(void *data)

> > > @@ -138,6 +139,33 @@ static int enable_psci_bp_hardening(void *data)

> > >  

> > >  	return 0;

> > >  }

> > > +

> > > +#define CAVIUM_TX2_SIP_SMC_CALL		0xC200FF00

> > > +#define CAVIUM_TX2_BTB_HARDEN_CAP	0xB0A0

> > > +

> > > +static int enable_tx2_psci_bp_hardening(void *data)

> > > +{

> > > +	const struct arm64_cpu_capabilities *entry = data;

> > > +	struct arm_smccc_res res;

> > > +

> > > +	if (!entry->matches(entry, SCOPE_LOCAL_CPU))

> > > +		return;

> > > +

> > > +	arm_smccc_smc(CAVIUM_TX2_SIP_SMC_CALL, CAVIUM_TX2_BTB_HARDEN_CAP, 0, 0, 0, 0, 0, 0, &res);

> > 

> > One thing to be aware of here is that if somebody configures qemu to emulate

> > a TX2, this may actually disappear into EL3 and not return. You're better

> > off sticking with PSCI GET_VERSION in terms of portability, but it's your

> > call -- I'd expect you to deal with any breakage reports on the list due

> > to the SMC above. Fair?

> 

> I don't like having a custom SMC here either. But Overloading PSCI get version

> is the problem as I wrote earlier - there is no way to check if the firmware

> implements BTB hardening with overloading. There is a good chance that users

> with old firmware will just fail without any warning.


That's true, but there is precedent for this elsewhere. For example, CPU
errata that require a firmware change are often not probable. Also, your SMC
call won't always work (see the qemu comment below). Note that I'm not
saying I won't take this code, just that you need to be aware of what
you're doing.

> Is there a reason for overloading PSCI get version? Allocating a new standard

> SMC number would make checking for existance and usage much simpler.


PSCI get version is what we have today. We're working on extending PSCI to
allocate a new standard SMC number, but we need something that can be used
with existing firmware too and standardisation doesn't happen overnight.

> I did not quite understand the possible issue in qemu, unimplemented SMC calls

> are expected to return an error code. What am I missing here?


Qemu will inject them into EL3, and there might not be anything there.

Will
Jayachandran C Jan. 9, 2018, 2:26 a.m. UTC | #7
On Mon, Jan 08, 2018 at 05:23:41PM +0000, Will Deacon wrote:
> On Mon, Jan 08, 2018 at 09:19:43AM -0800, Jayachandran C wrote:

> > On Mon, Jan 08, 2018 at 04:46:52PM +0000, Will Deacon wrote:

> > > On Sun, Jan 07, 2018 at 10:53:36PM -0800, Jayachandran C wrote:

> > > > Use PSCI based mitigation for speculative execution attacks targeting

> > > > the branch predictor. The approach is similar to the one used for

> > > > Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

> > > > test if the firmware supports the capability.

> > > > 

> > > > If the secure firmware has been updated with the mitigation code to

> > > > invalidate the branch target buffer, we use the PSCI version call to

> > > > invoke it.

> > > > 

> > > > Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

> > > > ---

> > > >  arch/arm64/kernel/cpu_errata.c | 38 ++++++++++++++++++++++++++++++++++++++

> > > >  1 file changed, 38 insertions(+)

> > > > 

> > > > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c

> > > > index cb0fb37..abceb5d 100644

> > > > --- a/arch/arm64/kernel/cpu_errata.c

> > > > +++ b/arch/arm64/kernel/cpu_errata.c

> > > > @@ -124,6 +124,7 @@ static void  install_bp_hardening_cb(const struct arm64_cpu_capabilities *entry,

> > > >  	__install_bp_hardening_cb(fn, hyp_vecs_start, hyp_vecs_end);

> > > >  }

> > > >  

> > > > +#include <linux/arm-smccc.h>

> > > >  #include <linux/psci.h>

> > > >  

> > > >  static int enable_psci_bp_hardening(void *data)

> > > > @@ -138,6 +139,33 @@ static int enable_psci_bp_hardening(void *data)

> > > >  

> > > >  	return 0;

> > > >  }

> > > > +

> > > > +#define CAVIUM_TX2_SIP_SMC_CALL		0xC200FF00

> > > > +#define CAVIUM_TX2_BTB_HARDEN_CAP	0xB0A0

> > > > +

> > > > +static int enable_tx2_psci_bp_hardening(void *data)

> > > > +{

> > > > +	const struct arm64_cpu_capabilities *entry = data;

> > > > +	struct arm_smccc_res res;

> > > > +

> > > > +	if (!entry->matches(entry, SCOPE_LOCAL_CPU))

> > > > +		return;

> > > > +

> > > > +	arm_smccc_smc(CAVIUM_TX2_SIP_SMC_CALL, CAVIUM_TX2_BTB_HARDEN_CAP, 0, 0, 0, 0, 0, 0, &res);

> > > 

> > > One thing to be aware of here is that if somebody configures qemu to emulate

> > > a TX2, this may actually disappear into EL3 and not return. You're better

> > > off sticking with PSCI GET_VERSION in terms of portability, but it's your

> > > call -- I'd expect you to deal with any breakage reports on the list due

> > > to the SMC above. Fair?

> > 

> > I don't like having a custom SMC here either. But Overloading PSCI get version

> > is the problem as I wrote earlier - there is no way to check if the firmware

> > implements BTB hardening with overloading. There is a good chance that users

> > with old firmware will just fail without any warning.

> 

> That's true, but there is precedent for this elsewhere. For example, CPU

> errata that require a firmware change are often not probable. Also, your SMC

> call won't always work (see the qemu comment below). Note that I'm not

> saying I won't take this code, just that you need to be aware of what

> you're doing.

> 

> > Is there a reason for overloading PSCI get version? Allocating a new standard

> > SMC number would make checking for existance and usage much simpler.

> 

> PSCI get version is what we have today. We're working on extending PSCI to

> allocate a new standard SMC number, but we need something that can be used

> with existing firmware too and standardisation doesn't happen overnight.


Can you hold this patchset until the SMC number is published? Otherwise we
will end up with two incompatible interfaces, and the mess of supporting
both.

Or if there is a plan standardize this later, I can pickup a vendor specific
SMC for now, and switch over to the standard one later. Any suggestions here?

JC.
Will Deacon Jan. 9, 2018, 9:53 a.m. UTC | #8
On Mon, Jan 08, 2018 at 06:26:20PM -0800, Jayachandran C wrote:
> On Mon, Jan 08, 2018 at 05:23:41PM +0000, Will Deacon wrote:

> > On Mon, Jan 08, 2018 at 09:19:43AM -0800, Jayachandran C wrote:

> > > On Mon, Jan 08, 2018 at 04:46:52PM +0000, Will Deacon wrote:

> > > > On Sun, Jan 07, 2018 at 10:53:36PM -0800, Jayachandran C wrote:

> > > > > +	arm_smccc_smc(CAVIUM_TX2_SIP_SMC_CALL, CAVIUM_TX2_BTB_HARDEN_CAP, 0, 0, 0, 0, 0, 0, &res);

> > > > 

> > > > One thing to be aware of here is that if somebody configures qemu to emulate

> > > > a TX2, this may actually disappear into EL3 and not return. You're better

> > > > off sticking with PSCI GET_VERSION in terms of portability, but it's your

> > > > call -- I'd expect you to deal with any breakage reports on the list due

> > > > to the SMC above. Fair?

> > > 

> > > I don't like having a custom SMC here either. But Overloading PSCI get version

> > > is the problem as I wrote earlier - there is no way to check if the firmware

> > > implements BTB hardening with overloading. There is a good chance that users

> > > with old firmware will just fail without any warning.

> > 

> > That's true, but there is precedent for this elsewhere. For example, CPU

> > errata that require a firmware change are often not probable. Also, your SMC

> > call won't always work (see the qemu comment below). Note that I'm not

> > saying I won't take this code, just that you need to be aware of what

> > you're doing.

> > 

> > > Is there a reason for overloading PSCI get version? Allocating a new standard

> > > SMC number would make checking for existance and usage much simpler.

> > 

> > PSCI get version is what we have today. We're working on extending PSCI to

> > allocate a new standard SMC number, but we need something that can be used

> > with existing firmware too and standardisation doesn't happen overnight.

> 

> Can you hold this patchset until the SMC number is published? Otherwise we

> will end up with two incompatible interfaces, and the mess of supporting

> both.


This has already been queued, and will be necessary for older PSCI versions
that can be extended to perform the BP invalidation in get version, but
which cannot be upgraded to a newer version of the spec. But that's fine; we
can support both interfaces because the new one will need to be discoverable
anyway.

> Or if there is a plan standardize this later, I can pickup a vendor specific

> SMC for now, and switch over to the standard one later. Any suggestions here?


I would suggest using GET VERSION for now and switching later, but it's
entirely up to you.

Wil
Jon Masters Jan. 16, 2018, 9:50 p.m. UTC | #9
On 01/09/2018 07:47 AM, Jayachandran C wrote:
> Use PSCI based mitigation for speculative execution attacks targeting

> the branch predictor. The approach is similar to the one used for

> Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

> test if the firmware supports the capability.

> 

> If the secure firmware has been updated with the mitigation code to

> invalidate the branch target buffer, we use the PSCI version call to

> invoke it.


What's the status of this patch for TX2? Previously you were holding on
the new SMC number, but I think the consensus was to pull this in now?

Jon.

-- 
Computer Architect
Jon Masters Jan. 16, 2018, 9:52 p.m. UTC | #10
On 01/09/2018 07:47 AM, Jayachandran C wrote:

> Use PSCI based mitigation for speculative execution attacks targeting

> the branch predictor. The approach is similar to the one used for

> Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

> test if the firmware supports the capability.

> 

> If the secure firmware has been updated with the mitigation code to

> invalidate the branch target buffer, we use the PSCI version call to

> invoke it.


What's the status of this patch currently? Previously you had suggested
to hold while the SMC got standardized, but then you seemed happy with
pulling in. What's the latest?

Jon.
Jayachandran C Jan. 16, 2018, 11:45 p.m. UTC | #11
On Tue, Jan 16, 2018 at 04:52:53PM -0500, Jon Masters wrote:
> On 01/09/2018 07:47 AM, Jayachandran C wrote:

> 

> > Use PSCI based mitigation for speculative execution attacks targeting

> > the branch predictor. The approach is similar to the one used for

> > Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

> > test if the firmware supports the capability.

> > 

> > If the secure firmware has been updated with the mitigation code to

> > invalidate the branch target buffer, we use the PSCI version call to

> > invoke it.

> 

> What's the status of this patch currently? Previously you had suggested

> to hold while the SMC got standardized, but then you seemed happy with

> pulling in. What's the latest?


My understanding is that the SMC standardization is being worked on
but will take more time, and the KPTI current patchset will go to
mainline before that.

Given that, I would expect arm64 maintainers to pick up this patch for
ThunderX2, but I have not seen any comments so far.

Will/Marc, please let me know if you are planning to pick this patch
into the KPTI tree.

Thanks,
JC.
Jon Masters Jan. 17, 2018, 6:34 p.m. UTC | #12
On 01/16/2018 06:45 PM, Jayachandran C wrote:
> On Tue, Jan 16, 2018 at 04:52:53PM -0500, Jon Masters wrote:

>> On 01/09/2018 07:47 AM, Jayachandran C wrote:

>>

>>> Use PSCI based mitigation for speculative execution attacks targeting

>>> the branch predictor. The approach is similar to the one used for

>>> Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

>>> test if the firmware supports the capability.

>>>

>>> If the secure firmware has been updated with the mitigation code to

>>> invalidate the branch target buffer, we use the PSCI version call to

>>> invoke it.

>>

>> What's the status of this patch currently? Previously you had suggested

>> to hold while the SMC got standardized, but then you seemed happy with

>> pulling in. What's the latest?

> 

> My understanding is that the SMC standardization is being worked on

> but will take more time, and the KPTI current patchset will go to

> mainline before that.

> 

> Given that, I would expect arm64 maintainers to pick up this patch for

> ThunderX2, but I have not seen any comments so far.

> 

> Will/Marc, please let me know if you are planning to pick this patch

> into the KPTI tree.


We've pulled in mitigations for QCOM Falkor into our internal
development branch (for future releases, this isn't about existing
stuff), but we can't pull in mitigations for other vendors until they're
upstream, and this patch isn't in any tree we track yet.

Therefore, I encourage all of the vendors to get this upstream. Until
that's true, it will be difficult to continue to carry out of tree bits.

Jon.
Will Deacon Jan. 18, 2018, 1:53 p.m. UTC | #13
Hi JC,

On Tue, Jan 16, 2018 at 03:45:54PM -0800, Jayachandran C wrote:
> On Tue, Jan 16, 2018 at 04:52:53PM -0500, Jon Masters wrote:

> > On 01/09/2018 07:47 AM, Jayachandran C wrote:

> > 

> > > Use PSCI based mitigation for speculative execution attacks targeting

> > > the branch predictor. The approach is similar to the one used for

> > > Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

> > > test if the firmware supports the capability.

> > > 

> > > If the secure firmware has been updated with the mitigation code to

> > > invalidate the branch target buffer, we use the PSCI version call to

> > > invoke it.

> > 

> > What's the status of this patch currently? Previously you had suggested

> > to hold while the SMC got standardized, but then you seemed happy with

> > pulling in. What's the latest?

> 

> My understanding is that the SMC standardization is being worked on

> but will take more time, and the KPTI current patchset will go to

> mainline before that.

> 

> Given that, I would expect arm64 maintainers to pick up this patch for

> ThunderX2, but I have not seen any comments so far.

> 

> Will/Marc, please let me know if you are planning to pick this patch

> into the KPTI tree.


Are you really sure you want us to apply this? If we do, then you can't run
KVM guests anymore because your IMPDEF SMC results in an UNDEF being
injected (crash below).

I really think that you should just hook up the enable_psci_bp_hardening
callback like we've done for the Cortex CPUs. We can optimise this later
once the SMC standarisation work has been completed (which is nearly final
now and works in a backwards-compatible manner).

Will

--->8

[    0.319123] Code: 2a080042 b8236885 29008829 17ffffc0 (d4000003) 
[    0.319125] Code: 2a080042 b8236885 29008829 17ffffc0 (d4000003) 
[    0.319147] Modules linked in:
[    0.319152] CPU: 2 PID: 19 Comm: migration/2 Not tainted 4.15.0-rc8-00103-g9409c1e175be-dirty #1
[    0.319154] Hardware name: linux,dummy-virt (DT)
[    0.319156] pstate: 00000085 (nzcv daIf -PAN -UAO)
[    0.319163] pc : __arm_smccc_smc+0x0/0x2c
[    0.319166] lr : enable_tx2_psci_bp_hardening+0x6c/0x108
[    0.319167] sp : ffff000009dcbd30
[    0.319168] x29: ffff000009dcbd40 x28: 0000000000000000 
[    0.319171] x27: ffff00000803bc88 x26: 0000000000000001 
[    0.319174] x25: ffff000008d13980 x24: ffff00000907b575 
[    0.319176] x23: 0000000000000001 x22: 0000000000000000 
[    0.319179] x21: ffff00000803bd3c x20: ffff00000803bd18 
[    0.319181] x19: ffff0000089f2438 x18: 0000000000000030 
[    0.319183] x17: 0000000000000000 x16: 0000000000000000 
[    0.319185] x15: 0000000000000000 x14: 0000000000000400 
[    0.319187] x13: 0000000000000400 x12: 0000000000000000 
[    0.319189] x11: 0000000000000000 x10: 0000000000000a00 
[    0.319192] x9 : ffff000009dcbd80 x8 : ffff8001f691b460 
[    0.319194] x7 : 0000000000000000 x6 : 0000000000000000 
[    0.319196] x5 : 0000000000000000 x4 : 0000000000000000 
[    0.319198] x3 : 0000000000000000 x2 : 0000000000000000 
[    0.319200] x1 : 000000000000b0a0 x0 : 00000000c200ff00 
[    0.319203] Process migration/2 (pid: 19, stack limit = 0x000000004aa336a5)
[    0.319204] Call trace:
[    0.319207]  __arm_smccc_smc+0x0/0x2c
[    0.319211]  multi_cpu_stop+0x8c/0x110
[    0.319213]  cpu_stopper_thread+0xac/0x120
[    0.319219]  smpboot_thread_fn+0x158/0x240
[    0.319220]  kthread+0x128/0x130
[    0.319223]  ret_from_fork+0x10/0x18
[    0.319226] Code: 2a080042 b8236885 29008829 17ffffc0 (d4000003) 
[    0.319230] ---[ end trace 169f08213b3163bb ]---
[    0.319234] Internal error: undefined instruction: 0 [#2] PREEMPT SMP
[    0.319259] note: migration/2[19] exited with preempt_count 1
[    0.319284] Modules linked in:
[    0.319288] CPU: 3 PID: 24 Comm: migration/3 Tainted: G      D          4.15.0-rc8-00103-g9409c1e175be-dirty #1
[    0.319289] Hardware name: linux,dummy-virt (DT)
[    0.319291] pstate: 00000085 (nzcv daIf -PAN -UAO)
[    0.319295] pc : __arm_smccc_smc+0x0/0x2c
[    0.319298] lr : enable_tx2_psci_bp_hardening+0x6c/0x108
[    0.319298] sp : ffff000009df3d30
[    0.319300] x29: ffff000009df3d40 x28: 0000000000000000 
[    0.319303] x27: ffff00000803bc88 x26: 0000000000000001 
[    0.319305] x25: ffff000008d13980 x24: ffff00000907b575 
[    0.319307] x23: 0000000000000001 x22: 0000000000000000 
[    0.319310] x21: ffff00000803bd3c x20: ffff00000803bd18 
[    0.319312] x19: ffff0000089f2438 x18: 0000000000000030 
[    0.319314] x17: 0000000000000000 x16: 0000000000000000 
[    0.319316] x15: 0000000000000000 x14: 0000000000000400 
[    0.319318] x13: 0000000000000400 x12: 0000000000000001 
[    0.319321] x11: 000000009ad0065e x10: 0000000000000a00 
[    0.319323] x9 : ffff000009df3d80 x8 : ffff8001f691fa60 
[    0.319325] x7 : 0000000000000000 x6 : 0000000000000000 
[    0.319327] x5 : 0000000000000000 x4 : 0000000000000000 
[    0.319329] x3 : 0000000000000000 x2 : 0000000000000000 
[    0.319331] x1 : 000000000000b0a0 x0 : 00000000c200ff00 
[    0.319334] Process migration/3 (pid: 24, stack limit = 0x00000000be13f0f9)
[    0.319335] Call trace:
[    0.319338]  __arm_smccc_smc+0x0/0x2c
[    0.319340]  multi_cpu_stop+0x8c/0x110
[    0.319342]  cpu_stopper_thread+0xac/0x120
[    0.319345]  smpboot_thread_fn+0x158/0x240
[    0.319346]  kthread+0x128/0x130
[    0.319348]  ret_from_fork+0x10/0x18
[    0.319351] Code: 2a080042 b8236885 29008829 17ffffc0 (d4000003) 
[    0.319352] ---[ end trace 169f08213b3163bc ]---
[    0.319355] note: migration/3[24] exited with preempt_count 1
[    0.319371] Internal error: undefined instruction: 0 [#3] PREEMPT SMP
[    0.319373] Modules linked in:
[    0.319376] CPU: 1 PID: 14 Comm: migration/1 Tainted: G      D          4.15.0-rc8-00103-g9409c1e175be-dirty #1
[    0.319377] Hardware name: linux,dummy-virt (DT)
[    0.319379] pstate: 00000085 (nzcv daIf -PAN -UAO)
[    0.319383] pc : __arm_smccc_smc+0x0/0x2c
[    0.319385] lr : enable_tx2_psci_bp_hardening+0x6c/0x108
[    0.319386] sp : ffff000009da3d30
[    0.319387] x29: ffff000009da3d40 x28: 0000000000000000 
[    0.319390] x27: ffff00000803bc88 x26: 0000000000000001 
[    0.319393] x25: ffff000008d13980 x24: ffff00000907b575 
[    0.319395] x23: 0000000000000001 x22: 0000000000000000 
[    0.319397] x21: ffff00000803bd3c x20: ffff00000803bd18 
[    0.319399] x19: ffff0000089f2438 x18: 0000000000000030 
[    0.319402] x17: 0000000000000000 x16: 0000000000000000 
[    0.319404] x15: 0000000000000000 x14: 0000000000000400 
[    0.319406] x13: 0000000000000400 x12: 0000000000000000 
[    0.319408] x11: 0000000000000000 x10: 0000000000000a00 
[    0.319410] x9 : ffff000009da3d80 x8 : ffff8001f68e6c60 
[    0.319412] x7 : 0000000000000000 x6 : 0000000000000000 
[    0.319414] x5 : 0000000000000000 x4 : 0000000000000000 
[    0.319416] x3 : 0000000000000000 x2 : 0000000000000000 
[    0.319418] x1 : 000000000000b0a0 x0 : 00000000c200ff00 
[    0.319422] Process migration/1 (pid: 14, stack limit = 0x00000000596b9e92)
[    0.319423] Call trace:
[    0.319425]  __arm_smccc_smc+0x0/0x2c
[    0.319427]  multi_cpu_stop+0x8c/0x110
[    0.319429]  cpu_stopper_thread+0xac/0x120
[    0.319431]  smpboot_thread_fn+0x158/0x240
[    0.319433]  kthread+0x128/0x130
[    0.319435]  ret_from_fork+0x10/0x18
[    0.319437] Code: 2a080042 b8236885 29008829 17ffffc0 (d4000003) 
[    0.319439] ---[ end trace 169f08213b3163bd ]---
[    0.319441] note: migration/1[14] exited with preempt_count 1
[    0.857389] Code: 2a080042 b8236885 29008829 17ffffc0 (d4000003) 
[    0.863472] Internal error: undefined instruction: 0 [#4] PREEMPT SMP
[    0.869937] Modules linked in:
[    0.872969] CPU: 0 PID: 11 Comm: migration/0 Tainted: G      D          4.15.0-rc8-00103-g9409c1e175be-dirty #1
[    0.883064] Hardware name: linux,dummy-virt (DT)
[    0.887680] pstate: 00000085 (nzcv daIf -PAN -UAO)
[    0.892453] pc : __arm_smccc_smc+0x0/0x2c
[    0.896466] lr : enable_tx2_psci_bp_hardening+0x6c/0x108
[    0.901767] sp : ffff000009d73d30
[    0.905078] x29: ffff000009d73d40 x28: 0000000000000000 
[    0.910404] x27: ffff00000803bc88 x26: 0000000000000001 
[    0.915706] x25: ffff000008d13980 x24: ffff00000907b575 
[    0.921040] x23: 0000000000000001 x22: 0000000000000000 
[    0.926357] x21: ffff00000803bd3c x20: ffff00000803bd18 
[    0.931660] x19: ffff0000089f2438 x18: 0000000000000010 
[    0.936954] x17: 00000000ffffff80 x16: 00000000bad0c696 
[    0.942280] x15: 0000000000000000 x14: 0000000000000400 
[    0.947567] x13: 0000000000000400 x12: 0000000000000001 
[    0.952861] x11: 0000000002014024 x10: 0000000000000a00 
[    0.958179] x9 : ffff000009d73d80 x8 : ffff8001f68e1860 
[    0.963460] x7 : 0000000000000000 x6 : 0000000000000000 
[    0.968761] x5 : 0000000000000000 x4 : 0000000000000000 
[    0.974071] x3 : 0000000000000000 x2 : 0000000000000000 
[    0.979341] x1 : 000000000000b0a0 x0 : 00000000c200ff00 
[    0.984676] Process migration/0 (pid: 11, stack limit = 0x0000000054c91580)
[    0.991623] Call trace:
[    0.994062]  __arm_smccc_smc+0x0/0x2c
[    0.997749]  multi_cpu_stop+0x8c/0x110
[    1.001495]  cpu_stopper_thread+0xac/0x120
[    1.005567]  smpboot_thread_fn+0x158/0x240
[    1.009665]  kthread+0x128/0x130
[    1.012881]  ret_from_fork+0x10/0x18
[    1.016435] Code: 2a080042 b8236885 29008829 17ffffc0 (d4000003) 
[    1.022526] ---[ end trace 169f08213b3163be ]---
[    1.027146] note: migration/0[11] exited with preempt_count 1
Jayachandran C Jan. 18, 2018, 5:56 p.m. UTC | #14
On Thu, Jan 18, 2018 at 01:53:55PM +0000, Will Deacon wrote:
> Hi JC,

> 

> On Tue, Jan 16, 2018 at 03:45:54PM -0800, Jayachandran C wrote:

> > On Tue, Jan 16, 2018 at 04:52:53PM -0500, Jon Masters wrote:

> > > On 01/09/2018 07:47 AM, Jayachandran C wrote:

> > > 

> > > > Use PSCI based mitigation for speculative execution attacks targeting

> > > > the branch predictor. The approach is similar to the one used for

> > > > Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

> > > > test if the firmware supports the capability.

> > > > 

> > > > If the secure firmware has been updated with the mitigation code to

> > > > invalidate the branch target buffer, we use the PSCI version call to

> > > > invoke it.

> > > 

> > > What's the status of this patch currently? Previously you had suggested

> > > to hold while the SMC got standardized, but then you seemed happy with

> > > pulling in. What's the latest?

> > 

> > My understanding is that the SMC standardization is being worked on

> > but will take more time, and the KPTI current patchset will go to

> > mainline before that.

> > 

> > Given that, I would expect arm64 maintainers to pick up this patch for

> > ThunderX2, but I have not seen any comments so far.

> > 

> > Will/Marc, please let me know if you are planning to pick this patch

> > into the KPTI tree.

> 

> Are you really sure you want us to apply this? If we do, then you can't run

> KVM guests anymore because your IMPDEF SMC results in an UNDEF being

> injected (crash below).

> 

> I really think that you should just hook up the enable_psci_bp_hardening

> callback like we've done for the Cortex CPUs. We can optimise this later

> once the SMC standarisation work has been completed (which is nearly final

> now and works in a backwards-compatible manner).


I think Marc's patch here:
https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/kpti&id=d35e77fae4b70331310c3bc1796bb43b93f9a85e
handles returning for undefined smc calls in guest.

I think in this case we have to choose between crashing or giving a false
sense of security when a guest compiled with HARDEN_BRANCH_PREDICTOR is
booted on an hypervisor that does not support hardening. Crashing maybe
a reasonable option.

JC.
Jon Masters Jan. 18, 2018, 6:27 p.m. UTC | #15
On 01/18/2018 12:56 PM, Jayachandran C wrote:
> On Thu, Jan 18, 2018 at 01:53:55PM +0000, Will Deacon wrote:

>> Hi JC,

>>

>> On Tue, Jan 16, 2018 at 03:45:54PM -0800, Jayachandran C wrote:

>>> On Tue, Jan 16, 2018 at 04:52:53PM -0500, Jon Masters wrote:

>>>> On 01/09/2018 07:47 AM, Jayachandran C wrote:

>>>>

>>>>> Use PSCI based mitigation for speculative execution attacks targeting

>>>>> the branch predictor. The approach is similar to the one used for

>>>>> Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

>>>>> test if the firmware supports the capability.

>>>>>

>>>>> If the secure firmware has been updated with the mitigation code to

>>>>> invalidate the branch target buffer, we use the PSCI version call to

>>>>> invoke it.

>>>>

>>>> What's the status of this patch currently? Previously you had suggested

>>>> to hold while the SMC got standardized, but then you seemed happy with

>>>> pulling in. What's the latest?

>>>

>>> My understanding is that the SMC standardization is being worked on

>>> but will take more time, and the KPTI current patchset will go to

>>> mainline before that.

>>>

>>> Given that, I would expect arm64 maintainers to pick up this patch for

>>> ThunderX2, but I have not seen any comments so far.

>>>

>>> Will/Marc, please let me know if you are planning to pick this patch

>>> into the KPTI tree.

>>

>> Are you really sure you want us to apply this? If we do, then you can't run

>> KVM guests anymore because your IMPDEF SMC results in an UNDEF being

>> injected (crash below).

>>

>> I really think that you should just hook up the enable_psci_bp_hardening

>> callback like we've done for the Cortex CPUs. We can optimise this later

>> once the SMC standarisation work has been completed (which is nearly final

>> now and works in a backwards-compatible manner).

> 

> I think Marc's patch here:

> https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/kpti&id=d35e77fae4b70331310c3bc1796bb43b93f9a85e

> handles returning for undefined smc calls in guest.

> 

> I think in this case we have to choose between crashing or giving a false

> sense of security when a guest compiled with HARDEN_BRANCH_PREDICTOR is

> booted on an hypervisor that does not support hardening. Crashing maybe

> a reasonable option.


Crashing is a completely unreasonable option and is totally
unacceptable. We never do this in enterprise, period.

It's reasonable to give an output in dmesg that a system isn't hardened,
but it's not reasonable to crash. On x86, we added a new qemu machine
type for those guests that would have IBRS exposed, and ask users to
switch that on explicitly, but even if they boot the new kernels on
unpatched infrastructure, we'll detect the lack of the branch predictor
control interface and just log that.

The exact same thing should happen on ARM.

Jon.
Jayachandran C Jan. 18, 2018, 11:28 p.m. UTC | #16
Hi Jon,

On Thu, Jan 18, 2018 at 01:27:15PM -0500, Jon Masters wrote:
> On 01/18/2018 12:56 PM, Jayachandran C wrote:

> > On Thu, Jan 18, 2018 at 01:53:55PM +0000, Will Deacon wrote:

> >> Hi JC,

> >>

> >> On Tue, Jan 16, 2018 at 03:45:54PM -0800, Jayachandran C wrote:

> >>> On Tue, Jan 16, 2018 at 04:52:53PM -0500, Jon Masters wrote:

> >>>> On 01/09/2018 07:47 AM, Jayachandran C wrote:

> >>>>

> >>>>> Use PSCI based mitigation for speculative execution attacks targeting

> >>>>> the branch predictor. The approach is similar to the one used for

> >>>>> Cortex-A CPUs, but in case of ThunderX2 we add another SMC call to

> >>>>> test if the firmware supports the capability.

> >>>>>

> >>>>> If the secure firmware has been updated with the mitigation code to

> >>>>> invalidate the branch target buffer, we use the PSCI version call to

> >>>>> invoke it.

> >>>>

> >>>> What's the status of this patch currently? Previously you had suggested

> >>>> to hold while the SMC got standardized, but then you seemed happy with

> >>>> pulling in. What's the latest?

> >>>

> >>> My understanding is that the SMC standardization is being worked on

> >>> but will take more time, and the KPTI current patchset will go to

> >>> mainline before that.

> >>>

> >>> Given that, I would expect arm64 maintainers to pick up this patch for

> >>> ThunderX2, but I have not seen any comments so far.

> >>>

> >>> Will/Marc, please let me know if you are planning to pick this patch

> >>> into the KPTI tree.

> >>

> >> Are you really sure you want us to apply this? If we do, then you can't run

> >> KVM guests anymore because your IMPDEF SMC results in an UNDEF being

> >> injected (crash below).

> >>

> >> I really think that you should just hook up the enable_psci_bp_hardening

> >> callback like we've done for the Cortex CPUs. We can optimise this later

> >> once the SMC standarisation work has been completed (which is nearly final

> >> now and works in a backwards-compatible manner).

> > 

> > I think Marc's patch here:

> > https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/commit/?h=kvm-arm64/kpti&id=d35e77fae4b70331310c3bc1796bb43b93f9a85e

> > handles returning for undefined smc calls in guest.

> > 

> > I think in this case we have to choose between crashing or giving a false

> > sense of security when a guest compiled with HARDEN_BRANCH_PREDICTOR is

> > booted on an hypervisor that does not support hardening. Crashing maybe

> > a reasonable option.

> 

> Crashing is a completely unreasonable option and is totally

> unacceptable. We never do this in enterprise, period.

> 

> It's reasonable to give an output in dmesg that a system isn't hardened,

> but it's not reasonable to crash. On x86, we added a new qemu machine

> type for those guests that would have IBRS exposed, and ask users to

> switch that on explicitly, but even if they boot the new kernels on

> unpatched infrastructure, we'll detect the lack of the branch predictor

> control interface and just log that.

> 

> The exact same thing should happen on ARM.


With the current patchset from ARM, there is no way of detecting if the
hypervisor is hardened or not, to provide the warning.  The only other
option I have call get version blindly and provide a false sense of
security.

Since both options are bad, I don't have a good solution here. If RedHat
has a preference here on what would be better, I can go with that.

JC.
Jon Masters Jan. 19, 2018, 1:17 a.m. UTC | #17
Hi JC, Will,

On 01/18/2018 06:28 PM, Jayachandran C wrote:
> On Thu, Jan 18, 2018 at 01:27:15PM -0500, Jon Masters wrote:

>> On 01/18/2018 12:56 PM, Jayachandran C wrote:

>>> On Thu, Jan 18, 2018 at 01:53:55PM +0000, Will Deacon wrote:


>>> I think in this case we have to choose between crashing or giving a false

>>> sense of security when a guest compiled with HARDEN_BRANCH_PREDICTOR is

>>> booted on an hypervisor that does not support hardening. Crashing maybe

>>> a reasonable option.

>>

>> Crashing is a completely unreasonable option and is totally

>> unacceptable. We never do this in enterprise, period.

>>

>> It's reasonable to give an output in dmesg that a system isn't hardened,

>> but it's not reasonable to crash. On x86, we added a new qemu machine

>> type for those guests that would have IBRS exposed, and ask users to

>> switch that on explicitly, but even if they boot the new kernels on

>> unpatched infrastructure, we'll detect the lack of the branch predictor

>> control interface and just log that.

>>

>> The exact same thing should happen on ARM.

> 

> With the current patchset from ARM, there is no way of detecting if the

> hypervisor is hardened or not, to provide the warning.  The only other

> option I have call get version blindly and provide a false sense of

> security.


Agreed that (unless) I'm missing something, the current arm patchset
doesn't have an enumeration mechanism to see if firmware supports the
branch predictor hardening or not. Am I missing something there?

On the three other affected arches we're tracking, there's an
enumeration mechanism. On x86, there's a new set of CPUID bits. On
POWER, there's a new hcall that tells us whether the millicode supports
what we need, and on z there's a new facility code we can test for that
is also passed into VMs. So we need to have a similar enumeration
mechanism on ARM that is passed into guests as well.

> Since both options are bad, I don't have a good solution here. If RedHat

> has a preference here on what would be better, I can go with that.


We need an enumeration mechanism that determines whether the hypervisor
is patched. In the absence of that, blindly calling in and hoping that
the firmware is updated is better than nothing. I'll look to see if
there's a generic upstream solution for enumeration that I've missed (or
that can be added, perhaps a new SMC enumeration mechanism). If there
isn't a short term fix, we'll work with you guys directly to add
something RHEL specific by checking some firmware version somehow.

Jon.
Jon Masters Jan. 19, 2018, 7:08 p.m. UTC | #18
On 01/19/2018 07:22 AM, Jayachandran C wrote:
> Use PSCI based mitigation for speculative execution attacks targeting

> the branch predictor. We use the same mechanism as the one used for

> Cortex-A CPUs, we expect the PSCI version call to have a side effect

> of clearing the BTBs.

> 

> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

> ---

>  arch/arm64/kernel/cpu_errata.c | 10 ++++++++++

>  1 file changed, 10 insertions(+)

> 

> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c

> index 70e5f18..45ff9a2 100644

> --- a/arch/arm64/kernel/cpu_errata.c

> +++ b/arch/arm64/kernel/cpu_errata.c

> @@ -338,6 +338,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {

>  		.capability = ARM64_HARDEN_BP_POST_GUEST_EXIT,

>  		MIDR_ALL_VERSIONS(MIDR_QCOM_FALKOR_V1),

>  	},

> +	{

> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

> +		MIDR_ALL_VERSIONS(MIDR_BRCM_VULCAN),

> +		.enable = enable_psci_bp_hardening,

> +	},

> +	{

> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

> +		MIDR_ALL_VERSIONS(MIDR_CAVIUM_THUNDERX2),

> +		.enable = enable_psci_bp_hardening,

> +	},

>  #endif

>  	{

>  	}

> 


Both of these patches seem reasonable to me.
Will Deacon Jan. 22, 2018, 11:33 a.m. UTC | #19
On Fri, Jan 19, 2018 at 04:22:47AM -0800, Jayachandran C wrote:
> Use PSCI based mitigation for speculative execution attacks targeting

> the branch predictor. We use the same mechanism as the one used for

> Cortex-A CPUs, we expect the PSCI version call to have a side effect

> of clearing the BTBs.

> 

> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

> ---

>  arch/arm64/kernel/cpu_errata.c | 10 ++++++++++

>  1 file changed, 10 insertions(+)

> 

> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c

> index 70e5f18..45ff9a2 100644

> --- a/arch/arm64/kernel/cpu_errata.c

> +++ b/arch/arm64/kernel/cpu_errata.c

> @@ -338,6 +338,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {

>  		.capability = ARM64_HARDEN_BP_POST_GUEST_EXIT,

>  		MIDR_ALL_VERSIONS(MIDR_QCOM_FALKOR_V1),

>  	},

> +	{

> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

> +		MIDR_ALL_VERSIONS(MIDR_BRCM_VULCAN),

> +		.enable = enable_psci_bp_hardening,

> +	},

> +	{

> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

> +		MIDR_ALL_VERSIONS(MIDR_CAVIUM_THUNDERX2),

> +		.enable = enable_psci_bp_hardening,

> +	},

>  #endif


Thanks.

Acked-by: Will Deacon <will.deacon@arm.com>


Will
Will Deacon Jan. 22, 2018, 11:41 a.m. UTC | #20
On Fri, Jan 19, 2018 at 04:22:48AM -0800, Jayachandran C wrote:
> Whitelist Broadcom Vulcan/Cavium ThunderX2 processors in

> unmap_kernel_at_el0(). These CPUs are not vulnerable to

> CVE-2017-5754 and do not need KPTI when KASLR is off.

> 

> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

> ---

>  arch/arm64/kernel/cpufeature.c | 7 +++++++

>  1 file changed, 7 insertions(+)

> 

> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c

> index 647d44b..fb698ca 100644

> --- a/arch/arm64/kernel/cpufeature.c

> +++ b/arch/arm64/kernel/cpufeature.c

> @@ -866,6 +866,13 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,

>  	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))

>  		return true;

>  

> +	/* Don't force KPTI for CPUs that are not vulnerable */

> +	switch (read_cpuid_id() & MIDR_CPU_MODEL_MASK) {

> +	case MIDR_CAVIUM_THUNDERX2:

> +	case MIDR_BRCM_VULCAN:

> +		return false;

> +	}

> +

>  	/* Defer to CPU feature registers */

>  	return !cpuid_feature_extract_unsigned_field(pfr0,

>  						     ID_AA64PFR0_CSV3_SHIFT);


We'll need to re-jig this to work properly with big/little because this is
only called once, but that's ok for now:

Acked-by: Will Deacon <will.deacon@arm.com>


Suzuki has a series reworking much of the cpufeatures code so that we can
do this properly for 4.17.

Will
Ard Biesheuvel Jan. 22, 2018, 11:51 a.m. UTC | #21
On 22 January 2018 at 11:41, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Jan 19, 2018 at 04:22:48AM -0800, Jayachandran C wrote:

>> Whitelist Broadcom Vulcan/Cavium ThunderX2 processors in

>> unmap_kernel_at_el0(). These CPUs are not vulnerable to

>> CVE-2017-5754 and do not need KPTI when KASLR is off.

>>

>> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

>> ---

>>  arch/arm64/kernel/cpufeature.c | 7 +++++++

>>  1 file changed, 7 insertions(+)

>>

>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c

>> index 647d44b..fb698ca 100644

>> --- a/arch/arm64/kernel/cpufeature.c

>> +++ b/arch/arm64/kernel/cpufeature.c

>> @@ -866,6 +866,13 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,

>>       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))

>>               return true;

>>

>> +     /* Don't force KPTI for CPUs that are not vulnerable */

>> +     switch (read_cpuid_id() & MIDR_CPU_MODEL_MASK) {

>> +     case MIDR_CAVIUM_THUNDERX2:

>> +     case MIDR_BRCM_VULCAN:

>> +             return false;

>> +     }

>> +

>>       /* Defer to CPU feature registers */

>>       return !cpuid_feature_extract_unsigned_field(pfr0,

>>                                                    ID_AA64PFR0_CSV3_SHIFT);

>

> We'll need to re-jig this to work properly with big/little because this is

> only called once, but that's ok for now:

>

> Acked-by: Will Deacon <will.deacon@arm.com>

>

> Suzuki has a series reworking much of the cpufeatures code so that we can

> do this properly for 4.17.

>


If we start adding opt outs here, we should at least include A53, and
probably replace

>>       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))

>>               return true;


with

>>       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_offset() > 0)

>>               return true;


so that adding 'nokaslr' to the command line also disables KPTI.
Will Deacon Jan. 22, 2018, 11:55 a.m. UTC | #22
On Mon, Jan 22, 2018 at 11:51:34AM +0000, Ard Biesheuvel wrote:
> On 22 January 2018 at 11:41, Will Deacon <will.deacon@arm.com> wrote:

> > On Fri, Jan 19, 2018 at 04:22:48AM -0800, Jayachandran C wrote:

> >> Whitelist Broadcom Vulcan/Cavium ThunderX2 processors in

> >> unmap_kernel_at_el0(). These CPUs are not vulnerable to

> >> CVE-2017-5754 and do not need KPTI when KASLR is off.

> >>

> >> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

> >> ---

> >>  arch/arm64/kernel/cpufeature.c | 7 +++++++

> >>  1 file changed, 7 insertions(+)

> >>

> >> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c

> >> index 647d44b..fb698ca 100644

> >> --- a/arch/arm64/kernel/cpufeature.c

> >> +++ b/arch/arm64/kernel/cpufeature.c

> >> @@ -866,6 +866,13 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,

> >>       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))

> >>               return true;

> >>

> >> +     /* Don't force KPTI for CPUs that are not vulnerable */

> >> +     switch (read_cpuid_id() & MIDR_CPU_MODEL_MASK) {

> >> +     case MIDR_CAVIUM_THUNDERX2:

> >> +     case MIDR_BRCM_VULCAN:

> >> +             return false;

> >> +     }

> >> +

> >>       /* Defer to CPU feature registers */

> >>       return !cpuid_feature_extract_unsigned_field(pfr0,

> >>                                                    ID_AA64PFR0_CSV3_SHIFT);

> >

> > We'll need to re-jig this to work properly with big/little because this is

> > only called once, but that's ok for now:

> >

> > Acked-by: Will Deacon <will.deacon@arm.com>

> >

> > Suzuki has a series reworking much of the cpufeatures code so that we can

> > do this properly for 4.17.

> >

> 

> If we start adding opt outs here, we should at least include A53, and

> probably replace

> 

> >>       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))

> >>               return true;

> 

> with

> 

> >>       if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_offset() > 0)

> >>               return true;

> 

> so that adding 'nokaslr' to the command line also disables KPTI.


Yup, I was going to do this once we have the new cpufeatures code from
Suzuki and can safely whitelist cores that can appear in big/little
configurations.

Will
Jon Masters Jan. 22, 2018, 6:59 p.m. UTC | #23
On 01/22/2018 06:41 AM, Will Deacon wrote:
> On Fri, Jan 19, 2018 at 04:22:48AM -0800, Jayachandran C wrote:

>> Whitelist Broadcom Vulcan/Cavium ThunderX2 processors in

>> unmap_kernel_at_el0(). These CPUs are not vulnerable to

>> CVE-2017-5754 and do not need KPTI when KASLR is off.

>>

>> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

>> ---

>>  arch/arm64/kernel/cpufeature.c | 7 +++++++

>>  1 file changed, 7 insertions(+)

>>

>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c

>> index 647d44b..fb698ca 100644

>> --- a/arch/arm64/kernel/cpufeature.c

>> +++ b/arch/arm64/kernel/cpufeature.c

>> @@ -866,6 +866,13 @@ static bool unmap_kernel_at_el0(const struct arm64_cpu_capabilities *entry,

>>  	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))

>>  		return true;

>>  

>> +	/* Don't force KPTI for CPUs that are not vulnerable */

>> +	switch (read_cpuid_id() & MIDR_CPU_MODEL_MASK) {

>> +	case MIDR_CAVIUM_THUNDERX2:

>> +	case MIDR_BRCM_VULCAN:

>> +		return false;

>> +	}

>> +

>>  	/* Defer to CPU feature registers */

>>  	return !cpuid_feature_extract_unsigned_field(pfr0,

>>  						     ID_AA64PFR0_CSV3_SHIFT);

> 

> We'll need to re-jig this to work properly with big/little because this is

> only called once, but that's ok for now:

> 

> Acked-by: Will Deacon <will.deacon@arm.com>

> 

> Suzuki has a series reworking much of the cpufeatures code so that we can

> do this properly for 4.17.


Thanks, much appreciated.

Jon.
Jon Masters Jan. 22, 2018, 7 p.m. UTC | #24
On 01/22/2018 06:33 AM, Will Deacon wrote:
> On Fri, Jan 19, 2018 at 04:22:47AM -0800, Jayachandran C wrote:

>> Use PSCI based mitigation for speculative execution attacks targeting

>> the branch predictor. We use the same mechanism as the one used for

>> Cortex-A CPUs, we expect the PSCI version call to have a side effect

>> of clearing the BTBs.

>>

>> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

>> ---

>>  arch/arm64/kernel/cpu_errata.c | 10 ++++++++++

>>  1 file changed, 10 insertions(+)

>>

>> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c

>> index 70e5f18..45ff9a2 100644

>> --- a/arch/arm64/kernel/cpu_errata.c

>> +++ b/arch/arm64/kernel/cpu_errata.c

>> @@ -338,6 +338,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {

>>  		.capability = ARM64_HARDEN_BP_POST_GUEST_EXIT,

>>  		MIDR_ALL_VERSIONS(MIDR_QCOM_FALKOR_V1),

>>  	},

>> +	{

>> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

>> +		MIDR_ALL_VERSIONS(MIDR_BRCM_VULCAN),

>> +		.enable = enable_psci_bp_hardening,

>> +	},

>> +	{

>> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

>> +		MIDR_ALL_VERSIONS(MIDR_CAVIUM_THUNDERX2),

>> +		.enable = enable_psci_bp_hardening,

>> +	},

>>  #endif

> 

> Thanks.

> 

> Acked-by: Will Deacon <will.deacon@arm.com>


Thanks. I have separately asked for a specification tweak to allow us to
discover whether firmware has been augmented to provide the necessary
support that we need. That applies beyond Cavium.

(for now in RHEL, we've asked the vendors to give us a temporary patch
that we can match DMI or other data later in boot and warn users on)

Jon.
Will Deacon Jan. 23, 2018, 9:51 a.m. UTC | #25
On Mon, Jan 22, 2018 at 02:00:59PM -0500, Jon Masters wrote:
> On 01/22/2018 06:33 AM, Will Deacon wrote:

> > On Fri, Jan 19, 2018 at 04:22:47AM -0800, Jayachandran C wrote:

> >> Use PSCI based mitigation for speculative execution attacks targeting

> >> the branch predictor. We use the same mechanism as the one used for

> >> Cortex-A CPUs, we expect the PSCI version call to have a side effect

> >> of clearing the BTBs.

> >>

> >> Signed-off-by: Jayachandran C <jnair@caviumnetworks.com>

> >> ---

> >>  arch/arm64/kernel/cpu_errata.c | 10 ++++++++++

> >>  1 file changed, 10 insertions(+)

> >>

> >> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c

> >> index 70e5f18..45ff9a2 100644

> >> --- a/arch/arm64/kernel/cpu_errata.c

> >> +++ b/arch/arm64/kernel/cpu_errata.c

> >> @@ -338,6 +338,16 @@ const struct arm64_cpu_capabilities arm64_errata[] = {

> >>  		.capability = ARM64_HARDEN_BP_POST_GUEST_EXIT,

> >>  		MIDR_ALL_VERSIONS(MIDR_QCOM_FALKOR_V1),

> >>  	},

> >> +	{

> >> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

> >> +		MIDR_ALL_VERSIONS(MIDR_BRCM_VULCAN),

> >> +		.enable = enable_psci_bp_hardening,

> >> +	},

> >> +	{

> >> +		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,

> >> +		MIDR_ALL_VERSIONS(MIDR_CAVIUM_THUNDERX2),

> >> +		.enable = enable_psci_bp_hardening,

> >> +	},

> >>  #endif

> > 

> > Thanks.

> > 

> > Acked-by: Will Deacon <will.deacon@arm.com>

> 

> Thanks. I have separately asked for a specification tweak to allow us to

> discover whether firmware has been augmented to provide the necessary

> support that we need. That applies beyond Cavium.


AFAIK, there's already an SMCCC/PSCI proposal doing the rounds that is
discoverable and does what we need. Have you seen it? We should be posting
code this week.

Will
diff mbox series

Patch

diff --git a/arch/arm64/kernel/bpi.S b/arch/arm64/kernel/bpi.S
index 06a931eb2673..2e9146534174 100644
--- a/arch/arm64/kernel/bpi.S
+++ b/arch/arm64/kernel/bpi.S
@@ -53,3 +53,27 @@  ENTRY(__bp_harden_hyp_vecs_start)
 	vectors __kvm_hyp_vector
 	.endr
 ENTRY(__bp_harden_hyp_vecs_end)
+ENTRY(__psci_hyp_bp_inval_start)
+	sub	sp, sp, #(8 * 18)
+	stp	x16, x17, [sp, #(16 * 0)]
+	stp	x14, x15, [sp, #(16 * 1)]
+	stp	x12, x13, [sp, #(16 * 2)]
+	stp	x10, x11, [sp, #(16 * 3)]
+	stp	x8, x9, [sp, #(16 * 4)]
+	stp	x6, x7, [sp, #(16 * 5)]
+	stp	x4, x5, [sp, #(16 * 6)]
+	stp	x2, x3, [sp, #(16 * 7)]
+	stp	x0, x1, [sp, #(18 * 8)]
+	mov	x0, #0x84000000
+	smc	#0
+	ldp	x16, x17, [sp, #(16 * 0)]
+	ldp	x14, x15, [sp, #(16 * 1)]
+	ldp	x12, x13, [sp, #(16 * 2)]
+	ldp	x10, x11, [sp, #(16 * 3)]
+	ldp	x8, x9, [sp, #(16 * 4)]
+	ldp	x6, x7, [sp, #(16 * 5)]
+	ldp	x4, x5, [sp, #(16 * 6)]
+	ldp	x2, x3, [sp, #(16 * 7)]
+	ldp	x0, x1, [sp, #(18 * 8)]
+	add	sp, sp, #(8 * 18)
+ENTRY(__psci_hyp_bp_inval_end)
diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c
index 16ea5c6f314e..cb0fb3796bb8 100644
--- a/arch/arm64/kernel/cpu_errata.c
+++ b/arch/arm64/kernel/cpu_errata.c
@@ -53,6 +53,8 @@  static int cpu_enable_trap_ctr_access(void *__unused)
 DEFINE_PER_CPU_READ_MOSTLY(struct bp_hardening_data, bp_hardening_data);
 
 #ifdef CONFIG_KVM
+extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
+
 static void __copy_hyp_vect_bpi(int slot, const char *hyp_vecs_start,
 				const char *hyp_vecs_end)
 {
@@ -94,6 +96,9 @@  static void __install_bp_hardening_cb(bp_hardening_cb_t fn,
 	spin_unlock(&bp_lock);
 }
 #else
+#define __psci_hyp_bp_inval_start	NULL
+#define __psci_hyp_bp_inval_end		NULL
+
 static void __install_bp_hardening_cb(bp_hardening_cb_t fn,
 				      const char *hyp_vecs_start,
 				      const char *hyp_vecs_end)
@@ -118,6 +123,21 @@  static void  install_bp_hardening_cb(const struct arm64_cpu_capabilities *entry,
 
 	__install_bp_hardening_cb(fn, hyp_vecs_start, hyp_vecs_end);
 }
+
+#include <linux/psci.h>
+
+static int enable_psci_bp_hardening(void *data)
+{
+	const struct arm64_cpu_capabilities *entry = data;
+
+	if (psci_ops.get_version)
+		install_bp_hardening_cb(entry,
+				       (bp_hardening_cb_t)psci_ops.get_version,
+				       __psci_hyp_bp_inval_start,
+				       __psci_hyp_bp_inval_end);
+
+	return 0;
+}
 #endif	/* CONFIG_HARDEN_BRANCH_PREDICTOR */
 
 #define MIDR_RANGE(model, min, max) \
@@ -261,6 +281,28 @@  const struct arm64_cpu_capabilities arm64_errata[] = {
 		MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
 	},
 #endif
+#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
+	{
+		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
+		.enable = enable_psci_bp_hardening,
+	},
+	{
+		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
+		.enable = enable_psci_bp_hardening,
+	},
+	{
+		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
+		.enable = enable_psci_bp_hardening,
+	},
+	{
+		.capability = ARM64_HARDEN_BRANCH_PREDICTOR,
+		MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
+		.enable = enable_psci_bp_hardening,
+	},
+#endif
 	{
 	}
 };