ARM errata 430973 on multi platform kernels (was: OMAP3-N900: Add microphone bias voltages)

[Tony Lindgren]

Hi,

OK I think debian is using v3.16 kernel

Yes. It will be used for Debian jessie (not yet released) and the
N900 related drivers are enabled in the armmp flavour. Unfortunately
it does not work together with thumb using userland because the
errata 430973 workaround is not enabled.
See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=768890

Hmm I believe many ARMv8 boards will be randomly oopsing
with armhf without that. I sort of recall random oopses just

Since I don't know of any ARMv8 omaps, I will read ARMv8 as ARMv7.

Sorry right, s/ARMv8/Cortex-A8/ :)

And yes, for armhf userland one gets random oopses at least on the
Nokia N900. AFAIK this is not true for all ARMv7 processors
(especially non omaps), though.

http://www.spinics.net/lists/linux-omap/msg108511.html
See also 5c86c5339c56 ("ARM: omap2plus_defconfig: Enable ARM erratum
430973 for omap3").

For me the random oopses occur without this config flag and are
fixed by it. The workaround is not very suitable for multi platform
kernels, though, since its enabled also for unaffected platforms.
As far as I can see the CONFIG_ARM_ERRATA_430973 flag is checked
in proc-v7.S and in proc-v7-2level.S. I think the first file is
irrelevant, since it can be fixed later (see workaround in
nokia_n900_legacy_init in pdata-quirks.c).

Yes so it seems, and the bootloaders should really set it. It's
also disabled for multiplatform builds.

So basically the problem comes down to proc-v7-2level.S

I wonder if the ARMv8 revision range might be wrong 430973 in
kernel or errata?

what revision range? I think the errata is enabled unconditionally
or disabled completly.

The Cortex-A8 range claimed to be affected in Kconfig is listed
as r1p0..r1p2. But I recall seeing this also on omap3 (37xx)
which is r3p2. Also searching for r3p2 430973 produces:

http://e2e.ti.com/support/arm/sitara_arm/f/791/p/254742/891611

And that points to Siarhei's test program that should expose it
run together with other processes:

https://cloud.github.com/downloads/ssvb/ssvb.github.com/ssvb-cpuburn-a8.S

Also I recall that 430973 change to the
arch/arm/mm/proc-v7-2level.S fixed the issue, this should be
verified though.

If I understand the errata correctly the BTAC/BTB flushing is
important. It would be nice if it could be limited to affected
devices, though.

Agreed.

I guess it should be tried to change the workaround, so that it does
only change the behaviour of affected platforms. Otherwise its a
hard decision for distributions to enable the workaround.

Well we should figure out first why flush BTAC/BTB is needed in
cpu_v7_switch_mm.. And if what I'm describing above is still
reproducable.

Reading the help text in the kernel the flushing is the actual
workaround. The other changes only make it possible to do the
flushing.
Maybe an option would be to provide two cpu_v7_switch_mm
implementations (one with the errata and one without). Then
the system can start with the simple implementation. Once
the boot as progressed far enough to know, that the hardware
is affected by the errata, it could switch to the implementation
with the flushing.

That seems like a good way to deal with it but we should first
verify it Siarhei's test program.

Also, I think that the armel distro did not have these issue
while armhf did, so there may something more to this bug.

Regards,

Tony

[Robert Nelson]

Post by Tony Lindgren

And yes, for armhf userland one gets random oopses at least on the
Nokia N900. AFAIK this is not true for all ARMv7 processors
(especially non omaps), though.

http://www.spinics.net/lists/linux-omap/msg108511.html
See also 5c86c5339c56 ("ARM: omap2plus_defconfig: Enable ARM erratum
430973 for omap3").

For me the random oopses occur without this config flag and are
fixed by it. The workaround is not very suitable for multi platform
kernels, though, since its enabled also for unaffected platforms.
As far as I can see the CONFIG_ARM_ERRATA_430973 flag is checked
in proc-v7.S and in proc-v7-2level.S. I think the first file is
irrelevant, since it can be fixed later (see workaround in
nokia_n900_legacy_init in pdata-quirks.c).

Yes so it seems, and the bootloaders should really set it. It's
also disabled for multiplatform builds.

These are now done in u-boot as of: v2015.04-rc4

http://git.denx.de/?p=u-boot.git;a=commit;h=c6f90e1418a84fe5fa463b38403bd1845cb6a59c

Regards,

[Sebastian Reichel]

Post by Robert Nelson

Post by Tony Lindgren

And yes, for armhf userland one gets random oopses at least on the
Nokia N900. AFAIK this is not true for all ARMv7 processors
(especially non omaps), though.

http://www.spinics.net/lists/linux-omap/msg108511.html
See also 5c86c5339c56 ("ARM: omap2plus_defconfig: Enable ARM erratum
430973 for omap3").

For me the random oopses occur without this config flag and are
fixed by it. The workaround is not very suitable for multi platform
kernels, though, since its enabled also for unaffected platforms.
As far as I can see the CONFIG_ARM_ERRATA_430973 flag is checked
in proc-v7.S and in proc-v7-2level.S. I think the first file is
irrelevant, since it can be fixed later (see workaround in
nokia_n900_legacy_init in pdata-quirks.c).

Yes so it seems, and the bootloaders should really set it. It's
also disabled for multiplatform builds.

These are now done in u-boot as of: v2015.04-rc4
http://git.denx.de/?p=u-boot.git;a=commit;h=c6f90e1418a84fe5fa463b38403bd1845cb6a59c

Seems like "include/configs/nokia_rx51.h" has been forgotten. Note,
that the kernel must still be able to enable the bit itself (in case
NOLO is used without u-boot).

-- Sebastian

[Pali Rohár]

Post by Sebastian Reichel

Post by Robert Nelson

Post by Tony Lindgren

And yes, for armhf userland one gets random oopses at
least on the Nokia N900. AFAIK this is not true for all
ARMv7 processors (especially non omaps), though.

http://www.spinics.net/lists/linux-omap/msg108511.html
Enable ARM erratum 430973 for omap3").

For me the random oopses occur without this config flag
and are fixed by it. The workaround is not very suitable
for multi platform kernels, though, since its enabled
also for unaffected platforms.
As far as I can see the CONFIG_ARM_ERRATA_430973 flag is
checked in proc-v7.S and in proc-v7-2level.S. I think
the first file is irrelevant, since it can be fixed
later (see workaround in nokia_n900_legacy_init in
pdata-quirks.c).

Yes so it seems, and the bootloaders should really set it.
It's also disabled for multiplatform builds.

These are now done in u-boot as of: v2015.04-rc4
http://git.denx.de/?p=u-boot.git;a=commit;h=c6f90e1418a84fe5
fa463b38403bd1845cb6a59c

Seems like "include/configs/nokia_rx51.h" has been forgotten.
Note, that the kernel must still be able to enable the bit
itself (in case NOLO is used without u-boot).
-- Sebastian

You must not expect that bootloader set something. Nokia N900 has
closed and signed bootloader which cannot be easily replaced and
or fixed.

For N900 there is U-Boot but it does not support lot of things
(load kernel via usb or serial) so for developers it is better to
use original Nokia bootloader.

[Sebastian Reichel]

Hi,

Post by Pali Rohár

Post by Sebastian Reichel

Post by Robert Nelson

Post by Tony Lindgren
Yes so it seems, and the bootloaders should really set it.
It's also disabled for multiplatform builds.

These are now done in u-boot as of: v2015.04-rc4
http://git.denx.de/?p=u-boot.git;a=commit;h=c6f90e1418a84fe5
fa463b38403bd1845cb6a59c

Seems like "include/configs/nokia_rx51.h" has been forgotten.
Note, that the kernel must still be able to enable the bit
itself (in case NOLO is used without u-boot).

You must not expect that bootloader set something. Nokia N900 has
closed and signed bootloader which cannot be easily replaced and
or fixed.
For N900 there is U-Boot but it does not support lot of things
(load kernel via usb or serial) so for developers it is better to
use original Nokia bootloader.

Yes, I'm aware of that. I think having the IBE bit set in u-boot has
advantages nevertheless. I think the kernel can't enable the bit
sufficiently early if its compiled in thumb-mode itself
(CONFIG_THUMB2_KERNEL).

So I suggest to enable the IBE bit in the kernel (that part is
already done) *and* in u-boot.

-- Sebastian

[Pali Rohár]

Post by Sebastian Reichel
Hi,

Post by Pali Rohár

Post by Sebastian Reichel

Post by Robert Nelson

Post by Tony Lindgren
Yes so it seems, and the bootloaders should really set
it. It's also disabled for multiplatform builds.

These are now done in u-boot as of: v2015.04-rc4
http://git.denx.de/?p=u-boot.git;a=commit;h=c6f90e1418a8
4fe5 fa463b38403bd1845cb6a59c

Seems like "include/configs/nokia_rx51.h" has been
forgotten. Note, that the kernel must still be able to
enable the bit itself (in case NOLO is used without
u-boot).

You must not expect that bootloader set something. Nokia
N900 has closed and signed bootloader which cannot be
easily replaced and or fixed.
For N900 there is U-Boot but it does not support lot of
things (load kernel via usb or serial) so for developers it
is better to use original Nokia bootloader.

Yes, I'm aware of that. I think having the IBE bit set in
u-boot has advantages nevertheless. I think the kernel can't
enable the bit sufficiently early if its compiled in
thumb-mode itself (CONFIG_THUMB2_KERNEL).
So I suggest to enable the IBE bit in the kernel (that part is
already done) *and* in u-boot.
-- Sebastian

Yes. Code for both (U-Boot and linux kernel) exists and are
already included.

In linux kernel code is since 3.14-rc6 and in U-Boot code is
there since beginning of Nokia N900 support (2013.04) directly in
board code.

I believe that you guys do not remove or break that code :-)

Nokia N900 needs slightly different code for setting IBE bit as
other omap3 boards. For info is in commit message of commit
4748a7240284b0f68bd47a66365c2cd561939830.

[Sebastian Reichel]

Hi,

Post by Pali Rohár

Post by Sebastian Reichel
So I suggest to enable the IBE bit in the kernel (that part is
already done) *and* in u-boot.
-- Sebastian

Yes. Code for both (U-Boot and linux kernel) exists and are
already included.
In linux kernel code is since 3.14-rc6 and in U-Boot code is
there since beginning of Nokia N900 support (2013.04) directly in
board code.

ah. I wasn't aware, that u-boot already sets the IBE bit on the N900.
Thanks for the information.

-- Sebastian

[Pavel Machek]

Hi!

Maybe an option would be to provide two cpu_v7_switch_mm
implementations (one with the errata and one without). Then
the system can start with the simple implementation. Once
the boot as progressed far enough to know, that the hardware
is affected by the errata, it could switch to the implementation
with the flushing.

Actually... we should start with the slow&safe option, and then switch
to faster implementation if workaround is not needed.
Pavel

[Ivaylo Dimitrov]

Hi

Maybe an option would be to provide two cpu_v7_switch_mm
implementations (one with the errata and one without). Then
the system can start with the simple implementation. Once
the boot as progressed far enough to know, that the hardware
is affected by the errata, it could switch to the implementation
with the flushing.

Or rather the opposite, as if the kernel itself is thumb-compiled, it
most probably will have no chance to progress enough to switch to the
correct implementation before hanging.

-- Sebastian

Ivo

[Tony Lindgren]

Hi

Maybe an option would be to provide two cpu_v7_switch_mm
implementations (one with the errata and one without). Then
the system can start with the simple implementation. Once
the boot as progressed far enough to know, that the hardware
is affected by the errata, it could switch to the implementation
with the flushing.

Or rather the opposite, as if the kernel itself is thumb-compiled, it most
probably will have no chance to progress enough to switch to the correct
implementation before hanging.

We should first verify the same bug happens with armel also.
I just verified the CPU load in the background makes armhf
apps segfault without $subject workaround enabled.

If the segfaulting does not happen with armel, then chances
it's some kind of neon related issue and the fix can be more
targeted.

Regards,

Tony

[Ivaylo Dimitrov]

Hi,

Post by Tony Lindgren
We should first verify the same bug happens with armel also.
I just verified the CPU load in the background makes armhf
apps segfault without $subject workaround enabled.
If the segfaulting does not happen with armel, then chances
it's some kind of neon related issue and the fix can be more
targeted.
Regards,
Tony

I can assure you that at least SoC in N900 is affected by ARM errata
430973, no matter armel or armhf. Though the crash is usually with
SIGILL(because of the wrong T bit in CPSR the code is executed with), I
don't remember seeing SIGSEGV, but that could be my ageing memory. I am
the maintainer of the so-called cssu-thumb Fremantle upgrade [1], which
is armel, thumb2 compiled, so I have some experience with the matter :)
. Without that errata workaround enabled in kernel, it is impossible to
boot Fremantle with cssu-thumb on top. BTW it was the same back in the
days when there was Ubuntu 12.04 for N900 - it was hardfp, thumb2
compiled. Again, without that errata workaround enabled, there was no
way to boot it.

Regards,
Ivo

[1] http://talk.maemo.org/showthread.php?t=84829

[Tony Lindgren]

Hi,

Post by Tony Lindgren
We should first verify the same bug happens with armel also.
I just verified the CPU load in the background makes armhf
apps segfault without $subject workaround enabled.
If the segfaulting does not happen with armel, then chances
it's some kind of neon related issue and the fix can be more
targeted.
Regards,
Tony

I can assure you that at least SoC in N900 is affected by ARM errata 430973,
no matter armel or armhf. Though the crash is usually with SIGILL(because of
the wrong T bit in CPSR the code is executed with), I don't remember seeing
SIGSEGV, but that could be my ageing memory. I am the maintainer of the
so-called cssu-thumb Fremantle upgrade [1], which is armel, thumb2 compiled,
so I have some experience with the matter :) . Without that errata
workaround enabled in kernel, it is impossible to boot Fremantle with
cssu-thumb on top. BTW it was the same back in the days when there was
Ubuntu 12.04 for N900 - it was hardfp, thumb2 compiled. Again, without that
errata workaround enabled, there was no way to boot it.

