[PATCH RESEND] ARM: dts: OMAP3-N900: Add microphone bias voltages

[PATCH RESEND] ARM: dts: OMAP3-N900: Add microphone bias voltages

[Jarkko Nikula]

From: Pavel Machek <pavel@ucw.cz>

N900 audio recording needs that codec provides bias voltage for integrated
digital microphone and headset microphone depending which one is used.
Digital microphone uses 2 V bias and it comes from the codec A part. Codec
B part drives the headset microphone bias and that is set to 2.5 V.

Signed-off-by: Pavel Machek <pavel@ucw.cz>
[Jarkko: Headset mic bias changed to 2 (2.5 V) as it was before commit
e2e8bfdf6157 ("ASoC: tlv320aic3x: Convert mic bias to a supply widget")]
Signed-off-by: Jarkko Nikula <jarkko.nikula@bitmer.com>
---
 arch/arm/boot/dts/omap3-n900.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-n900.dts b/arch/arm/boot/dts/omap3-n900.dts
index db80f9d376fa..9c8bdf2c93a1 100644
--- a/arch/arm/boot/dts/omap3-n900.dts
+++ b/arch/arm/boot/dts/omap3-n900.dts
@@ -484,6 +484,8 @@
 		DRVDD-supply = <&vmmc2>;
 		IOVDD-supply = <&vio>;
 		DVDD-supply = <&vio>;
+
+		ai3x-micbias-vg = <1>;
 	};
 
 	tlv320aic3x_aux: tlv320aic3x@19 {
@@ -495,6 +497,8 @@
 		DRVDD-supply = <&vmmc2>;
 		IOVDD-supply = <&vio>;
 		DVDD-supply = <&vio>;
+
+		ai3x-micbias-vg = <2>;
 	};
 
 	tsl2563: tsl2563@29 {
-- 
2.1.4

Re: [PATCH RESEND] ARM: dts: OMAP3-N900: Add microphone bias voltages

[Jarkko Nikula]

On 03/01/2015 09:07 PM, Jarkko Nikula wrote:
> From: Pavel Machek <pavel@ucw.cz>
> 
> N900 audio recording needs that codec provides bias voltage for integrated
> digital microphone and headset microphone depending which one is used.
> Digital microphone uses 2 V bias and it comes from the codec A part. Codec
> B part drives the headset microphone bias and that is set to 2.5 V.
> 
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> [Jarkko: Headset mic bias changed to 2 (2.5 V) as it was before commit
> e2e8bfdf6157 ("ASoC: tlv320aic3x: Convert mic bias to a supply widget")]
> Signed-off-by: Jarkko Nikula <jarkko.nikula@bitmer.com>
> ---
>  arch/arm/boot/dts/omap3-n900.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
Ping?

-- 
Jarkko

Re: [PATCH RESEND] ARM: dts: OMAP3-N900: Add microphone bias voltages

[Tony Lindgren]

* Jarkko Nikula <jarkko.nikula-FVTvWyuFUl3QT0dZR+AlfA@public.gmane.org> [150330 09:31]:
> On 03/01/2015 09:07 PM, Jarkko Nikula wrote:
> > From: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> > 
> > N900 audio recording needs that codec provides bias voltage for integrated
> > digital microphone and headset microphone depending which one is used.
> > Digital microphone uses 2 V bias and it comes from the codec A part. Codec
> > B part drives the headset microphone bias and that is set to 2.5 V.
> > 
> > Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> > [Jarkko: Headset mic bias changed to 2 (2.5 V) as it was before commit
> > e2e8bfdf6157 ("ASoC: tlv320aic3x: Convert mic bias to a supply widget")]
> > Signed-off-by: Jarkko Nikula <jarkko.nikula-FVTvWyuFUl3QT0dZR+AlfA@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/omap3-n900.dts | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> Ping?

Oops sorry I have missed this one. This looks like a regression fix
for the v4.0-rc series? Should it be cc stable v3.16+ or something?

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND] ARM: dts: OMAP3-N900: Add microphone bias voltages

[Jarkko Nikula]

