[U-Boot] [PATCH 0/4] ARM: Provide workaround setup bits for CVE-2017-5715 (A8/A15)

[U-Boot] [PATCH 0/4] ARM: Provide workaround setup bits for CVE-2017-5715 (A8/A15)

[Nishanth Menon]

Hi,

This is a follow on from https://marc.info/?l=u-boot&m=151691688828176&w=2 (RFC)

NOTE:
* As per ARM recommendations[2], and discussions in list[1] ARM
  Cortex-A9/12/17 do not need additional steps in u-boot to enable the
  OS level workarounds.
* This itself is'nt a complete solution and is based on recommendation
  This from Arm[2] for variant 2 CVE-2017-5715 -> Kernel changes can be seen on
  linux next (next-20180612) or on linux master (upcoming v4.18-rc1 tag).
* I think it is necessary on older SoCs without firmware support
  (such as older OMAPs and AM*) to have kernel support mirroring what we do in
  u-boot to support additional cores AND/OR low power states where contexts are
  lost (assuming ACR states are'nt saved). just my 2 cents.

Few of the tests (with linux next-20180612):
AM571-IDK: https://pastebin.ubuntu.com/p/sr5X6sN3Tr/ (single core A15)
OMAP5-uEVM: https://pastebin.ubuntu.com/p/9yDM22bJ6n/ (dual core A15)
OMAP3-beagle-xm: https://pastebin.ubuntu.com/p/9DfDkpyxym/ (Single A8)
AM335x-Beaglebone-black: https://pastebin.ubuntu.com/p/DczT9jPMwb/ (Single A8)

Nishanth Menon (4):
  ARM: Introduce ability to enable ACR::IBE on Cortex-A8 for
    CVE-2017-5715
  ARM: Introduce ability to enable invalidate of BTB with ICIALLU on
    Cortex-A15 for CVE-2017-5715
  ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of
    BTB) to facilitate CVE_2017-5715 WA in OS
  ARM: mach-omap2: omap3/am335x: Enable ACR::IBE on Cortex-A8 SoCs for
    CVE-2017-5715

 arch/arm/Kconfig            |  9 +++++++++
 arch/arm/cpu/armv7/start.S  | 15 +++++++++++++--
 arch/arm/mach-omap2/Kconfig |  3 +++
 3 files changed, 25 insertions(+), 2 deletions(-)

[1] https://marc.info/?t=151639906500002&r=1&w=2
[2] https://developer.arm.com/support/security-update
[3] https://marc.info/?t=151543790400007&r=1&w=2 and the latest in:
	https://marc.info/?l=linux-arm-kernel&m=151689379521082&w=2
[4]
	https://github.com/ARM-software/arm-trusted-firmware/wiki/ARM-Trusted-Firmware-Security-Advisory-TFV-6
	https://www.op-tee.org/security-advisories/
	https://www.linaro.org/blog/meltdown-spectre/
-- 
2.15.1

[U-Boot] [PATCH 1/4] ARM: Introduce ability to enable ACR::IBE on Cortex-A8 for CVE-2017-5715

[Nishanth Menon]

As recommended by Arm in [1], IBE[2] has to be enabled unconditionally
for BPIALL to be functional on Cortex-A8 processors. Provide a config
option for platforms to enable this option based on impact analysis
for products.

NOTE: This patch in itself is NOT the final solution, this requires:
a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
   provide direct access to ACR register.
b) Operating Systems such as Linux to provide adequate workaround in the right
   locations.
c) This workaround applies to only the boot processor. It is important
   to apply workaround as necessary (context-save-restore) around low
   power context loss OR additional processors as necessary in either
   firmware support OR elsewhere in OS.

[1] https://developer.arm.com/support/security-update
[2] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0344k/Bgbffjhh.html

Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Andre Przywara <Andre.Przywara@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/Kconfig           | 5 +++++
 arch/arm/cpu/armv7/start.S | 7 +++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index dde422bc5d53..9e32d5b43cb0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -108,6 +108,8 @@ config SYS_ARM_MPU
 # CONFIG_ARM_ERRATA_621766
 # CONFIG_ARM_ERRATA_798870
 # CONFIG_ARM_ERRATA_801819
+# CONFIG_ARM_CORTEX_A8_CVE_2017_5715
+
 config ARM_ERRATA_430973
 	bool
 
@@ -177,6 +179,9 @@ config ARM_ERRATA_852423
 config ARM_ERRATA_855873
 	bool
 
+config ARM_CORTEX_A8_CVE_2017_5715
+	bool
+
 config CPU_ARM720T
 	bool
 	select SYS_CACHE_SHIFT_5
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index c996525f861e..3beaf5a93d81 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -252,12 +252,15 @@ skip_errata_801819:
 	pop	{r1-r5}			@ Restore the cpu info - fall through
 #endif
 
-#ifdef CONFIG_ARM_ERRATA_430973
+#if defined(CONFIG_ARM_ERRATA_430973) || defined (CONFIG_ARM_CORTEX_A8_CVE_2017_5715)
 	mrc	p15, 0, r0, c1, c0, 1	@ Read ACR
 
+#ifdef CONFIG_ARM_CORTEX_A8_CVE_2017_5715
+	orr	r0, r0, #(0x1 << 6)	@ Set IBE bit always to enable OS WA
+#else
 	cmp	r2, #0x21		@ Only on < r2p1
 	orrlt	r0, r0, #(0x1 << 6)	@ Set IBE bit
-
+#endif
 	push	{r1-r5}			@ Save the cpu info registers
 	bl	v7_arch_cp15_set_acr
 	pop	{r1-r5}			@ Restore the cpu info - fall through
-- 
2.15.1

[U-Boot] [PATCH 1/4] ARM: Introduce ability to enable ACR::IBE on Cortex-A8 for CVE-2017-5715

[Fabio Estevam]

On Tue, Jun 12, 2018 at 5:24 PM, Nishanth Menon <nm@ti.com> wrote:
> As recommended by Arm in [1], IBE[2] has to be enabled unconditionally
> for BPIALL to be functional on Cortex-A8 processors. Provide a config
> option for platforms to enable this option based on impact analysis
> for products.
>
> NOTE: This patch in itself is NOT the final solution, this requires:
> a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
>    provide direct access to ACR register.
> b) Operating Systems such as Linux to provide adequate workaround in the right
>    locations.
> c) This workaround applies to only the boot processor. It is important
>    to apply workaround as necessary (context-save-restore) around low
>    power context loss OR additional processors as necessary in either
>    firmware support OR elsewhere in OS.
>
> [1] https://developer.arm.com/support/security-update
> [2] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0344k/Bgbffjhh.html
>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Andre Przywara <Andre.Przywara@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

On a imx51-babbage board:

Tested-by: Fabio Estevam <fabio.estevam@nxp.com>

[U-Boot] [U-Boot, 1/4] ARM: Introduce ability to enable ACR::IBE on Cortex-A8 for CVE-2017-5715