Right, it affects n900 for sure. My point is that it also seems to
affect 37xx versions not listed to suffer from this issue.

Regards,

Tony

[1] http://talk.maemo.org/showthread.php?t=84829

[Matthijs van Duin]

Post by Tony Lindgren
Right, it affects n900 for sure. My point is that it also seems to
affect 37xx versions not listed to suffer from this issue.

They shouldn't... erratum 430973 only affected Cortex-A8 r1, and the
dm37xx should have an r3p2 right?

A word of caution though: at least on the DM814x and AM335x, secure
ROM sets bit 6 (IBE) in the Auxiliary Control Register, thereby
enabling BTB invalidate instructions (which normally execute as nops).
This is presumably a leftover of the erratum 430973 workaround, but it
means there is a risk of running afoul of erratum 687067 if BTB
invalidate by MVA instructions are actually used.

I would actually suggest clearing IBE if it set on Cortex-A8 r2 or
later processors and a secure monitor call is available to do so
(there is on the DM814x and AM335x, dunno about the 37xx), also for
performance reasons: BTB invalidates are quite expensive instructions
(when enabled).

Matthijs

[Ivaylo Dimitrov]

Post by Matthijs van Duin
I would actually suggest clearing IBE if it set on Cortex-A8 r2 or
later processors and a secure monitor call is available to do so
(there is on the DM814x and AM335x, dunno about the 37xx), also for
performance reasons: BTB invalidates are quite expensive instructions
(when enabled).

There is (or should be) SM call, it is explained in 36xx TRM(SWPU177AA)
as well, 26.4.1, "Booting Overview". Though I wonder why SMC is needed
to write ACR on non-HS devices. A simple MRC should suffice, unless I
miss something.

Clearing the IBE bit on devices that don't need erratum 430973
workaround, along with enabling that workaround in kernel is the best
approach IMO. That way we will gain both stability on cores that need
the workaround (like on N900 and the other devices with p1) and won't
lose performance on cores that are not affected.

Post by Matthijs van Duin
Matthijs

Regards,
Ivo

[Matthijs van Duin]

Though I wonder why SMC is needed to write ACR on non-HS devices. A simple
MRC should suffice, unless I miss something.

Public-world access to ACR varies per bit:
bit 1 (L2EN) is documented as banked, but at least on r3p2 turns out
to be common r/w.
bits 30-31 are secure read-only and public RAZ.
remaining bits are secure read/write and public read-only.

The net effect is that doing an MRC from public world will only modify
the L2EN bit.

There's no bit in the non-secure access control register to affect all
of this, so GP vs HS doesn't matter here (from a CPU point of view; it
may matter for the availability of SM calls obviously).

Matthijs

[Matthijs van Duin]

MRC

I mean MCR of course

[Ivaylo Dimitrov]

Post by Matthijs van Duin

Though I wonder why SMC is needed to write ACR on non-HS devices. A simple
MRC should suffice, unless I miss something.

bit 1 (L2EN) is documented as banked, but at least on r3p2 turns out
to be common r/w.
bits 30-31 are secure read-only and public RAZ.
remaining bits are secure read/write and public read-only.
The net effect is that doing an MRC from public world will only modify
the L2EN bit.
There's no bit in the non-secure access control register to affect all
of this, so GP vs HS doesn't matter here (from a CPU point of view; it
may matter for the availability of SM calls obviously).
Matthijs

But then the first part(setting the IBE bit in ACR to 1) of the errata
workaround is wrong, as it uses a plain MCR to set the IBE bit, see
http://lxr.free-electrons.com/source/arch/arm/mm/proc-v7.S#L340. Which
is weird, given that the workaround was posted by ARM iirc.

Ivo

[Matthijs van Duin]

By default, the BTB Invalidate instruction is treated as a NOP on Cortex-A8.
However, it is possible to enable the BTB Invalidate instruction such that it
actually does a full invalidate of the BTB by setting the IBE bit (bit 6) in
the CP15 Auxiliary Control Register. As a consequence of erratum 687067, the
L1 System Array Debug Register should be cleared to 0 before the IBE bit is
MOV r1, #0
MCR p15, 0, r1, c15, c1, 0 ; write instruction data 0 register
MRC p15, 0, R1, c1, c0, 1 ; read Aux Ctl Register
ORR R1, R1 #(1 << 6) ; set IBE to 1
MCR p15, 0, R1, c1, c0, 1 ; write Aux Ctl Register
The above code needs to be executed in Secure state. ARM Limited recommends
that this code is added to the boot monitor.

The 430973 workaround code in proc-v7.S will do absolutely nothing if
executed in non-secure state. Ditto for the 458693 workaround, and the
460075 workaround should trigger an undefined instruction exception.
Maybe linux is started in secure mode on some targets and this code
was written for one of those?

I scanned DM814x secure ROM for any (ARM or Thumb) write to
Instruction L1 System Array Debug Register 0, but I found none, hence
my warning to watch out for erratum 687067.

Adding the full set of BTB invalidates while making sure IBE is
disabled on sufficiently recent Cortex-A8 revisions would be optimal
for the Cortex-A8. But, apparently (based on the description of the
ARMv7 CPUID registers) there are also processors which only require
BTB invalidates when code is modified, but not when context-switching,
so there may be performance considerations there...

Matthijs

[Tony Lindgren]

Post by Matthijs van Duin

By default, the BTB Invalidate instruction is treated as a NOP on Cortex-A8.
However, it is possible to enable the BTB Invalidate instruction such that it
actually does a full invalidate of the BTB by setting the IBE bit (bit 6) in
the CP15 Auxiliary Control Register. As a consequence of erratum 687067, the
L1 System Array Debug Register should be cleared to 0 before the IBE bit is
MOV r1, #0
MCR p15, 0, r1, c15, c1, 0 ; write instruction data 0 register
MRC p15, 0, R1, c1, c0, 1 ; read Aux Ctl Register
ORR R1, R1 #(1 << 6) ; set IBE to 1
MCR p15, 0, R1, c1, c0, 1 ; write Aux Ctl Register
The above code needs to be executed in Secure state. ARM Limited recommends
that this code is added to the boot monitor.

The 430973 workaround code in proc-v7.S will do absolutely nothing if
executed in non-secure state. Ditto for the 458693 workaround, and the
460075 workaround should trigger an undefined instruction exception.
Maybe linux is started in secure mode on some targets and this code
was written for one of those?

That's only for HS omaps, for those we currently only do it in the
nokia_n900_legacy_init that calls rx51_secure_update_aux_cr.

Post by Matthijs van Duin
I scanned DM814x secure ROM for any (ARM or Thumb) write to
Instruction L1 System Array Debug Register 0, but I found none, hence
my warning to watch out for erratum 687067.

OK

Post by Matthijs van Duin
Adding the full set of BTB invalidates while making sure IBE is
disabled on sufficiently recent Cortex-A8 revisions would be optimal
for the Cortex-A8. But, apparently (based on the description of the
ARMv7 CPUID registers) there are also processors which only require
BTB invalidates when code is modified, but not when context-switching,
so there may be performance considerations there...

Attempting to summarize all that's been discussed.. It sounds like we
need the following implemented:

1. For cortex-a8 revisions affected by 458693, we can do a custom
cpu_v7_switch_mm function that always does flush BTAC/BTB.

2. For HS cortex-a8 processors other than n900 affected by 458693,
we need to implement functions similar to rx51_secure_update_aux_cr,
the bootrom on n900 is different from TI HS omaps so the SMC call
numbering may be different.

3. For later cortex-a8 processors not affected by 458693, we need
to clear IBE bit to avoid erratum 687067.

Regards,

Tony

[Tony Lindgren]

Post by Tony Lindgren

Post by Matthijs van Duin

By default, the BTB Invalidate instruction is treated as a NOP on Cortex-A8.
However, it is possible to enable the BTB Invalidate instruction such that it
actually does a full invalidate of the BTB by setting the IBE bit (bit 6) in
the CP15 Auxiliary Control Register. As a consequence of erratum 687067, the
L1 System Array Debug Register should be cleared to 0 before the IBE bit is
MOV r1, #0
MCR p15, 0, r1, c15, c1, 0 ; write instruction data 0 register
MRC p15, 0, R1, c1, c0, 1 ; read Aux Ctl Register
ORR R1, R1 #(1 << 6) ; set IBE to 1
MCR p15, 0, R1, c1, c0, 1 ; write Aux Ctl Register
The above code needs to be executed in Secure state. ARM Limited recommends
that this code is added to the boot monitor.

The 430973 workaround code in proc-v7.S will do absolutely nothing if
executed in non-secure state. Ditto for the 458693 workaround, and the
460075 workaround should trigger an undefined instruction exception.
Maybe linux is started in secure mode on some targets and this code
was written for one of those?

That's only for HS omaps, for those we currently only do it in the
nokia_n900_legacy_init that calls rx51_secure_update_aux_cr.

Post by Matthijs van Duin
I scanned DM814x secure ROM for any (ARM or Thumb) write to
Instruction L1 System Array Debug Register 0, but I found none, hence
my warning to watch out for erratum 687067.

OK

Post by Matthijs van Duin
Adding the full set of BTB invalidates while making sure IBE is
disabled on sufficiently recent Cortex-A8 revisions would be optimal
for the Cortex-A8. But, apparently (based on the description of the
ARMv7 CPUID registers) there are also processors which only require
BTB invalidates when code is modified, but not when context-switching,
so there may be performance considerations there...

Attempting to summarize all that's been discussed.. It sounds like we
1. For cortex-a8 revisions affected by 458693, we can do a custom
cpu_v7_switch_mm function that always does flush BTAC/BTB.
2. For HS cortex-a8 processors other than n900 affected by 458693,
we need to implement functions similar to rx51_secure_update_aux_cr,
the bootrom on n900 is different from TI HS omaps so the SMC call
numbering may be different.
3. For later cortex-a8 processors not affected by 458693, we need
to clear IBE bit to avoid erratum 687067.

Oops sorry, wrong numbers for errata above.. s/458693/430973/, here's
a better version:

1. For cortex-a8 revisions affected by 430973, we can do a custom
cpu_v7_switch_mm function that always does flush BTAC/BTB.

2. For HS cortex-a8 processors other than n900 affected by 430973,
we need to implement functions similar to rx51_secure_update_aux_cr,
the bootrom on n900 is different from TI HS omaps so the SMC call
numbering may be different.

3. For later cortex-a8 processors not affected by 430973, we need
to clear IBE bit to avoid erratum 687067.

Regards,

Tony

[Ivaylo Dimitrov]

Post by Tony Lindgren

Post by Tony Lindgren

Post by Matthijs van Duin