On 03/30/2015 07:42 PM, Tony Lindgren wrote:
> * Jarkko Nikula <jarkko.nikula@bitmer.com> [150330 09:31]:
>> On 03/01/2015 09:07 PM, Jarkko Nikula wrote:
>>> From: Pavel Machek <pavel@ucw.cz>
>>>
>>> N900 audio recording needs that codec provides bias voltage for integrated
>>> digital microphone and headset microphone depending which one is used.
>>> Digital microphone uses 2 V bias and it comes from the codec A part. Codec
>>> B part drives the headset microphone bias and that is set to 2.5 V.
>>>
>>> Signed-off-by: Pavel Machek <pavel@ucw.cz>
>>> [Jarkko: Headset mic bias changed to 2 (2.5 V) as it was before commit
>>> e2e8bfdf6157 ("ASoC: tlv320aic3x: Convert mic bias to a supply widget")]
>>> Signed-off-by: Jarkko Nikula <jarkko.nikula@bitmer.com>
>>> ---
>>>  arch/arm/boot/dts/omap3-n900.dts | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>> Ping?
> 
> Oops sorry I have missed this one. This looks like a regression fix
> for the v4.0-rc series? Should it be cc stable v3.16+ or something?
> 
Well, there has been regression but finding exactly how far should the
fix go didn't look instantly straightforward due all DT, codec driver
mic bias etc changes and I ended up not cc'ing stable.

But well, I guess first kernel where this commit makes sense is 3.16+
due commit f7d0f2a08567 ("ARM: dts: omap3-n900: Add sound support").
Although it applies on top of commit 14e3e295b2b9 ("ARM: dts:
omap3-n900: Add TLV320AIC3X support") too (3.12+) but not before that.

-- 
Jarkko

Re: [PATCH RESEND] ARM: dts: OMAP3-N900: Add microphone bias voltages

[Tony Lindgren]

* Jarkko Nikula <jarkko.nikula-FVTvWyuFUl3QT0dZR+AlfA@public.gmane.org> [150330 10:46]:
> On 03/30/2015 07:42 PM, Tony Lindgren wrote:
> > * Jarkko Nikula <jarkko.nikula-FVTvWyuFUl3QT0dZR+AlfA@public.gmane.org> [150330 09:31]:
> >> On 03/01/2015 09:07 PM, Jarkko Nikula wrote:
> >>> From: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> >>>
> >>> N900 audio recording needs that codec provides bias voltage for integrated
> >>> digital microphone and headset microphone depending which one is used.
> >>> Digital microphone uses 2 V bias and it comes from the codec A part. Codec
> >>> B part drives the headset microphone bias and that is set to 2.5 V.
> >>>
> >>> Signed-off-by: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> >>> [Jarkko: Headset mic bias changed to 2 (2.5 V) as it was before commit
> >>> e2e8bfdf6157 ("ASoC: tlv320aic3x: Convert mic bias to a supply widget")]
> >>> Signed-off-by: Jarkko Nikula <jarkko.nikula-FVTvWyuFUl3QT0dZR+AlfA@public.gmane.org>
> >>> ---
> >>>  arch/arm/boot/dts/omap3-n900.dts | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >> Ping?
> > 
> > Oops sorry I have missed this one. This looks like a regression fix
> > for the v4.0-rc series? Should it be cc stable v3.16+ or something?
> > 
> Well, there has been regression but finding exactly how far should the
> fix go didn't look instantly straightforward due all DT, codec driver
> mic bias etc changes and I ended up not cc'ing stable.
> 
> But well, I guess first kernel where this commit makes sense is 3.16+
> due commit f7d0f2a08567 ("ARM: dts: omap3-n900: Add sound support").
> Although it applies on top of commit 14e3e295b2b9 ("ARM: dts:
> omap3-n900: Add TLV320AIC3X support") too (3.12+) but not before that.

OK I think debian is using v3.16 kernel and that's pretty much the
first kernel that is usable with dts on many omap3 devices so might
make sense for that. I can add it if you think it makes sense.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH RESEND] ARM: dts: OMAP3-N900: Add microphone bias voltages

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1584 bytes --]

Hi,