[Tom Rini]

On Tue, Jun 12, 2018 at 03:24:08PM -0500, Nishanth Menon wrote:

> As recommended by Arm in [1], IBE[2] has to be enabled unconditionally
> for BPIALL to be functional on Cortex-A8 processors. Provide a config
> option for platforms to enable this option based on impact analysis
> for products.
> 
> NOTE: This patch in itself is NOT the final solution, this requires:
> a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
>    provide direct access to ACR register.
> b) Operating Systems such as Linux to provide adequate workaround in the right
>    locations.
> c) This workaround applies to only the boot processor. It is important
>    to apply workaround as necessary (context-save-restore) around low
>    power context loss OR additional processors as necessary in either
>    firmware support OR elsewhere in OS.
> 
> [1] https://developer.arm.com/support/security-update
> [2] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0344k/Bgbffjhh.html
> 
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Andre Przywara <Andre.Przywara@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180629/08d07bcf/attachment.sig>

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Nishanth Menon]

As recommended by Arm in [1], ACTLR[0] (Enable invalidates of BTB)
needs to be set[2] for BTB to be invalidated on ICIALLU. This needs to
be done unconditionally for Cortex-A15 processors. Provide a config
option for platforms to enable this option based on impact analysis
for products.

NOTE: This patch in itself is NOT the final solution, this requires:
a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
   provide direct access to ACR register.
b) Operating Systems such as Linux to provide adequate workaround in the
   right locations.
c) This workaround applies to only the boot processor. It is important
   to apply workaround as necessary (context-save-restore) around low
   power context loss OR additional processors as necessary in either
   firmware support OR elsewhere in OS.

[1] https://developer.arm.com/support/security-update
[2] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0438c/BABGHIBG.html

Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Christoffer Dall <christoffer.dall@linaro.org>
Cc: Andre Przywara <Andre.Przywara@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/Kconfig           | 4 ++++
 arch/arm/cpu/armv7/start.S | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 9e32d5b43cb0..98f58fd27696 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -109,6 +109,7 @@ config SYS_ARM_MPU
 # CONFIG_ARM_ERRATA_798870
 # CONFIG_ARM_ERRATA_801819
 # CONFIG_ARM_CORTEX_A8_CVE_2017_5715
+# CONFIG_ARM_CORTEX_A15_CVE_2017_5715
 
 config ARM_ERRATA_430973
 	bool
@@ -182,6 +183,9 @@ config ARM_ERRATA_855873
 config ARM_CORTEX_A8_CVE_2017_5715
 	bool
 
+config ARM_CORTEX_A15_CVE_2017_5715
+	bool
+
 config CPU_ARM720T
 	bool
 	select SYS_CACHE_SHIFT_5
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 3beaf5a93d81..81edec01bf32 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -241,6 +241,14 @@ skip_errata_798870:
 skip_errata_801819:
 #endif
 
+#ifdef CONFIG_ARM_CORTEX_A15_CVE_2017_5715
+	mrc	p15, 0, r0, c1, c0, 1	@ read auxilary control register
+	orr	r0, r0, #1 << 0		@ Enable invalidates of BTB
+	push	{r1-r5}			@ Save the cpu info registers
+	bl	v7_arch_cp15_set_acr
+	pop	{r1-r5}			@ Restore the cpu info - fall through
+#endif
+
 #ifdef CONFIG_ARM_ERRATA_454179
 	mrc	p15, 0, r0, c1, c0, 1	@ Read ACR
 
-- 
2.15.1

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Marek Vasut]

On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> As recommended by Arm in [1], ACTLR[0] (Enable invalidates of BTB)
> needs to be set[2] for BTB to be invalidated on ICIALLU. This needs to
> be done unconditionally for Cortex-A15 processors. Provide a config
> option for platforms to enable this option based on impact analysis
> for products.
> 
> NOTE: This patch in itself is NOT the final solution, this requires:
> a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
>    provide direct access to ACR register.
> b) Operating Systems such as Linux to provide adequate workaround in the
>    right locations.
> c) This workaround applies to only the boot processor. It is important
>    to apply workaround as necessary (context-save-restore) around low
>    power context loss OR additional processors as necessary in either
>    firmware support OR elsewhere in OS.
> 
> [1] https://developer.arm.com/support/security-update
> [2] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0438c/BABGHIBG.html
> 
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Andre Przywara <Andre.Przywara@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  arch/arm/Kconfig           | 4 ++++
>  arch/arm/cpu/armv7/start.S | 8 ++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 9e32d5b43cb0..98f58fd27696 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -109,6 +109,7 @@ config SYS_ARM_MPU
>  # CONFIG_ARM_ERRATA_798870
>  # CONFIG_ARM_ERRATA_801819
>  # CONFIG_ARM_CORTEX_A8_CVE_2017_5715
> +# CONFIG_ARM_CORTEX_A15_CVE_2017_5715
>  
>  config ARM_ERRATA_430973
>  	bool
> @@ -182,6 +183,9 @@ config ARM_ERRATA_855873
>  config ARM_CORTEX_A8_CVE_2017_5715
>  	bool
>  
> +config ARM_CORTEX_A15_CVE_2017_5715
> +	bool
> +
>  config CPU_ARM720T
>  	bool
>  	select SYS_CACHE_SHIFT_5
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 3beaf5a93d81..81edec01bf32 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -241,6 +241,14 @@ skip_errata_798870:
>  skip_errata_801819:
>  #endif
>  
> +#ifdef CONFIG_ARM_CORTEX_A15_CVE_2017_5715
> +	mrc	p15, 0, r0, c1, c0, 1	@ read auxilary control register
> +	orr	r0, r0, #1 << 0		@ Enable invalidates of BTB

Can we use BIT() macro in the assembler code too ?

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Nishanth Menon]

On 23:05-20180612, Marek Vasut wrote:
> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
[..]
> > +#ifdef CONFIG_ARM_CORTEX_A15_CVE_2017_5715
> > +	mrc	p15, 0, r0, c1, c0, 1	@ read auxilary control register
> > +	orr	r0, r0, #1 << 0		@ Enable invalidates of BTB
> 
> Can we use BIT() macro in the assembler code too ?

Probably, but just following convention in the rest of the file. Do we
want to change from existing code?

-- 
Regards,
Nishanth Menon

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Tom Rini]

On Wed, Jun 13, 2018 at 08:32:15AM -0500, Nishanth Menon wrote:
> On 23:05-20180612, Marek Vasut wrote:
> > On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> [..]
> > > +#ifdef CONFIG_ARM_CORTEX_A15_CVE_2017_5715
> > > +	mrc	p15, 0, r0, c1, c0, 1	@ read auxilary control register
> > > +	orr	r0, r0, #1 << 0		@ Enable invalidates of BTB
> > 
> > Can we use BIT() macro in the assembler code too ?
> 
> Probably, but just following convention in the rest of the file. Do we
> want to change from existing code?

