[4.9,V2,09/24] ARM: spectre-v2: add firmware based hardening

Message ID 20181107164402.9380-10-dave.long@linaro.org
State New
Headers show
Series
  • V4.9 backport of 32-bit arm spectre patches
Related show

Commit Message

David Long Nov. 7, 2018, 4:43 p.m.
From: Russell King <rmk+kernel@armlinux.org.uk>


Commit 10115105cb3aa17b5da1cb726ae8dd5f6854bd93 upstream.
Commit 6282e916f774e37845c65d1eae9f8c649004f033 upstream.

Add firmware based hardening for cores that require more complex
handling in firmware.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Boot-tested-by: Tony Lindgren <tony@atomide.com>
Reviewed-by: Tony Lindgren <tony@atomide.com>

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

Signed-off-by: David A. Long <dave.long@linaro.org>

---
 arch/arm/mm/proc-v7-bugs.c | 60 ++++++++++++++++++++++++++++++++++++++
 arch/arm/mm/proc-v7.S      | 21 +++++++++++++
 2 files changed, 81 insertions(+)

-- 
2.17.1

Comments

Russell King - ARM Linux Nov. 12, 2018, 4:54 p.m. | #1
Marc,

Can you please ack this to say that you are now happy with it after
your comments on version 1, so we can move forward and have Greg
merge it.

Thanks.

On Wed, Nov 07, 2018 at 11:43:47AM -0500, David Long wrote:
> From: Russell King <rmk+kernel@armlinux.org.uk>

> 

> Commit 10115105cb3aa17b5da1cb726ae8dd5f6854bd93 upstream.

> Commit 6282e916f774e37845c65d1eae9f8c649004f033 upstream.

> 

> Add firmware based hardening for cores that require more complex

> handling in firmware.

> 

> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> Boot-tested-by: Tony Lindgren <tony@atomide.com>

> Reviewed-by: Tony Lindgren <tony@atomide.com>

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

> Signed-off-by: David A. Long <dave.long@linaro.org>

> ---

>  arch/arm/mm/proc-v7-bugs.c | 60 ++++++++++++++++++++++++++++++++++++++

>  arch/arm/mm/proc-v7.S      | 21 +++++++++++++

>  2 files changed, 81 insertions(+)

> 

> diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c

> index 85a2e3d6263c..da25a38e1897 100644

> --- a/arch/arm/mm/proc-v7-bugs.c

> +++ b/arch/arm/mm/proc-v7-bugs.c

> @@ -1,14 +1,20 @@

>  // SPDX-License-Identifier: GPL-2.0

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

>  #include <linux/kernel.h>

> +#include <linux/psci.h>

>  #include <linux/smp.h>

>  

>  #include <asm/cp15.h>

>  #include <asm/cputype.h>

> +#include <asm/proc-fns.h>

>  #include <asm/system_misc.h>

>  

>  #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR

>  DEFINE_PER_CPU(harden_branch_predictor_fn_t, harden_branch_predictor_fn);

>  

> +extern void cpu_v7_smc_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);

> +extern void cpu_v7_hvc_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);

> +

>  static void harden_branch_predictor_bpiall(void)

>  {

>  	write_sysreg(0, BPIALL);

> @@ -19,6 +25,16 @@ static void harden_branch_predictor_iciallu(void)

>  	write_sysreg(0, ICIALLU);

>  }

>  

> +static void __maybe_unused call_smc_arch_workaround_1(void)

> +{

> +	arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);

> +}

> +

> +static void __maybe_unused call_hvc_arch_workaround_1(void)

> +{

> +	arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);

> +}

> +

>  static void cpu_v7_spectre_init(void)