On Mon, Mar 30, 2015 at 10:50:52AM -0700, Tony Lindgren wrote:
> * Jarkko Nikula <jarkko.nikula@bitmer.com> [150330 10:46]:
> > Well, there has been regression but finding exactly how far should the
> > fix go didn't look instantly straightforward due all DT, codec driver
> > mic bias etc changes and I ended up not cc'ing stable.
> > 
> > But well, I guess first kernel where this commit makes sense is 3.16+
> > due commit f7d0f2a08567 ("ARM: dts: omap3-n900: Add sound support").
> > Although it applies on top of commit 14e3e295b2b9 ("ARM: dts:
> > omap3-n900: Add TLV320AIC3X support") too (3.12+) but not before that.
> 
> 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

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.

> and that's pretty much the first kernel that is usable with dts on
> many omap3 devices so might make sense for that.

DT support for N900's soundcard has been added in 3.16, so before
that the audio stuff didn't work at all.

> I can add it if you think it makes sense.

I guess backporting this makes sense because of fewer "broken" DTB
files in the wild.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH RESEND] ARM: dts: OMAP3-N900: Add microphone bias voltages

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [150331 05:33]:
> Hi,
> 
> On Mon, Mar 30, 2015 at 10:50:52AM -0700, Tony Lindgren wrote:
> > * Jarkko Nikula <jarkko.nikula@bitmer.com> [150330 10:46]:
> > > Well, there has been regression but finding exactly how far should the
> > > fix go didn't look instantly straightforward due all DT, codec driver
> > > mic bias etc changes and I ended up not cc'ing stable.
> > > 
> > > But well, I guess first kernel where this commit makes sense is 3.16+
> > > due commit f7d0f2a08567 ("ARM: dts: omap3-n900: Add sound support").
> > > Although it applies on top of commit 14e3e295b2b9 ("ARM: dts:
> > > omap3-n900: Add TLV320AIC3X support") too (3.12+) but not before that.
> > 
> > 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
with running apt-get update on ARMv8 omaps on armhf without that:

http://www.spinics.net/lists/linux-omap/msg108511.html

See also 5c86c5339c56 ("ARM: omap2plus_defconfig: Enable ARM erratum
430973 for omap3").

I wonder if the ARMv8 revision range might be wrong 430973 in
kernel or errata? Also I recall that 430973 change to the
arch/arm/mm/proc-v7-2level.S fixed the issue, this should be
verified though.
 
> 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.
 
> > and that's pretty much the first kernel that is usable with dts on
> > many omap3 devices so might make sense for that.
> 
> DT support for N900's soundcard has been added in 3.16, so before
> that the audio stuff didn't work at all.
> 
> > I can add it if you think it makes sense.
> 
> I guess backporting this makes sense because of fewer "broken" DTB
> files in the wild.

Anyways yeah adding $subject patch into omap-for-v4.0/fixes with
cc stable v3.16+.

Regards,

Tony

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2848 bytes --]

Hi,

On Wed, Apr 01, 2015 at 12:47:36PM -0700, Tony Lindgren wrote:
> > > 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
> with running apt-get update on ARMv8 omaps on armhf without that:

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

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).

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.

> 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.

> > 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.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [150403 09:37]:
> Hi,
> 
> On Wed, Apr 01, 2015 at 12:47:36PM -0700, Tony Lindgren wrote:
> > > > 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
> > with running apt-get update on ARMv8 omaps on armhf without that:
> 
> 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

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

[Robert Nelson]

>> 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,

-- 
Robert Nelson
https://rcn-ee.com/

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

On Fri, Apr 03, 2015 at 02:21:48PM -0500, Robert Nelson wrote:
> >> 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1866 bytes --]

On Sunday 05 April 2015 15:00:45 Sebastian Reichel wrote:
> On Fri, Apr 03, 2015 at 02:21:48PM -0500, Robert Nelson wrote:
> > >> 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=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.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]

Hi,

On Sun, Apr 05, 2015 at 03:26:14PM +0200, Pali Rohár wrote:
> > > > 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Pali Rohár]

[-- Attachment #1: Type: Text/Plain, Size: 1851 bytes --]

On Sunday 05 April 2015 15:45:28 Sebastian Reichel wrote:
> Hi,
> 
> On Sun, Apr 05, 2015 at 03:26:14PM +0200, Pali Rohár wrote:
> > > > > 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.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

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

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 557 bytes --]

Hi,