Agreed, we should follow the existing style (and I'm not 100% sure I
like using BIT() in asm files).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180613/cebb11ee/attachment.sig>

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Nishanth Menon]

On 15:46-20180613, Tom Rini wrote:
> On Wed, Jun 13, 2018 at 08:32:15AM -0500, Nishanth Menon wrote:
> > On 23:05-20180612, Marek Vasut wrote:
> > > On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> > [..]
> > > > +#ifdef CONFIG_ARM_CORTEX_A15_CVE_2017_5715
> > > > +	mrc	p15, 0, r0, c1, c0, 1	@ read auxilary control register
> > > > +	orr	r0, r0, #1 << 0		@ Enable invalidates of BTB
> > > 
> > > Can we use BIT() macro in the assembler code too ?
> > 
> > Probably, but just following convention in the rest of the file. Do we
> > want to change from existing code?
> 
> Agreed, we should follow the existing style (and I'm not 100% sure I
> like using BIT() in asm files).

OK. Will drop this feedback about BIT() macro if I have to do a v2.

-- 
Regards,
Nishanth Menon

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Marek Vasut]

On 06/13/2018 11:32 PM, Nishanth Menon wrote:
> On 15:46-20180613, Tom Rini wrote:
>> On Wed, Jun 13, 2018 at 08:32:15AM -0500, Nishanth Menon wrote:
>>> On 23:05-20180612, Marek Vasut wrote:
>>>> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
>>> [..]
>>>>> +#ifdef CONFIG_ARM_CORTEX_A15_CVE_2017_5715
>>>>> +	mrc	p15, 0, r0, c1, c0, 1	@ read auxilary control register
>>>>> +	orr	r0, r0, #1 << 0		@ Enable invalidates of BTB
>>>>
>>>> Can we use BIT() macro in the assembler code too ?
>>>
>>> Probably, but just following convention in the rest of the file. Do we
>>> want to change from existing code?
>>
>> Agreed, we should follow the existing style (and I'm not 100% sure I
>> like using BIT() in asm files).
> 
> OK. Will drop this feedback about BIT() macro if I have to do a v2.

Fine by me

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Florian Fainelli]

On June 12, 2018 1:24:09 PM PDT, Nishanth Menon <nm@ti.com> wrote:
>As recommended by Arm in [1], ACTLR[0] (Enable invalidates of BTB)
>needs to be set[2] for BTB to be invalidated on ICIALLU. This needs to
>be done unconditionally for Cortex-A15 processors. Provide a config
>option for platforms to enable this option based on impact analysis
>for products.
>
>NOTE: This patch in itself is NOT the final solution, this requires:
>a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
>   provide direct access to ACR register.
>b) Operating Systems such as Linux to provide adequate workaround in
>the
>   right locations.

This is the case as of 4.18 so you could probably reference CONFIG_CPU_SPECTRE and CONFIG_HARDEN_BRANCH_PREDICTOR in a v2.

>c) This workaround applies to only the boot processor. It is important
>   to apply workaround as necessary (context-save-restore) around low
>   power context loss OR additional processors as necessary in either
>   firmware support OR elsewhere in OS.

About that, I don't know enough of uboot but are there existing PSCI or other seemingly standard secondary core support in uboot that would make us go through the same initialization as the boot CPU? If not, is everything going to be largely implementation specific and scattered between uboot and the hypervisors or kernel?

FWIW, this is what prompted me to submit this:

https://patchwork.kernel.org/patch/10453643/


>
>[1] https://developer.arm.com/support/security-update
>[2]
>http://infocenter.arm.com/help/topic/com.arm.doc.ddi0438c/BABGHIBG.html
>
>Cc: Marc Zyngier <marc.zyngier@arm.com>
>Cc: Russell King <linux@arm.linux.org.uk>
>Cc: Tony Lindgren <tony@atomide.com>
>Cc: Robin Murphy <robin.murphy@arm.com>
>Cc: Florian Fainelli <f.fainelli@gmail.com>
>Cc: Catalin Marinas <catalin.marinas@arm.com>
>Cc: Will Deacon <will.deacon@arm.com>
>Cc: Christoffer Dall <christoffer.dall@linaro.org>
>Cc: Andre Przywara <Andre.Przywara@arm.com>
>Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>Cc: Tom Rini <trini@konsulko.com>
>Cc: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
>
>Signed-off-by: Nishanth Menon <nm@ti.com>
>---
> arch/arm/Kconfig           | 4 ++++
> arch/arm/cpu/armv7/start.S | 8 ++++++++
> 2 files changed, 12 insertions(+)
>
>diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>index 9e32d5b43cb0..98f58fd27696 100644
>--- a/arch/arm/Kconfig
>+++ b/arch/arm/Kconfig
>@@ -109,6 +109,7 @@ config SYS_ARM_MPU
> # CONFIG_ARM_ERRATA_798870
> # CONFIG_ARM_ERRATA_801819
> # CONFIG_ARM_CORTEX_A8_CVE_2017_5715
>+# CONFIG_ARM_CORTEX_A15_CVE_2017_5715
> 
> config ARM_ERRATA_430973
> 	bool
>@@ -182,6 +183,9 @@ config ARM_ERRATA_855873
> config ARM_CORTEX_A8_CVE_2017_5715
> 	bool
> 
>+config ARM_CORTEX_A15_CVE_2017_5715
>+	bool
>+
> config CPU_ARM720T
> 	bool
> 	select SYS_CACHE_SHIFT_5
>diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
>index 3beaf5a93d81..81edec01bf32 100644
>--- a/arch/arm/cpu/armv7/start.S
>+++ b/arch/arm/cpu/armv7/start.S
>@@ -241,6 +241,14 @@ skip_errata_798870:
> skip_errata_801819:
> #endif
> 
>+#ifdef CONFIG_ARM_CORTEX_A15_CVE_2017_5715
>+	mrc	p15, 0, r0, c1, c0, 1	@ read auxilary control register
>+	orr	r0, r0, #1 << 0		@ Enable invalidates of BTB
>+	push	{r1-r5}			@ Save the cpu info registers
>+	bl	v7_arch_cp15_set_acr
>+	pop	{r1-r5}			@ Restore the cpu info - fall through
>+#endif
>+
> #ifdef CONFIG_ARM_ERRATA_454179
> 	mrc	p15, 0, r0, c1, c0, 1	@ Read ACR
> 


-- 
Florian

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Nishanth Menon]

On 00:30-20180613, Florian Fainelli wrote:
> On June 12, 2018 1:24:09 PM PDT, Nishanth Menon <nm@ti.com> wrote:
> >As recommended by Arm in [1], ACTLR[0] (Enable invalidates of BTB)
> >needs to be set[2] for BTB to be invalidated on ICIALLU. This needs to
> >be done unconditionally for Cortex-A15 processors. Provide a config
> >option for platforms to enable this option based on impact analysis
> >for products.
> >
> >NOTE: This patch in itself is NOT the final solution, this requires:
> >a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
> >   provide direct access to ACR register.
> >b) Operating Systems such as Linux to provide adequate workaround in
> >the
> >   right locations.
> 
> This is the case as of 4.18 so you could probably reference CONFIG_CPU_SPECTRE and CONFIG_HARDEN_BRANCH_PREDICTOR in a v2.