>  {

>  	const char *spectre_v2_method = NULL;

> @@ -45,7 +61,51 @@ static void cpu_v7_spectre_init(void)

>  			harden_branch_predictor_iciallu;

>  		spectre_v2_method = "ICIALLU";

>  		break;

> +

> +#ifdef CONFIG_ARM_PSCI

> +	default:

> +		/* Other ARM CPUs require no workaround */

> +		if (read_cpuid_implementor() == ARM_CPU_IMP_ARM)

> +			break;

> +		/* fallthrough */

> +		/* Cortex A57/A72 require firmware workaround */

> +	case ARM_CPU_PART_CORTEX_A57:

> +	case ARM_CPU_PART_CORTEX_A72: {

> +		struct arm_smccc_res res;

> +

> +		if (psci_ops.smccc_version == SMCCC_VERSION_1_0)

> +			break;

> +

> +		switch (psci_ops.conduit) {

> +		case PSCI_CONDUIT_HVC:

> +			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,

> +					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);

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

> +				break;

> +			per_cpu(harden_branch_predictor_fn, cpu) =

> +				call_hvc_arch_workaround_1;

> +			processor.switch_mm = cpu_v7_hvc_switch_mm;

> +			spectre_v2_method = "hypervisor";

> +			break;

> +

> +		case PSCI_CONDUIT_SMC:

> +			arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,

> +					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);

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

> +				break;

> +			per_cpu(harden_branch_predictor_fn, cpu) =

> +				call_smc_arch_workaround_1;

> +			processor.switch_mm = cpu_v7_smc_switch_mm;

> +			spectre_v2_method = "firmware";

> +			break;

> +

> +		default:

> +			break;

> +		}

>  	}

> +#endif

> +	}

> +

>  	if (spectre_v2_method)

>  		pr_info("CPU%u: Spectre v2: using %s workaround\n",

>  			smp_processor_id(), spectre_v2_method);

> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S

> index 2d2e5ae85816..850c22bca19c 100644

> --- a/arch/arm/mm/proc-v7.S

> +++ b/arch/arm/mm/proc-v7.S

> @@ -9,6 +9,7 @@

>   *

>   *  This is the "shell" of the ARMv7 processor support.

>   */

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

>  #include <linux/init.h>

>  #include <linux/linkage.h>

>  #include <asm/assembler.h>

> @@ -88,6 +89,26 @@ ENTRY(cpu_v7_dcache_clean_area)

>  	ret	lr

>  ENDPROC(cpu_v7_dcache_clean_area)

>  

> +#ifdef CONFIG_ARM_PSCI

> +	.arch_extension sec

> +ENTRY(cpu_v7_smc_switch_mm)

> +	stmfd	sp!, {r0 - r3}

> +	movw	r0, #:lower16:ARM_SMCCC_ARCH_WORKAROUND_1

> +	movt	r0, #:upper16:ARM_SMCCC_ARCH_WORKAROUND_1

> +	smc	#0

> +	ldmfd	sp!, {r0 - r3}

> +	b	cpu_v7_switch_mm

> +ENDPROC(cpu_v7_smc_switch_mm)

> +	.arch_extension virt

> +ENTRY(cpu_v7_hvc_switch_mm)

> +	stmfd	sp!, {r0 - r3}

> +	movw	r0, #:lower16:ARM_SMCCC_ARCH_WORKAROUND_1

> +	movt	r0, #:upper16:ARM_SMCCC_ARCH_WORKAROUND_1

> +	hvc	#0

> +	ldmfd	sp!, {r0 - r3}

> +	b	cpu_v7_switch_mm

> +ENDPROC(cpu_v7_hvc_switch_mm)

> +#endif

>  ENTRY(cpu_v7_iciallu_switch_mm)

>  	mov	r3, #0

>  	mcr	p15, 0, r3, c7, c5, 0		@ ICIALLU

> -- 

> 2.17.1

> 


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
Marc Zyngier Nov. 13, 2018, 2:23 p.m. | #2
Russell,

On Mon, 12 Nov 2018 16:54:10 +0000,
Russell King - ARM Linux <linux@armlinux.org.uk> wrote:
> 

> Marc,

> 

> Can you please ack this to say that you are now happy with it after

> your comments on version 1, so we can move forward and have Greg

> merge it.

> 

> Thanks.

> 

> On Wed, Nov 07, 2018 at 11:43:47AM -0500, David Long wrote:

> > From: Russell King <rmk+kernel@armlinux.org.uk>

> > 

> > Commit 10115105cb3aa17b5da1cb726ae8dd5f6854bd93 upstream.

> > Commit 6282e916f774e37845c65d1eae9f8c649004f033 upstream.