On Sun, Apr 05, 2015 at 03:52:10PM +0200, Pali Rohár wrote:
> > 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[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
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: ARM errata 430973 on multi platform kernels

[Ivaylo Dimitrov]

Hi

On  3.04.2015 19:35, Sebastian Reichel wrote:
>
> 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

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [150403 15:08]:
> Hi
> 
> On  3.04.2015 19:35, Sebastian Reichel wrote:
> >
> >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

Re: ARM errata 430973 on multi platform kernels

[Ivaylo Dimitrov]

Hi,

> 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

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [150403 15:47]:
> 
> Hi,
> 
> >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

Re: ARM errata 430973 on multi platform kernels

[Matthijs van Duin]

On 4 April 2015 at 00:52, Tony Lindgren <tony@atomide.com> wrote:
> 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

Re: ARM errata 430973 on multi platform kernels

[Ivaylo Dimitrov]

On  5.04.2015 07:13, Matthijs van Duin wrote:
> 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.

>
> Matthijs
>

Regards,
Ivo

Re: ARM errata 430973 on multi platform kernels

[Matthijs van Duin]

On 5 April 2015 at 09:23, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> wrote:
> 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

Re: ARM errata 430973 on multi platform kernels

[Matthijs van Duin]

On 5 April 2015 at 18:50, Matthijs van Duin <matthijsvanduin@gmail.com> wrote:
> MRC

I mean MCR of course

Re: ARM errata 430973 on multi platform kernels

[Ivaylo Dimitrov]

On  5.04.2015 19:50, Matthijs van Duin wrote:
> On 5 April 2015 at 09:23, Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> wrote:
>> 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
>

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

Re: ARM errata 430973 on multi platform kernels

[Matthijs van Duin]

Cortex-A8 errata doc states in its workaround for erratum 430973:

> 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
> set using the following code sequence:
> 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

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Matthijs van Duin <matthijsvanduin@gmail.com> [150405 16:53]:
> Cortex-A8 errata doc states in its workaround for erratum 430973:
> 
> > 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
> > set using the following code sequence:
> > 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.

> 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
 
> 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

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [150406 08:24]:
> * Matthijs van Duin <matthijsvanduin@gmail.com> [150405 16:53]:
> > Cortex-A8 errata doc states in its workaround for erratum 430973:
> > 
> > > 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
> > > set using the following code sequence:
> > > 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.
> 
> > 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
>  
> > 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.

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

Re: ARM errata 430973 on multi platform kernels

[Ivaylo Dimitrov]

On  6.04.2015 18:40, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [150406 08:24]:
>> * Matthijs van Duin <matthijsvanduin@gmail.com> [150405 16:53]:
>>> Cortex-A8 errata doc states in its workaround for erratum 430973:
>>>
>>>> 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
>>>> set using the following code sequence:
>>>> 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.
>>
>>> 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
>>
>>> 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.
>
> 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.
>

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?

> 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.

> Regards,
>
> Tony
>

Regards,

Ivo

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [150406 10:15]:
> On  6.04.2015 18:40, Tony Lindgren wrote:
> >
> >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.
> >
> 
> 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.
 
> >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

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.
 
> 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

Re: ARM errata 430973 on multi platform kernels

[Matthijs van Duin]

* Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [150406 10:15]:
> 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).


On 6 April 2015 at 19:42, Tony Lindgren <tony@atomide.com> wrote:
> 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...


>> 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.

>>  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

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Matthijs van Duin <matthijsvanduin@gmail.com> [150406 11:15]:
> * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [150406 10:15]:
> > 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).
> 
> 
> On 6 April 2015 at 19:42, Tony Lindgren <tony@atomide.com> wrote:
> > 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.
 
> >> 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.
 
> >>  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

Re: ARM errata 430973 on multi platform kernels

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

Hi,

On Mon, Apr 06, 2015 at 07:23:13PM -0700, Tony Lindgren wrote:
> 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: ARM errata 430973 on multi platform kernels

[Matthijs van Duin]

On 7 April 2015 at 04:23, Tony Lindgren <tony@atomide.com> wrote:
> 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...


> 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...


> 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.

On 7 April 2015 at 05:12, Sebastian Reichel <sre@kernel.org> wrote:
> 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.

> 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

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Matthijs van Duin <matthijsvanduin@gmail.com> [150406 20:50]:
> On 7 April 2015 at 04:23, Tony Lindgren <tony@atomide.com> wrote:
> > 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. 

> 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.
 
