ARM: Fix errata 751472 handling on Cortex-A9 r1p*

Looks like enabling CONFIG_ARM_ERRATA_751472 causes omap4 blaze to not boot when enabled. The ARM core on it is an earlier r1p2:

CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=10c53c7d

Unfortunately I don't have the details of errata 751472, but I'm guessing we need to disable it for r1p*.

Quoted 7 lines · Tony Lindgren

Looks like enabling CONFIG_ARM_ERRATA_751472 causes omap4 blaze to not boot when enabled. The ARM core on it is an earlier r1p2:

CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=10c53c7d

Unfortunately I don't have the details of errata 751472, but I'm guessing we need to disable it for r1p*.

Fails because it can't write the diagnostic register from non-secure mode or fails later?

Looks like enabling CONFIG_ARM_ERRATA_751472 causes omap4 blaze to not boot when enabled. The ARM core on it is an earlier r1p2:

CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=10c53c7d

Unfortunately I don't have the details of errata 751472, but I'm guessing we need to disable it for r1p*.

Quoted 5 lines · Tony Lindgren

Looks like enabling CONFIG_ARM_ERRATA_751472 causes omap4 blaze to not boot when enabled. The ARM core on it is an earlier r1p2:

CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=10c53c7d

Quoted 2 lines

Unfortunately I don't have the details of errata 751472, but I'm guessing we need to disable it for r1p*.