Did'nt want to tie the description too deep to Linux specifics.. Linux
documents itself and users are encouraged to read that documentation,
correct?

> 
> >c) This workaround applies to only the boot processor. It is important
> >   to apply workaround as necessary (context-save-restore) around low
> >   power context loss OR additional processors as necessary in either
> >   firmware support OR elsewhere in OS.
> 
> About that, I don't know enough of uboot but are there existing PSCI or
> other seemingly standard secondary core support in uboot that would make
> us go through the same initialization as the boot CPU? If not, is
> everything going to be largely implementation specific and
> scattered between uboot and the hypervisors or kernel?

in ARMV7 SoCs, unfortunately, we lived in a world of no-exact-standard.
even within TI, Few of the SoCs use PSCI, others did implement custom
SMC calls (since they existed in an architecture prior to PSCI).

> 
> FWIW, this is what prompted me to submit this:
> 
> https://patchwork.kernel.org/patch/10453643/

That wont work in a generic manner for precisely the same reason I had to do
it with weak function in u-boot (some SoCs will only permit 'mcr
p15, 0, r0, c1, c0, 1' in secure world and you need to make a custom smc
call to make it happen). Unfortunately, IMHO, at least at this
point, there'd be custom implementations per SoC and layers depending on
where to implement it.

-- 
Regards,
Nishanth Menon

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Florian Fainelli]

On 06/13/2018 06:37 AM, Nishanth Menon wrote:
> On 00:30-20180613, Florian Fainelli wrote:
>> On June 12, 2018 1:24:09 PM PDT, Nishanth Menon <nm@ti.com> wrote:
>>> As recommended by Arm in [1], ACTLR[0] (Enable invalidates of BTB)
>>> needs to be set[2] for BTB to be invalidated on ICIALLU. This needs to
>>> be done unconditionally for Cortex-A15 processors. Provide a config
>>> option for platforms to enable this option based on impact analysis
>>> for products.
>>>
>>> NOTE: This patch in itself is NOT the final solution, this requires:
>>> a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
>>>   provide direct access to ACR register.
>>> b) Operating Systems such as Linux to provide adequate workaround in
>>> the
>>>   right locations.
>>
>> This is the case as of 4.18 so you could probably reference CONFIG_CPU_SPECTRE and CONFIG_HARDEN_BRANCH_PREDICTOR in a v2.
> 
> Did'nt want to tie the description too deep to Linux specifics.. Linux
> documents itself and users are encouraged to read that documentation,
> correct?

That's fair enough I guess, we also don't know how the other OSes do
provide that mitigation and whether they have run-time/build-time
configuration options gating those.

> 
>>
>>> c) This workaround applies to only the boot processor. It is important
>>>   to apply workaround as necessary (context-save-restore) around low
>>>   power context loss OR additional processors as necessary in either
>>>   firmware support OR elsewhere in OS.
>>
>> About that, I don't know enough of uboot but are there existing PSCI or
>> other seemingly standard secondary core support in uboot that would make
>> us go through the same initialization as the boot CPU? If not, is
>> everything going to be largely implementation specific and
>> scattered between uboot and the hypervisors or kernel?
> 
> in ARMV7 SoCs, unfortunately, we lived in a world of no-exact-standard.
> even within TI, Few of the SoCs use PSCI, others did implement custom
> SMC calls (since they existed in an architecture prior to PSCI).
> 
>>
>> FWIW, this is what prompted me to submit this:
>>
>> https://patchwork.kernel.org/patch/10453643/
> 
> That wont work in a generic manner for precisely the same reason I had to do
> it with weak function in u-boot (some SoCs will only permit 'mcr
> p15, 0, r0, c1, c0, 1' in secure world and you need to make a custom smc
> call to make it happen). Unfortunately, IMHO, at least at this
> point, there'd be custom implementations per SoC and layers depending on
> where to implement it.

It won't work in a generic manner but it will work for some platforms
where updating the firmware is impractical, and since the bits are write
ignore if your PL does not allow it, this still seems like a net win for
platforms where this is effective, and it does take care of Linux doing
the SMP bring-up of secondary cores as well. That's what we have in our
downstream tree at least, and I was hoping this could land upstream too.
-- 
Florian

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Nishanth Menon]

On 21:36-20180613, Florian Fainelli wrote:
[...]
> >>> c) This workaround applies to only the boot processor. It is important
> >>>   to apply workaround as necessary (context-save-restore) around low
> >>>   power context loss OR additional processors as necessary in either
> >>>   firmware support OR elsewhere in OS.
> >>
> >> About that, I don't know enough of uboot but are there existing PSCI or
> >> other seemingly standard secondary core support in uboot that would make
> >> us go through the same initialization as the boot CPU? If not, is
> >> everything going to be largely implementation specific and
> >> scattered between uboot and the hypervisors or kernel?
> > 
> > in ARMV7 SoCs, unfortunately, we lived in a world of no-exact-standard.
> > even within TI, Few of the SoCs use PSCI, others did implement custom
> > SMC calls (since they existed in an architecture prior to PSCI).
> > 
> >>
> >> FWIW, this is what prompted me to submit this:
> >>
> >> https://patchwork.kernel.org/patch/10453643/
> > 
> > That wont work in a generic manner for precisely the same reason I had to do
> > it with weak function in u-boot (some SoCs will only permit 'mcr
> > p15, 0, r0, c1, c0, 1' in secure world and you need to make a custom smc
> > call to make it happen). Unfortunately, IMHO, at least at this
> > point, there'd be custom implementations per SoC and layers depending on
> > where to implement it.
> 
> It won't work in a generic manner but it will work for some platforms
> where updating the firmware is impractical, and since the bits are write
> ignore if your PL does not allow it, this still seems like a net win for
> platforms where this is effective, and it does take care of Linux doing
> the SMP bring-up of secondary cores as well. That's what we have in our
> downstream tree at least, and I was hoping this could land upstream too.


I think it is clear from Russell's summary that we dont want "may work"
workaround in kernel/bootloaders. in case of u-boot (which this patch is
about), I'd suggest adding the CONFIG_*CVE* input to the Kconfig for the
SoC where you know for sure this works.

Does that sound a fair tradeoff?

-- 
Regards,
Nishanth Menon

[U-Boot] [PATCH 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Fabio Estevam]