> > 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.
 
> 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.
 
> There's more secrom misconfiguration that the bootloader should fix
> anyhow to make optimal use of the processor...

Yeah. 
 
> > 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.
 
> On 7 April 2015 at 05:12, Sebastian Reichel <sre@kernel.org> wrote:
> > 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.
> 
> > 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

Re: ARM errata 430973 on multi platform kernels

[Grazvydas Ignotas]

On Tue, Apr 7, 2015 at 5:23 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Matthijs van Duin <matthijsvanduin@gmail.com> [150406 11:15]:
>>
>> On 6 April 2015 at 19:42, Tony Lindgren <tony@atomide.com> wrote:
>> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Grazvydas Ignotas <notasas@gmail.com> [150409 15:37]:
> On Tue, Apr 7, 2015 at 5:23 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Matthijs van Duin <matthijsvanduin@gmail.com> [150406 11:15]:
> >>
> >> On 6 April 2015 at 19:42, Tony Lindgren <tony@atomide.com> wrote:
> >> > 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?

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

Re: ARM errata 430973 on multi platform kernels

[Nishanth Menon]

On 04/09/2015 05:44 PM, Tony Lindgren wrote:
> * Grazvydas Ignotas <notasas@gmail.com> [150409 15:37]:
>> On Tue, Apr 7, 2015 at 5:23 AM, Tony Lindgren <tony@atomide.com> wrote:
>>> * Matthijs van Duin <matthijsvanduin@gmail.com> [150406 11:15]:
>>>>
>>>> On 6 April 2015 at 19:42, Tony Lindgren <tony@atomide.com> wrote:
>>>>> 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?
> 
> 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.

> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


-- 
Regards,
Nishanth Menon

Re: ARM errata 430973 on multi platform kernels

[Grazvydas Ignotas]

On Fri, Apr 10, 2015 at 1:44 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Grazvydas Ignotas <notasas@gmail.com> [150409 15:37]:
>> On Tue, Apr 7, 2015 at 5:23 AM, Tony Lindgren <tony@atomide.com> wrote:
>> > * Matthijs van Duin <matthijsvanduin@gmail.com> [150406 11:15]:
>> >>
>> >> On 6 April 2015 at 19:42, Tony Lindgren <tony@atomide.com> wrote:
>> >> > 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?
>
> 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)

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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Grazvydas Ignotas <notasas@gmail.com> [150410 15:06]:
> On Fri, Apr 10, 2015 at 1:44 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Grazvydas Ignotas <notasas@gmail.com> [150409 15:37]:
> >> On Tue, Apr 7, 2015 at 5:23 AM, Tony Lindgren <tony@atomide.com> wrote:
> >> > * Matthijs van Duin <matthijsvanduin@gmail.com> [150406 11:15]:
> >> >>
> >> >> On 6 April 2015 at 19:42, Tony Lindgren <tony@atomide.com> wrote:
> >> >> > 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?
> >
> > 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)
> 
> 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 :)

> 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?
 
> 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

Re: ARM errata 430973 on multi platform kernels

[Matthijs van Duin]

On 10 April 2015 at 00:37, Grazvydas Ignotas <notasas@gmail.com> wrote:
> 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.

Re: ARM errata 430973 on multi platform kernels

[Russell King - ARM Linux]

On Mon, Apr 06, 2015 at 08:14:30PM +0200, Matthijs van Duin wrote:
> * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [150406 10:15]:
> > 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.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: ARM errata 430973 on multi platform kernels

[Russell King - ARM Linux]

On Mon, Apr 06, 2015 at 10:42:45AM -0700, Tony Lindgren wrote:
> * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [150406 10:15]:
> > On  6.04.2015 18:40, Tony Lindgren wrote:
> > >
> > >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.
> > >
> > 
> > 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.
>  
> > >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
> 
> 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.
>  
> > 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 */
 
 	/*


-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [150407 06:58]:
> On Mon, Apr 06, 2015 at 10:42:45AM -0700, Tony Lindgren wrote:
> > * Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com> [150406 10:15]:
> > > On  6.04.2015 18:40, Tony Lindgren wrote:
> > > >
> > > >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.
> > > >
> > > 
> > > 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.
> >  
> > > >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
> > 
> > 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.
> >  
> > > 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):
> 
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -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 */
>  
>  	/*

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

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [150407 08:27]:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [150407 06:58]:
> > 
> > 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):
> > 
> > --- a/arch/arm/mm/proc-v7.S
> > +++ b/arch/arm/mm/proc-v7.S
> > @@ -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 */
> >  
> >  	/*
> 
> 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 */

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 <tony@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 <tony@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

