Message ID | 1425236828-28349-1-git-send-email-jarkko.nikula@bitmer.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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 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? Regards, Tony -- 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
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 Nikula <jarkko.nikula@bitmer.com> [150330 10:46]: > 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. 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 linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
* 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 -- 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
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
* 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 -- 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
>> 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,
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
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 -- 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
* 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 -- 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
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 -- 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
* 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 -- 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
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 -- 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
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 -- 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
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
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.
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
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
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.
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 -- 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
On 5 April 2015 at 18:50, Matthijs van Duin <matthijsvanduin@gmail.com> wrote:
> MRC
I mean MCR of course
--
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
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 -- 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
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 -- 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
* 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 -- 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
* 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 -- 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
* 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 -- 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
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 -- 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
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
* 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 -- 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
* 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 -- 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
* 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 -- 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
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
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 -- 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
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.
* 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 -- 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
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
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.
--
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
* 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 -- 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
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 {