On Tue, Jun 12, 2018 at 5:24 PM, Nishanth Menon <nm@ti.com> wrote:
> As recommended by Arm in [1], ACTLR[0] (Enable invalidates of BTB)
> needs to be set[2] for BTB to be invalidated on ICIALLU. This needs to
> be done unconditionally for Cortex-A15 processors. Provide a config
> option for platforms to enable this option based on impact analysis
> for products.
>
> NOTE: This patch in itself is NOT the final solution, this requires:
> a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
>    provide direct access to ACR register.
> b) Operating Systems such as Linux to provide adequate workaround in the
>    right locations.
> c) This workaround applies to only the boot processor. It is important
>    to apply workaround as necessary (context-save-restore) around low
>    power context loss OR additional processors as necessary in either
>    firmware support OR elsewhere in OS.
>
> [1] https://developer.arm.com/support/security-update
> [2] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0438c/BABGHIBG.html
>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Andre Przywara <Andre.Przywara@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
>
> Signed-off-by: Nishanth Menon <nm@ti.com>

On a imx51-babbage board:

Tested-by: Fabio Estevam <fabio.estevam@nxp.com>

[U-Boot] [U-Boot, 2/4] ARM: Introduce ability to enable invalidate of BTB with ICIALLU on Cortex-A15 for CVE-2017-5715

[Tom Rini]

On Tue, Jun 12, 2018 at 03:24:09PM -0500, Nishanth Menon wrote:

> As recommended by Arm in [1], ACTLR[0] (Enable invalidates of BTB)
> needs to be set[2] for BTB to be invalidated on ICIALLU. This needs to
> be done unconditionally for Cortex-A15 processors. Provide a config
> option for platforms to enable this option based on impact analysis
> for products.
> 
> NOTE: This patch in itself is NOT the final solution, this requires:
> a) Implementation of v7_arch_cp15_set_acr on SoCs which may not
>    provide direct access to ACR register.
> b) Operating Systems such as Linux to provide adequate workaround in the
>    right locations.
> c) This workaround applies to only the boot processor. It is important
>    to apply workaround as necessary (context-save-restore) around low
>    power context loss OR additional processors as necessary in either
>    firmware support OR elsewhere in OS.
> 
> [1] https://developer.arm.com/support/security-update
> [2] http://infocenter.arm.com/help/topic/com.arm.doc.ddi0438c/BABGHIBG.html
> 
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Christoffer Dall <christoffer.dall@linaro.org>
> Cc: Andre Przywara <Andre.Przywara@arm.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Michael Nazzareno Trimarchi <michael@amarulasolutions.com>
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180629/0c6cdcb8/attachment.sig>

[U-Boot] [PATCH 3/4] ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of BTB) to facilitate CVE_2017-5715 WA in OS

[Nishanth Menon]

Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
function to setup the bits, we are able to override the settings.

Without this enabled, Linux kernel reports:
CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable

With this enabled, Linux kernel reports:
CPU0: Spectre v2: using ICIALLU workaround

NOTE: This by itself does not enable the workaround for CPU1 (on
OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 3bb1ecb58de0..77820cc8d1e4 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -53,6 +53,7 @@ config OMAP54XX
 	bool "OMAP54XX SoC"
 	select ARM_ERRATA_798870
 	select SYS_THUMB_BUILD
+	select ARM_CORTEX_A15_CVE_2017_5715
 	imply NAND_OMAP_ELM
 	imply NAND_OMAP_GPMC
 	imply SPL_DISPLAY_PRINT
-- 
2.15.1

[U-Boot] [PATCH 3/4] ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of BTB) to facilitate CVE_2017-5715 WA in OS

[Marek Vasut]

On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> function to setup the bits, we are able to override the settings.
> 
> Without this enabled, Linux kernel reports:
> CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> 
> With this enabled, Linux kernel reports:
> CPU0: Spectre v2: using ICIALLU workaround
> 
> NOTE: This by itself does not enable the workaround for CPU1 (on
> OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>
> ---
>  arch/arm/mach-omap2/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index 3bb1ecb58de0..77820cc8d1e4 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -53,6 +53,7 @@ config OMAP54XX
>  	bool "OMAP54XX SoC"
>  	select ARM_ERRATA_798870
>  	select SYS_THUMB_BUILD
> +	select ARM_CORTEX_A15_CVE_2017_5715
>  	imply NAND_OMAP_ELM
>  	imply NAND_OMAP_GPMC
>  	imply SPL_DISPLAY_PRINT
> 

Can this be enabled for all CA15 systems somehow ? I am sure there are
more that are vulnerable.

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 3/4] ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of BTB) to facilitate CVE_2017-5715 WA in OS

[Nishanth Menon]

On 23:06-20180612, Marek Vasut wrote:
> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> > Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> > function to setup the bits, we are able to override the settings.
> > 
> > Without this enabled, Linux kernel reports:
> > CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> > 
> > With this enabled, Linux kernel reports:
> > CPU0: Spectre v2: using ICIALLU workaround
> > 
> > NOTE: This by itself does not enable the workaround for CPU1 (on
> > OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> > 
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> >  arch/arm/mach-omap2/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > index 3bb1ecb58de0..77820cc8d1e4 100644
> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -53,6 +53,7 @@ config OMAP54XX
> >  	bool "OMAP54XX SoC"
> >  	select ARM_ERRATA_798870
> >  	select SYS_THUMB_BUILD
> > +	select ARM_CORTEX_A15_CVE_2017_5715
> >  	imply NAND_OMAP_ELM
> >  	imply NAND_OMAP_GPMC
> >  	imply SPL_DISPLAY_PRINT
> > 
> 
> Can this be enabled for all CA15 systems somehow ? I am sure there are
> more that are vulnerable.

I just dont know how to make smc call convention generic. This is the
reason why v7_arch_cp15_set_acr is setup as a weak function. you'd have
to implement it specific to SoC (in many newer SoCs, you might
potentially be able to make at least few implementations generic using
PSCI). NOTE: this is the same trouble with erratum 801819 implementation
as well.

-- 
Regards,
Nishanth Menon

[U-Boot] [PATCH 3/4] ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of BTB) to facilitate CVE_2017-5715 WA in OS

[Russell King - ARM Linux]

On Wed, Jun 13, 2018 at 01:06:13AM +0200, Marek Vasut wrote:
> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> > Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> > function to setup the bits, we are able to override the settings.
> > 
> > Without this enabled, Linux kernel reports:
> > CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> > 
> > With this enabled, Linux kernel reports:
> > CPU0: Spectre v2: using ICIALLU workaround
> > 
> > NOTE: This by itself does not enable the workaround for CPU1 (on
> > OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> > 
> > Signed-off-by: Nishanth Menon <nm@ti.com>
> > ---
> >  arch/arm/mach-omap2/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> > index 3bb1ecb58de0..77820cc8d1e4 100644
> > --- a/arch/arm/mach-omap2/Kconfig
> > +++ b/arch/arm/mach-omap2/Kconfig
> > @@ -53,6 +53,7 @@ config OMAP54XX
> >  	bool "OMAP54XX SoC"
> >  	select ARM_ERRATA_798870
> >  	select SYS_THUMB_BUILD
> > +	select ARM_CORTEX_A15_CVE_2017_5715
> >  	imply NAND_OMAP_ELM
> >  	imply NAND_OMAP_GPMC
> >  	imply SPL_DISPLAY_PRINT
> > 
> 
> Can this be enabled for all CA15 systems somehow ? I am sure there are
> more that are vulnerable.

I think you're missing the point.

Spectre affects the _entire_ system.  Working around it in just the
kernel does not mean that the system is no longer vulnerable.

Fixing the "system" means implementing the fixes also in the secure
world, which on A15 and A8 also means setting the IBE bit there.  If
the IBE bit is set in the secure world, it will also be set in the
non-secure world.

The fact that the kernel is complaining is telling you that the
system as a whole does not have the workarounds in place to mitigate
against the vulnerability.  Merely setting the IBE bit via some
secure API doesn't "magically" fix the secure world.

So, even if you were to set the IBE bit via some magic secure API,
the fact still remains: even with these workarounds in place, as I
understand it, the _system as a whole_ remains vulnerable - you
might as well _not_ have the kernel workarounds.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[U-Boot] [PATCH 3/4] ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of BTB) to facilitate CVE_2017-5715 WA in OS