By default, the BTB Invalidate instruction is treated as a NOP on Cortex-A8.
However, it is possible to enable the BTB Invalidate instruction such that it
actually does a full invalidate of the BTB by setting the IBE bit (bit 6) in
the CP15 Auxiliary Control Register. As a consequence of erratum 687067, the
L1 System Array Debug Register should be cleared to 0 before the IBE bit is
MOV r1, #0
MCR p15, 0, r1, c15, c1, 0 ; write instruction data 0 register
MRC p15, 0, R1, c1, c0, 1 ; read Aux Ctl Register
ORR R1, R1 #(1 << 6) ; set IBE to 1
MCR p15, 0, R1, c1, c0, 1 ; write Aux Ctl Register
The above code needs to be executed in Secure state. ARM Limited recommends
that this code is added to the boot monitor.

The 430973 workaround code in proc-v7.S will do absolutely nothing if
executed in non-secure state. Ditto for the 458693 workaround, and the
460075 workaround should trigger an undefined instruction exception.
Maybe linux is started in secure mode on some targets and this code
was written for one of those?

That's only for HS omaps, for those we currently only do it in the
nokia_n900_legacy_init that calls rx51_secure_update_aux_cr.

Post by Matthijs van Duin
I scanned DM814x secure ROM for any (ARM or Thumb) write to
Instruction L1 System Array Debug Register 0, but I found none, hence
my warning to watch out for erratum 687067.

OK

Post by Matthijs van Duin
Adding the full set of BTB invalidates while making sure IBE is
disabled on sufficiently recent Cortex-A8 revisions would be optimal
for the Cortex-A8. But, apparently (based on the description of the
ARMv7 CPUID registers) there are also processors which only require
BTB invalidates when code is modified, but not when context-switching,
so there may be performance considerations there...

Attempting to summarize all that's been discussed.. It sounds like we
1. For cortex-a8 revisions affected by 458693, we can do a custom
cpu_v7_switch_mm function that always does flush BTAC/BTB.
2. For HS cortex-a8 processors other than n900 affected by 458693,
we need to implement functions similar to rx51_secure_update_aux_cr,
the bootrom on n900 is different from TI HS omaps so the SMC call
numbering may be different.
3. For later cortex-a8 processors not affected by 458693, we need
to clear IBE bit to avoid erratum 687067.

Oops sorry, wrong numbers for errata above.. s/458693/430973/, here's
1. For cortex-a8 revisions affected by 430973, we can do a custom
cpu_v7_switch_mm function that always does flush BTAC/BTB.

Why custom function, if IBE bit is zero, BTB invalidate instruction is a
NOP. Do you think that "mcr p15, 0, r2, c7, c5, 6" executed as a NOP
will put so much overhead, that it deserves a custom function?

Post by Tony Lindgren
2. For HS cortex-a8 processors other than n900 affected by 430973,
we need to implement functions similar to rx51_secure_update_aux_cr,
the bootrom on n900 is different from TI HS omaps so the SMC call
numbering may be different.
3. For later cortex-a8 processors not affected by 430973, we need
to clear IBE bit to avoid erratum 687067.

Maybe it should be implemented something like:

1. if Cortex-A8, always execute invalidate BTB instruction in
cpu_v7_switch_mm

2. For Cortex-A8 revisions affected by 430973, set IBE bit to 1, set it
to 0 for all others. That should happen as soon as possible,
otherwise kernel may crash on affected revisions if thumb-
compiled.

Post by Tony Lindgren
Regards,
Tony

Regards,

Ivo

[Tony Lindgren]

Post by Ivaylo Dimitrov

Post by Tony Lindgren
Oops sorry, wrong numbers for errata above.. s/458693/430973/, here's
1. For cortex-a8 revisions affected by 430973, we can do a custom
cpu_v7_switch_mm function that always does flush BTAC/BTB.

Why custom function, if IBE bit is zero, BTB invalidate instruction is a
NOP. Do you think that "mcr p15, 0, r2, c7, c5, 6" executed as a NOP will
put so much overhead, that it deserves a custom function?

Hmm but it still seems to do something also on cortex-a8 r3p2 that
is supposedly not affected by 430973.. Based on my tests so far, at least
armhf running cpuburn-a8 in the background and doing apt-get update
segfaults constantly without flush BTAC/BTB. This seems to be the case
no matter how the aux ctrl reg bits are set.. This should be reproducable
on any pandboard xm BTW.

Post by Ivaylo Dimitrov

Post by Tony Lindgren
2. For HS cortex-a8 processors other than n900 affected by 430973,
we need to implement functions similar to rx51_secure_update_aux_cr,
the bootrom on n900 is different from TI HS omaps so the SMC call
numbering may be different.
3. For later cortex-a8 processors not affected by 430973, we need
to clear IBE bit to avoid erratum 687067.

1. if Cortex-A8, always execute invalidate BTB instruction in
cpu_v7_switch_mm

This part still seems to need more investigating for why it's still
needed also r3p2 as I describe above. Otherwise we may be hiding some
other bug.

Post by Ivaylo Dimitrov
2. For Cortex-A8 revisions affected by 430973, set IBE bit to 1, set it
to 0 for all others. That should happen as soon as possible,
otherwise kernel may crash on affected revisions if thumb-
compiled.

Yes this makes sense.

Regards,

Tony

[Matthijs van Duin]

Post by Ivaylo Dimitrov
Why custom function, if IBE bit is zero, BTB invalidate instruction is a
NOP. Do you think that "mcr p15, 0, r2, c7, c5, 6" executed as a NOP will
put so much overhead, that it deserves a custom function?

Executing them as nop is Cortex-A8 specific. On other ARMv7 CPUs, BTB
invalidation on context switch may be unnecessary yet expensive.

In general I think you'll want a version with and one without BTB
flushing, and select depending on CPUID (ID_MMFR1, bits 28-31), with
overrides for known processor issues (i.e. the cortex-a8 has a wrong
value there in all cases: it reports value 2, while it should be
treated as 1 or 4 depending on cpu revision).

Post by Ivaylo Dimitrov
Hmm but it still seems to do something also on cortex-a8 r3p2 that
is supposedly not affected by 430973.. Based on my tests so far, at least
armhf running cpuburn-a8 in the background and doing apt-get update
segfaults constantly without flush BTAC/BTB. This seems to be the case
no matter how the aux ctrl reg bits are set..

That sounds.... really odd. The TRM is fairly explicit about BTB
flush executing as nop when IBE is not set. Of course the TRM is not
exactly flawless, but still...

Post by Ivaylo Dimitrov

Post by Ivaylo Dimitrov
2. For Cortex-A8 revisions affected by 430973, set IBE bit to 1, set it
to 0 for all others.

Note btw that erratum 687067 affects *all* Cortex-A8 revisions, so
there's a bit of a catch-22 there. The proper workaround for it
(zeroing some particular debug register) can only be done in secure
privileged mode, and there's no straightforward way to check whether
this has been done.

However, it only affects BTB invalidate by MVA, afaik BTB invalidate
all is safe.

Post by Ivaylo Dimitrov

Post by Ivaylo Dimitrov
That should happen as soon as possible,
otherwise kernel may crash on affected revisions if thumb-compiled.

The right place to do this to be honest would be in the bootloader,
but I guess that's not always convenient to arrange...

Matthijs

[Tony Lindgren]

Post by Matthijs van Duin

Post by Ivaylo Dimitrov
Why custom function, if IBE bit is zero, BTB invalidate instruction is a
NOP. Do you think that "mcr p15, 0, r2, c7, c5, 6" executed as a NOP will
put so much overhead, that it deserves a custom function?

Executing them as nop is Cortex-A8 specific. On other ARMv7 CPUs, BTB
invalidation on context switch may be unnecessary yet expensive.
In general I think you'll want a version with and one without BTB
flushing, and select depending on CPUID (ID_MMFR1, bits 28-31), with
overrides for known processor issues (i.e. the cortex-a8 has a wrong
value there in all cases: it reports value 2, while it should be
treated as 1 or 4 depending on cpu revision).

Post by Ivaylo Dimitrov
Hmm but it still seems to do something also on cortex-a8 r3p2 that
is supposedly not affected by 430973.. Based on my tests so far, at least
armhf running cpuburn-a8 in the background and doing apt-get update
segfaults constantly without flush BTAC/BTB. This seems to be the case
no matter how the aux ctrl reg bits are set..

That sounds.... really odd. The TRM is fairly explicit about BTB
flush executing as nop when IBE is not set. Of course the TRM is not
exactly flawless, but still...

Oops, sorry user error.. I was trying to clear IBE as a banked register
like L2 enable bit and of course it did not get cleared.. Clearing it
with a smc call really clears it. And in that case my test case seems to
work reliably on r3p2 without erratum 430973 enabled.

Anyways, if the bootloader enables IBE bit, then the kernel must also
always enable the erratum 430973 flush BTAC/BTB parts on all cortex-a8.

Or else the kernel must clear the IBE bit based on the version, which
may be trickier with all the SoC and vendor specific smc calls.

Post by Matthijs van Duin

Post by Ivaylo Dimitrov

Post by Ivaylo Dimitrov
2. For Cortex-A8 revisions affected by 430973, set IBE bit to 1, set it
to 0 for all others.

Note btw that erratum 687067 affects *all* Cortex-A8 revisions, so
there's a bit of a catch-22 there. The proper workaround for it
(zeroing some particular debug register) can only be done in secure
privileged mode, and there's no straightforward way to check whether
this has been done.
However, it only affects BTB invalidate by MVA, afaik BTB invalidate
all is safe.

I'm now thinking the kernel should just always set the 430973 specific
cpu_v7_switch_mm for all cortex-a8 if IBE bit is set. That avoids
the whole mess of early SoC detection and smc calls. And if IBE bit
is not set, then we jsut use the rgular cpu_v7_switch_mm.

This will work as long as we can read the aux ctrl register IBE
bit using mrc, which I believe is the case for all cortex-a8 based
omap variants.

Post by Matthijs van Duin

Post by Ivaylo Dimitrov

Post by Ivaylo Dimitrov
That should happen as soon as possible,
otherwise kernel may crash on affected revisions if thumb-compiled.

The right place to do this to be honest would be in the bootloader,
but I guess that's not always convenient to arrange...

Yeah I agree.

Regards,

Tony

[Sebastian Reichel]

Hi,

Post by Tony Lindgren
I'm now thinking the kernel should just always set the 430973 specific
cpu_v7_switch_mm for all cortex-a8 if IBE bit is set. That avoids
the whole mess of early SoC detection and smc calls. And if IBE bit
is not set, then we just use the regular cpu_v7_switch_mm.
This will work as long as we can read the aux ctrl register IBE
bit using mrc, which I believe is the case for all cortex-a8 based
omap variants.

If I understood it correctly we can simply call the BTB flush on
cortex-a8 if IBE bit is not set, since it would be translated as
nop.

So it should be safe to include the call on all cortex-a8 based
cpus. We may need a non-BTB-flushing function for non-cortex-a8
based cpus, though.

-- Sebastian

[Matthijs van Duin]

Post by Tony Lindgren
Oops, sorry user error.. I was trying to clear IBE as a banked register
like L2 enable bit and of course it did not get cleared.. Clearing it
with a smc call really clears it. And in that case my test case seems to
work reliably on r3p2 without erratum 430973 enabled.

So if I understand correctly, you actually had crashes which only
occurred with IBE enabled and the 430973 workaround disabled? That's
quite interesting, since it seems to me that can only be the result of
erratum 687067, which means
1. secrom indeed fails to implement the 687067 workaround.
2. "BTB invalidate by MVA" is used somewhere in the kernel.
The 430973 workaround would likely conceal this problem due to
regularly flushing the whole BTB, but I'm not sure how wise it is to
rely on that...

Post by Tony Lindgren
I'm now thinking the kernel should just always set the 430973 specific
cpu_v7_switch_mm for all cortex-a8 if IBE bit is set.

And simply take the performance hit if secrom bogusly sets it and the
bootloader doesn't fix it?

Sounds reasonable enough to me, given how platform-specific the
appropriate auxcr config is, as well as the means by which it can be
changed.

There's more secrom misconfiguration that the bootloader should fix
anyhow to make optimal use of the processor...

Post by Tony Lindgren
This will work as long as we can read the aux ctrl register IBE
bit using mrc, which I believe is the case for all cortex-a8 based
omap variants.

Aux control is always readable, only write is an issue.

Post by Tony Lindgren
If I understood it correctly we can simply call the BTB flush on
cortex-a8 if IBE bit is not set, since it would be translated as
nop.

Indeed you can safely use the BTB-flushing context switch on any cortex-a8.

There's still value in checking if IBE is set on r2p1 or later, and if
so emit a warning about suboptimal performance.

Post by Tony Lindgren
So it should be safe to include the call on all cortex-a8 based
cpus. We may need a non-BTB-flushing function for non-cortex-a8
based cpus, though.