Re: ARM errata 430973 on multi platform kernels

[Russell King - ARM Linux]

On Tue, Apr 07, 2015 at 08:44:05AM -0700, Tony Lindgren wrote:
> 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?

> 8< --------------------
> From: Tony Lindgren <tony@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 <tony@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

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [150408 16:09]:
> On Tue, Apr 07, 2015 at 08:44:05AM -0700, Tony Lindgren wrote:
> > 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
 
> > 8< --------------------
> > From: Tony Lindgren <tony@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 <tony@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
> 
> -- 
> 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
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ARM errata 430973 on multi platform kernels

[Russell King - ARM Linux]

On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> 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 */

Thanks, merged into the original patch.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: ARM errata 430973 on multi platform kernels

[Russell King - ARM Linux]

On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > 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 */
> 
> Thanks, merged into the original patch.

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

8<===
From: Russell King <rmk+kernel@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+kernel@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 */
 
 	/*
-- 
1.8.3.1

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [150409 06:49]:
> On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > 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 */
> > 
> > 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 <tony@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
 
> 8<===
> From: Russell King <rmk+kernel@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+kernel@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 */
>  
>  	/*
> -- 
> 1.8.3.1
> 
> -- 
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.

Re: ARM errata 430973 on multi platform kernels

[Russell King - ARM Linux]

On Thu, Apr 09, 2015 at 08:09:19AM -0700, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [150409 06:49]:
> > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > 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 */
> > > 
> > > 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 <tony@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.

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

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: ARM errata 430973 on multi platform kernels

[Sebastian Reichel]

[-- Attachment #1.1: Type: text/plain, Size: 1829 bytes --]

Hi,

On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > 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 */
> > 
> > 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:

sre@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

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [150415 09:32]:
> Hi,
> 
> On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > 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 */
> > > 
> > > 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:
> 
> sre@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....

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

Re: ARM errata 430973 on multi platform kernels

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2506 bytes --]