[Marek Vasut]

On 06/13/2018 07:36 PM, Russell King - ARM Linux wrote:
> On Wed, Jun 13, 2018 at 01:06:13AM +0200, Marek Vasut wrote:
>> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
>>> Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
>>> function to setup the bits, we are able to override the settings.
>>>
>>> Without this enabled, Linux kernel reports:
>>> CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
>>>
>>> With this enabled, Linux kernel reports:
>>> CPU0: Spectre v2: using ICIALLU workaround
>>>
>>> NOTE: This by itself does not enable the workaround for CPU1 (on
>>> OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
>>>
>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>> ---
>>>  arch/arm/mach-omap2/Kconfig | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
>>> index 3bb1ecb58de0..77820cc8d1e4 100644
>>> --- a/arch/arm/mach-omap2/Kconfig
>>> +++ b/arch/arm/mach-omap2/Kconfig
>>> @@ -53,6 +53,7 @@ config OMAP54XX
>>>  	bool "OMAP54XX SoC"
>>>  	select ARM_ERRATA_798870
>>>  	select SYS_THUMB_BUILD
>>> +	select ARM_CORTEX_A15_CVE_2017_5715
>>>  	imply NAND_OMAP_ELM
>>>  	imply NAND_OMAP_GPMC
>>>  	imply SPL_DISPLAY_PRINT
>>>
>>
>> Can this be enabled for all CA15 systems somehow ? I am sure there are
>> more that are vulnerable.
> 
> I think you're missing the point.

Please read the patch again.

This enables it only for a specific SoC. My point being, this should be
enabled for all SoCs with CA15, not just some select few.

> Spectre affects the _entire_ system.  Working around it in just the
> kernel does not mean that the system is no longer vulnerable.
> 
> Fixing the "system" means implementing the fixes also in the secure
> world, which on A15 and A8 also means setting the IBE bit there.  If
> the IBE bit is set in the secure world, it will also be set in the
> non-secure world.
> 
> The fact that the kernel is complaining is telling you that the
> system as a whole does not have the workarounds in place to mitigate
> against the vulnerability.  Merely setting the IBE bit via some
> secure API doesn't "magically" fix the secure world.
> 
> So, even if you were to set the IBE bit via some magic secure API,
> the fact still remains: even with these workarounds in place, as I
> understand it, the _system as a whole_ remains vulnerable - you
> might as well _not_ have the kernel workarounds.
> 


-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 3/4] ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of BTB) to facilitate CVE_2017-5715 WA in OS

[Nishanth Menon]

On 20:36-20180613, Marek Vasut wrote:
> On 06/13/2018 07:36 PM, Russell King - ARM Linux wrote:
> > On Wed, Jun 13, 2018 at 01:06:13AM +0200, Marek Vasut wrote:
> >> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> >>> Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> >>> function to setup the bits, we are able to override the settings.
> >>>
> >>> Without this enabled, Linux kernel reports:
> >>> CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> >>>
> >>> With this enabled, Linux kernel reports:
> >>> CPU0: Spectre v2: using ICIALLU workaround
> >>>
> >>> NOTE: This by itself does not enable the workaround for CPU1 (on
> >>> OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> >>>
> >>> Signed-off-by: Nishanth Menon <nm@ti.com>
> >>> ---
> >>>  arch/arm/mach-omap2/Kconfig | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> >>> index 3bb1ecb58de0..77820cc8d1e4 100644
> >>> --- a/arch/arm/mach-omap2/Kconfig
> >>> +++ b/arch/arm/mach-omap2/Kconfig
> >>> @@ -53,6 +53,7 @@ config OMAP54XX
> >>>  	bool "OMAP54XX SoC"
> >>>  	select ARM_ERRATA_798870
> >>>  	select SYS_THUMB_BUILD
> >>> +	select ARM_CORTEX_A15_CVE_2017_5715
> >>>  	imply NAND_OMAP_ELM
> >>>  	imply NAND_OMAP_GPMC
> >>>  	imply SPL_DISPLAY_PRINT
> >>>
> >>
> >> Can this be enabled for all CA15 systems somehow ? I am sure there are
> >> more that are vulnerable.
> > 
> > I think you're missing the point.
> 
> Please read the patch again.
> 
> This enables it only for a specific SoC. My point being, this should be
> enabled for all SoCs with CA15, not just some select few.
> 

As I had previously responded in
https://marc.info/?l=u-boot&m=152889727127549&w=2

I am not disagreeing this needs to be done for all CA15 based SoCs
(and A8s for previous patches ...), but.. I am not sure what you'd
like me to do here -> I just dont know what the SMC convention is
for all SoCs with CA15! I can help with TI SoCs for sure.. but then,
Russell has a point that this is just one part of the solution -> on
devices that provide secure services, there is definitely a need to
lock the secure entry points down as well. But, specifically to this
patch, do recommend an alternative if one exists.. will gladly follow.

-- 
Regards,
Nishanth Menon

[U-Boot] [PATCH 3/4] ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of BTB) to facilitate CVE_2017-5715 WA in OS

[Russell King - ARM Linux]