I just looked it up, apparently BTB-flushing on context switch is not
needed architecturally in ARMv7 (though it was in ARMv6), so that
version should probably indeed only be used for the cortex-a8.

Matthijs

[Tony Lindgren]

Post by Matthijs van Duin

Post by Tony Lindgren
Oops, sorry user error.. I was trying to clear IBE as a banked register
like L2 enable bit and of course it did not get cleared.. Clearing it
with a smc call really clears it. And in that case my test case seems to
work reliably on r3p2 without erratum 430973 enabled.

So if I understand correctly, you actually had crashes which only
occurred with IBE enabled and the 430973 workaround disabled?

That's right. It seems to happen at least with r3p2 that has 430973
fixed.

Post by Matthijs van Duin
That's quite interesting, since it seems to me that can only be the
result of erratum 687067, which means
1. secrom indeed fails to implement the 687067 workaround.
2. "BTB invalidate by MVA" is used somewhere in the kernel.
The 430973 workaround would likely conceal this problem due to
regularly flushing the whole BTB, but I'm not sure how wise it is to
rely on that...

Yes it seems to be hidden behind 430973. Anyways, we can print some
warnings in the kernel for incorrect revision and IBE handling.

Post by Matthijs van Duin

Post by Tony Lindgren
I'm now thinking the kernel should just always set the 430973 specific
cpu_v7_switch_mm for all cortex-a8 if IBE bit is set.

And simply take the performance hit if secrom bogusly sets it and the
bootloader doesn't fix it?

Yes it seems Russell's patch should do that for cortex-a8.

Post by Matthijs van Duin
Sounds reasonable enough to me, given how platform-specific the
appropriate auxcr config is, as well as the means by which it can be
changed.

Right, we have quite a few combinations already for omap3.. 34xx/36xx,
HS/GP, TI vs Nokia bootrom.. Just proves how useless all these
"security" "features" are in the long run :) They will just keep
biting people over and over in the long run even if not used.

Post by Matthijs van Duin
There's more secrom misconfiguration that the bootloader should fix
anyhow to make optimal use of the processor...

Yeah.

Post by Matthijs van Duin

Post by Tony Lindgren
This will work as long as we can read the aux ctrl register IBE
bit using mrc, which I believe is the case for all cortex-a8 based
omap variants.

Aux control is always readable, only write is an issue.

OK, hopefully that's the case for 36xx HS version too.. I recall some
registers reading as zero on N9 but hopefully not for the aux control
register.

Post by Matthijs van Duin

Post by Tony Lindgren
If I understood it correctly we can simply call the BTB flush on
cortex-a8 if IBE bit is not set, since it would be translated as
nop.

Indeed you can safely use the BTB-flushing context switch on any cortex-a8.
There's still value in checking if IBE is set on r2p1 or later, and if
so emit a warning about suboptimal performance.

Post by Tony Lindgren
So it should be safe to include the call on all cortex-a8 based
cpus. We may need a non-BTB-flushing function for non-cortex-a8
based cpus, though.

I just looked it up, apparently BTB-flushing on context switch is not
needed architecturally in ARMv7 (though it was in ARMv6), so that
version should probably indeed only be used for the cortex-a8.

OK

Regards,

Tony

[Grazvydas Ignotas]

Post by Tony Lindgren

Post by Matthijs van Duin

Post by Tony Lindgren
Hmm but it still seems to do something also on cortex-a8 r3p2 that
is supposedly not affected by 430973.. Based on my tests so far, at least
armhf running cpuburn-a8 in the background and doing apt-get update
segfaults constantly without flush BTAC/BTB. This seems to be the case
no matter how the aux ctrl reg bits are set..

That sounds.... really odd. The TRM is fairly explicit about BTB
flush executing as nop when IBE is not set. Of course the TRM is not
exactly flawless, but still...

Oops, sorry user error.. I was trying to clear IBE as a banked register
like L2 enable bit and of course it did not get cleared.. Clearing it
with a smc call really clears it. And in that case my test case seems to
work reliably on r3p2 without erratum 430973 enabled.

May I ask how do you perform the smc call? I wanted to clear IBE too
to experiment, but it just hangs my board, even if I just write back
the same value. Here is what I do:

asm ("mrc p15, 0, %0, c1, c0, 1" : "=r"(val));

asm (".arch_extension sec\n\t"
"mov r0, %0\n"
"mov r12, #3\n"
"smc #0\n"
:: "r"(val) : "r0", "r12");

I just run this from a sysfs write handler, does it need to be run on
SRAM or something?

Gražvydas

[Tony Lindgren]

Post by Grazvydas Ignotas

Post by Tony Lindgren

Post by Matthijs van Duin

Post by Tony Lindgren
Hmm but it still seems to do something also on cortex-a8 r3p2 that
is supposedly not affected by 430973.. Based on my tests so far, at least
armhf running cpuburn-a8 in the background and doing apt-get update
segfaults constantly without flush BTAC/BTB. This seems to be the case
no matter how the aux ctrl reg bits are set..

That sounds.... really odd. The TRM is fairly explicit about BTB
flush executing as nop when IBE is not set. Of course the TRM is not
exactly flawless, but still...

Oops, sorry user error.. I was trying to clear IBE as a banked register
like L2 enable bit and of course it did not get cleared.. Clearing it
with a smc call really clears it. And in that case my test case seems to
work reliably on r3p2 without erratum 430973 enabled.

May I ask how do you perform the smc call? I wanted to clear IBE too
to experiment, but it just hangs my board, even if I just write back
asm ("mrc p15, 0, %0, c1, c0, 1" : "=r"(val));
asm (".arch_extension sec\n\t"
"mov r0, %0\n"
"mov r12, #3\n"
"smc #0\n"
:: "r"(val) : "r0", "r12");
I just run this from a sysfs write handler, does it need to be run on
SRAM or something?

Best done in the bootloader.. I just hacked it into the restore from
off-idle to test, see below. But for that you naturally need to have
a device with working idle and it's usable for just testing for lazy
people.

Regards,

Tony