> > 

> > Add firmware based hardening for cores that require more complex

> > handling in firmware.

> > 

> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> > Boot-tested-by: Tony Lindgren <tony@atomide.com>

> > Reviewed-by: Tony Lindgren <tony@atomide.com>

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

> > Signed-off-by: David A. Long <dave.long@linaro.org>


Sure. Feel free to add my

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


I assume someone has tested these patches (I haven't, and I'm unlikely
to do so in the near future as I'm travelling). I'm not sure Tony's
"Boot-tested-by" is still valid, and Florian's earlier set of tests
didn't show the issues of the initial backport.

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.
David Long Nov. 13, 2018, 3:16 p.m. | #3
On 11/13/18 9:23 AM, Marc Zyngier wrote:
> Russell,

> 

> On Mon, 12 Nov 2018 16:54:10 +0000,

> Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

>>

>> Marc,

>>

>> Can you please ack this to say that you are now happy with it after

>> your comments on version 1, so we can move forward and have Greg

>> merge it.

>>

>> Thanks.

>>

>> On Wed, Nov 07, 2018 at 11:43:47AM -0500, David Long wrote:

>>> From: Russell King <rmk+kernel@armlinux.org.uk>

>>>

>>> Commit 10115105cb3aa17b5da1cb726ae8dd5f6854bd93 upstream.

>>> Commit 6282e916f774e37845c65d1eae9f8c649004f033 upstream.

>>>

>>> Add firmware based hardening for cores that require more complex

>>> handling in firmware.

>>>

>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

>>> Boot-tested-by: Tony Lindgren <tony@atomide.com>

>>> Reviewed-by: Tony Lindgren <tony@atomide.com>

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

>>> Signed-off-by: David A. Long <dave.long@linaro.org>

> 

> Sure. Feel free to add my

> 

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

> 

> I assume someone has tested these patches (I haven't, and I'm unlikely

> to do so in the near future as I'm travelling). I'm not sure Tony's

> "Boot-tested-by" is still valid, and Florian's earlier set of tests

> didn't show the issues of the initial backport.

> 

> Thanks,

> 

> 	M.

> 


I tested the patch set through kernelci and (belatedly) kvm-unit-tests, 
the latter of which revealed the problem in V1 #11/24.  I have to assume 
Florian didn't specifically test kvm, something I myself had originally 
assumed would be covered by kernelci.

I didn't scrub any of the ack/tested/reviewed lines from the original 
patches.  I've always assumed this is the correct way to do this but 
maybe it's not?

-dl
Tony Lindgren Nov. 13, 2018, 4:43 p.m. | #4
* Marc Zyngier <marc.zyngier@arm.com> [181113 14:23]:
> I assume someone has tested these patches (I haven't, and I'm unlikely

> to do so in the near future as I'm travelling). I'm not sure Tony's

> "Boot-tested-by" is still valid, and Florian's earlier set of tests

> didn't show the issues of the initial backport.


To make the "Boot-tested-by" tags still valid, I just applied this
whole series on v4.19.136 and gave it a quick boot test on my boards
between C-A8 to C-A15 and they still boot just fine with this
series and show the spectre workaround status on dmesg.

Thanks,

Tony
Marc Zyngier Nov. 13, 2018, 5:36 p.m. | #5
On Tue, 13 Nov 2018 15:16:03 +0000,
David Long <dave.long@linaro.org> wrote:
> 

> On 11/13/18 9:23 AM, Marc Zyngier wrote:

> > Russell,

> > 

> > On Mon, 12 Nov 2018 16:54:10 +0000,

> > Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> >> 

> >> Marc,

> >> 

> >> Can you please ack this to say that you are now happy with it after

> >> your comments on version 1, so we can move forward and have Greg

> >> merge it.

> >> 

> >> Thanks.

> >> 

> >> On Wed, Nov 07, 2018 at 11:43:47AM -0500, David Long wrote:

> >>> From: Russell King <rmk+kernel@armlinux.org.uk>

> >>> 

> >>> Commit 10115105cb3aa17b5da1cb726ae8dd5f6854bd93 upstream.

> >>> Commit 6282e916f774e37845c65d1eae9f8c649004f033 upstream.

> >>> 

> >>> Add firmware based hardening for cores that require more complex

> >>> handling in firmware.

> >>> 

> >>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> >>> Boot-tested-by: Tony Lindgren <tony@atomide.com>

> >>> Reviewed-by: Tony Lindgren <tony@atomide.com>

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

> >>> Signed-off-by: David A. Long <dave.long@linaro.org>

> > 

> > Sure. Feel free to add my

> > 

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

> > 

> > I assume someone has tested these patches (I haven't, and I'm unlikely

> > to do so in the near future as I'm travelling). I'm not sure Tony's

> > "Boot-tested-by" is still valid, and Florian's earlier set of tests

> > didn't show the issues of the initial backport.

> > 

> > Thanks,

> > 

> > 	M.

> > 

> 

> I tested the patch set through kernelci and (belatedly)

> kvm-unit-tests, the latter of which revealed the problem in V1 #11/24.

> I have to assume Florian didn't specifically test kvm, something I

> myself had originally assumed would be covered by kernelci.

> 

> I didn't scrub any of the ack/tested/reviewed lines from the original

> patches.  I've always assumed this is the correct way to do this but

> maybe it's not?


Leaving the tags is absolutely fine, they indicate that the original
patch was actually tested.

I'm more worried of potential regressions: we've already found two
problems, and although I cannot spot any other, it is fairly obvious
that there has only been a limited amount of testing. It may not be a
problem, but I'd rather be cautious.

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.
Russell King - ARM Linux Nov. 13, 2018, 5:54 p.m. | #6
On Tue, Nov 13, 2018 at 05:36:37PM +0000, Marc Zyngier wrote:
> On Tue, 13 Nov 2018 15:16:03 +0000,

> David Long <dave.long@linaro.org> wrote:

> > 

> > On 11/13/18 9:23 AM, Marc Zyngier wrote:

> > > Russell,

> > > 

> > > On Mon, 12 Nov 2018 16:54:10 +0000,

> > > Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> > >> 

> > >> Marc,

> > >> 

> > >> Can you please ack this to say that you are now happy with it after

> > >> your comments on version 1, so we can move forward and have Greg

> > >> merge it.

> > >> 

> > >> Thanks.

> > >> 

> > >> On Wed, Nov 07, 2018 at 11:43:47AM -0500, David Long wrote:

> > >>> From: Russell King <rmk+kernel@armlinux.org.uk>

> > >>> 

> > >>> Commit 10115105cb3aa17b5da1cb726ae8dd5f6854bd93 upstream.

> > >>> Commit 6282e916f774e37845c65d1eae9f8c649004f033 upstream.

> > >>> 

> > >>> Add firmware based hardening for cores that require more complex

> > >>> handling in firmware.

> > >>> 

> > >>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> > >>> Boot-tested-by: Tony Lindgren <tony@atomide.com>

> > >>> Reviewed-by: Tony Lindgren <tony@atomide.com>

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

> > >>> Signed-off-by: David A. Long <dave.long@linaro.org>

> > > 

> > > Sure. Feel free to add my

> > > 

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

> > > 

> > > I assume someone has tested these patches (I haven't, and I'm unlikely

> > > to do so in the near future as I'm travelling). I'm not sure Tony's

> > > "Boot-tested-by" is still valid, and Florian's earlier set of tests

> > > didn't show the issues of the initial backport.

> > > 

> > > Thanks,

> > > 

> > > 	M.

> > > 

> > 

> > I tested the patch set through kernelci and (belatedly)

> > kvm-unit-tests, the latter of which revealed the problem in V1 #11/24.

> > I have to assume Florian didn't specifically test kvm, something I

> > myself had originally assumed would be covered by kernelci.

> > 

> > I didn't scrub any of the ack/tested/reviewed lines from the original

> > patches.  I've always assumed this is the correct way to do this but

> > maybe it's not?

> 

> Leaving the tags is absolutely fine, they indicate that the original

> patch was actually tested.

> 

> I'm more worried of potential regressions: we've already found two

> problems, and although I cannot spot any other, it is fairly obvious

> that there has only been a limited amount of testing. It may not be a

> problem, but I'd rather be cautious.


The biggest problem is getting people to actually test the damn patches,
and not relying on the autobuilders/autobooters to do the work for you.
The autobooters are hopeless when it comes to identifying problems (as
I've now said multiple times since the regressions were first spotted.)

I'm of the opinion that the autobooters actually give _misleading_
results, and have been doing every since they were setup.

They claim that a platform which boots to a shell prompt, but which the
kernel log contains a stack trace is a "pass" result.  When you receive
an email that says that nothing failed, you don't bother going to (eg)
kernelci to read the boot logs, you assume there's no problem.  So the
failures (eg, due to WARN_ON() or worse, an oops that doesn't prevent
reaching a shell prompt) will go unnoticed.

That's exactly what has happened with the big.Little changes.

The ENDPROC() issue is much harder, none of the autobooters cover the
case of running a kernel under KVM afaik.  So that path never gets
executed - it's worse than that, because even if we _did_ have that,
we need to have both an ARM kernel and Thumb2 kernel exercised.  We
really can't rely on "eyes" spotting it, because as has been shown
many times, we have lots of cases where things get missed in review.
The claim that "open source is better because you have many eyes
looking at the source" seems to be provably false.

The same goes for the mess up in vfp_preserve_user_clear_hwstate(),
no one spotted that either, and it was only when some other compiler
issued a warning that it was found.

We keep asking people to test, but I don't think it does any good in
the long run other than delaying patches.  Unfortunately, the point
at which people notice problems is only _after_ the patches have been
merged into mainline and they are effectively forced via that method
to test the merged result.  Again, there's plenty of evidence to this.

For example, I've been posting asking people to test the big.Little
Spectre patches that caused problems last time around.  I get the
impression I'm completely wasting my time, because I'm getting zero
replies on that, not even from the individual who reported the
problem last time around.

I might as well have merged the patches last week and get it over
with, rather than delaying them until Friday.  At least we'll find
any problems quicker that way, because, as I've said above, it
_forces_ people to test.

I'm afraid that our "ask people to test" model just doesn't work
anymore now that we have autobuilders - I guess people end up
thinking that the autobuilders will do the work for them... :(

In summary, what I'm saying is that getting patches tested so we
can have confidence that they are correct is damn hard to nigh on
impossible.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
Florian Fainelli Nov. 13, 2018, 6:08 p.m. | #7
On 11/13/18 6:23 AM, Marc Zyngier wrote:
> Russell,

> 

> On Mon, 12 Nov 2018 16:54:10 +0000,

> Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

>>

>> Marc,

>>

>> Can you please ack this to say that you are now happy with it after

>> your comments on version 1, so we can move forward and have Greg

>> merge it.

>>

>> Thanks.

>>

>> On Wed, Nov 07, 2018 at 11:43:47AM -0500, David Long wrote:

>>> From: Russell King <rmk+kernel@armlinux.org.uk>

>>>

>>> Commit 10115105cb3aa17b5da1cb726ae8dd5f6854bd93 upstream.

>>> Commit 6282e916f774e37845c65d1eae9f8c649004f033 upstream.

>>>

>>> Add firmware based hardening for cores that require more complex

>>> handling in firmware.

>>>

>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

>>> Boot-tested-by: Tony Lindgren <tony@atomide.com>

>>> Reviewed-by: Tony Lindgren <tony@atomide.com>

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

>>> Signed-off-by: David A. Long <dave.long@linaro.org>

> 

> Sure. Feel free to add my

> 

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

> 

> I assume someone has tested these patches (I haven't, and I'm unlikely

> to do so in the near future as I'm travelling). I'm not sure Tony's

> "Boot-tested-by" is still valid, and Florian's earlier set of tests

> didn't show the issues of the initial backport.


Correct, I was not testing any KVM path at all, which is why this did
not show up as a problem for me, I am not really well equipped to
perform KVM testing at the moment.
-- 
Florian
Russell King - ARM Linux Nov. 20, 2018, 10:59 a.m. | #8
All,

It is now almost two weeks since David posted the patches, and a week
since the last message in this thread.

What is happening with these patches?  What about the 4.4-stable
backport as well?  Does anyone care about these anymore?  Do we have
any product customers using 32-bit Cortex CPUs anymore?

David - FYI - everything in my Spectre branch is now in mainline as
of 4.20-rc3, and I have nothing further planned for core 32-bit ARM
Spectre workarounds.

On Tue, Nov 13, 2018 at 10:08:48AM -0800, Florian Fainelli wrote:
> On 11/13/18 6:23 AM, Marc Zyngier wrote:

> > Russell,

> > 

> > On Mon, 12 Nov 2018 16:54:10 +0000,

> > Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> >>

> >> Marc,

> >>

> >> Can you please ack this to say that you are now happy with it after

> >> your comments on version 1, so we can move forward and have Greg

> >> merge it.

> >>

> >> Thanks.

> >>

> >> On Wed, Nov 07, 2018 at 11:43:47AM -0500, David Long wrote:

> >>> From: Russell King <rmk+kernel@armlinux.org.uk>

> >>>

> >>> Commit 10115105cb3aa17b5da1cb726ae8dd5f6854bd93 upstream.

> >>> Commit 6282e916f774e37845c65d1eae9f8c649004f033 upstream.

> >>>

> >>> Add firmware based hardening for cores that require more complex

> >>> handling in firmware.

> >>>

> >>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

> >>> Boot-tested-by: Tony Lindgren <tony@atomide.com>

> >>> Reviewed-by: Tony Lindgren <tony@atomide.com>

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

> >>> Signed-off-by: David A. Long <dave.long@linaro.org>

> > 

> > Sure. Feel free to add my

> > 

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

> > 

> > I assume someone has tested these patches (I haven't, and I'm unlikely

> > to do so in the near future as I'm travelling). I'm not sure Tony's

> > "Boot-tested-by" is still valid, and Florian's earlier set of tests

> > didn't show the issues of the initial backport.

> 

> Correct, I was not testing any KVM path at all, which is why this did

> not show up as a problem for me, I am not really well equipped to

> perform KVM testing at the moment.

> -- 

> Florian


-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
Greg KH Nov. 20, 2018, 11:15 a.m. | #9
On Tue, Nov 20, 2018 at 10:59:44AM +0000, Russell King - ARM Linux wrote:
> All,

> 

> It is now almost two weeks since David posted the patches, and a week

> since the last message in this thread.

> 

> What is happening with these patches?  What about the 4.4-stable

> backport as well?  Does anyone care about these anymore?  Do we have

> any product customers using 32-bit Cortex CPUs anymore?


I can queue these 4.9 patches up after this next release, I was waiting
to see if anyone else had any comments on them.

thanks,

greg k-h
David Long Nov. 20, 2018, 3:30 p.m. | #10
On 11/20/18 6:15 AM, Greg KH wrote:
> On Tue, Nov 20, 2018 at 10:59:44AM +0000, Russell King - ARM Linux wrote:

>> All,

>>

>> It is now almost two weeks since David posted the patches, and a week

>> since the last message in this thread.

>>

>> What is happening with these patches?  What about the 4.4-stable

>> backport as well?  Does anyone care about these anymore?  Do we have

>> any product customers using 32-bit Cortex CPUs anymore?

> 

> I can queue these 4.9 patches up after this next release, I was waiting

> to see if anyone else had any comments on them.

> 

> thanks,

> 

> greg k-h

> 



FWIW I have since run kvm-unit-tests again, this time with a thumb-2 
host kernel.  It looked OK to me.

-dl
David Long Nov. 20, 2018, 4:24 p.m. | #11
On 11/20/18 5:59 AM, Russell King - ARM Linux wrote:
> All,

> 

> It is now almost two weeks since David posted the patches, and a week

> since the last message in this thread.

> 

> What is happening with these patches?  What about the 4.4-stable

> backport as well?  Does anyone care about these anymore?  Do we have

> any product customers using 32-bit Cortex CPUs anymore?


Please note that the v4.4 patches I sent out do not include the kvm 
changes from the original patch set, therefore they were never affected 
by either of the two issues we've been discussing.  The kvm-unit-tests 
for v4.4 look OK, with or without a thumb-2 host kernel.

> 

> David - FYI - everything in my Spectre branch is now in mainline as

> of 4.20-rc3, and I have nothing further planned for core 32-bit ARM

> Spectre workarounds.

> 

> On Tue, Nov 13, 2018 at 10:08:48AM -0800, Florian Fainelli wrote:

>> On 11/13/18 6:23 AM, Marc Zyngier wrote:

>>> Russell,

>>>

>>> On Mon, 12 Nov 2018 16:54:10 +0000,

>>> Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

>>>>

>>>> Marc,

>>>>

>>>> Can you please ack this to say that you are now happy with it after

>>>> your comments on version 1, so we can move forward and have Greg

>>>> merge it.

>>>>

>>>> Thanks.

>>>>

>>>> On Wed, Nov 07, 2018 at 11:43:47AM -0500, David Long wrote:

>>>>> From: Russell King <rmk+kernel@armlinux.org.uk>

>>>>>

>>>>> Commit 10115105cb3aa17b5da1cb726ae8dd5f6854bd93 upstream.

>>>>> Commit 6282e916f774e37845c65d1eae9f8c649004f033 upstream.

>>>>>

>>>>> Add firmware based hardening for cores that require more complex

>>>>> handling in firmware.

>>>>>

>>>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

>>>>> Boot-tested-by: Tony Lindgren <tony@atomide.com>

>>>>> Reviewed-by: Tony Lindgren <tony@atomide.com>

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

>>>>> Signed-off-by: David A. Long <dave.long@linaro.org>

>>>

>>> Sure. Feel free to add my

>>>

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

>>>

>>> I assume someone has tested these patches (I haven't, and I'm unlikely

>>> to do so in the near future as I'm travelling). I'm not sure Tony's

>>> "Boot-tested-by" is still valid, and Florian's earlier set of tests

>>> didn't show the issues of the initial backport.

>>

>> Correct, I was not testing any KVM path at all, which is why this did

>> not show up as a problem for me, I am not really well equipped to

>> perform KVM testing at the moment.

>> -- 

>> Florian

> 


-dl
Marc Zyngier Nov. 20, 2018, 4:42 p.m. | #12
On 20/11/2018 15:30, David Long wrote:
> On 11/20/18 6:15 AM, Greg KH wrote:

>> On Tue, Nov 20, 2018 at 10:59:44AM +0000, Russell King - ARM Linux wrote:

>>> All,

>>>

>>> It is now almost two weeks since David posted the patches, and a week

>>> since the last message in this thread.

>>>

>>> What is happening with these patches?  What about the 4.4-stable

>>> backport as well?  Does anyone care about these anymore?  Do we have

>>> any product customers using 32-bit Cortex CPUs anymore?

>>

>> I can queue these 4.9 patches up after this next release, I was waiting

>> to see if anyone else had any comments on them.

>>

>> thanks,

>>

>> greg k-h

>>

> 

> 

> FWIW I have since run kvm-unit-tests again, this time with a thumb-2 

> host kernel.  It looked OK to me.


The issue previously reported wasn't with a Thumb-2 host, but with a
Thumb-2 *guest*. I've since verified that this particular case was
correctly working.

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

Patch

diff --git a/arch/arm/mm/proc-v7-bugs.c b/arch/arm/mm/proc-v7-bugs.c
index 85a2e3d6263c..da25a38e1897 100644
--- a/arch/arm/mm/proc-v7-bugs.c
+++ b/arch/arm/mm/proc-v7-bugs.c
@@ -1,14 +1,20 @@ 
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/arm-smccc.h>
 #include <linux/kernel.h>
+#include <linux/psci.h>
 #include <linux/smp.h>
 
 #include <asm/cp15.h>
 #include <asm/cputype.h>
+#include <asm/proc-fns.h>
 #include <asm/system_misc.h>
 
 #ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
 DEFINE_PER_CPU(harden_branch_predictor_fn_t, harden_branch_predictor_fn);
 
+extern void cpu_v7_smc_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);
+extern void cpu_v7_hvc_switch_mm(phys_addr_t pgd_phys, struct mm_struct *mm);
+
 static void harden_branch_predictor_bpiall(void)
 {
 	write_sysreg(0, BPIALL);
@@ -19,6 +25,16 @@  static void harden_branch_predictor_iciallu(void)
 	write_sysreg(0, ICIALLU);
 }
 
+static void __maybe_unused call_smc_arch_workaround_1(void)
+{
+	arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+}
+
+static void __maybe_unused call_hvc_arch_workaround_1(void)
+{
+	arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_WORKAROUND_1, NULL);
+}
+
 static void cpu_v7_spectre_init(void)
 {
 	const char *spectre_v2_method = NULL;
@@ -45,7 +61,51 @@  static void cpu_v7_spectre_init(void)
 			harden_branch_predictor_iciallu;
 		spectre_v2_method = "ICIALLU";
 		break;
+
+#ifdef CONFIG_ARM_PSCI
+	default:
+		/* Other ARM CPUs require no workaround */
+		if (read_cpuid_implementor() == ARM_CPU_IMP_ARM)
+			break;
+		/* fallthrough */
+		/* Cortex A57/A72 require firmware workaround */
+	case ARM_CPU_PART_CORTEX_A57:
+	case ARM_CPU_PART_CORTEX_A72: {
+		struct arm_smccc_res res;
+
+		if (psci_ops.smccc_version == SMCCC_VERSION_1_0)
+			break;
+
+		switch (psci_ops.conduit) {
+		case PSCI_CONDUIT_HVC:
+			arm_smccc_1_1_hvc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 != 0)
+				break;
+			per_cpu(harden_branch_predictor_fn, cpu) =
+				call_hvc_arch_workaround_1;
+			processor.switch_mm = cpu_v7_hvc_switch_mm;
+			spectre_v2_method = "hypervisor";
+			break;
+
+		case PSCI_CONDUIT_SMC:
+			arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FUNC_ID,
+					  ARM_SMCCC_ARCH_WORKAROUND_1, &res);
+			if ((int)res.a0 != 0)
+				break;
+			per_cpu(harden_branch_predictor_fn, cpu) =
+				call_smc_arch_workaround_1;
+			processor.switch_mm = cpu_v7_smc_switch_mm;
+			spectre_v2_method = "firmware";
+			break;
+
+		default:
+			break;
+		}
 	}
+#endif
+	}
+
 	if (spectre_v2_method)
 		pr_info("CPU%u: Spectre v2: using %s workaround\n",
 			smp_processor_id(), spectre_v2_method);
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 2d2e5ae85816..850c22bca19c 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -9,6 +9,7 @@ 
  *
  *  This is the "shell" of the ARMv7 processor support.
  */
+#include <linux/arm-smccc.h>
 #include <linux/init.h>
 #include <linux/linkage.h>
 #include <asm/assembler.h>
@@ -88,6 +89,26 @@  ENTRY(cpu_v7_dcache_clean_area)
 	ret	lr
 ENDPROC(cpu_v7_dcache_clean_area)
 
+#ifdef CONFIG_ARM_PSCI
+	.arch_extension sec
+ENTRY(cpu_v7_smc_switch_mm)
+	stmfd	sp!, {r0 - r3}
+	movw	r0, #:lower16:ARM_SMCCC_ARCH_WORKAROUND_1
+	movt	r0, #:upper16:ARM_SMCCC_ARCH_WORKAROUND_1
+	smc	#0
+	ldmfd	sp!, {r0 - r3}
+	b	cpu_v7_switch_mm
+ENDPROC(cpu_v7_smc_switch_mm)
+	.arch_extension virt
+ENTRY(cpu_v7_hvc_switch_mm)
+	stmfd	sp!, {r0 - r3}
+	movw	r0, #:lower16:ARM_SMCCC_ARCH_WORKAROUND_1
+	movt	r0, #:upper16:ARM_SMCCC_ARCH_WORKAROUND_1
+	hvc	#0
+	ldmfd	sp!, {r0 - r3}
+	b	cpu_v7_switch_mm
+ENDPROC(cpu_v7_hvc_switch_mm)
+#endif
 ENTRY(cpu_v7_iciallu_switch_mm)
 	mov	r3, #0
 	mcr	p15, 0, r3, c7, c5, 0		@ ICIALLU