Message ID | 20181107164402.9380-10-dave.long@linaro.org |
---|---|
State | New |
Headers | show |
Series | V4.9 backport of 32-bit arm spectre patches | expand |
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
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.
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
* 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
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.
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
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
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
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
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
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
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...
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