I checked the CA9MP errata document and this erratum impacts all r0/r1/r2 CPUs. I am wondering if the problem is because the workaround requires you to set a bit in the Diagnostic Control register and the read-modify-write sequence provided in the workaround is for secure mode. Not sure if there is a non-secure workaround available :-(

Quoted 5 lines · Tony Lindgren

Looks like enabling CONFIG_ARM_ERRATA_751472 causes omap4 blaze to not boot when enabled. The ARM core on it is an earlier r1p2:

CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=10c53c7d

Quoted 2 lines

Unfortunately I don't have the details of errata 751472, but I'm guessing we need to disable it for r1p*.

I checked the CA9MP errata document and this erratum impacts all r0/r1/r2 CPUs. I am wondering if the problem is because the workaround requires you to set a bit in the Diagnostic Control register and the read-modify-write sequence provided in the workaround is for secure mode. Not sure if there is a non-secure workaround available :-(

Quoted 7 lines · Jon Hunter

Quoted 5 lines · Tony Lindgren

Looks like enabling CONFIG_ARM_ERRATA_751472 causes omap4 blaze to not boot when enabled. The ARM core on it is an earlier r1p2:

CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=10c53c7d

Quoted 8 lines

Quoted 2 lines

Unfortunately I don't have the details of errata 751472, but I'm guessing we need to disable it for r1p*.

I checked the CA9MP errata document and this erratum impacts all r0/r1/r2 CPUs. I am wondering if the problem is because the workaround requires you to set a bit in the Diagnostic Control register and the read-modify-write sequence provided in the workaround is for secure mode. Not sure if there is a non-secure workaround available :-(

Most likely, and there's not a lot that the kernel can sanely do about that. We have ended up deciding (through being forced to because of how the security stuff works) that the stages prior to the kernel will implement the work-around enables because those stages are already platform specific, and the kernel will implement a "test for the work-around already enabled."

The net result is, if you enable an Errata in the kernel which your earlier boot stages has not already configured, the kernel will hang. Not much we can do about the hanging aspect, because the kernel takes an exception which we can't trap at those early stages in the boot process.

I'm not particularly happy about that design, but that's what we've ended up with through the 'design' of the security stuff forced onto us.

Quoted 30 lines · Russell King - ARM Linux

Quoted 7 lines · Jon Hunter

Quoted 5 lines · Tony Lindgren

Looks like enabling CONFIG_ARM_ERRATA_751472 causes omap4 blaze to not boot when enabled. The ARM core on it is an earlier r1p2:

CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=10c53c7d

Quoted 8 lines

Quoted 2 lines

Unfortunately I don't have the details of errata 751472, but I'm guessing we need to disable it for r1p*.

I checked the CA9MP errata document and this erratum impacts all r0/r1/r2 CPUs. I am wondering if the problem is because the workaround requires you to set a bit in the Diagnostic Control register and the read-modify-write sequence provided in the workaround is for secure mode. Not sure if there is a non-secure workaround available :-(

Most likely, and there's not a lot that the kernel can sanely do about that. We have ended up deciding (through being forced to because of how the security stuff works) that the stages prior to the kernel will implement the work-around enables because those stages are already platform specific, and the kernel will implement a "test for the work-around already enabled."

The net result is, if you enable an Errata in the kernel which your earlier boot stages has not already configured, the kernel will hang. Not much we can do about the hanging aspect, because the kernel takes an exception which we can't trap at those early stages in the boot process.

Quoted 3 lines

I'm not particularly happy about that design, but that's what we've ended up with through the 'design' of the security stuff forced onto us.

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Quoted 3 lines · Tony Lindgren

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Can you actually read the state of the diagnostic register in non-secure mode? If you can on the A9, is the same true on A8 or others?

Multi-platform kernels present a new problem in that we basically need to enable all errata work-arounds. I've been meaning to look thru the errata work-arounds to figure out which ones can be selected for multi-platform kernels without side effects on unaffected parts (i.e. set a chicken bit based on core revision). For any in runtime paths, we may need to do runtime patching if they have performance impact.

Quoted 7 lines · Rob Herring

Quoted 3 lines · Tony Lindgren

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Can you actually read the state of the diagnostic register in non-secure mode? If you can on the A9, is the same true on A8 or others?

Looks like it can be read on at least TI omap 4430 which is A9. But it reads as zero, so the below patch is what I came up with.

No idea if assuming that zero value for the diagnostic register is safe.. What's the default value of the diagnostic register supposed to be?

Quoted 6 lines

Multi-platform kernels present a new problem in that we basically need to enable all errata work-arounds. I've been meaning to look thru the errata work-arounds to figure out which ones can be selected for multi-platform kernels without side effects on unaffected parts (i.e. set a chicken bit based on core revision). For any in runtime paths, we may need to do runtime patching if they have performance impact.

Yeah that's how I ran into it as multiplatform enabled omap booted on other omaps but not on omap4.

Regards,

Tony

From: Tony Lindgren <[email protected]>
Date: Tue, 13 Nov 2012 16:57:42 -0800
Subject: [PATCH] ARM: Fix errata 751472 handling on Cortex-A9 for secure mode

Looks like enabling CONFIG_ARM_ERRATA_751472 causes TI omap4 blaze to not boot when enabled. The ARM core on it is an earlier r1p2.

This seems to be caused by the write to the diagnostic register that shortly after causes an exception as writing to the diagnostic register on systems with secure mode is not allowed.

Also it seems that reading the diagnostic register results zero so we may not be able to check if bit #11 is already set.

Let's assume that if the diagnostic register is zero, we don't want to touch it as it seems to hint secure mode at least on TI omaps. If it's non-zero, check if bit #11 is set and only attempt to set it if not set.

arch/arm/mm/proc-v7.S +7 -3

@@ -238,9 +238,13 @@ __v7_setup:
 #if defined(CONFIG_ARM_ERRATA_751472) && defined(CONFIG_SMP)
 	ALT_SMP(cmp r6, #0x30)			@ present prior to r3p0
 	ALT_UP_B(1f)
-	mrclt	p15, 0, r10, c15, c0, 1		@ read diagnostic register
-	orrlt	r10, r10, #1 << 11		@ set bit #11
-	mcrlt	p15, 0, r10, c15, c0, 1		@ write diagnostic register
+	bge	1f				@ not needed for r3p0 and later
+	mrc	p15, 0, r10, c15, c0, 1		@ read diagnostic register
+	teq	r10, #0				@ zero for secure mode?
+	beq	1f				@ bail out for secure mode
+	tst	r10, #1 << 11			@ bit #11 already set?
+	orreq	r10, r10, #1 << 11		@ set bit #11 if not set
+	mcreq	p15, 0, r10, c15, c0, 1		@ write diagnostic register
 1:
 #endif
 

Quoted 12 lines · Tony Lindgren

Quoted 7 lines · Rob Herring

Quoted 3 lines · Tony Lindgren

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Can you actually read the state of the diagnostic register in non-secure mode? If you can on the A9, is the same true on A8 or others?

Looks like it can be read on at least TI omap 4430 which is A9. But it reads as zero, so the below patch is what I came up with.

Quoted 3 lines

No idea if assuming that zero value for the diagnostic register is safe.. What's the default value of the diagnostic register supposed to be?

RTFM. Oh, wait it's a super secret, undocumented register. We shouldn't even be talking about it.

It could vary by rev, but I see 0 for the reset value, so this would not work if the bootloader did not do any setup of the diagnostic register.

One way to determine secure mode on the A9 would be seeing if you can change the auxcr register. Something like this (untested):

mrc p15, 0, r0, c1, c0, 1; Read ACTLR
eor r1, r0, #0x100 ; Modify alloc in 1 way
mcr p15, 0, r1, c1, c0, 1
mrc p15, 0, r2, c1, c0, 1; Read ACTLR
mcr p15, 0, r0, c1, c0, 1 ; Restore original value
cmp r1, r2
bne skip_errata

It would be good to do this test for all the errata rather than just this one.

Quoted 9 lines · Tony Lindgren

Quoted 7 lines · Rob Herring

Quoted 3 lines · Tony Lindgren

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Can you actually read the state of the diagnostic register in non-secure mode? If you can on the A9, is the same true on A8 or others?

Quoted 6 lines

Looks like it can be read on at least TI omap 4430 which is A9. But it reads as zero, so the below patch is what I came up with.

No idea if assuming that zero value for the diagnostic register is safe.. What's the default value of the diagnostic register supposed to be?

RTFM. Oh, wait it's a super secret, undocumented register. We shouldn't even be talking about it.

It could vary by rev, but I see 0 for the reset value, so this would not work if the bootloader did not do any setup of the diagnostic register.

One way to determine secure mode on the A9 would be seeing if you can change the auxcr register. Something like this (untested):

mrc p15, 0, r0, c1, c0, 1; Read ACTLR
eor r1, r0, #0x100 ; Modify alloc in 1 way
mcr p15, 0, r1, c1, c0, 1
mrc p15, 0, r2, c1, c0, 1; Read ACTLR
mcr p15, 0, r0, c1, c0, 1 ; Restore original value
cmp r1, r2
bne skip_errata

Quoted 26 lines · Rob Herring

Quoted 9 lines · Tony Lindgren

Quoted 7 lines · Rob Herring

Quoted 3 lines · Tony Lindgren

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Can you actually read the state of the diagnostic register in non-secure mode? If you can on the A9, is the same true on A8 or others?

Quoted 6 lines

Looks like it can be read on at least TI omap 4430 which is A9. But it reads as zero, so the below patch is what I came up with.

No idea if assuming that zero value for the diagnostic register is safe.. What's the default value of the diagnostic register supposed to be?

RTFM. Oh, wait it's a super secret, undocumented register. We shouldn't even be talking about it.

It could vary by rev, but I see 0 for the reset value, so this would not work if the bootloader did not do any setup of the diagnostic register.

One way to determine secure mode on the A9 would be seeing if you can change the auxcr register. Something like this (untested):

Quoted 7 lines

mrc   p15, 0, r0, c1, c0, 1; Read ACTLR
eor   r1, r0, #0x100          ; Modify alloc in 1 way
mcr   p15, 0, r1, c1, c0, 1
mrc   p15, 0, r2, c1, c0, 1; Read ACTLR
mcr   p15, 0, r0, c1, c0, 1   ; Restore original value
cmp   r1, r2
bne   skip_errata

This would fail on platforms where Linux runs in non-secure mode. What we do for some errata workarounds is to test whether the bit was already set and avoid writing the register. But this assumes that, for a given workaround in the kernel, there is a corresponding workaround in the code running before the kernel (boot-loader, firmware) which sets that bit.

Since the kernel will run more often in non-secure mode (on Cortex-A15 you need this for the virtualisation extensions) I strongly suggest that the workaround (usually undocumented bit setting) is done before the kernel is started and we simply remove it from Linux (or add a clear comment that it only works if running in secure mode; if unsure say 'N').

I don't think it's worth the hassle detecting whether the kernel runs in secure or non-secure mode, just assume the latter and get SoC vendors to update the boot loaders or firmware (if possible) with any errata workarounds.

Having a common SMC API for errata workarounds is not feasible since not all registers are public, most are implementation specific and it could have secure implications with exposing them.

BTW, I always wondered about what could be preventing TI and the other silicon vendors from using something like an SMC API based on asymmetric cryptography?

Quoted 49 lines · Catalin Marinas

Quoted 26 lines · Rob Herring

Quoted 9 lines · Tony Lindgren

Quoted 7 lines · Rob Herring

Quoted 3 lines · Tony Lindgren

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Can you actually read the state of the diagnostic register in non-secure mode? If you can on the A9, is the same true on A8 or others?

Quoted 6 lines

Looks like it can be read on at least TI omap 4430 which is A9. But it reads as zero, so the below patch is what I came up with.

No idea if assuming that zero value for the diagnostic register is safe.. What's the default value of the diagnostic register supposed to be?

RTFM. Oh, wait it's a super secret, undocumented register. We shouldn't even be talking about it.

It could vary by rev, but I see 0 for the reset value, so this would not work if the bootloader did not do any setup of the diagnostic register.

One way to determine secure mode on the A9 would be seeing if you can change the auxcr register. Something like this (untested):

Quoted 7 lines

mrc   p15, 0, r0, c1, c0, 1; Read ACTLR
eor   r1, r0, #0x100          ; Modify alloc in 1 way
mcr   p15, 0, r1, c1, c0, 1
mrc   p15, 0, r2, c1, c0, 1; Read ACTLR
mcr   p15, 0, r0, c1, c0, 1   ; Restore original value
cmp   r1, r2
bne   skip_errata

This would fail on platforms where Linux runs in non-secure mode. What we do for some errata workarounds is to test whether the bit was already set and avoid writing the register. But this assumes that, for a given workaround in the kernel, there is a corresponding workaround in the code running before the kernel (boot-loader, firmware) which sets that bit.

Since the kernel will run more often in non-secure mode (on Cortex-A15 you need this for the virtualisation extensions) I strongly suggest that the workaround (usually undocumented bit setting) is done before the kernel is started and we simply remove it from Linux (or add a clear comment that it only works if running in secure mode; if unsure say 'N').

Quoted 8 lines

I don't think it's worth the hassle detecting whether the kernel runs in secure or non-secure mode, just assume the latter and get SoC vendors to update the boot loaders or firmware (if possible) with any errata workarounds.

Having a common SMC API for errata workarounds is not feasible since not all registers are public, most are implementation specific and it could have secure implications with exposing them.

BTW, I always wondered about what could be preventing TI and the other silicon vendors from using something like an SMC API based on asymmetric cryptography? My understanding is that for OMAP4 GP chips, ROM code switches to non-secure mode before passing control to the bootloader and there is simply no way to workaround bugs like this.

Quoted 5 lines · Siarhei Siamashka

BTW, I always wondered about what could be preventing TI and the other silicon vendors from using something like an SMC API based on asymmetric cryptography? My understanding is that for OMAP4 GP chips, ROM code switches to non-secure mode before passing control to the bootloader and there is simply no way to workaround bugs like this.

AFAIK, there are some SMCs to the OMAP secure firmware that allow such bits to be set (see omap4_l2x0_set_debug() for example). I'm not sure they are documented.

Quoted 26 lines · Rob Herring

Quoted 9 lines · Tony Lindgren

Quoted 7 lines · Rob Herring

Quoted 3 lines · Tony Lindgren

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Can you actually read the state of the diagnostic register in non-secure mode? If you can on the A9, is the same true on A8 or others?

Quoted 6 lines

Looks like it can be read on at least TI omap 4430 which is A9. But it reads as zero, so the below patch is what I came up with.

No idea if assuming that zero value for the diagnostic register is safe.. What's the default value of the diagnostic register supposed to be?

RTFM. Oh, wait it's a super secret, undocumented register. We shouldn't even be talking about it.

It could vary by rev, but I see 0 for the reset value, so this would not work if the bootloader did not do any setup of the diagnostic register.

One way to determine secure mode on the A9 would be seeing if you can change the auxcr register. Something like this (untested):

Quoted 7 lines

mrc p15, 0, r0, c1, c0, 1; Read ACTLR
eor r1, r0, #0x100 ; Modify alloc in 1 way
mcr p15, 0, r1, c1, c0, 1
mrc p15, 0, r2, c1, c0, 1; Read ACTLR
mcr p15, 0, r0, c1, c0, 1 ; Restore original value
cmp r1, r2
bne skip_errata

This would fail on platforms where Linux runs in non-secure mode. What we do for some errata workarounds is to test whether the bit was already set and avoid writing the register. But this assumes that, for a given workaround in the kernel, there is a corresponding workaround in the code running before the kernel (boot-loader, firmware) which sets that bit.

Since the kernel will run more often in non-secure mode (on Cortex-A15 you need this for the virtualisation extensions) I strongly suggest that the workaround (usually undocumented bit setting) is done before the kernel is started and we simply remove it from Linux (or add a clear comment that it only works if running in secure mode; if unsure say 'N').

I don't think it's worth the hassle detecting whether the kernel runs in secure or non-secure mode, just assume the latter and get SoC vendors to update the boot loaders or firmware (if possible) with any errata workarounds.

Having a common SMC API for errata workarounds is not feasible since not all registers are public, most are implementation specific and it could have secure implications with exposing them.

Quoted 49 lines · Catalin Marinas

Quoted 26 lines · Rob Herring

Quoted 9 lines · Tony Lindgren

Quoted 7 lines · Rob Herring

Quoted 3 lines · Tony Lindgren

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Can you actually read the state of the diagnostic register in non-secure mode? If you can on the A9, is the same true on A8 or others?

Quoted 6 lines

Looks like it can be read on at least TI omap 4430 which is A9. But it reads as zero, so the below patch is what I came up with.

No idea if assuming that zero value for the diagnostic register is safe.. What's the default value of the diagnostic register supposed to be?

RTFM. Oh, wait it's a super secret, undocumented register. We shouldn't even be talking about it.

It could vary by rev, but I see 0 for the reset value, so this would not work if the bootloader did not do any setup of the diagnostic register.

One way to determine secure mode on the A9 would be seeing if you can change the auxcr register. Something like this (untested):

Quoted 7 lines

mrc p15, 0, r0, c1, c0, 1; Read ACTLR
eor r1, r0, #0x100 ; Modify alloc in 1 way
mcr p15, 0, r1, c1, c0, 1
mrc p15, 0, r2, c1, c0, 1; Read ACTLR
mcr p15, 0, r0, c1, c0, 1 ; Restore original value
cmp r1, r2
bne skip_errata

This would fail on platforms where Linux runs in non-secure mode. What we do for some errata workarounds is to test whether the bit was already set and avoid writing the register. But this assumes that, for a given workaround in the kernel, there is a corresponding workaround in the code running before the kernel (boot-loader, firmware) which sets that bit.

Since the kernel will run more often in non-secure mode (on Cortex-A15 you need this for the virtualisation extensions) I strongly suggest that the workaround (usually undocumented bit setting) is done before the kernel is started and we simply remove it from Linux (or add a clear comment that it only works if running in secure mode; if unsure say 'N').

Quoted 4 lines

I don't think it's worth the hassle detecting whether the kernel runs in secure or non-secure mode, just assume the latter and get SoC vendors to update the boot loaders or firmware (if possible) with any errata workarounds.

There's other places we need to know secure vs. non-secure mode like whether we can do smc calls or not.

So we should make all these work-arounds depend on !MULTI_PLATFORM then.

Does that work for Versatile Express CA9? It needs ARM_ERRATA_751472.

Quoted 55 lines · Rob Herring

Quoted 53 lines · Catalin Marinas

Quoted 26 lines · Rob Herring

Quoted 9 lines · Tony Lindgren

Quoted 7 lines · Rob Herring

Quoted 3 lines · Tony Lindgren

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Can you actually read the state of the diagnostic register in non-secure mode? If you can on the A9, is the same true on A8 or others?

Quoted 6 lines

Looks like it can be read on at least TI omap 4430 which is A9. But it reads as zero, so the below patch is what I came up with.

No idea if assuming that zero value for the diagnostic register is safe.. What's the default value of the diagnostic register supposed to be?

RTFM. Oh, wait it's a super secret, undocumented register. We shouldn't even be talking about it.

It could vary by rev, but I see 0 for the reset value, so this would not work if the bootloader did not do any setup of the diagnostic register.

One way to determine secure mode on the A9 would be seeing if you can change the auxcr register. Something like this (untested):

Quoted 7 lines

mrc p15, 0, r0, c1, c0, 1; Read ACTLR
eor r1, r0, #0x100 ; Modify alloc in 1 way
mcr p15, 0, r1, c1, c0, 1
mrc p15, 0, r2, c1, c0, 1; Read ACTLR
mcr p15, 0, r0, c1, c0, 1 ; Restore original value
cmp r1, r2
bne skip_errata

This would fail on platforms where Linux runs in non-secure mode. What we do for some errata workarounds is to test whether the bit was already set and avoid writing the register. But this assumes that, for a given workaround in the kernel, there is a corresponding workaround in the code running before the kernel (boot-loader, firmware) which sets that bit.

Since the kernel will run more often in non-secure mode (on Cortex-A15 you need this for the virtualisation extensions) I strongly suggest that the workaround (usually undocumented bit setting) is done before the kernel is started and we simply remove it from Linux (or add a clear comment that it only works if running in secure mode; if unsure say 'N').

I don't think it's worth the hassle detecting whether the kernel runs in secure or non-secure mode, just assume the latter and get SoC vendors to update the boot loaders or firmware (if possible) with any errata workarounds.

Quoted 4 lines

There's other places we need to know secure vs. non-secure mode like whether we can do smc calls or not.

So we should make all these work-arounds depend on !MULTI_PLATFORM then.

Only the workarounds that set bits in secure-only registers.

Quoted 1 line

Does that work for Versatile Express CA9? It needs ARM_ERRATA_751472.

On VE Linux runs in secure mode, so it's fine.

Quoted 4 lines · Catalin Marinas

Quoted 1 line · Rob Herring

Does that work for Versatile Express CA9? It needs ARM_ERRATA_751472.

On VE Linux runs in secure mode, so it's fine.

WTF? You are contradicting yourself.

Don't determine secure mode or not,
but apply work-arounds only in secure mode? How does a kernel built to boot on secure and non-secure chips know that? The requirement would be that every platform have proper work-arounds setup by the bootloader regardless of running in secure or non-secure mode.

Quoted 3 lines

Quoted 1 line

Does that work for Versatile Express CA9? It needs ARM_ERRATA_751472.

On VE Linux runs in secure mode, so it's fine.

WTF? You are contradicting yourself.

So, we don't detect whether we're running in secure mode or not; as I've already stated, we don't have a way to do that. We instead only apply work-arounds which aren't already enabled prior to the kernel booting. So, even on a secure mode platform, we will avoid writing the bits if the work-around has already been applied.

Quoted 6 lines · Russell King - ARM Linux

So, we don't detect whether we're running in secure mode or not; as I've already stated, we don't have a way to do that. We instead only apply work-arounds which aren't already enabled prior to the kernel booting. So, even on a secure mode platform, we will avoid writing the bits if the work-around has already been applied.

This all assumes that we can read the value of the diagnostic register, and on my 4430 blaze the read returns zero. I have no idea if this is the correct value for the register, or if reads always just returns 0.

So we should make all these work-arounds depend on !MULTI_PLATFORM then.

Quoted 1 line · Rob Herring

So we should make all these work-arounds depend on !MULTI_PLATFORM then.

No, because some of the work-arounds don't require setting bits in magic registers.

Also realise this: we test for the revision of the CPU to which they're applicable before attempting to set them. If you have a work-around enabled in the kernel, and it fails at boot, _that_ is an indicator not that the kernel is doing something wrong, but that you need to ensure that the work-around has been applied by the earlier stages.

It's not about "but platform X doesn't need it" - it's about platform X not having the earlier stages updated to fix the errata.

Quoted 6 lines · Russell King - ARM Linux

Quoted 1 line · Rob Herring

So we should make all these work-arounds depend on !MULTI_PLATFORM then.

No, because some of the work-arounds don't require setting bits in magic registers.

Quoted 8 lines

Also realise this: we test for the revision of the CPU to which they're applicable before attempting to set them. If you have a work-around enabled in the kernel, and it fails at boot, _that_ is an indicator not that the kernel is doing something wrong, but that you need to ensure that the work-around has been applied by the earlier stages.

It's not about "but platform X doesn't need it" - it's about platform X not having the earlier stages updated to fix the errata.

There is another aspect. Many CPUs in the field, even if they are a certain rxpy revision, have ECO fixes applied for critical bugs and don't require the workaround. Sometimes those undocumented bits have considerable performance impact and vendors may apply an ECO (unless they are very late in the tape-out process). But the ECO fix does not change the CPU revision, hence the workaround becomes platform specific.

That's why I think it's better if those workarounds are just pushed to the boot-loader for multi-platform kernels. Linux could still check the bits and warn about them rather than failing to boot.

Quoted 23 lines · Catalin Marinas

Quoted 6 lines · Russell King - ARM Linux

Quoted 1 line · Rob Herring

So we should make all these work-arounds depend on !MULTI_PLATFORM then.

No, because some of the work-arounds don't require setting bits in magic registers.

Quoted 8 lines

Also realise this: we test for the revision of the CPU to which they're applicable before attempting to set them. If you have a work-around enabled in the kernel, and it fails at boot, _that_ is an indicator not that the kernel is doing something wrong, but that you need to ensure that the work-around has been applied by the earlier stages.

It's not about "but platform X doesn't need it" - it's about platform X not having the earlier stages updated to fix the errata.

There is another aspect. Many CPUs in the field, even if they are a certain rxpy revision, have ECO fixes applied for critical bugs and don't require the workaround. Sometimes those undocumented bits have considerable performance impact and vendors may apply an ECO (unless they are very late in the tape-out process). But the ECO fix does not change the CPU revision, hence the workaround becomes platform specific.

Quoted 3 lines

That's why I think it's better if those workarounds are just pushed to the boot-loader for multi-platform kernels. Linux could still check the bits and warn about them rather than failing to boot.

... and that's a U-turn if ever there was one... it's ARM Ltd who've been pushing to have these work-arounds in the kernel in the first place.

Should we just remove all the work-arounds, and require boot loaders, including the one on Versatile platforms, to implement this? Why should we treat secure platforms be any different from the non-secure ones in this regard? After all, this _does_ stand in the way of a single kernel image working properly on secure and non-secure platforms.

The more this thread progresses, the more I think the decision to put errata into the kernel was the wrong one.

No idea if assuming that zero value for the diagnostic register is safe.. What's the default value of the diagnostic register supposed to be?

arch/arm/mm/proc-v7.S +3 -0

+	tst	r10, #1 << 11			@ bit #11 already set?
+	orreq	r10, r10, #1 << 11		@ set bit #11 if not set
+	mcreq	p15, 0, r10, c15, c0, 1		@ write diagnostic register
 1:
 #endif

Quoted 31 lines · Tony Lindgren

Quoted 30 lines · Russell King - ARM Linux

Quoted 7 lines · Jon Hunter

Quoted 5 lines · Tony Lindgren

Looks like enabling CONFIG_ARM_ERRATA_751472 causes omap4 blaze to not boot when enabled. The ARM core on it is an earlier r1p2:

CPU: ARMv7 Processor [411fc092] revision 2 (ARMv7), cr=10c53c7d

Quoted 8 lines

Quoted 2 lines

Unfortunately I don't have the details of errata 751472, but I'm guessing we need to disable it for r1p*.

I checked the CA9MP errata document and this erratum impacts all r0/r1/r2 CPUs. I am wondering if the problem is because the workaround requires you to set a bit in the Diagnostic Control register and the read-modify-write sequence provided in the workaround is for secure mode. Not sure if there is a non-secure workaround available :-(

Most likely, and there's not a lot that the kernel can sanely do about that. We have ended up deciding (through being forced to because of how the security stuff works) that the stages prior to the kernel will implement the work-around enables because those stages are already platform specific, and the kernel will implement a "test for the work-around already enabled."

The net result is, if you enable an Errata in the kernel which your earlier boot stages has not already configured, the kernel will hang. Not much we can do about the hanging aspect, because the kernel takes an exception which we can't trap at those early stages in the boot process.

Quoted 6 lines

Quoted 3 lines

I'm not particularly happy about that design, but that's what we've ended up with through the 'design' of the security stuff forced onto us.

Checking for the bit already set should work in this case, I'll post a patch for that shortly.

Can you actually read the state of the diagnostic register in non-secure mode? If you can on the A9, is the same true on A8 or others?

Multi-platform kernels present a new problem in that we basically need to enable all errata work-arounds. I've been meaning to look thru the errata work-arounds to figure out which ones can be selected for multi-platform kernels without side effects on unaffected parts (i.e. set a chicken bit based on core revision). For any in runtime paths, we may need to do runtime patching if they have performance impact.