On Thu, Apr 16, 2015 at 09:08:58AM -0700, Tony Lindgren wrote:
> * Sebastian Reichel <sre@kernel.org> [150415 09:32]:
> > Hi,
> > 
> > On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> > > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > > 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 */
> > > > 
> > > > 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:
> > 
> > sre@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....
> 
> Maybe test the version in Linux next:
> 
> 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 <sre@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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [150417 11:43]:
> On Thu, Apr 16, 2015 at 09:08:58AM -0700, Tony Lindgren wrote:
> > * Sebastian Reichel <sre@kernel.org> [150415 09:32]:
> > > Hi,
> > > 
> > > On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> > > > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > > > 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 */
> > > > > 
> > > > > 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:
> > > 
> > > sre@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....
> > 
> > Maybe test the version in Linux next:
> > 
> > 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 <sre@kernel.org>
> 
> (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
 
> 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

Re: ARM errata 430973 on multi platform kernels

[Russell King - ARM Linux]

On Mon, Apr 20, 2015 at 04:40:32PM -0700, Tony Lindgren wrote:
> * Sebastian Reichel <sre@kernel.org> [150417 11:43]:
> > On Thu, Apr 16, 2015 at 09:08:58AM -0700, Tony Lindgren wrote:
> > > * Sebastian Reichel <sre@kernel.org> [150415 09:32]:
> > > > Hi,
> > > > 
> > > > On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> > > > > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > > > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > > > > 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 */
> > > > > > 
> > > > > > 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:
> > > > 
> > > > sre@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....
> > > 
> > > Maybe test the version in Linux next:
> > > 
> > > 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 <sre@kernel.org>
> > 
> > (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

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.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [150423 03:26]:
> On Mon, Apr 20, 2015 at 04:40:32PM -0700, Tony Lindgren wrote:
> > * Sebastian Reichel <sre@kernel.org> [150417 11:43]:
> > > On Thu, Apr 16, 2015 at 09:08:58AM -0700, Tony Lindgren wrote:
> > > > * Sebastian Reichel <sre@kernel.org> [150415 09:32]:
> > > > > Hi,
> > > > > 
> > > > > On Thu, Apr 09, 2015 at 02:48:43PM +0100, Russell King - ARM Linux wrote:
> > > > > > On Thu, Apr 09, 2015 at 12:06:58AM +0100, Russell King - ARM Linux wrote:
> > > > > > > On Tue, Apr 07, 2015 at 08:22:08AM -0700, Tony Lindgren wrote:
> > > > > > > > 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 */
> > > > > > > 
> > > > > > > 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:
> > > > > 
> > > > > sre@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....
> > > > 
> > > > Maybe test the version in Linux next:
> > > > 
> > > > 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 <sre@kernel.org>
> > > 
> > > (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
> 
> 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

Re: ARM errata 430973 on multi platform kernels

[Russell King - ARM Linux]

On Thu, Apr 23, 2015 at 07:17:28AM -0700, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [150423 03:26]:
> > 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.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Russell King - ARM Linux <linux@arm.linux.org.uk> [150428 11:14]:
> On Thu, Apr 23, 2015 at 07:17:28AM -0700, Tony Lindgren wrote:
> > * Russell King - ARM Linux <linux@arm.linux.org.uk> [150423 03:26]:
> > > 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 <tony@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 <sre@kernel.org>
Signed-off-by: Tony Lindgren <tony@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

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Tony Lindgren <tony@atomide.com> [150429 07:42]:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [150428 11:14]:
> > On Thu, Apr 23, 2015 at 07:17:28AM -0700, Tony Lindgren wrote:
> > > * Russell King - ARM Linux <linux@arm.linux.org.uk> [150423 03:26]:
> > > > 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
 
> 8< ---------------------
> From: Tony Lindgren <tony@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 <sre@kernel.org>
> Signed-off-by: Tony Lindgren <tony@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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: ARM errata 430973 on multi platform kernels

[Matthijs van Duin]

On 23 April 2015 at 12:25, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> 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

Re: ARM errata 430973 on multi platform kernels

[Russell King - ARM Linux]

On Fri, Apr 24, 2015 at 10:54:29AM +0200, Matthijs van Duin wrote:
> On 23 April 2015 at 12:25, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > 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.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

Re: ARM errata 430973 on multi platform kernels

[Matthijs van Duin]

On 28 April 2015 at 20:11, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> "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

Re: ARM errata 430973 on multi platform kernels

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]

Hi,

On Sun, Apr 05, 2015 at 06:13:47AM +0200, Matthijs van Duin wrote:
> On 4 April 2015 at 00:52, Tony Lindgren <tony@atomide.com> wrote:
> > 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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: ARM errata 430973 on multi platform kernels

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [150405 06:40]:
> Hi,
> 
> On Sun, Apr 05, 2015 at 06:13:47AM +0200, Matthijs van Duin wrote:
> > On 4 April 2015 at 00:52, Tony Lindgren <tony@atomide.com> wrote:
> > > 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).

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.
 
> 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

Re: [PATCH RESEND] ARM: dts: OMAP3-N900: Add microphone bias voltages

[Tony Lindgren]

* Sebastian Reichel <sre@kernel.org> [150331 05:33]:
> Hi,
> 
> On Mon, Mar 30, 2015 at 10:50:52AM -0700, Tony Lindgren wrote:
> > * Jarkko Nikula <jarkko.nikula@bitmer.com> [150330 10:46]:
> > > Well, there has been regression but finding exactly how far should the
> > > fix go didn't look instantly straightforward due all DT, codec driver
> > > mic bias etc changes and I ended up not cc'ing stable.
> > > 
> > > But well, I guess first kernel where this commit makes sense is 3.16+
> > > due commit f7d0f2a08567 ("ARM: dts: omap3-n900: Add sound support").
> > > Although it applies on top of commit 14e3e295b2b9 ("ARM: dts:
> > > omap3-n900: Add TLV320AIC3X support") too (3.12+) but not before that.
> > 
> > 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.

Oops I just noticed the original $subject fix is still pending
as we got distracted with all this 430973 stuff. Applying Jarkko's
fix into omap-for-v4.1/fixes with Cc stable v3.16+.

Regards,

Tony