--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -516,6 +516,7 @@ l2_inv_gp:
ldr r4, scratchpad_base
ldr r3, [r4,#0xBC]
ldr r0, [r3,#4]
+ bic r0, r1, #(1 << 6)
mov r12, #0x3
smc #0 @ Call SMI monitor (smieq)
ldr r4, scratchpad_base

[Nishanth Menon]

Post by Tony Lindgren

Post by Grazvydas Ignotas

Post by Tony Lindgren

Post by Matthijs van Duin

Post by Tony Lindgren
Hmm but it still seems to do something also on cortex-a8 r3p2 that
is supposedly not affected by 430973.. Based on my tests so far, at least
armhf running cpuburn-a8 in the background and doing apt-get update
segfaults constantly without flush BTAC/BTB. This seems to be the case
no matter how the aux ctrl reg bits are set..

That sounds.... really odd. The TRM is fairly explicit about BTB
flush executing as nop when IBE is not set. Of course the TRM is not
exactly flawless, but still...

Oops, sorry user error.. I was trying to clear IBE as a banked register
like L2 enable bit and of course it did not get cleared.. Clearing it
with a smc call really clears it. And in that case my test case seems to
work reliably on r3p2 without erratum 430973 enabled.

May I ask how do you perform the smc call? I wanted to clear IBE too
to experiment, but it just hangs my board, even if I just write back
asm ("mrc p15, 0, %0, c1, c0, 1" : "=r"(val));
asm (".arch_extension sec\n\t"
"mov r0, %0\n"
"mov r12, #3\n"
"smc #0\n"
:: "r"(val) : "r0", "r12");
I just run this from a sysfs write handler, does it need to be run on
SRAM or something?

Best done in the bootloader.. I just hacked it into the restore from

I recently did that in u-boot:

http://git.denx.de/?p=u-boot.git;a=commitdiff;h=5902f4ce0f2bd1411e40dc0ece3598a0fc19b2ae

http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/omap3/board.c;h=b064c0cc834356b33e14e0f36774108fa6a6c580;hb=HEAD#l428

I believe it should be scalable to other SoCs as well (weak function
that may be overriden as needed). Hope that helps.

Post by Tony Lindgren
off-idle to test, see below. But for that you naturally need to have
a device with working idle and it's usable for just testing for lazy
people.
Regards,
Tony
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
ldr r4, scratchpad_base
ldr r3, [r4,#0xBC]
ldr r0, [r3,#4]
+ bic r0, r1, #(1 << 6)
mov r12, #0x3
ldr r4, scratchpad_base
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Grazvydas Ignotas]

Post by Tony Lindgren

Post by Grazvydas Ignotas

Post by Tony Lindgren

Post by Matthijs van Duin

Post by Tony Lindgren
Hmm but it still seems to do something also on cortex-a8 r3p2 that
is supposedly not affected by 430973.. Based on my tests so far, at least
armhf running cpuburn-a8 in the background and doing apt-get update
segfaults constantly without flush BTAC/BTB. This seems to be the case
no matter how the aux ctrl reg bits are set..

That sounds.... really odd. The TRM is fairly explicit about BTB
flush executing as nop when IBE is not set. Of course the TRM is not
exactly flawless, but still...

Oops, sorry user error.. I was trying to clear IBE as a banked register
like L2 enable bit and of course it did not get cleared.. Clearing it
with a smc call really clears it. And in that case my test case seems to
work reliably on r3p2 without erratum 430973 enabled.

May I ask how do you perform the smc call? I wanted to clear IBE too
to experiment, but it just hangs my board, even if I just write back
asm ("mrc p15, 0, %0, c1, c0, 1" : "=r"(val));
asm (".arch_extension sec\n\t"
"mov r0, %0\n"
"mov r12, #3\n"
"smc #0\n"
:: "r"(val) : "r0", "r12");
I just run this from a sysfs write handler, does it need to be run on
SRAM or something?

Best done in the bootloader.. I just hacked it into the restore from
off-idle to test, see below. But for that you naturally need to have
a device with working idle and it's usable for just testing for lazy
people.
Regards,
Tony
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
ldr r4, scratchpad_base
ldr r3, [r4,#0xBC]
ldr r0, [r3,#4]
+ bic r0, r1, #(1 << 6)

Hmm did you mean r0 instead of r1 here? I hope your test results
didn't come from some other random bit from r1 being written to
aux_ctrl.
And according to readback this doesn't seem to work for me, even when
my board has idle working. Or is it not supposed to be visible in
readback?

Anyway I've managed to clear that damn bit in the bootloader, but
failed to measure any performance impact from clearing this bit and
getting rid of BTB flush mcr in cpu_v7_switch_mm() (this is on DM3730
with r3p2 A8). My test was to simply run 2 processes that would spin a
counter (running more processes doesn't seem to increase context
switches per second, so running 2 seemed enough).


Gražvydas

[Tony Lindgren]

Post by Grazvydas Ignotas

Post by Tony Lindgren

Post by Grazvydas Ignotas

Post by Tony Lindgren

Post by Matthijs van Duin

Post by Tony Lindgren
Hmm but it still seems to do something also on cortex-a8 r3p2 that
is supposedly not affected by 430973.. Based on my tests so far, at least
armhf running cpuburn-a8 in the background and doing apt-get update
segfaults constantly without flush BTAC/BTB. This seems to be the case
no matter how the aux ctrl reg bits are set..

That sounds.... really odd. The TRM is fairly explicit about BTB
flush executing as nop when IBE is not set. Of course the TRM is not
exactly flawless, but still...

Oops, sorry user error.. I was trying to clear IBE as a banked register
like L2 enable bit and of course it did not get cleared.. Clearing it
with a smc call really clears it. And in that case my test case seems to
work reliably on r3p2 without erratum 430973 enabled.

May I ask how do you perform the smc call? I wanted to clear IBE too
to experiment, but it just hangs my board, even if I just write back
asm ("mrc p15, 0, %0, c1, c0, 1" : "=r"(val));
asm (".arch_extension sec\n\t"
"mov r0, %0\n"
"mov r12, #3\n"
"smc #0\n"
:: "r"(val) : "r0", "r12");
I just run this from a sysfs write handler, does it need to be run on
SRAM or something?

Best done in the bootloader.. I just hacked it into the restore from
off-idle to test, see below. But for that you naturally need to have
a device with working idle and it's usable for just testing for lazy
people.
Regards,
Tony
--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
ldr r4, scratchpad_base
ldr r3, [r4,#0xBC]
ldr r0, [r3,#4]
+ bic r0, r1, #(1 << 6)

Hmm did you mean r0 instead of r1 here? I hope your test results
didn't come from some other random bit from r1 being written to
aux_ctrl.

Oh right sorry, yeah it should be r0 above. Luck based coding :)

Post by Grazvydas Ignotas
And according to readback this doesn't seem to work for me, even when
my board has idle working. Or is it not supposed to be visible in
readback?

Hmm I've verified between apps segfaulting depending on how bit 6
is set on r3p2.

Anyways, did a retry just in case, below is an updated test patch.
For me aux ctrl changes after enabling idle stuff:

aux ctrl: 0x000000e2
...
aux ctrl: 0x000000a2

Did you enable the UART timeout etc so it really hits off mode
when testing?

Post by Grazvydas Ignotas
Anyway I've managed to clear that damn bit in the bootloader, but
failed to measure any performance impact from clearing this bit and
getting rid of BTB flush mcr in cpu_v7_switch_mm() (this is on DM3730
with r3p2 A8). My test was to simply run 2 processes that would spin a
counter (running more processes doesn't seem to increase context
switches per second, so running 2 seemed enough).

Well that's a good test result :) It means it's OK to keep the
430973 enabled without a performance impatct.

Regards,

Tony

8< -----------------
--- a/arch/arm/mach-omap2/pm-debug.c
+++ b/arch/arm/mach-omap2/pm-debug.c
@@ -93,6 +93,7 @@ static int pwrdm_dbg_show_counter(struct powerdomain *pwrdm, void *user)
{
struct seq_file *s = (struct seq_file *)user;
int i;
+ u32 val;

if (strcmp(pwrdm->name, "emu_pwrdm") == 0 ||
strcmp(pwrdm->name, "wkup_pwrdm") == 0 ||
@@ -116,6 +117,9 @@ static int pwrdm_dbg_show_counter(struct powerdomain *pwrdm, void *user)

seq_printf(s, "\n");

+ asm ("mrc p15, 0, %0, c1, c0, 1" : "=r"(val));
+ seq_printf(s, "aux ctrl: 0x%08x\n", val);
+
return 0;
}

--- a/arch/arm/mach-omap2/sleep34xx.S
+++ b/arch/arm/mach-omap2/sleep34xx.S
@@ -516,6 +516,7 @@ l2_inv_gp:
ldr r4, scratchpad_base
ldr r3, [r4,#0xBC]
ldr r0, [r3,#4]
+ bic r0, r0, #(1 << 6)
mov r12, #0x3
smc #0 @ Call SMI monitor (smieq)
ldr r4, scratchpad_base

[Matthijs van Duin]

Post by Grazvydas Ignotas
May I ask how do you perform the smc call?

A point worth mentioning is that TI advises that r1-r4 may be
clobbered in general, and at least on GP dm814x and am335x devices r4
is in fact clobbered, even though it is normally a callee-save
register.

Also, on aforementioned devices, the secure-world MMU is enabled, with
translation table 0 being 1KB @ 0x402f2000, used for secure VA
0x00000000 - 0x0fffffff (the rest goes via translation table 1 in
secure ROM). It is generally not consulted in practice since secrom
locks two dTLB and two iTLB entries. The GP secure monitor doesn't
actually perform any data access whatsoever, so the two dTLB can and
should be unlocked considering that TLB entries are a rather scarce
resource (32 per side) and a dTLB miss incurs a 24-cycle minimum
penalty (compare with 8-cycle minimum penalty for L1 cache miss).

Unlocking the second iTLB entry is also safe, but if both iTLB entries
are unlocked, you need to preserve or repopulate the relevant
translation entry (VA 0x00300000 -> PA 0x00000000) to be able to
perform an SMC. Everything that happens in secure monitor mode is
heavily double-checked by the SSM, so while you're free to choose a
cache policy, any creativity beyond that will piss off the SSM and
cause it to hit the "MPU security violation" global reset.

Once appropriate fixes to the auxiliary control register have been
applied, generally no further SMCs are needed hence all TLB entries
can be unlocked. Since it would be reasonable for a bootloader to do
this, I recommend checking whether a fix is needed and not
unconditionally performing an SMC.

[Russell King - ARM Linux]

Post by Matthijs van Duin

Post by Ivaylo Dimitrov
Why custom function, if IBE bit is zero, BTB invalidate instruction is a
NOP. Do you think that "mcr p15, 0, r2, c7, c5, 6" executed as a NOP will
put so much overhead, that it deserves a custom function?

Executing them as nop is Cortex-A8 specific. On other ARMv7 CPUs, BTB
invalidation on context switch may be unnecessary yet expensive.

That can be solved (see the patch I just posted) so I wouldn't worry
too much about that issue.

[Russell King - ARM Linux]

Post by Tony Lindgren

Post by Ivaylo Dimitrov

Post by Tony Lindgren
Oops sorry, wrong numbers for errata above.. s/458693/430973/, here's
1. For cortex-a8 revisions affected by 430973, we can do a custom
cpu_v7_switch_mm function that always does flush BTAC/BTB.

Why custom function, if IBE bit is zero, BTB invalidate instruction is a
NOP. Do you think that "mcr p15, 0, r2, c7, c5, 6" executed as a NOP will
put so much overhead, that it deserves a custom function?

Hmm but it still seems to do something also on cortex-a8 r3p2 that
is supposedly not affected by 430973.. Based on my tests so far, at least
armhf running cpuburn-a8 in the background and doing apt-get update
segfaults constantly without flush BTAC/BTB. This seems to be the case
no matter how the aux ctrl reg bits are set.. This should be reproducable
on any pandboard xm BTW.

Post by Ivaylo Dimitrov

Post by Tony Lindgren
2. For HS cortex-a8 processors other than n900 affected by 430973,
we need to implement functions similar to rx51_secure_update_aux_cr,
the bootrom on n900 is different from TI HS omaps so the SMC call
numbering may be different.
3. For later cortex-a8 processors not affected by 430973, we need
to clear IBE bit to avoid erratum 687067.

1. if Cortex-A8, always execute invalidate BTB instruction in
cpu_v7_switch_mm

This part still seems to need more investigating for why it's still
needed also r3p2 as I describe above. Otherwise we may be hiding some
other bug.

Post by Ivaylo Dimitrov
2. For Cortex-A8 revisions affected by 430973, set IBE bit to 1, set it
to 0 for all others. That should happen as soon as possible,
otherwise kernel may crash on affected revisions if thumb-
compiled.

Yes this makes sense.

Well, one thing we can do is to tweak the proc-v7*.S such that we detect
Cortex-A8 separately, and only execute the BTB flush for CA8 processors
if the errata is enabled. Something like this (untested):

diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
index bc86be205c04..fa385140715f 100644
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -37,15 +37,18 @@
* It is assumed that:
* - we are not using split page tables
*/
-ENTRY(cpu_v7_switch_mm)
+ENTRY(cpu_ca8_switch_mm)
#ifdef CONFIG_MMU
mov r2, #0
- mmid r1, r1 @ get mm->context.id
- ALT_SMP(orr r0, r0, #TTB_FLAGS_SMP)
- ALT_UP(orr r0, r0, #TTB_FLAGS_UP)
#ifdef CONFIG_ARM_ERRATA_430973
mcr p15, 0, r2, c7, c5, 6 @ flush BTAC/BTB
#endif
+#endif
+ENTRY(cpu_v7_switch_mm)
+#ifdef CONFIG_MMU
+ mmid r1, r1 @ get mm->context.id
+ ALT_SMP(orr r0, r0, #TTB_FLAGS_SMP)
+ ALT_UP(orr r0, r0, #TTB_FLAGS_UP)
#ifdef CONFIG_PID_IN_CONTEXTIDR
mrc p15, 0, r2, c13, c0, 1 @ read current context ID
lsr r2, r2, #8 @ extract the PID
@@ -61,6 +64,7 @@ ENTRY(cpu_v7_switch_mm)
#endif
bx lr
ENDPROC(cpu_v7_switch_mm)
+ENDPROC(cpu_ca8_switch_mm)

/*
* cpu_v7_set_pte_ext(ptep, pte)
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index b1d19ea5e1af..6bec3cfbea39 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -153,6 +153,21 @@ ENDPROC(cpu_v7_do_resume)
#endif

/*
+ * Cortex-A8
+ */
+ globl_equ cpu_ca8_proc_init, cpu_v7_proc_init
+ globl_equ cpu_ca8_proc_fin, cpu_v7_proc_fin
+ globl_equ cpu_ca8_reset, cpu_v7_reset
+ globl_equ cpu_ca8_do_idle, cpu_v7_do_idle
+ globl_equ cpu_ca8_dcache_clean_area, cpu_v7_dcache_clean_area
+ globl_equ cpu_ca8_set_pte_ext, cpu_v7_set_pte_ext
+ globl_equ cpu_ca8_suspend_size, cpu_v7_suspend_size
+#ifdef CONFIG_ARM_CPU_SUSPEND
+ globl_equ cpu_ca8_do_suspend, cpu_v7_do_suspend
+ globl_equ cpu_ca8_do_resume, cpu_v7_do_resume
+#endif
+
+/*
* Cortex-A9 processor functions
*/
globl_equ cpu_ca9mp_proc_init, cpu_v7_proc_init
@@ -471,7 +486,10 @@ __v7_setup_stack:

@ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+#ifndef CONFIG_ARM_LPAE
+ define_processor_functions ca8, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+#endif
#ifdef CONFIG_CPU_PJ4B
define_processor_functions pj4b, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
#endif
@@ -527,6 +545,16 @@ __v7_ca9mp_proc_info:
__v7_proc __v7_ca9mp_proc_info, __v7_ca9mp_setup, proc_fns = ca9mp_processor_functions
.size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info

+ /*
+ * ARM Ltd. Cortex A8 processor.
+ */
+ .type __v7_ca8_proc_info, #object
+__v7_ca8_proc_info:
+ .long 0x410fc080
+ .long 0xff0ffff0
+ __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ .size __v7_ca8_proc_info, . - __v7_ca8_proc_info
+
#endif /* CONFIG_ARM_LPAE */

/*

[Tony Lindgren]

Post by Russell King - ARM Linux

Post by Tony Lindgren

Post by Ivaylo Dimitrov

Post by Tony Lindgren
Oops sorry, wrong numbers for errata above.. s/458693/430973/, here's
1. For cortex-a8 revisions affected by 430973, we can do a custom
cpu_v7_switch_mm function that always does flush BTAC/BTB.

Why custom function, if IBE bit is zero, BTB invalidate instruction is a
NOP. Do you think that "mcr p15, 0, r2, c7, c5, 6" executed as a NOP will
put so much overhead, that it deserves a custom function?

Hmm but it still seems to do something also on cortex-a8 r3p2 that
is supposedly not affected by 430973.. Based on my tests so far, at least
armhf running cpuburn-a8 in the background and doing apt-get update
segfaults constantly without flush BTAC/BTB. This seems to be the case
no matter how the aux ctrl reg bits are set.. This should be reproducable
on any pandboard xm BTW.

Post by Ivaylo Dimitrov

Post by Tony Lindgren
2. For HS cortex-a8 processors other than n900 affected by 430973,
we need to implement functions similar to rx51_secure_update_aux_cr,
the bootrom on n900 is different from TI HS omaps so the SMC call
numbering may be different.
3. For later cortex-a8 processors not affected by 430973, we need
to clear IBE bit to avoid erratum 687067.

1. if Cortex-A8, always execute invalidate BTB instruction in
cpu_v7_switch_mm

This part still seems to need more investigating for why it's still
needed also r3p2 as I describe above. Otherwise we may be hiding some
other bug.

Post by Ivaylo Dimitrov
2. For Cortex-A8 revisions affected by 430973, set IBE bit to 1, set it
to 0 for all others. That should happen as soon as possible,
otherwise kernel may crash on affected revisions if thumb-
compiled.

Yes this makes sense.

Well, one thing we can do is to tweak the proc-v7*.S such that we detect
Cortex-A8 separately, and only execute the BTB flush for CA8 processors
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
__v7_proc __v7_ca9mp_proc_info, __v7_ca9mp_setup, proc_fns = ca9mp_processor_functions
.size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info
+ /*
+ * ARM Ltd. Cortex A8 processor.
+ */
+ .type __v7_ca8_proc_info, #object
+ .long 0x410fc080
+ .long 0xff0ffff0
+ __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ .size __v7_ca8_proc_info, . - __v7_ca8_proc_info
+
#endif /* CONFIG_ARM_LPAE */
/*

Works for me. The above needs the following fix folded in to build:

--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -532,7 +532,7 @@ __v7_ca9mp_proc_info:
__v7_ca8_proc_info:
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info

#endif /* CONFIG_ARM_LPAE */

Regards,

Tony

[Tony Lindgren]

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux
Well, one thing we can do is to tweak the proc-v7*.S such that we detect
Cortex-A8 separately, and only execute the BTB flush for CA8 processors
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
__v7_proc __v7_ca9mp_proc_info, __v7_ca9mp_setup, proc_fns = ca9mp_processor_functions
.size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info
+ /*
+ * ARM Ltd. Cortex A8 processor.
+ */
+ .type __v7_ca8_proc_info, #object
+ .long 0x410fc080
+ .long 0xff0ffff0
+ __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ .size __v7_ca8_proc_info, . - __v7_ca8_proc_info
+
#endif /* CONFIG_ARM_LPAE */
/*

--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

And then on top of that patch, we can fix the sefaulting issues with the
following, what do you guys think?

Regards,

Tony

8< --------------------
From: Tony Lindgren <***@atomide.com>
Date: Tue, 7 Apr 2015 07:52:39 -0700
Subject: [PATCH] ARM: mm: Fix Cortex-A8 erratum 430973 segfaults for bootloaders and multiarch

Looks like apps can be made to segfault easily on armhf distros
just by running cpuburn-a8 in the background, then starting apt
get update unless erratum 430973 workaround is enabled. This happens
on r3p2 also, which has 430973 fixed in hardware.

Turns out the reason for this is some bootloaders incorrectly
setting the auxilary register IBE bit, which probably causes us
to hit erratum 687067 on Cortex-A8 later than r1p2.

Now that Cortex-A8 has it's own cpu_ca8_switch_mm, we can fix these
issues:

1. If the bootloader incorrectly sets the IBE bit in the auxilary
control register for Cortex-A8 revisions with 430973 fixed
in hardware, we need to call flush BTAC/BTB to avoid segfaults
probably caused by erratum 687067. So let's flush BTAC/BTB
unconditionally for Cortex-A8. It won't do anything unless the
IBE bit is set.

2. We can and should now keep 430973 enabled for multiarch builds
as it no longer causes issues with Cortex-A9 as we have a separate
cpu_ca8_switch_mm.

Note that SoCs probably should also add checks and print warnings
for the misconfigured IBE bit depending on the Cortex-A8 revision
so the bootloaders can be fixed Cortex-A8 revisions later than
r1p2 to not set the IBE bit.

Signed-off-by: Tony Lindgren <***@atomide.com>

--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -36,14 +36,16 @@
*
* It is assumed that:
* - we are not using split page tables
+ *
+ * Note that we always need to flush BTAC/BTB if IBE is set
+ * even on Cortex-A8 revisions not affected by 430973.
+ * If IBE is not set, the flush BTAC/BTB won't do anything.
*/
ENTRY(cpu_ca8_switch_mm)
#ifdef CONFIG_MMU
mov r2, #0
-#ifdef CONFIG_ARM_ERRATA_430973
mcr p15, 0, r2, c7, c5, 6 @ flush BTAC/BTB
#endif
-#endif
ENTRY(cpu_v7_switch_mm)
#ifdef CONFIG_MMU
mmid r1, r1 @ get mm->context.id
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -352,7 +352,7 @@ __v7_setup:
ldr r10, =0x00000c08 @ Cortex-A8 primary part number
teq r0, r10
bne 2f
-#if defined(CONFIG_ARM_ERRATA_430973) && !defined(CONFIG_ARCH_MULTIPLATFORM)
+#if defined(CONFIG_ARM_ERRATA_430973)

teq r5, #0x00100000 @ only present in r1p*
mrceq p15, 0, r10, c1, c0, 1 @ read aux control register

[Russell King - ARM Linux]

Post by Tony Lindgren
And then on top of that patch, we can fix the sefaulting issues with the
following, what do you guys think?

Has this change been tested on OMAP secure parts?

Post by Tony Lindgren
8< --------------------
Date: Tue, 7 Apr 2015 07:52:39 -0700
Subject: [PATCH] ARM: mm: Fix Cortex-A8 erratum 430973 segfaults for bootloaders and multiarch
Looks like apps can be made to segfault easily on armhf distros
just by running cpuburn-a8 in the background, then starting apt
get update unless erratum 430973 workaround is enabled. This happens
on r3p2 also, which has 430973 fixed in hardware.
Turns out the reason for this is some bootloaders incorrectly
setting the auxilary register IBE bit, which probably causes us
to hit erratum 687067 on Cortex-A8 later than r1p2.
Now that Cortex-A8 has it's own cpu_ca8_switch_mm, we can fix these
1. If the bootloader incorrectly sets the IBE bit in the auxilary
control register for Cortex-A8 revisions with 430973 fixed
in hardware, we need to call flush BTAC/BTB to avoid segfaults
probably caused by erratum 687067. So let's flush BTAC/BTB
unconditionally for Cortex-A8. It won't do anything unless the
IBE bit is set.
2. We can and should now keep 430973 enabled for multiarch builds
as it no longer causes issues with Cortex-A9 as we have a separate
cpu_ca8_switch_mm.
Note that SoCs probably should also add checks and print warnings
for the misconfigured IBE bit depending on the Cortex-A8 revision
so the bootloaders can be fixed Cortex-A8 revisions later than
r1p2 to not set the IBE bit.
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -36,14 +36,16 @@
*
* - we are not using split page tables
+ *
+ * Note that we always need to flush BTAC/BTB if IBE is set
+ * even on Cortex-A8 revisions not affected by 430973.
+ * If IBE is not set, the flush BTAC/BTB won't do anything.
*/
ENTRY(cpu_ca8_switch_mm)
#ifdef CONFIG_MMU
mov r2, #0
-#ifdef CONFIG_ARM_ERRATA_430973
#endif
-#endif
ENTRY(cpu_v7_switch_mm)
#ifdef CONFIG_MMU
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
teq r0, r10
bne 2f
-#if defined(CONFIG_ARM_ERRATA_430973) && !defined(CONFIG_ARCH_MULTIPLATFORM)
+#if defined(CONFIG_ARM_ERRATA_430973)

[Tony Lindgren]

Post by Russell King - ARM Linux

Post by Tony Lindgren
And then on top of that patch, we can fix the sefaulting issues with the
following, what do you guys think?

Has this change been tested on OMAP secure parts?

Boots just fine for me on n900, but let's wait for comments
from Sebastian.

Regards,

Tony

Post by Russell King - ARM Linux

Post by Tony Lindgren
8< --------------------
Date: Tue, 7 Apr 2015 07:52:39 -0700
Subject: [PATCH] ARM: mm: Fix Cortex-A8 erratum 430973 segfaults for bootloaders and multiarch
Looks like apps can be made to segfault easily on armhf distros
just by running cpuburn-a8 in the background, then starting apt
get update unless erratum 430973 workaround is enabled. This happens
on r3p2 also, which has 430973 fixed in hardware.
Turns out the reason for this is some bootloaders incorrectly
setting the auxilary register IBE bit, which probably causes us
to hit erratum 687067 on Cortex-A8 later than r1p2.
Now that Cortex-A8 has it's own cpu_ca8_switch_mm, we can fix these
1. If the bootloader incorrectly sets the IBE bit in the auxilary
control register for Cortex-A8 revisions with 430973 fixed
in hardware, we need to call flush BTAC/BTB to avoid segfaults
probably caused by erratum 687067. So let's flush BTAC/BTB
unconditionally for Cortex-A8. It won't do anything unless the
IBE bit is set.
2. We can and should now keep 430973 enabled for multiarch builds
as it no longer causes issues with Cortex-A9 as we have a separate
cpu_ca8_switch_mm.
Note that SoCs probably should also add checks and print warnings
for the misconfigured IBE bit depending on the Cortex-A8 revision
so the bootloaders can be fixed Cortex-A8 revisions later than
r1p2 to not set the IBE bit.
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -36,14 +36,16 @@
*
* - we are not using split page tables
+ *
+ * Note that we always need to flush BTAC/BTB if IBE is set
+ * even on Cortex-A8 revisions not affected by 430973.
+ * If IBE is not set, the flush BTAC/BTB won't do anything.
*/
ENTRY(cpu_ca8_switch_mm)
#ifdef CONFIG_MMU
mov r2, #0
-#ifdef CONFIG_ARM_ERRATA_430973
#endif
-#endif
ENTRY(cpu_v7_switch_mm)
#ifdef CONFIG_MMU
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
teq r0, r10
bne 2f
-#if defined(CONFIG_ARM_ERRATA_430973) && !defined(CONFIG_ARCH_MULTIPLATFORM)
+#if defined(CONFIG_ARM_ERRATA_430973)

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Russell King - ARM Linux]

Post by Russell King - ARM Linux
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

Thanks, merged into the original patch.

[Russell King - ARM Linux]

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

Thanks, merged into the original patch.

Do you want to give me an ack for this, thanks?

8<===
From: Russell King <rmk+***@arm.linux.org.uk>
Subject: [PATCH] ARM: proc-v7: avoid errata 430973 workaround for non-Cortex
A8 CPUs

Avoid the errata 430973 workaround for non-Cortex A8 CPUs. Having this
workaround enabled introduces an additional branch target buffer flush
into the context switching path, something we wish to avoid. To allow
this errata to be enabled in multiplatform kernels while reducing its
impact, rearrange the Cortex-A8 CPU support to avoid impacting on other
Version 7 CPUs.

Signed-off-by: Russell King <rmk+***@arm.linux.org.uk>
---
arch/arm/mm/proc-v7-2level.S | 12 ++++++++----
arch/arm/mm/proc-v7.S | 28 ++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
index bc86be205c04..fa385140715f 100644
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -37,15 +37,18 @@
* It is assumed that:
* - we are not using split page tables
*/
-ENTRY(cpu_v7_switch_mm)
+ENTRY(cpu_ca8_switch_mm)
#ifdef CONFIG_MMU
mov r2, #0
- mmid r1, r1 @ get mm->context.id
- ALT_SMP(orr r0, r0, #TTB_FLAGS_SMP)
- ALT_UP(orr r0, r0, #TTB_FLAGS_UP)
#ifdef CONFIG_ARM_ERRATA_430973
mcr p15, 0, r2, c7, c5, 6 @ flush BTAC/BTB
#endif
+#endif
+ENTRY(cpu_v7_switch_mm)
+#ifdef CONFIG_MMU
+ mmid r1, r1 @ get mm->context.id
+ ALT_SMP(orr r0, r0, #TTB_FLAGS_SMP)
+ ALT_UP(orr r0, r0, #TTB_FLAGS_UP)
#ifdef CONFIG_PID_IN_CONTEXTIDR
mrc p15, 0, r2, c13, c0, 1 @ read current context ID
lsr r2, r2, #8 @ extract the PID
@@ -61,6 +64,7 @@ ENTRY(cpu_v7_switch_mm)
#endif
bx lr
ENDPROC(cpu_v7_switch_mm)
+ENDPROC(cpu_ca8_switch_mm)

/*
* cpu_v7_set_pte_ext(ptep, pte)
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index b1d19ea5e1af..003190ae9cd8 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -153,6 +153,21 @@ ENDPROC(cpu_v7_do_resume)
#endif

/*
+ * Cortex-A8
+ */
+ globl_equ cpu_ca8_proc_init, cpu_v7_proc_init
+ globl_equ cpu_ca8_proc_fin, cpu_v7_proc_fin
+ globl_equ cpu_ca8_reset, cpu_v7_reset
+ globl_equ cpu_ca8_do_idle, cpu_v7_do_idle
+ globl_equ cpu_ca8_dcache_clean_area, cpu_v7_dcache_clean_area
+ globl_equ cpu_ca8_set_pte_ext, cpu_v7_set_pte_ext
+ globl_equ cpu_ca8_suspend_size, cpu_v7_suspend_size
+#ifdef CONFIG_ARM_CPU_SUSPEND
+ globl_equ cpu_ca8_do_suspend, cpu_v7_do_suspend
+ globl_equ cpu_ca8_do_resume, cpu_v7_do_resume
+#endif
+
+/*
* Cortex-A9 processor functions
*/
globl_equ cpu_ca9mp_proc_init, cpu_v7_proc_init
@@ -471,7 +486,10 @@ __v7_setup_stack:

@ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+#ifndef CONFIG_ARM_LPAE
+ define_processor_functions ca8, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+#endif
#ifdef CONFIG_CPU_PJ4B
define_processor_functions pj4b, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
#endif
@@ -527,6 +545,16 @@ __v7_ca9mp_proc_info:
__v7_proc __v7_ca9mp_proc_info, __v7_ca9mp_setup, proc_fns = ca9mp_processor_functions
.size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info

+ /*
+ * ARM Ltd. Cortex A8 processor.
+ */
+ .type __v7_ca8_proc_info, #object
+__v7_ca8_proc_info:
+ .long 0x410fc080
+ .long 0xff0ffff0
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
+ .size __v7_ca8_proc_info, . - __v7_ca8_proc_info
+
#endif /* CONFIG_ARM_LPAE */

/*

[Tony Lindgren]

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

Thanks, merged into the original patch.

Do you want to give me an ack for this, thanks?

The patch below works for me:

Tested-by: Tony Lindgren <***@atomide.com>

I'm wondering if this and the follow-up patch should be tagged
cc: stable?

They together fix apps segfaulting both with and without 430973
for some common use cases for distro kernels.

Regards,

Tony

Post by Russell King - ARM Linux
8<===
Subject: [PATCH] ARM: proc-v7: avoid errata 430973 workaround for non-Cortex
A8 CPUs
Avoid the errata 430973 workaround for non-Cortex A8 CPUs. Having this
workaround enabled introduces an additional branch target buffer flush
into the context switching path, something we wish to avoid. To allow
this errata to be enabled in multiplatform kernels while reducing its
impact, rearrange the Cortex-A8 CPU support to avoid impacting on other
Version 7 CPUs.
---
arch/arm/mm/proc-v7-2level.S | 12 ++++++++----
arch/arm/mm/proc-v7.S | 28 ++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mm/proc-v7-2level.S b/arch/arm/mm/proc-v7-2level.S
index bc86be205c04..fa385140715f 100644
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -37,15 +37,18 @@
* - we are not using split page tables
*/
-ENTRY(cpu_v7_switch_mm)
+ENTRY(cpu_ca8_switch_mm)
#ifdef CONFIG_MMU
mov r2, #0
- ALT_SMP(orr r0, r0, #TTB_FLAGS_SMP)
- ALT_UP(orr r0, r0, #TTB_FLAGS_UP)
#ifdef CONFIG_ARM_ERRATA_430973
#endif
+#endif
+ENTRY(cpu_v7_switch_mm)
+#ifdef CONFIG_MMU
+ ALT_SMP(orr r0, r0, #TTB_FLAGS_SMP)
+ ALT_UP(orr r0, r0, #TTB_FLAGS_UP)
#ifdef CONFIG_PID_IN_CONTEXTIDR
@@ -61,6 +64,7 @@ ENTRY(cpu_v7_switch_mm)
#endif
bx lr
ENDPROC(cpu_v7_switch_mm)
+ENDPROC(cpu_ca8_switch_mm)
/*
* cpu_v7_set_pte_ext(ptep, pte)
diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index b1d19ea5e1af..003190ae9cd8 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -153,6 +153,21 @@ ENDPROC(cpu_v7_do_resume)
#endif
/*
+ * Cortex-A8
+ */
+ globl_equ cpu_ca8_proc_init, cpu_v7_proc_init
+ globl_equ cpu_ca8_proc_fin, cpu_v7_proc_fin
+ globl_equ cpu_ca8_reset, cpu_v7_reset
+ globl_equ cpu_ca8_do_idle, cpu_v7_do_idle
+ globl_equ cpu_ca8_dcache_clean_area, cpu_v7_dcache_clean_area
+ globl_equ cpu_ca8_set_pte_ext, cpu_v7_set_pte_ext
+ globl_equ cpu_ca8_suspend_size, cpu_v7_suspend_size
+#ifdef CONFIG_ARM_CPU_SUSPEND
+ globl_equ cpu_ca8_do_suspend, cpu_v7_do_suspend
+ globl_equ cpu_ca8_do_resume, cpu_v7_do_resume
+#endif
+
+/*
* Cortex-A9 processor functions
*/
globl_equ cpu_ca9mp_proc_init, cpu_v7_proc_init
@ define struct processor (see <asm/proc-fns.h> and proc-macros.S)
define_processor_functions v7, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+#ifndef CONFIG_ARM_LPAE
+ define_processor_functions ca8, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
define_processor_functions ca9mp, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
+#endif
#ifdef CONFIG_CPU_PJ4B
define_processor_functions pj4b, dabort=v7_early_abort, pabort=v7_pabort, suspend=1
#endif
__v7_proc __v7_ca9mp_proc_info, __v7_ca9mp_setup, proc_fns = ca9mp_processor_functions
.size __v7_ca9mp_proc_info, . - __v7_ca9mp_proc_info
+ /*
+ * ARM Ltd. Cortex A8 processor.
+ */
+ .type __v7_ca8_proc_info, #object
+ .long 0x410fc080
+ .long 0xff0ffff0
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
+ .size __v7_ca8_proc_info, . - __v7_ca8_proc_info
+
#endif /* CONFIG_ARM_LPAE */
/*
--
1.8.3.1
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[Russell King - ARM Linux]

Post by Tony Lindgren

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

Thanks, merged into the original patch.

Do you want to give me an ack for this, thanks?

I'm wondering if this and the follow-up patch should be tagged
cc: stable?
They together fix apps segfaulting both with and without 430973
for some common use cases for distro kernels.

If we do, I'll want to merge both patches together... For the time being,
I'll queue it without the stable tag.

[Sebastian Reichel]

Hi,

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

Thanks, merged into the original patch.

Do you want to give me an ack for this, thanks?

I tried to test this together with Tony's follow up patch, but I get
this after applying the patch to v4.0:

***@earth ~/src/linux [430973-fix] % make -j4
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
make[1]: 'include/generated/mach-types.h' is up to date.
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
AS arch/arm/mm/proc-v7.o
arch/arm/mm/proc-v7.S: Assembler messages:
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
Makefile:947: recipe for target 'arch/arm/mm' failed
make: *** [arch/arm/mm] Error 2
make: *** Waiting for unfinished jobs....

-- Sebastian

[Tony Lindgren]

Post by Sebastian Reichel
Hi,

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

Thanks, merged into the original patch.

Do you want to give me an ack for this, thanks?

I tried to test this together with Tony's follow up patch, but I get
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
make[1]: 'include/generated/mach-types.h' is up to date.
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
AS arch/arm/mm/proc-v7.o
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
Makefile:947: recipe for target 'arch/arm/mm' failed
make: *** [arch/arm/mm] Error 2
make: *** Waiting for unfinished jobs....

Maybe test the version in Linux next:

a6d746789825 ("ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs")

And then apply my patch on top.

Regards,

Tony

[Sebastian Reichel]

Post by Tony Lindgren

Post by Sebastian Reichel
Hi,

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

Thanks, merged into the original patch.

Do you want to give me an ack for this, thanks?

I tried to test this together with Tony's follow up patch, but I get
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
make[1]: 'include/generated/mach-types.h' is up to date.
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
AS arch/arm/mm/proc-v7.o
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
Makefile:947: recipe for target 'arch/arm/mm' failed
make: *** [arch/arm/mm] Error 2
make: *** Waiting for unfinished jobs....

a6d746789825 ("ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs")

DONE with your your patch added on top:

Tested-By: Sebastian Reichel <***@kernel.org>

(on N900)

I guess we should also drop the CONFIG_ARM_ERRATA_430973 check from
pdata-quirks' nokia_n900_legacy_init() and just enable it unconditionally.

-- Sebastian

[Tony Lindgren]

Post by Sebastian Reichel

Post by Tony Lindgren

Post by Sebastian Reichel
Hi,

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

Thanks, merged into the original patch.

Do you want to give me an ack for this, thanks?

I tried to test this together with Tony's follow up patch, but I get
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
make[1]: 'include/generated/mach-types.h' is up to date.
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
AS arch/arm/mm/proc-v7.o
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
Makefile:947: recipe for target 'arch/arm/mm' failed
make: *** [arch/arm/mm] Error 2
make: *** Waiting for unfinished jobs....

a6d746789825 ("ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs")

(on N900)

OK thanks, patch now uploaded to Russell's patch system:

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8345/1

Post by Sebastian Reichel
I guess we should also drop the CONFIG_ARM_ERRATA_430973 check from
pdata-quirks' nokia_n900_legacy_init() and just enable it unconditionally.

Yeah makes sense to me.

Regards,

Tony

[Russell King - ARM Linux]

Post by Tony Lindgren

Post by Sebastian Reichel

Post by Tony Lindgren

Post by Sebastian Reichel
Hi,

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

Thanks, merged into the original patch.

Do you want to give me an ack for this, thanks?

I tried to test this together with Tony's follow up patch, but I get
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
make[1]: 'include/generated/mach-types.h' is up to date.
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
AS arch/arm/mm/proc-v7.o
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
Makefile:947: recipe for target 'arch/arm/mm' failed
make: *** [arch/arm/mm] Error 2
make: *** Waiting for unfinished jobs....

a6d746789825 ("ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs")

(on N900)

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8345/1

I have a concern with that patch.

The reason that it's disabled for multiplatform is because we can't
guarantee that the auxctrl register will be writable. The solution
we came up with for multiplatform was to require firmware to be
updated to enable this bit.

Enabling it on a platform where firmware has not been updated, but
runs in the non-secure world will lead to the kernel hanging in the
early assembly code.

I've discussed it with Catalin, and Catalin's position is that we
should not remove the !multiplatform conditional. That's something
which I find that I'm agreeing with Catalin on - any other non-secure
Cortex A8 user who is setting this bit in firmware will instantly
break if this patch is applied.

However, I don't think anyone is willing to say that they have a
solution to this problem - obviously, you can't build OMAP as a
non-multiplatform kernel anymore, so in effect you can never have
the kernel enable this errata. And you can't detect whether you're
running in secure mode or not.

We could do the "only write the bit if it was originally clear" but
we still have the problem that doing so may cause other people
regressions.

[Tony Lindgren]

Post by Russell King - ARM Linux

Post by Tony Lindgren

Post by Sebastian Reichel

Post by Tony Lindgren

Post by Sebastian Reichel
Hi,

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux

Post by Russell King - ARM Linux
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
.long 0x410fc080
.long 0xff0ffff0
- __v7_proc __v7_ca8mp_proc_info, proc_fns = ca8_processor_functions
+ __v7_proc __v7_ca8_proc_info, __v7_setup, proc_fns = ca8_processor_functions
.size __v7_ca8_proc_info, . - __v7_ca8_proc_info
#endif /* CONFIG_ARM_LPAE */

Thanks, merged into the original patch.

Do you want to give me an ack for this, thanks?

I tried to test this together with Tony's follow up patch, but I get
CHK include/config/kernel.release
CHK include/generated/uapi/linux/version.h
CHK include/generated/utsrelease.h
make[1]: 'include/generated/mach-types.h' is up to date.
CALL scripts/checksyscalls.sh
CHK include/generated/compile.h
AS arch/arm/mm/proc-v7.o
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
arch/arm/mm/proc-v7.S:535: Error: invalid operands (*ABS* and .text sections) for `|'
scripts/Makefile.build:294: recipe for target 'arch/arm/mm/proc-v7.o' failed
make[1]: *** [arch/arm/mm/proc-v7.o] Error 1
Makefile:947: recipe for target 'arch/arm/mm' failed
make: *** [arch/arm/mm] Error 2
make: *** Waiting for unfinished jobs....

a6d746789825 ("ARM: proc-v7: avoid errata 430973 workaround for non-Cortex A8 CPUs")

(on N900)

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8345/1

I have a concern with that patch.
The reason that it's disabled for multiplatform is because we can't
guarantee that the auxctrl register will be writable. The solution
we came up with for multiplatform was to require firmware to be
updated to enable this bit.
Enabling it on a platform where firmware has not been updated, but
runs in the non-secure world will lead to the kernel hanging in the
early assembly code.
I've discussed it with Catalin, and Catalin's position is that we
should not remove the !multiplatform conditional. That's something
which I find that I'm agreeing with Catalin on - any other non-secure
Cortex A8 user who is setting this bit in firmware will instantly
break if this patch is applied.
However, I don't think anyone is willing to say that they have a
solution to this problem - obviously, you can't build OMAP as a
non-multiplatform kernel anymore, so in effect you can never have
the kernel enable this errata. And you can't detect whether you're
running in secure mode or not.
We could do the "only write the bit if it was originally clear" but
we still have the problem that doing so may cause other people
regressions.

How about we keep the bit writing part !multiplatform conditional
(or even remove it) but always do the flush for ca-8?

Then we could also do a warning for a misconfigured ca-8 later on.

Regards,

Tony

[Russell King - ARM Linux]

Post by Tony Lindgren

Post by Russell King - ARM Linux
However, I don't think anyone is willing to say that they have a
solution to this problem - obviously, you can't build OMAP as a
non-multiplatform kernel anymore, so in effect you can never have
the kernel enable this errata. And you can't detect whether you're
running in secure mode or not.
We could do the "only write the bit if it was originally clear" but
we still have the problem that doing so may cause other people
regressions.

How about we keep the bit writing part !multiplatform conditional
(or even remove it) but always do the flush for ca-8?
Then we could also do a warning for a misconfigured ca-8 later on.

Yes, we could do this - we'll have to rely on the boot loader doing
the right thing and setting this bit appropriately. For those
platforms where this doesn't apply, I don't see any solution as (iirc)
OMAP now requires MULTIPLATFORM to be enabled.

[Tony Lindgren]

Post by Russell King - ARM Linux

Post by Tony Lindgren

Post by Russell King - ARM Linux
However, I don't think anyone is willing to say that they have a
solution to this problem - obviously, you can't build OMAP as a
non-multiplatform kernel anymore, so in effect you can never have
the kernel enable this errata. And you can't detect whether you're
running in secure mode or not.
We could do the "only write the bit if it was originally clear" but
we still have the problem that doing so may cause other people
regressions.

How about we keep the bit writing part !multiplatform conditional
(or even remove it) but always do the flush for ca-8?
Then we could also do a warning for a misconfigured ca-8 later on.

Yes, we could do this - we'll have to rely on the boot loader doing
the right thing and setting this bit appropriately. For those
platforms where this doesn't apply, I don't see any solution as (iirc)
OMAP now requires MULTIPLATFORM to be enabled.

OK, here's an updated patch. I've kept Sebastian's Tested-by as we know
the init part is a nop on n900 that he tested with.

Regards,

Tony

8< ---------------------
From: Tony Lindgren <***@atomide.com>
Date: Mon, 27 Apr 2015 10:18:14 -0700
Subject: [PATCH] ARM: mm: Fix Cortex-A8 erratum 430973 segfaults for bootloaders and multiarch

Looks like apps can be made to segfault easily on armhf distros
just by running cpuburn-a8 in the background, then starting apt
get update unless erratum 430973 workaround is enabled. This happens
on r3p2 also, which has 430973 fixed in hardware.

Turns out the reason for this is some bootloaders incorrectly
setting the auxilary register IBE bit, which probably causes us
to hit erratum 687067 on Cortex-A8 later than r1p2.

If the bootloader incorrectly sets the IBE bit in the auxilary control
register for Cortex-A8 revisions with 430973 fixed in hardware, we
need to call flush BTAC/BTB to avoid segfaults probably caused by
erratum 687067. So let's flush BTAC/BTB unconditionally for Cortex-A8.
It won't do anything unless the IBE bit is set.

Note that we keep the erratum 430973 Kconfig option still around and
disabled for multiarch as it may be unsafe to enable for some secure
SoC. It is known safe to be enabled for n900, but won't do anything
on n900 as the IBE bit needs to be set with SMC.

Also note that SoCs probably should also add checks and print warnings
for the misconfigured IBE bit depending on the Cortex-A8 revision
so the bootloaders can be fixed Cortex-A8 revisions later than
r1p2 to not set the IBE bit.

Tested-By: Sebastian Reichel <***@kernel.org>
Signed-off-by: Tony Lindgren <***@atomide.com>

--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -36,14 +36,16 @@
*
* It is assumed that:
* - we are not using split page tables
+ *
+ * Note that we always need to flush BTAC/BTB if IBE is set
+ * even on Cortex-A8 revisions not affected by 430973.
+ * If IBE is not set, the flush BTAC/BTB won't do anything.
*/
ENTRY(cpu_ca8_switch_mm)
#ifdef CONFIG_MMU
mov r2, #0
-#ifdef CONFIG_ARM_ERRATA_430973
mcr p15, 0, r2, c7, c5, 6 @ flush BTAC/BTB
#endif
-#endif
ENTRY(cpu_v7_switch_mm)
#ifdef CONFIG_MMU
mmid r1, r1 @ get mm->context.id

[Tony Lindgren]

Post by Tony Lindgren

Post by Russell King - ARM Linux

Post by Tony Lindgren

Post by Russell King - ARM Linux
However, I don't think anyone is willing to say that they have a
solution to this problem - obviously, you can't build OMAP as a
non-multiplatform kernel anymore, so in effect you can never have
the kernel enable this errata. And you can't detect whether you're
running in secure mode or not.
We could do the "only write the bit if it was originally clear" but
we still have the problem that doing so may cause other people
regressions.

How about we keep the bit writing part !multiplatform conditional
(or even remove it) but always do the flush for ca-8?
Then we could also do a warning for a misconfigured ca-8 later on.

Yes, we could do this - we'll have to rely on the boot loader doing
the right thing and setting this bit appropriately. For those
platforms where this doesn't apply, I don't see any solution as (iirc)
OMAP now requires MULTIPLATFORM to be enabled.

OK, here's an updated patch. I've kept Sebastian's Tested-by as we know
the init part is a nop on n900 that he tested with.

I've posted this updated version into Russell's patch system:

http://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=8353/1

Regards,

Tony

Post by Tony Lindgren
8< ---------------------
Date: Mon, 27 Apr 2015 10:18:14 -0700
Subject: [PATCH] ARM: mm: Fix Cortex-A8 erratum 430973 segfaults for bootloaders and multiarch
Looks like apps can be made to segfault easily on armhf distros
just by running cpuburn-a8 in the background, then starting apt
get update unless erratum 430973 workaround is enabled. This happens
on r3p2 also, which has 430973 fixed in hardware.
Turns out the reason for this is some bootloaders incorrectly
setting the auxilary register IBE bit, which probably causes us
to hit erratum 687067 on Cortex-A8 later than r1p2.
If the bootloader incorrectly sets the IBE bit in the auxilary control
register for Cortex-A8 revisions with 430973 fixed in hardware, we
need to call flush BTAC/BTB to avoid segfaults probably caused by
erratum 687067. So let's flush BTAC/BTB unconditionally for Cortex-A8.
It won't do anything unless the IBE bit is set.
Note that we keep the erratum 430973 Kconfig option still around and
disabled for multiarch as it may be unsafe to enable for some secure
SoC. It is known safe to be enabled for n900, but won't do anything
on n900 as the IBE bit needs to be set with SMC.
Also note that SoCs probably should also add checks and print warnings
for the misconfigured IBE bit depending on the Cortex-A8 revision
so the bootloaders can be fixed Cortex-A8 revisions later than
r1p2 to not set the IBE bit.
--- a/arch/arm/mm/proc-v7-2level.S
+++ b/arch/arm/mm/proc-v7-2level.S
@@ -36,14 +36,16 @@
*
* - we are not using split page tables
+ *
+ * Note that we always need to flush BTAC/BTB if IBE is set
+ * even on Cortex-A8 revisions not affected by 430973.
+ * If IBE is not set, the flush BTAC/BTB won't do anything.
*/
ENTRY(cpu_ca8_switch_mm)
#ifdef CONFIG_MMU
mov r2, #0
-#ifdef CONFIG_ARM_ERRATA_430973
#endif
-#endif
ENTRY(cpu_v7_switch_mm)
#ifdef CONFIG_MMU
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Matthijs van Duin]

On 23 April 2015 at 12:25, Russell King - ARM Linux

And you can't detect whether you're running in secure mode or not.

If not, you get an undefined instruction exception, which you could trap.

This may not be convenient, but "can't detect" is an overstatement.

Matthijs

[Russell King - ARM Linux]

Post by Matthijs van Duin
On 23 April 2015 at 12:25, Russell King - ARM Linux

And you can't detect whether you're running in secure mode or not.

If not, you get an undefined instruction exception, which you could trap.
This may not be convenient, but "can't detect" is an overstatement.

It's these kinds of statements that really piss me off.

At this stage in the boot, there's no memory allocators. There's no
MMU. There's really not very much. There's no guarantee that the
location where the vectors are is writable on all platforms.

It's pretty much _impossible_ to do generically.

"Can't detect" is _not_ an overstatement. It's a statement that I'm
giving you through my experience and knowledge of the Linux kernel,
the ARM archtecture, the capabilities of the platforms we have to
deal with, and how we want the kernel to work.

Sure, we _can_ detect it if (and only if) we code specifically for a
platform which has RAM at the CPU vector location. Unfortunately,
that's a _very_ small proportion which approximates a number very
close to zero.

[Matthijs van Duin]

On 28 April 2015 at 20:11, Russell King - ARM Linux

Post by Russell King - ARM Linux
"Can't detect" is _not_ an overstatement. It's a statement that I'm
giving you through my experience and knowledge of the Linux kernel,
the ARM archtecture, the capabilities of the platforms we have to
deal with, and how we want the kernel to work.
Sure, we _can_ detect it if (and only if) we code specifically for a
platform which has RAM at the CPU vector location. Unfortunately,
that's a _very_ small proportion which approximates a number very
close to zero.

Sorry, I clearly had never adequately appreciated how much of a luxury
it is to be targeting only a limited range of platforms, or the burden
of the constraints imposed by needing to support the bewildering
diversity out there.

However, your objection isn't quite valid either since any ARM cpu
supporting the security extensions also supports changing the vector
base address.

Matthijs

[Sebastian Reichel]

Hi,

Post by Matthijs van Duin

Post by Tony Lindgren
Right, it affects n900 for sure. My point is that it also seems to
affect 37xx versions not listed to suffer from this issue.

They shouldn't... erratum 430973 only affected Cortex-A8 r1, and the
dm37xx should have an r3p2 right?
A word of caution though: at least on the DM814x and AM335x, secure
ROM sets bit 6 (IBE) in the Auxiliary Control Register, thereby
enabling BTB invalidate instructions (which normally execute as nops).
This is presumably a leftover of the erratum 430973 workaround, but it
means there is a risk of running afoul of erratum 687067 if BTB
invalidate by MVA instructions are actually used.
I would actually suggest clearing IBE if it set on Cortex-A8 r2 or
later processors and a secure monitor call is available to do so
(there is on the DM814x and AM335x, dunno about the 37xx), also for
performance reasons: BTB invalidates are quite expensive instructions
(when enabled).

So if I understand the issue correct, it would be ok to enable the
430973 workaround in cpu_v7_switch_mm for arm multiplatform kernels
(mcr p15, 0, r2, c7, c5, 6 @ flush BTAC/BTB). On unaffected
Cortex-A8 platforms the IBE bit should be unset (resulting in a nop
instead of the BTB flush).

So assuming, that the same is true for non Cortex-A8 platforms: Can
the workaround simply be enabled by default if CPU_V7 is selected?

-- Sebastian

[Tony Lindgren]

Post by Sebastian Reichel
Hi,

Post by Matthijs van Duin

Post by Tony Lindgren
Right, it affects n900 for sure. My point is that it also seems to
affect 37xx versions not listed to suffer from this issue.

They shouldn't... erratum 430973 only affected Cortex-A8 r1, and the
dm37xx should have an r3p2 right?
A word of caution though: at least on the DM814x and AM335x, secure
ROM sets bit 6 (IBE) in the Auxiliary Control Register, thereby
enabling BTB invalidate instructions (which normally execute as nops).
This is presumably a leftover of the erratum 430973 workaround, but it
means there is a risk of running afoul of erratum 687067 if BTB
invalidate by MVA instructions are actually used.
I would actually suggest clearing IBE if it set on Cortex-A8 r2 or
later processors and a secure monitor call is available to do so
(there is on the DM814x and AM335x, dunno about the 37xx), also for
performance reasons: BTB invalidates are quite expensive instructions
(when enabled).

So if I understand the issue correct, it would be ok to enable the
430973 workaround in cpu_v7_switch_mm for arm multiplatform kernels
Cortex-A8 platforms the IBE bit should be unset (resulting in a nop
instead of the BTB flush).

Well we only want to enable for cortex-a8 revisions affected by
430973, so a custom cpu_v7_switch_mm seems like the way to go there.

And then we need checks for clearing IBE for unaffected cortex-a8
revisions. I'll check to today if I have IBE bit set on the systems
where I'm seeing issues that should not be affected by 430973.

Post by Sebastian Reichel
So assuming, that the same is true for non Cortex-A8 platforms: Can
the workaround simply be enabled by default if CPU_V7 is selected?

It should be enabled conditionally.

Regards,

Tony