On Wed, Jun 13, 2018 at 10:36:56PM +0200, Marek Vasut wrote:
> On 06/13/2018 07:36 PM, Russell King - ARM Linux wrote:
> > On Wed, Jun 13, 2018 at 01:06:13AM +0200, Marek Vasut wrote:
> >> On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> >>> Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> >>> function to setup the bits, we are able to override the settings.
> >>>
> >>> Without this enabled, Linux kernel reports:
> >>> CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> >>>
> >>> With this enabled, Linux kernel reports:
> >>> CPU0: Spectre v2: using ICIALLU workaround
> >>>
> >>> NOTE: This by itself does not enable the workaround for CPU1 (on
> >>> OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> >>>
> >>> Signed-off-by: Nishanth Menon <nm@ti.com>
> >>> ---
> >>>  arch/arm/mach-omap2/Kconfig | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> >>> index 3bb1ecb58de0..77820cc8d1e4 100644
> >>> --- a/arch/arm/mach-omap2/Kconfig
> >>> +++ b/arch/arm/mach-omap2/Kconfig
> >>> @@ -53,6 +53,7 @@ config OMAP54XX
> >>>  	bool "OMAP54XX SoC"
> >>>  	select ARM_ERRATA_798870
> >>>  	select SYS_THUMB_BUILD
> >>> +	select ARM_CORTEX_A15_CVE_2017_5715
> >>>  	imply NAND_OMAP_ELM
> >>>  	imply NAND_OMAP_GPMC
> >>>  	imply SPL_DISPLAY_PRINT
> >>>
> >>
> >> Can this be enabled for all CA15 systems somehow ? I am sure there are
> >> more that are vulnerable.
> > 
> > I think you're missing the point.
> 
> Please read the patch again.

Stop this madness - I /know/ precisely what _this_ patch is doing.
My reply was to *your* comment about extending it "for all CA15
systems".

> This enables it only for a specific SoC. My point being, this should be
> enabled for all SoCs with CA15, not just some select few.

Let's try again... the short answer: NO.

The long answer:

Enabling IBE does *not* universally solve the problem for all SoCs
using CA15.  It merely enables the instructions required for
workarounds in the *kernel* part of the system to take effect.  That
leaves the rest of the system *vulnerable*.

Just in the same way that we have to apply the workarounds /not only/
at the kernel level, but also the hypervisor level for KVM to prevent
KVM being vulnerable, the workarounds _also_ need to be appled at
secure firmware level, as I tried to explain.

Nishanth's OMAP5 case is kind of special because, from what he's
said (a) there's nothing in the secure world that really matters,
and (b) there's nothing that can be done to fix the secure world
because that firmware is in ROM and can never be changed.

That isn't true "for all CA15 systems", and if we're wanting
systems to be properly fixed, then fixing the problem properly
(by fixing the secure world to set IBE *and* implement the
workarounds there) is the right thing.

Setting the IBE bit in the kernel for all CA15 means that, while
we solve the kernel part (and KVM part), the secure world will
remain vulnerable if it has no protection - and worse, people
probably haven't thought enough about this, or know enough about it,
to realise that the vulnerability still exists all the time that any
part of the system has not been fixed.  So, having the kernel print
a warning is critical.

If it was just the case that the kernel was all that was affected,
then KVM wouldn't have needed to be fixed, but the reality is it
needed to be fixed and has been.  The same applies to the secure
world.

Think about this: if you can trick the secure world into speculatively
executing a set of gadgets by manipulating the ARM register values
passed to the SMC call to read secure world memory - or any memory
you shouldn't have access to (like the kernel) then setting the IBE
bit and having the kernel fixes in place is completely meaningless.
As I said below, the system _remains_ vulnerable.

Take a look at the work going on with ARM64 syscalls - they're now
explicitly zeroing all registers on entry that are not an explicit
argument to any syscall.  The reason is to prevent userspace doing
exactly what I've described above, except with the kernel.

So, should we extend it "for all CA15 systems".  No, definitely not
without knowing exactly what the situation is for each and every one.
Having it done in firmware - the same firmware that switches the
CPU out of secure mode - is the right answer where it's possible to
do so.  That won't happen if we apply a "fix" to set IBE as a big
hammer to the kernel.

> > Spectre affects the _entire_ system.  Working around it in just the
> > kernel does not mean that the system is no longer vulnerable.
> > 
> > Fixing the "system" means implementing the fixes also in the secure
> > world, which on A15 and A8 also means setting the IBE bit there.  If
> > the IBE bit is set in the secure world, it will also be set in the
> > non-secure world.
> > 
> > The fact that the kernel is complaining is telling you that the
> > system as a whole does not have the workarounds in place to mitigate
> > against the vulnerability.  Merely setting the IBE bit via some
> > secure API doesn't "magically" fix the secure world.
> > 
> > So, even if you were to set the IBE bit via some magic secure API,
> > the fact still remains: even with these workarounds in place, as I
> > understand it, the _system as a whole_ remains vulnerable - you
> > might as well _not_ have the kernel workarounds.

And the long answer is basically what I said ^^^^^ there.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[U-Boot] [U-Boot, 3/4] ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of BTB) to facilitate CVE_2017-5715 WA in OS

[Tom Rini]

On Tue, Jun 12, 2018 at 03:24:10PM -0500, Nishanth Menon wrote:

> Enable CVE_2017_5715 and since we have our own v7_arch_cp15_set_acr
> function to setup the bits, we are able to override the settings.
> 
> Without this enabled, Linux kernel reports:
> CPU0: Spectre v2: firmware did not set auxiliary control register IBE bit, system vulnerable
> 
> With this enabled, Linux kernel reports:
> CPU0: Spectre v2: using ICIALLU workaround
> 
> NOTE: This by itself does not enable the workaround for CPU1 (on
> OMAP5 and DRA72/AM572 SoCs) and may require additional kernel patches.
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180629/413e1c95/attachment.sig>

[U-Boot] [PATCH 4/4] ARM: mach-omap2: omap3/am335x: Enable ACR::IBE on Cortex-A8 SoCs for CVE-2017-5715

[Nishanth Menon]

Enable CVE-2017-5715 option to set the IBE bit. This enables kernel
workarounds necessary for the said CVE.

With this enabled, Linux reports:
CPU0: Spectre v2: using BPIALL workaround

This workaround may need to be re-applied in OS environment around low
power transition resume states where context of ACR would be lost (off-mode
etc).

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 arch/arm/mach-omap2/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 77820cc8d1e4..f4babc8d2600 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -10,6 +10,7 @@ config OMAP34XX
 	select ARM_ERRATA_454179
 	select ARM_ERRATA_621766
 	select ARM_ERRATA_725233
+	select ARM_CORTEX_A8_CVE_2017_5715
 	select USE_TINY_PRINTF
 	imply NAND_OMAP_GPMC
 	imply SPL_EXT_SUPPORT
@@ -116,6 +117,7 @@ config AM43XX
 config AM33XX
 	bool "AM33XX SoC"
 	select SPECIFY_CONSOLE_INDEX
+	select ARM_CORTEX_A8_CVE_2017_5715
 	imply NAND_OMAP_ELM
 	imply NAND_OMAP_GPMC
 	imply SPL_NAND_AM33XX_BCH
-- 
2.15.1

[U-Boot] [U-Boot, 4/4] ARM: mach-omap2: omap3/am335x: Enable ACR::IBE on Cortex-A8 SoCs for CVE-2017-5715

[Tom Rini]

On Tue, Jun 12, 2018 at 03:24:11PM -0500, Nishanth Menon wrote:

> Enable CVE-2017-5715 option to set the IBE bit. This enables kernel
> workarounds necessary for the said CVE.
> 
> With this enabled, Linux reports:
> CPU0: Spectre v2: using BPIALL workaround
> 
> This workaround may need to be re-applied in OS environment around low
> power transition resume states where context of ACR would be lost (off-mode
> etc).
> 
> Signed-off-by: Nishanth Menon <nm@ti.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180629/9b9c813c/attachment.sig>

[U-Boot] [PATCH 0/4] ARM: Provide workaround setup bits for CVE-2017-5715 (A8/A15)

[Marek Vasut]

On 06/12/2018 10:24 PM, Nishanth Menon wrote:
> Hi,
> 
> This is a follow on from https://marc.info/?l=u-boot&m=151691688828176&w=2 (RFC)
> 
> NOTE:
> * As per ARM recommendations[2], and discussions in list[1] ARM
>   Cortex-A9/12/17 do not need additional steps in u-boot to enable the
>   OS level workarounds.
> * This itself is'nt a complete solution and is based on recommendation
>   This from Arm[2] for variant 2 CVE-2017-5715 -> Kernel changes can be seen on
>   linux next (next-20180612) or on linux master (upcoming v4.18-rc1 tag).
> * I think it is necessary on older SoCs without firmware support
>   (such as older OMAPs and AM*) to have kernel support mirroring what we do in
>   u-boot to support additional cores AND/OR low power states where contexts are
>   lost (assuming ACR states are'nt saved). just my 2 cents.
> 
> Few of the tests (with linux next-20180612):
> AM571-IDK: https://pastebin.ubuntu.com/p/sr5X6sN3Tr/ (single core A15)
> OMAP5-uEVM: https://pastebin.ubuntu.com/p/9yDM22bJ6n/ (dual core A15)
> OMAP3-beagle-xm: https://pastebin.ubuntu.com/p/9DfDkpyxym/ (Single A8)
> AM335x-Beaglebone-black: https://pastebin.ubuntu.com/p/DczT9jPMwb/ (Single A8)
> 
> Nishanth Menon (4):
>   ARM: Introduce ability to enable ACR::IBE on Cortex-A8 for
>     CVE-2017-5715
>   ARM: Introduce ability to enable invalidate of BTB with ICIALLU on
>     Cortex-A15 for CVE-2017-5715
>   ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of
>     BTB) to facilitate CVE_2017-5715 WA in OS
>   ARM: mach-omap2: omap3/am335x: Enable ACR::IBE on Cortex-A8 SoCs for
>     CVE-2017-5715
> 
>  arch/arm/Kconfig            |  9 +++++++++
>  arch/arm/cpu/armv7/start.S  | 15 +++++++++++++--
>  arch/arm/mach-omap2/Kconfig |  3 +++
>  3 files changed, 25 insertions(+), 2 deletions(-)
> 
> [1] https://marc.info/?t=151639906500002&r=1&w=2
> [2] https://developer.arm.com/support/security-update
> [3] https://marc.info/?t=151543790400007&r=1&w=2 and the latest in:
> 	https://marc.info/?l=linux-arm-kernel&m=151689379521082&w=2
> [4]
> 	https://github.com/ARM-software/arm-trusted-firmware/wiki/ARM-Trusted-Firmware-Security-Advisory-TFV-6
> 	https://www.op-tee.org/security-advisories/
> 	https://www.linaro.org/blog/meltdown-spectre/
> 

Except for that minor insignificant nit about BIT() macro, entire series

Acked-by: Marek Vasut <marek.vasut@gmail.com>

-- 
Best regards,
Marek Vasut

[U-Boot] [PATCH 0/4] ARM: Provide workaround setup bits for CVE-2017-5715 (A8/A15)

[Tom Rini]

On Tue, Jun 12, 2018 at 03:24:07PM -0500, Nishanth Menon wrote:

> Hi,
> 
> This is a follow on from https://marc.info/?l=u-boot&m=151691688828176&w=2 (RFC)
> 
> NOTE:
> * As per ARM recommendations[2], and discussions in list[1] ARM
>   Cortex-A9/12/17 do not need additional steps in u-boot to enable the
>   OS level workarounds.
> * This itself is'nt a complete solution and is based on recommendation
>   This from Arm[2] for variant 2 CVE-2017-5715 -> Kernel changes can be seen on
>   linux next (next-20180612) or on linux master (upcoming v4.18-rc1 tag).
> * I think it is necessary on older SoCs without firmware support
>   (such as older OMAPs and AM*) to have kernel support mirroring what we do in
>   u-boot to support additional cores AND/OR low power states where contexts are
>   lost (assuming ACR states are'nt saved). just my 2 cents.
> 
> Few of the tests (with linux next-20180612):
> AM571-IDK: https://pastebin.ubuntu.com/p/sr5X6sN3Tr/ (single core A15)
> OMAP5-uEVM: https://pastebin.ubuntu.com/p/9yDM22bJ6n/ (dual core A15)
> OMAP3-beagle-xm: https://pastebin.ubuntu.com/p/9DfDkpyxym/ (Single A8)
> AM335x-Beaglebone-black: https://pastebin.ubuntu.com/p/DczT9jPMwb/ (Single A8)
> 
> Nishanth Menon (4):
>   ARM: Introduce ability to enable ACR::IBE on Cortex-A8 for
>     CVE-2017-5715
>   ARM: Introduce ability to enable invalidate of BTB with ICIALLU on
>     Cortex-A15 for CVE-2017-5715
>   ARM: mach-omap2: omap5/dra7: Enable ACTLR[0] (Enable invalidates of
>     BTB) to facilitate CVE_2017-5715 WA in OS
>   ARM: mach-omap2: omap3/am335x: Enable ACR::IBE on Cortex-A8 SoCs for
>     CVE-2017-5715
> 
>  arch/arm/Kconfig            |  9 +++++++++
>  arch/arm/cpu/armv7/start.S  | 15 +++++++++++++--
>  arch/arm/mach-omap2/Kconfig |  3 +++
>  3 files changed, 25 insertions(+), 2 deletions(-)
> 
> [1] https://marc.info/?t=151639906500002&r=1&w=2
> [2] https://developer.arm.com/support/security-update
> [3] https://marc.info/?t=151543790400007&r=1&w=2 and the latest in:
> 	https://marc.info/?l=linux-arm-kernel&m=151689379521082&w=2
> [4]
> 	https://github.com/ARM-software/arm-trusted-firmware/wiki/ARM-Trusted-Firmware-Security-Advisory-TFV-6
> 	https://www.op-tee.org/security-advisories/
> 	https://www.linaro.org/blog/meltdown-spectre/

This series of changes for U-Boot, if I can briefly summarize the
feedback as I understand it, is that yes, this is correct and is a part
of what is required to work around the issues, but only covers as much
of the system as U-Boot can cover leaving other parts of the software
stack (still) in need of fixes.  Yes?  If so, is there anything else
that should be done before in U-Boot we grab these changes?  Would any
of the knowledgeable but not usually U-Boot folks on CC feel comfortable
adding an ack/reviewed-by to the series?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180618/fbdc1526/attachment.sig>