Multi-platform, and secure-only ARM errata workarounds

Multi-platform, and secure-only ARM errata workarounds

[Stephen Warren]

Rob,

I'm looking into enabling CONFIG_MULTIPLATFORM on Tegra for 3.10, and
the main blocking issue is due to commit 62e4d35 "ARM: 7609/1: disable
errata work-arounds which access secure registers". Various Tegra
versions need 3 of those workarounds, and our bootloader doesn't
implement them (at the least, upstream U-Boot; not sure about our
downstream code, but I'm fairly sure given the lack of any feedback I
got in the bug I filed to implement them).

Now, I can easily add those 3 errata workarounds to U-Boot, but that
will require people to reflash their bootloader. This is probably
acceptable for development/reference boards (although I'm sure people
will find it annoying) but for re-purposed production boards (such as
the Toshiba AC100 or various tablets) it will be impossible to update
the factory bootloader. Switching to upstream U-Boot would currently
lose some functionality, and significantly affect people's boot flow, so
is likely unacceptable.

Is there any other alternative I'm not seeing? Having the kernel
suddenly become incompatible with any currently extant bootloader when I
enable CONFIG_MULTIPLATFORM doesn't seem like a great idea.

Multi-platform, and secure-only ARM errata workarounds

[Marc Dietrich]

Stephen,

Am Montag, 25. Februar 2013, 16:47:38 schrieb Stephen Warren:
> I'm looking into enabling CONFIG_MULTIPLATFORM on Tegra for 3.10, and
> the main blocking issue is due to commit 62e4d35 "ARM: 7609/1: disable
> errata work-arounds which access secure registers". Various Tegra
> versions need 3 of those workarounds, and our bootloader doesn't
> implement them (at the least, upstream U-Boot; not sure about our
> downstream code, but I'm fairly sure given the lack of any feedback I
> got in the bug I filed to implement them).
> 
> Now, I can easily add those 3 errata workarounds to U-Boot, but that
> will require people to reflash their bootloader. This is probably
> acceptable for development/reference boards (although I'm sure people
> will find it annoying) but for re-purposed production boards (such as
> the Toshiba AC100 or various tablets) it will be impossible to update
> the factory bootloader. Switching to upstream U-Boot would currently
> lose some functionality, and significantly affect people's boot flow, so
> is likely unacceptable.

personally, I have no problem to require a certain u-boot version for a given 
kernel. From a distro point of view, you will likely update the 
bootloader/kernel on a distro update anyway.

> Is there any other alternative I'm not seeing? Having the kernel
> suddenly become incompatible with any currently extant bootloader when I
> enable CONFIG_MULTIPLATFORM doesn't seem like a great idea.

I *think* it's ok to deprecate fastboot on older devices. AFAIK, the in-kernel 
supported boards all have u-boot support already.

Marc

Multi-platform, and secure-only ARM errata workarounds

[Arnd Bergmann]

On Monday 25 February 2013, Stephen Warren wrote:
> Is there any other alternative I'm not seeing? Having the kernel
> suddenly become incompatible with any currently extant bootloader when I
> enable CONFIG_MULTIPLATFORM doesn't seem like a great idea.

Could we make those errata be run-time enabled only when not booting
in secure mode?

	Arnd

Multi-platform, and secure-only ARM errata workarounds

[Catalin Marinas]

On Tue, Feb 26, 2013 at 10:23:26AM +0000, Arnd Bergmann wrote:
> On Monday 25 February 2013, Stephen Warren wrote:
> > Is there any other alternative I'm not seeing? Having the kernel
> > suddenly become incompatible with any currently extant bootloader when I
> > enable CONFIG_MULTIPLATFORM doesn't seem like a great idea.
> 
> Could we make those errata be run-time enabled only when not booting
> in secure mode?

There is no easy way to detect whether Linux booted in secure mode,
unless we add some information in the DT. But I wouldn't add S vs NS
information, rather which errata need to be enabled by the kernel.

-- 
Catalin

Multi-platform, and secure-only ARM errata workarounds

[Catalin Marinas]

On Tue, Feb 26, 2013 at 10:31:14AM +0000, Catalin Marinas wrote:
> On Tue, Feb 26, 2013 at 10:23:26AM +0000, Arnd Bergmann wrote:
> > On Monday 25 February 2013, Stephen Warren wrote:
> > > Is there any other alternative I'm not seeing? Having the kernel
> > > suddenly become incompatible with any currently extant bootloader when I
> > > enable CONFIG_MULTIPLATFORM doesn't seem like a great idea.
> > 
> > Could we make those errata be run-time enabled only when not booting
> > in secure mode?
> 
> There is no easy way to detect whether Linux booted in secure mode,
> unless we add some information in the DT. But I wouldn't add S vs NS
> information, rather which errata need to be enabled by the kernel.

Another difficulty - parsing the DT is done later while some workarounds
are enabled before the MMU (or require the MMU to be disabled
temporarily).

-- 
Catalin

Multi-platform, and secure-only ARM errata workarounds

[Arnd Bergmann]

On Tuesday 26 February 2013, Catalin Marinas wrote:
> On Tue, Feb 26, 2013 at 10:31:14AM +0000, Catalin Marinas wrote:
> > On Tue, Feb 26, 2013 at 10:23:26AM +0000, Arnd Bergmann wrote:
> > > On Monday 25 February 2013, Stephen Warren wrote:
> > > > Is there any other alternative I'm not seeing? Having the kernel
> > > > suddenly become incompatible with any currently extant bootloader when I
> > > > enable CONFIG_MULTIPLATFORM doesn't seem like a great idea.
> > > 
> > > Could we make those errata be run-time enabled only when not booting
> > > in secure mode?
> > 
> > There is no easy way to detect whether Linux booted in secure mode,
> > unless we add some information in the DT. But I wouldn't add S vs NS
> > information, rather which errata need to be enabled by the kernel.
> 
> Another difficulty - parsing the DT is done later while some workarounds
> are enabled before the MMU (or require the MMU to be disabled
> temporarily).

Can we do it easily after detecting the machine_desc? Maybe we could
add a bit mask of errata to the struct.

	Arnd

Multi-platform, and secure-only ARM errata workarounds

[Catalin Marinas]

On Tue, Feb 26, 2013 at 10:48:06AM +0000, Arnd Bergmann wrote:
> On Tuesday 26 February 2013, Catalin Marinas wrote:
> > On Tue, Feb 26, 2013 at 10:31:14AM +0000, Catalin Marinas wrote:
> > > On Tue, Feb 26, 2013 at 10:23:26AM +0000, Arnd Bergmann wrote:
> > > > On Monday 25 February 2013, Stephen Warren wrote:
> > > > > Is there any other alternative I'm not seeing? Having the kernel
> > > > > suddenly become incompatible with any currently extant bootloader when I
> > > > > enable CONFIG_MULTIPLATFORM doesn't seem like a great idea.
> > > > 
> > > > Could we make those errata be run-time enabled only when not booting
> > > > in secure mode?
> > > 
> > > There is no easy way to detect whether Linux booted in secure mode,
> > > unless we add some information in the DT. But I wouldn't add S vs NS
> > > information, rather which errata need to be enabled by the kernel.
> > 
> > Another difficulty - parsing the DT is done later while some workarounds
> > are enabled before the MMU (or require the MMU to be disabled
> > temporarily).
> 
> Can we do it easily after detecting the machine_desc? Maybe we could
> add a bit mask of errata to the struct.

On DT platforms we detect the machine_desc after the MMU has been
initialised. Some errata workarounds currently go to __v7_setup and many
of them can't be enabled from non-secure mode. You would need to
simulate a lower power mode or reset to briefly disable the MMU.

Usually the workaround (setting implementation defined bits) is needed
for secondary CPUs as well, but at least at that point we have more
information.

-- 
Catalin

Multi-platform, and secure-only ARM errata workarounds

[Russell King - ARM Linux]

On Tue, Feb 26, 2013 at 10:23:26AM +0000, Arnd Bergmann wrote:
> On Monday 25 February 2013, Stephen Warren wrote:
> > Is there any other alternative I'm not seeing? Having the kernel
> > suddenly become incompatible with any currently extant bootloader when I
> > enable CONFIG_MULTIPLATFORM doesn't seem like a great idea.
> 
> Could we make those errata be run-time enabled only when not booting
> in secure mode?

The long and the short answer to this is... no.

1. It is impossible to tell whether we're running secure or non-secure.

2. Errata need to be applied before the MMU is initialized.  We need the
   MMU to be initialized to run any C code what so ever, so calling out
   to platform specific code to set errata is not possible.  Moreover,
   we no longer determine the platform in the assembly code since DT
   came along: this was removed because detecting it in DT from assembly
   is far from trivial (you'd need to write an assembly DT parser).

Now, as for having the secure mode errata enabled on a kernel running in
non-secure mode... what happens today is that we check whether something
before the kernel has enabled the workaround, and we omit to write to
the register.

What that means is that we expect whatever came before the kernel to have
appropriately enabled the bits in the secure registers.  If it hasn't,
and you have one of these secure mode workarounds enabled, the kernel
will fault at boot time.

So, if Stephen has working configs with these secure mode workarounds
enabled, this means that the bits in the secure registers must already
be appropriately set.

Could the problem be that someone has made all errata workarounds, even
those which apply after the system is up and running, depend on
!MULTIPLATFORM ?

Multi-platform, and secure-only ARM errata workarounds

[Rob Herring]

On 02/26/2013 05:35 AM, Russell King - ARM Linux wrote:
> On Tue, Feb 26, 2013 at 10:23:26AM +0000, Arnd Bergmann wrote:
>> On Monday 25 February 2013, Stephen Warren wrote:
>>> Is there any other alternative I'm not seeing? Having the kernel
>>> suddenly become incompatible with any currently extant bootloader when I
>>> enable CONFIG_MULTIPLATFORM doesn't seem like a great idea.
>>
>> Could we make those errata be run-time enabled only when not booting
>> in secure mode?
> 
> The long and the short answer to this is... no.
> 
> 1. It is impossible to tell whether we're running secure or non-secure.
> 
> 2. Errata need to be applied before the MMU is initialized.  We need the
>    MMU to be initialized to run any C code what so ever, so calling out
>    to platform specific code to set errata is not possible.  Moreover,
>    we no longer determine the platform in the assembly code since DT
>    came along: this was removed because detecting it in DT from assembly
>    is far from trivial (you'd need to write an assembly DT parser).
> 
> Now, as for having the secure mode errata enabled on a kernel running in
> non-secure mode... what happens today is that we check whether something
> before the kernel has enabled the workaround, and we omit to write to
> the register.
> 
> What that means is that we expect whatever came before the kernel to have
> appropriately enabled the bits in the secure registers.  If it hasn't,
> and you have one of these secure mode workarounds enabled, the kernel
> will fault at boot time.

Only when booting in non-secure mode...

> So, if Stephen has working configs with these secure mode workarounds
> enabled, this means that the bits in the secure registers must already
> be appropriately set.

...and Stephen is booting in secure mode.

> Could the problem be that someone has made all errata workarounds, even
> those which apply after the system is up and running, depend on
> !MULTIPLATFORM ?

I don't think so. All those work-arounds remain and can be enabled.
There was one (430973) which had both a boot time chicken bit setting
and runtime piece. Only the boot time part of it is disabled for multi-plat.

Rob

Multi-platform, and secure-only ARM errata workarounds

[Stephen Warren]

On 02/26/2013 02:36 AM, Marc Dietrich wrote:
> Stephen,
> 
> Am Montag, 25. Februar 2013, 16:47:38 schrieb Stephen Warren:
>> I'm looking into enabling CONFIG_MULTIPLATFORM on Tegra for 3.10, and
>> the main blocking issue is due to commit 62e4d35 "ARM: 7609/1: disable
>> errata work-arounds which access secure registers". Various Tegra
>> versions need 3 of those workarounds, and our bootloader doesn't
>> implement them (at the least, upstream U-Boot; not sure about our
>> downstream code, but I'm fairly sure given the lack of any feedback I
>> got in the bug I filed to implement them).
>>
>> Now, I can easily add those 3 errata workarounds to U-Boot, but that
>> will require people to reflash their bootloader. This is probably
>> acceptable for development/reference boards (although I'm sure people
>> will find it annoying) but for re-purposed production boards (such as
>> the Toshiba AC100 or various tablets) it will be impossible to update
>> the factory bootloader. Switching to upstream U-Boot would currently
>> lose some functionality, and significantly affect people's boot flow, so
>> is likely unacceptable.
> 
> personally, I have no problem to require a certain u-boot version for a given 
> kernel. From a distro point of view, you will likely update the 
> bootloader/kernel on a distro update anyway.

So a distro will certainly update the kernel.

But updating a bootloader would be very unusual, I believe.

Also, I hope that upstream U-Boot is never going to support the bizarre
"Tegra partition table" cruft that our "fastboot" bootloader supports,
so you'll end up completely re-programming the flash (BCT, bootloader,
partition table, all filesystems) if you want to switch to U-Boot. That
seems like a fair chunk for the installer to own, but I guess if you're
comfortable with doing that, I won't complain.

Multi-platform, and secure-only ARM errata workarounds

[Stephen Warren]

On 02/26/2013 04:35 AM, Russell King - ARM Linux wrote:
> On Tue, Feb 26, 2013 at 10:23:26AM +0000, Arnd Bergmann wrote:
>> On Monday 25 February 2013, Stephen Warren wrote:
>>> Is there any other alternative I'm not seeing? Having the kernel
>>> suddenly become incompatible with any currently extant bootloader when I
>>> enable CONFIG_MULTIPLATFORM doesn't seem like a great idea.
>>
>> Could we make those errata be run-time enabled only when not booting
>> in secure mode?
> 
> The long and the short answer to this is... no.
> 
> 1. It is impossible to tell whether we're running secure or non-secure.
> 
> 2. Errata need to be applied before the MMU is initialized.  We need the
>    MMU to be initialized to run any C code what so ever, so calling out
>    to platform specific code to set errata is not possible.  Moreover,
>    we no longer determine the platform in the assembly code since DT
>    came along: this was removed because detecting it in DT from assembly
>    is far from trivial (you'd need to write an assembly DT parser).
> 
> Now, as for having the secure mode errata enabled on a kernel running in
> non-secure mode... what happens today is that we check whether something
> before the kernel has enabled the workaround, and we omit to write to
> the register.
> 
> What that means is that we expect whatever came before the kernel to have
> appropriately enabled the bits in the secure registers.  If it hasn't,
> and you have one of these secure mode workarounds enabled, the kernel
> will fault at boot time.

The conditional in that statement makes me wonder which of the following
operations will fault in non-secure mode:

1) Reading from the diagnostic register.

2) Writing to the diagnostic register, of a value the same as what's
already there.

3) Writing to the diagnostic register, of a value different than what's
already there.

Would the following not fault in both secure and non-secure mode:

read diagnostic register
if desired bit already set:
    b 1f
set desired bit
write value back to diagnostic register
1:

If so, that would allow a multi-SoC kernel to keep the errata workaround
enabled, and allow the kernel to apply the WAR /if/ booted in secure
mode, but require the errata to be previously enabled if the kernel was
not booted in secure mode.

Multi-platform, and secure-only ARM errata workarounds

[Russell King - ARM Linux]

On Tue, Feb 26, 2013 at 11:01:30AM -0700, Stephen Warren wrote:
> The conditional in that statement makes me wonder which of the following
> operations will fault in non-secure mode:
> 
> 1) Reading from the diagnostic register.

Won't fault.

> 2) Writing to the diagnostic register, of a value the same as what's
> already there.

Will fault.

> 3) Writing to the diagnostic register, of a value different than what's
> already there.

Will fault.

> Would the following not fault in both secure and non-secure mode:
> 
> read diagnostic register
> if desired bit already set:
>     b 1f
> set desired bit
> write value back to diagnostic register
> 1:

That is exactly what we do - the problem is, that if you require
workaround X to be enabled, and a different platform has that errata
fixed, then the other platform will not enable the work-around, and
the bit will be clear.

So, that causes the early boot code to read-check-modify-write the
secure-only register, and the write causes _that_ platform to fail to
boot.

Yes, yours continues to work just fine - but everyone else starts
failing.

Now, factor into that that any particular platform may have a range of
different versions of the ARM CPU core (OMAP CPUs have gone through
various Cortex-A9 CPU revisions) and you'll find that you can't tell
just from the platform information what work-arounds should be enabled.

The whole errata business is a total nightmare - and it gets much worse
with the advent of multi-SoC kernels.  We don't have an answer to this
other than disabling these troublesome errata and requiring the code
which comes before the kernel to do whatever is necessary.

As I've said, the alternative is to encode the errata information into
DT, and then write a DT parser purely in assembler... no one fancies
doing that though.

Multi-platform, and secure-only ARM errata workarounds

[Stephen Warren]

On 02/26/2013 11:11 AM, Russell King - ARM Linux wrote:
> On Tue, Feb 26, 2013 at 11:01:30AM -0700, Stephen Warren wrote:
>> The conditional in that statement makes me wonder which of the following
>> operations will fault in non-secure mode:
>>
>> 1) Reading from the diagnostic register.
> 
> Won't fault.
> 
>> 2) Writing to the diagnostic register, of a value the same as what's
>> already there.
> 
> Will fault.
> 
>> 3) Writing to the diagnostic register, of a value different than what's
>> already there.
> 
> Will fault.
> 
>> Would the following not fault in both secure and non-secure mode:
>>
>> read diagnostic register
>> if desired bit already set:
>>     b 1f
>> set desired bit
>> write value back to diagnostic register
>> 1:
> 
> That is exactly what we do

Well, I asked because for the 3 WARs in question at least, that isn't
what the code does. For example, from proc-v7.s:

#ifdef CONFIG_ARM_ERRATA_742230
	cmp	r6, #0x22		@ only present up to r2p2
	mrcle	p15, 0, r10, c15, c0, 1	@ read diagnostic register
	orrle	r10, r10, #1 << 4	@ set bit #4
	mcrle	p15, 0, r10, c15, c0, 1	@ write diagnostic register
#endif

(unless that orrle affects the flags and hence skips the mcrle, but I
don't think so.)

> - the problem is, that if you require
> workaround X to be enabled, and a different platform has that errata
> fixed, then the other platform will not enable the work-around, and
> the bit will be clear.

The 3 WARs in question are conditional upon the CPU's revision and patch
number (which I'll call rNpN). I assume that any Cortex-A9 with the
affected rNpN would require the WAR enabled; is it possible that someone
could have fixed a particular core bug directly (and hence not changed
rNpN), rather than just updated to a new core revision? If not, the
sequence above should be safe, and avoid all the issues you pointed out.
I guess anything is possible though. Perhaps this depends a lot on how
ARM licenses their cores and fixes and/or if anyone with an
architectural license would have applied that to fix a regular A9 core...

Multi-platform, and secure-only ARM errata workarounds

[Russell King - ARM Linux]

On Tue, Feb 26, 2013 at 11:30:08AM -0700, Stephen Warren wrote:
> On 02/26/2013 11:11 AM, Russell King - ARM Linux wrote:
> > On Tue, Feb 26, 2013 at 11:01:30AM -0700, Stephen Warren wrote:
> >> The conditional in that statement makes me wonder which of the following
> >> operations will fault in non-secure mode:
> >>
> >> 1) Reading from the diagnostic register.
> > 
> > Won't fault.
> > 
> >> 2) Writing to the diagnostic register, of a value the same as what's
> >> already there.
> > 
> > Will fault.
> > 
> >> 3) Writing to the diagnostic register, of a value different than what's
> >> already there.
> > 
> > Will fault.
> > 
> >> Would the following not fault in both secure and non-secure mode:
> >>
> >> read diagnostic register
> >> if desired bit already set:
> >>     b 1f
> >> set desired bit
> >> write value back to diagnostic register
> >> 1:
> > 
> > That is exactly what we do
> 
> Well, I asked because for the 3 WARs in question at least, that isn't
> what the code does. For example, from proc-v7.s:
> 
> #ifdef CONFIG_ARM_ERRATA_742230
> 	cmp	r6, #0x22		@ only present up to r2p2
> 	mrcle	p15, 0, r10, c15, c0, 1	@ read diagnostic register
> 	orrle	r10, r10, #1 << 4	@ set bit #4
> 	mcrle	p15, 0, r10, c15, c0, 1	@ write diagnostic register
> #endif
> 
> (unless that orrle affects the flags and hence skips the mcrle, but I
> don't think so.)

Hmm.  I've not really been taking much notice of how these work-arounds
all work - maybe it's safe to write this diagnostic register from
non-secure mode then?

I have noticed this kind of fishy thing with OMAP4430 running in non-secure
mode - some registers I thought would cause an exception don't.  No idea
why not...

> > - the problem is, that if you require
> > workaround X to be enabled, and a different platform has that errata
> > fixed, then the other platform will not enable the work-around, and
> > the bit will be clear.
> 
> The 3 WARs in question are conditional upon the CPU's revision and patch
> number (which I'll call rNpN). I assume that any Cortex-A9 with the
> affected rNpN would require the WAR enabled; is it possible that someone
> could have fixed a particular core bug directly (and hence not changed
> rNpN),

SoC folk are apparantly free to incorporate fixes into the ARM CPU
when they fabricate it, which means that the rNpN number doesn't
accurately reflect what workarounds are required.

We've had this discussion before - a few months ago - and the conclusion
that was arrived at was that the _only_ way out of this mess is to
require whatever runs _before_ the kernel to enable the appropriate
work-arounds in these apparantly secure-accessible only registers.

We've now been around this problem about three or four times, and we
ultimately end up coming back to the above, and/or talking about
ripping out the errata workarounds from the kernel which just set
bits in the register(s).

Multi-platform, and secure-only ARM errata workarounds

[Nicolas Pitre]

On Tue, 26 Feb 2013, Stephen Warren wrote:

> On 02/26/2013 02:36 AM, Marc Dietrich wrote:
> > Stephen,
> > 
> > Am Montag, 25. Februar 2013, 16:47:38 schrieb Stephen Warren:
> >> I'm looking into enabling CONFIG_MULTIPLATFORM on Tegra for 3.10, and
> >> the main blocking issue is due to commit 62e4d35 "ARM: 7609/1: disable
> >> errata work-arounds which access secure registers". Various Tegra
> >> versions need 3 of those workarounds, and our bootloader doesn't
> >> implement them (at the least, upstream U-Boot; not sure about our
> >> downstream code, but I'm fairly sure given the lack of any feedback I
> >> got in the bug I filed to implement them).
> >>
> >> Now, I can easily add those 3 errata workarounds to U-Boot, but that
> >> will require people to reflash their bootloader. This is probably
> >> acceptable for development/reference boards (although I'm sure people
> >> will find it annoying) but for re-purposed production boards (such as
> >> the Toshiba AC100 or various tablets) it will be impossible to update
> >> the factory bootloader. Switching to upstream U-Boot would currently
> >> lose some functionality, and significantly affect people's boot flow, so
> >> is likely unacceptable.
> > 
> > personally, I have no problem to require a certain u-boot version for a given 
> > kernel. From a distro point of view, you will likely update the 
> > bootloader/kernel on a distro update anyway.
> 
> So a distro will certainly update the kernel.
> 
> But updating a bootloader would be very unusual, I believe.

The bootloader should grow some ability to script those errata 
workarounds.  Most bootloaders already have memory read and write 
commands, so only CP access is missing.


Nicolas

Multi-platform, and secure-only ARM errata workarounds

[Santosh Shilimkar]

On Wednesday 27 February 2013 12:19 AM, Russell King - ARM Linux wrote:
> On Tue, Feb 26, 2013 at 11:30:08AM -0700, Stephen Warren wrote:
>> On 02/26/2013 11:11 AM, Russell King - ARM Linux wrote:
>>> On Tue, Feb 26, 2013 at 11:01:30AM -0700, Stephen Warren wrote:
>>>> The conditional in that statement makes me wonder which of the following
>>>> operations will fault in non-secure mode:
>>>>
>>>> 1) Reading from the diagnostic register.
>>>
>>> Won't fault.
>>>
>>>> 2) Writing to the diagnostic register, of a value the same as what's
>>>> already there.
>>>
>>> Will fault.
>>>
>>>> 3) Writing to the diagnostic register, of a value different than what's
>>>> already there.
>>>
>>> Will fault.
>>>
>>>> Would the following not fault in both secure and non-secure mode:
>>>>
>>>> read diagnostic register
>>>> if desired bit already set:
>>>>      b 1f
>>>> set desired bit
>>>> write value back to diagnostic register
>>>> 1:
>>>
>>> That is exactly what we do
>>
>> Well, I asked because for the 3 WARs in question at least, that isn't
>> what the code does. For example, from proc-v7.s:
>>
>> #ifdef CONFIG_ARM_ERRATA_742230
>> 	cmp	r6, #0x22		@ only present up to r2p2
>> 	mrcle	p15, 0, r10, c15, c0, 1	@ read diagnostic register
>> 	orrle	r10, r10, #1 << 4	@ set bit #4
>> 	mcrle	p15, 0, r10, c15, c0, 1	@ write diagnostic register
>> #endif
>>
>> (unless that orrle affects the flags and hence skips the mcrle, but I
>> don't think so.)
>
> Hmm.  I've not really been taking much notice of how these work-arounds
> all work - maybe it's safe to write this diagnostic register from
> non-secure mode then?
>
> I have noticed this kind of fishy thing with OMAP4430 running in non-secure
> mode - some registers I thought would cause an exception don't.  No idea
> why not...
>
They do fault on OMAP. We discussed the issue in the past [1] [2].
The only way we could get around is to disable those WA flags in config.
I was told to move such requirements to boot-loaders then

Regards,
Santosh

[1] https://patchwork.kernel.org/patch/1743211/
[2] https://lkml.org/lkml/2012/12/10/321

Multi-platform, and secure-only ARM errata workarounds

[Marc Dietrich]

Am Dienstag, 26. Februar 2013, 09:39:15 schrieb Stephen Warren:
> On 02/26/2013 02:36 AM, Marc Dietrich wrote:
> > Am Montag, 25. Februar 2013, 16:47:38 schrieb Stephen Warren:
> >> ...
> >> Now, I can easily add those 3 errata workarounds to U-Boot, but that
> >> will require people to reflash their bootloader. This is probably
> >> acceptable for development/reference boards (although I'm sure people
> >> will find it annoying) but for re-purposed production boards (such as
> >> the Toshiba AC100 or various tablets) it will be impossible to update
> >> the factory bootloader. Switching to upstream U-Boot would currently
> >> lose some functionality, and significantly affect people's boot flow, so
> >> is likely unacceptable.
> > 
> > personally, I have no problem to require a certain u-boot version for a
> > given kernel. From a distro point of view, you will likely update the
> > bootloader/kernel on a distro update anyway.
> 
> So a distro will certainly update the kernel.
> 
> But updating a bootloader would be very unusual, I believe.

mmh? Every time I update to a new distro release, the bootloader gets also 
updated - even on arm, e.g. ftp://ports.ubuntu.com/ubuntu-ports/pool/main/u/u-boot-linaro lists four version of uboot - one for each supported distro 
release. I know for closed embedded device this is different, but that's not 
our target.

> Also, I hope that upstream U-Boot is never going to support the bizarre
> "Tegra partition table" cruft that our "fastboot" bootloader supports,
> so you'll end up completely re-programming the flash (BCT, bootloader,
> partition table, all filesystems) if you want to switch to U-Boot. That
> seems like a fair chunk for the installer to own, but I guess if you're
> comfortable with doing that, I won't complain.

Yes, that a huge pile of work and people are working on this already. One the 
other hand, why (if not for upgrade use) do we have u-boot support for all 
these legacy boards?

Marc

Multi-platform, and secure-only ARM errata workarounds

[Rob Herring]

On 02/27/2013 03:03 AM, Marc Dietrich wrote:
> Am Dienstag, 26. Februar 2013, 09:39:15 schrieb Stephen Warren:
>> On 02/26/2013 02:36 AM, Marc Dietrich wrote:
>>> Am Montag, 25. Februar 2013, 16:47:38 schrieb Stephen Warren:
>>>> ...
>>>> Now, I can easily add those 3 errata workarounds to U-Boot, but that
>>>> will require people to reflash their bootloader. This is probably
>>>> acceptable for development/reference boards (although I'm sure people
>>>> will find it annoying) but for re-purposed production boards (such as
>>>> the Toshiba AC100 or various tablets) it will be impossible to update
>>>> the factory bootloader. Switching to upstream U-Boot would currently
>>>> lose some functionality, and significantly affect people's boot flow, so
>>>> is likely unacceptable.
>>>
>>> personally, I have no problem to require a certain u-boot version for a
>>> given kernel. From a distro point of view, you will likely update the
>>> bootloader/kernel on a distro update anyway.
>>
>> So a distro will certainly update the kernel.
>>
>> But updating a bootloader would be very unusual, I believe.
> 
> mmh? Every time I update to a new distro release, the bootloader gets also 
> updated - even on arm, e.g. ftp://ports.ubuntu.com/ubuntu-ports/pool/main/u/u-boot-linaro lists four version of uboot - one for each supported distro 
> release. I know for closed embedded device this is different, but that's not 
> our target.

There is a package of u-boot bootloaders, but they are not installed by
the OS. On ubuntu, installing a kernel only writes the boot.scr script.
There is an assumption that u-boot will go and read this. So we already
have some requirements on u-boot which would require at least a u-boot
environment update. I guess updating the environment is easier than
u-boot itself, but that probably depends on the platform.

Rob

Multi-platform, and secure-only ARM errata workarounds

[Stephen Warren]

On 02/27/2013 02:03 AM, Marc Dietrich wrote:
> Am Dienstag, 26. Februar 2013, 09:39:15 schrieb Stephen Warren:
>> On 02/26/2013 02:36 AM, Marc Dietrich wrote:
>>> Am Montag, 25. Februar 2013, 16:47:38 schrieb Stephen Warren:
>>>> ...
>>>> Now, I can easily add those 3 errata workarounds to U-Boot, but that
>>>> will require people to reflash their bootloader. This is probably
>>>> acceptable for development/reference boards (although I'm sure people
>>>> will find it annoying) but for re-purposed production boards (such as
>>>> the Toshiba AC100 or various tablets) it will be impossible to update
>>>> the factory bootloader. Switching to upstream U-Boot would currently
>>>> lose some functionality, and significantly affect people's boot flow, so
>>>> is likely unacceptable.
>>>
>>> personally, I have no problem to require a certain u-boot version for a
>>> given kernel. From a distro point of view, you will likely update the
>>> bootloader/kernel on a distro update anyway.
>>
>> So a distro will certainly update the kernel.
>>
>> But updating a bootloader would be very unusual, I believe.
> 
> mmh? Every time I update to a new distro release, the bootloader gets also 
> updated - even on arm, e.g. ftp://ports.ubuntu.com/ubuntu-ports/pool/main/u/u-boot-linaro lists four version of uboot - one for each supported distro 
> release. I know for closed embedded device this is different, but that's not 
> our target.

I have no idea why that package exists or what it is. It clearly doesn't
install U-Boot on the device when the package is installed, or anyone
using an AC100 would already be running U-Boot not our binary
bootloader, right?

>> Also, I hope that upstream U-Boot is never going to support the bizarre
>> "Tegra partition table" cruft that our "fastboot" bootloader supports,
>> so you'll end up completely re-programming the flash (BCT, bootloader,
>> partition table, all filesystems) if you want to switch to U-Boot. That
>> seems like a fair chunk for the installer to own, but I guess if you're
>> comfortable with doing that, I won't complain.
> 
> Yes, that a huge pile of work and people are working on this already. One the 
> other hand, why (if not for upgrade use) do we have u-boot support for all 
> these legacy boards?

Mainly because I didn't want to use our binary bootloader when
developing or testing kernel support for the AC100, I think:-)

But yes, I would support all AC100 general Linux usage switching to U-Boot.

I think what I'll do when Tegra is converted to multi-platform is:

a) The WARs will be disabled in the kernel; no choice there really.

b) I've already submitted patches to U-Boot to implement the WARs there.

c) Everyone will need to update to the latest U-Boot:-(

d) For people wanting to continue running our binary bootloader (which
only implements 1 of the 3 WARs in question in the latest code, and I
have no idea if it even did that when the AC100 bootloader binary was
compiled), they'll need to create a small stub binary that implements
the WARs and prepend this to the zImage. I can help create this if
people need. Or, you can chain-load U-Boot. I've filed bugs against our
binary bootloader, so hopefully within a few years or something, new
products won't need this stub!

Multi-platform, and secure-only ARM errata workarounds

[Marc Dietrich]

Am Mittwoch, 27. Februar 2013, 10:42:27 schrieb Stephen Warren:
> On 02/27/2013 02:03 AM, Marc Dietrich wrote:
> > Am Dienstag, 26. Februar 2013, 09:39:15 schrieb Stephen Warren:
> >> On 02/26/2013 02:36 AM, Marc Dietrich wrote:
> >>> Am Montag, 25. Februar 2013, 16:47:38 schrieb Stephen Warren:
> >>>> ...
> >>>> Now, I can easily add those 3 errata workarounds to U-Boot, but that
> >>>> will require people to reflash their bootloader. This is probably
> >>>> acceptable for development/reference boards (although I'm sure people
> >>>> will find it annoying) but for re-purposed production boards (such as
> >>>> the Toshiba AC100 or various tablets) it will be impossible to update
> >>>> the factory bootloader. Switching to upstream U-Boot would currently
> >>>> lose some functionality, and significantly affect people's boot flow,
> >>>> so
> >>>> is likely unacceptable.
> >>> 
> >>> personally, I have no problem to require a certain u-boot version for a
> >>> given kernel. From a distro point of view, you will likely update the
> >>> bootloader/kernel on a distro update anyway.
> >> 
> >> So a distro will certainly update the kernel.
> >> 
> >> But updating a bootloader would be very unusual, I believe.
> > 
> > mmh? Every time I update to a new distro release, the bootloader gets also
> > updated - even on arm, e.g.
> > ftp://ports.ubuntu.com/ubuntu-ports/pool/main/u/u-boot-linaro lists four
> > version of uboot - one for each supported distro release. I know for
> > closed embedded device this is different, but that's not our target.
> 
> I have no idea why that package exists or what it is. It clearly doesn't
> install U-Boot on the device when the package is installed, or anyone
> using an AC100 would already be running U-Boot not our binary
> bootloader, right?

yes, you are right - it's left to user to break its board. I somehow missed 
this.
 
> >> Also, I hope that upstream U-Boot is never going to support the bizarre
> >> "Tegra partition table" cruft that our "fastboot" bootloader supports,
> >> so you'll end up completely re-programming the flash (BCT, bootloader,
> >> partition table, all filesystems) if you want to switch to U-Boot. That
> >> seems like a fair chunk for the installer to own, but I guess if you're
> >> comfortable with doing that, I won't complain.
> > 
> > Yes, that a huge pile of work and people are working on this already. One
> > the other hand, why (if not for upgrade use) do we have u-boot support
> > for all these legacy boards?
> 
> Mainly because I didn't want to use our binary bootloader when
> developing or testing kernel support for the AC100, I think:-)
> 
> But yes, I would support all AC100 general Linux usage switching to U-Boot.
> 
> I think what I'll do when Tegra is converted to multi-platform is:
> 
> a) The WARs will be disabled in the kernel; no choice there really.
> 
> b) I've already submitted patches to U-Boot to implement the WARs there.
> 
> c) Everyone will need to update to the latest U-Boot:-(
> 
> d) For people wanting to continue running our binary bootloader (which
> only implements 1 of the 3 WARs in question in the latest code, and I
> have no idea if it even did that when the AC100 bootloader binary was
> compiled), they'll need to create a small stub binary that implements
> the WARs and prepend this to the zImage. I can help create this if
> people need. Or, you can chain-load U-Boot. I've filed bugs against our
> binary bootloader, so hopefully within a few years or something, new
> products won't need this stub!

I don't want to drive this thread further off topic. I just like to add that 
the AC100 support is purly community driven. It is planed that we can use both 
bootloaders (fastboot and u-boot) with a 3.1 kernel (downstream nvidia based). 
In this situation, the user can upgrade his bootloader to u-boot and later 
install a 3.8 kernel based on mainline. At 3.10 kernel version times, all 
users are expected to have u-boot installed, whatever u-boot version is 
required then.

Marc

Multi-platform, and secure-only ARM errata workarounds

[Stephen Warren]

On 02/26/2013 11:11 AM, Russell King - ARM Linux wrote:
...
> The whole errata business is a total nightmare - and it gets much worse
> with the advent of multi-SoC kernels.  We don't have an answer to this
> other than disabling these troublesome errata and requiring the code
> which comes before the kernel to do whatever is necessary.

So for the initial boot, I can see that the bootloader can apply any
WARs that are required, irrespective of whether the code that comes
before the kernel is a secure monitor, a regular bootloader and/or
whether the kernel ends up running in secure or normal world.

I have one question on this case though: For erratum 751472 (An
interrupted ICIALLUIS operation may prevent the completion of a
following broadcasted operation), one of our engineers asserts:

This is valid only for SMP configurations and since bootloader has only
one CPU (CPU0) up and running, this is not valid for Bootloader.

Is that assertion correct? I assume that the WAR can be enabled
irrespective of whether SMP is actively in use, and shouldn't negatively
affect single-CPU operation in the bootloader. Hence, the bootloader or
secure monitor should always apply this WAR.

Now on to other scenarios: What about booting secondary CPUs, in cases
such as: initial kernel boot, CPU hotplug, CPU power saving.

Since some of the bits that enable WARs are banked per CPU, the WAR
needs to be enabled by code running on each individual CPU, each time
it's powered on. When a secure monitor exists, the CPU will boot through
it (at least on Tegra, there is a single register that defines the boot
vector for all CPUs; I don't know if that fact is ARM-architectural or
not), so the secure monitor can apply the WAR if needed. However, when
there is no secure monitor and the kernel runs in secure world, the
kernel would have to apply those WARs, since the only code that runs is
in the kernel.

But that leads to the question: How does the secondary CPU boot vector
code in the kernel know whether it can/should apply the WARs? It only
can if the kernel is running in secure mode, and that isn't always the
case, and there's no easy way to detect this at run-time. (It usually is
with any upstream SW, but we presumably want to support running the
upstream kernel on boards that were repurposed and have a secure monitor).

Apparently, the solution we have downstream is a Kconfig option that
selects whether the kernel should support running in secure world or
normal world, and hence whether to apply the WARs or not. Obviously that
won't work well with a multi-platform kernel.

The solutions that come to mind are:

1)

Assume that Tegra kernels always run in secure world, and so the WARs
can be applied in the Tegra-specific secondary CPU boot code. The
bootloader would still have to apply WARs for the initial CPU0 boot,
since the kernel code for that is common, so we can't assume we're
running in secure mode there. I guess that upstream we have no support
for running under a secure monitor on Tegra yet though, so perhaps this
isn't so bad (e.g. we don't make an SMC call to set the CPU boot vector,
but rather just write to the register directly). Still, making this
assumption would only be a temporary solution until we actually do
support running under a secure monitor upstream.

2)

Assume that we're always running in normal world, and mandate that a
secure monitor must exist, and push the WARs into it. That would require
implementing a simple secure monitor and using it for the cases where
one otherwise wouldn't exist.

Neither seems entirely ideal.

Perhaps one more option: I wonder if we can play games like reading the
CPU boot vector register; if it's equal to the kernel's value then we
must have been running in secure mode since we wrote it, but otherwise
we must have booted through a secure monitor?

Multi-platform, and secure-only ARM errata workarounds

[Russell King - ARM Linux]

On Fri, Mar 01, 2013 at 10:37:27AM -0700, Stephen Warren wrote:
> I have one question on this case though: For erratum 751472 (An
> interrupted ICIALLUIS operation may prevent the completion of a
> following broadcasted operation), one of our engineers asserts:
> 
> This is valid only for SMP configurations and since bootloader has only
> one CPU (CPU0) up and running, this is not valid for Bootloader.
> 
> Is that assertion correct? I assume that the WAR can be enabled
> irrespective of whether SMP is actively in use, and shouldn't negatively
> affect single-CPU operation in the bootloader. Hence, the bootloader or
> secure monitor should always apply this WAR.

I'd say no, for three reasons:

1. It requires no run-time workaround - by that I mean, it doesn't require
   software modification of the ICIALLUIS operation.

2. It only requires a bit set in a secure-only-accessible diagnostic
   register before the MMU is brought online.

3. This is the cruncher for this erratum - when we bring up a SMP system,
   we need this erratum applied before the caches and MMU are enabled.
   Why?  Because when we enable the caches and MMU, we have to then be
   able to reliably issue the ICIALLUIS operation.

   We know that we can't run platform specific code before these are
   brought up, so we can't go running SMC instructions this early on
   from the kernel code.

   As previously pointed out, having the work-around enabled in the
   kernel may not be appropriate for other platforms because even though
   they have a pre-r3p0 CPU, they may have fixed the problem in their
   silicon.

Therefore, for all of the above reasons, we _can't_ have this in a kernel
designed to run on multiple different platforms.

> Now on to other scenarios: What about booting secondary CPUs, in cases
> such as: initial kernel boot, CPU hotplug, CPU power saving.
> 
> Since some of the bits that enable WARs are banked per CPU, the WAR
> needs to be enabled by code running on each individual CPU, each time
> it's powered on. When a secure monitor exists, the CPU will boot through
> it (at least on Tegra, there is a single register that defines the boot
> vector for all CPUs; I don't know if that fact is ARM-architectural or
> not), so the secure monitor can apply the WAR if needed. However, when
> there is no secure monitor and the kernel runs in secure world, the
> kernel would have to apply those WARs, since the only code that runs is
> in the kernel.
> 
> But that leads to the question: How does the secondary CPU boot vector
> code in the kernel know whether it can/should apply the WARs? It only
> can if the kernel is running in secure mode, and that isn't always the
> case, and there's no easy way to detect this at run-time. (It usually is
> with any upstream SW, but we presumably want to support running the
> upstream kernel on boards that were repurposed and have a secure monitor).
> 
> Apparently, the solution we have downstream is a Kconfig option that
> selects whether the kernel should support running in secure world or
> normal world, and hence whether to apply the WARs or not. Obviously that
> won't work well with a multi-platform kernel.

I think what you're digging up here is one of the biggest obstacles we
still have, created through a "freedom of design" and the desire by
distros to have a single kernel image booting on multiple platforms.

> The solutions that come to mind are:
> 
> 1)
> 
> Assume that Tegra kernels always run in secure world, and so the WARs
> can be applied in the Tegra-specific secondary CPU boot code. The
> bootloader would still have to apply WARs for the initial CPU0 boot,
> since the kernel code for that is common, so we can't assume we're
> running in secure mode there. I guess that upstream we have no support
> for running under a secure monitor on Tegra yet though, so perhaps this
> isn't so bad (e.g. we don't make an SMC call to set the CPU boot vector,
> but rather just write to the register directly). Still, making this
> assumption would only be a temporary solution until we actually do
> support running under a secure monitor upstream.

This has the advantage that you (in theory) know which work-arounds are
appropriate for the revisions of CPU cores that you have on your platform.
So, although this would mean duplicating the erratum throughout the
platform code (which is distasteful) it would nevertheless work.

> 2)
> 
> Assume that we're always running in normal world, and mandate that a
> secure monitor must exist, and push the WARs into it. That would require
> implementing a simple secure monitor and using it for the cases where
> one otherwise wouldn't exist.

I think that's a very complex solution, and more fragile than just
duplicating the errata.

> Perhaps one more option: I wonder if we can play games like reading the
> CPU boot vector register; if it's equal to the kernel's value then we
> must have been running in secure mode since we wrote it, but otherwise
> we must have booted through a secure monitor?

That sounds platform specific again... I know that OMAP has to talk to
the secure world to tell it how to boot the secondary CPUs, so this
solution is specific to Tegra.

Multi-platform, and secure-only ARM errata workarounds

[Peter De Schrijver]

On Fri, Mar 01, 2013 at 06:37:27PM +0100, Stephen Warren wrote:

...

> Since some of the bits that enable WARs are banked per CPU, the WAR
> needs to be enabled by code running on each individual CPU, each time
> it's powered on. When a secure monitor exists, the CPU will boot through
> it (at least on Tegra, there is a single register that defines the boot
> vector for all CPUs; I don't know if that fact is ARM-architectural or
> not), so the secure monitor can apply the WAR if needed. However, when
> there is no secure monitor and the kernel runs in secure world, the
> kernel would have to apply those WARs, since the only code that runs is
> in the kernel.
> 

The boot vector register is Tegra specific. At least on OMAP all cores boot
from ROM afaik.

Cheers,

Peter.

Multi-platform, and secure-only ARM errata workarounds

[Peter De Schrijver]

On Mon, Mar 04, 2013 at 07:34:36AM +0100, Peter De Schrijver wrote:
> On Fri, Mar 01, 2013 at 06:37:27PM +0100, Stephen Warren wrote:
> 
> ...
> 
> > Since some of the bits that enable WARs are banked per CPU, the WAR
> > needs to be enabled by code running on each individual CPU, each time
> > it's powered on. When a secure monitor exists, the CPU will boot through
> > it (at least on Tegra, there is a single register that defines the boot
> > vector for all CPUs; I don't know if that fact is ARM-architectural or
> > not), so the secure monitor can apply the WAR if needed. However, when
> > there is no secure monitor and the kernel runs in secure world, the
> > kernel would have to apply those WARs, since the only code that runs is
> > in the kernel.
> > 
> 
> The boot vector register is Tegra specific. At least on OMAP all cores boot
> from ROM afaik.

Nicolas Pitre suggested a slightly different solution to me:

1) Handle CPU0 errata WARs in the bootloader
2) Indicate in device tree if linux is booting in secude mode or non-secure
   mode.
3) Use this information in the kernel to decide how to apply the WARs for
   secondary core bringup and after powerungating.

Cheers,

Peter.

Multi-platform, and secure-only ARM errata workarounds

[Stephen Warren]

On 03/04/2013 02:16 AM, Peter De Schrijver wrote:
> On Mon, Mar 04, 2013 at 07:34:36AM +0100, Peter De Schrijver wrote:
>> On Fri, Mar 01, 2013 at 06:37:27PM +0100, Stephen Warren wrote:
>>
>> ...
>>
>>> Since some of the bits that enable WARs are banked per CPU, the WAR
>>> needs to be enabled by code running on each individual CPU, each time
>>> it's powered on. When a secure monitor exists, the CPU will boot through
>>> it (at least on Tegra, there is a single register that defines the boot
>>> vector for all CPUs; I don't know if that fact is ARM-architectural or
>>> not), so the secure monitor can apply the WAR if needed. However, when
>>> there is no secure monitor and the kernel runs in secure world, the
>>> kernel would have to apply those WARs, since the only code that runs is
>>> in the kernel.
>>>
>>
>> The boot vector register is Tegra specific. At least on OMAP all cores boot
>> from ROM afaik.
> 
> Nicolas Pitre suggested a slightly different solution to me:
> 
> 1) Handle CPU0 errata WARs in the bootloader

OK - there's not much choice here, and I've posted a patch for this for
Tegra U-Boot already.

> 2) Indicate in device tree if linux is booting in secude mode or non-secure
>    mode.
> 3) Use this information in the kernel to decide how to apply the WARs for
>    secondary core bringup and after powerungating.

Hmmm. That seems like a lot of overhead to avoid duplicating roughly 8
assembly instructions per Tegra version. Also, some/all of the WARs in
question probably need to be applied very early by assembly code, e.g.
before MMU is re-enabled, so I think you'd end up parsing DT from
assembly again, which would be painful. I tend to think just including
the code in the kernel's SoC-specific reset handler is simplest, and
even with the slight duplication, probably most maintainable. I've
written a patch for this for Tegra already, which I hope to post later
today, depending on testing and what other stuff I get side-tracked on.

Multi-platform, and secure-only ARM errata workarounds

[Peter De Schrijver]

On Mon, Mar 04, 2013 at 06:08:27PM +0100, Stephen Warren wrote:
> On 03/04/2013 02:16 AM, Peter De Schrijver wrote:
> > On Mon, Mar 04, 2013 at 07:34:36AM +0100, Peter De Schrijver wrote:
> >> On Fri, Mar 01, 2013 at 06:37:27PM +0100, Stephen Warren wrote:
> >>

...

> > 1) Handle CPU0 errata WARs in the bootloader
> 
> OK - there's not much choice here, and I've posted a patch for this for
> Tegra U-Boot already.
> 
> > 2) Indicate in device tree if linux is booting in secude mode or non-secure
> >    mode.
> > 3) Use this information in the kernel to decide how to apply the WARs for
> >    secondary core bringup and after powerungating.
> 
> Hmmm. That seems like a lot of overhead to avoid duplicating roughly 8
> assembly instructions per Tegra version. Also, some/all of the WARs in

Unfortunately we can't write to the diag register if we are in non-secure
mode. So unless we never want to support running in non-secure mode, we will
need to make the distinction somehow and use a different method for non-secure
mode. Or assume the secure OS has applied the WARs. I'm afraid existing secure
OS implementations for Tegra don't work that way though. They just offer an
SMC which allows the kernel to read and write the diag register.

> question probably need to be applied very early by assembly code, e.g.
> before MMU is re-enabled, so I think you'd end up parsing DT from
> assembly again, which would be painful. I tend to think just including
> the code in the kernel's SoC-specific reset handler is simplest, and
> even with the slight duplication, probably most maintainable. I've
> written a patch for this for Tegra already, which I hope to post later
> today, depending on testing and what other stuff I get side-tracked on.

No. We could just set a flag in __tegra_cpu_reset_handler_data based on the
info in DT or use a different reset handler. DT is parsed before bringing up
secondary CPUs, so this approach should work I think.

Cheers,

Peter.

Multi-platform, and secure-only ARM errata workarounds

[Stephen Warren]

On 03/05/2013 12:40 AM, Peter De Schrijver wrote:
> On Mon, Mar 04, 2013 at 06:08:27PM +0100, Stephen Warren wrote:
>> On 03/04/2013 02:16 AM, Peter De Schrijver wrote:
>>> On Mon, Mar 04, 2013 at 07:34:36AM +0100, Peter De Schrijver wrote:
>>>> On Fri, Mar 01, 2013 at 06:37:27PM +0100, Stephen Warren wrote:
>>>>
> 
> ...
> 
>>> 1) Handle CPU0 errata WARs in the bootloader
>>
>> OK - there's not much choice here, and I've posted a patch for this for
>> Tegra U-Boot already.
>>
>>> 2) Indicate in device tree if linux is booting in secude mode or non-secure
>>>    mode.
>>> 3) Use this information in the kernel to decide how to apply the WARs for
>>>    secondary core bringup and after powerungating.
>>
>> Hmmm. That seems like a lot of overhead to avoid duplicating roughly 8
>> assembly instructions per Tegra version. Also, some/all of the WARs in
> 
> Unfortunately we can't write to the diag register if we are in non-secure
> mode. So unless we never want to support running in non-secure mode, we will
> need to make the distinction somehow and use a different method for non-secure
> mode. Or assume the secure OS has applied the WARs.

Yes. The secure OS really has to have enabled the appropriate WARs
before jumping into the kernel's reset vector. If/when we support the
upstream kernel running on Tegra in non-secure mode, the plan was to use
a Tegra-specific mechanism to detect secure-vs-normal mode in the Tegra
reset vector, and skip the application of secure-only WARs based on that.

> I'm afraid existing secure
> OS implementations for Tegra don't work that way though. They just offer an
> SMC which allows the kernel to read and write the diag register.

I had a downstream discussion about this, and Bo Yan said someone had
verified this was working correctly for at least for some WARs on some
CPUs and for the one particular secure OS we're using.

I think it's reasonable to require a fixed secure OS (i.e. one that
correctly enables any required WARs) be used with any upstream kernel,
since running in normal world would be a new feature that we'd be
supporting.

An SMC to read/write the diag register sounds the opposite of secure...

>> question probably need to be applied very early by assembly code, e.g.
>> before MMU is re-enabled, so I think you'd end up parsing DT from
>> assembly again, which would be painful. I tend to think just including
>> the code in the kernel's SoC-specific reset handler is simplest, and
>> even with the slight duplication, probably most maintainable. I've
>> written a patch for this for Tegra already, which I hope to post later
>> today, depending on testing and what other stuff I get side-tracked on.
> 
> No. We could just set a flag in __tegra_cpu_reset_handler_data based on the
> info in DT or use a different reset handler. DT is parsed before bringing up
> secondary CPUs, so this approach should work I think.

Yes, that could work.

Multi-platform, and secure-only ARM errata workarounds

[Peter De Schrijver]

On Tue, Mar 05, 2013 at 06:00:26PM +0100, Stephen Warren wrote:
> On 03/05/2013 12:40 AM, Peter De Schrijver wrote:
> > On Mon, Mar 04, 2013 at 06:08:27PM +0100, Stephen Warren wrote:
> >> On 03/04/2013 02:16 AM, Peter De Schrijver wrote:
> >>> On Mon, Mar 04, 2013 at 07:34:36AM +0100, Peter De Schrijver wrote:
> >>>> On Fri, Mar 01, 2013 at 06:37:27PM +0100, Stephen Warren wrote:
> >>>>

> > Unfortunately we can't write to the diag register if we are in non-secure
> > mode. So unless we never want to support running in non-secure mode, we will
> > need to make the distinction somehow and use a different method for non-secure
> > mode. Or assume the secure OS has applied the WARs.
> 
> Yes. The secure OS really has to have enabled the appropriate WARs
> before jumping into the kernel's reset vector. If/when we support the
> upstream kernel running on Tegra in non-secure mode, the plan was to use
> a Tegra-specific mechanism to detect secure-vs-normal mode in the Tegra
> reset vector, and skip the application of secure-only WARs based on that.
> 

Ok. If we have such a mechanism, that works too ofcourse. I was under the
impression there is no way to know if you're running in secure mode
or non-secure mode.

> > I'm afraid existing secure
> > OS implementations for Tegra don't work that way though. They just offer an
> > SMC which allows the kernel to read and write the diag register.
> 
> I had a downstream discussion about this, and Bo Yan said someone had
> verified this was working correctly for at least for some WARs on some
> CPUs and for the one particular secure OS we're using.
> 

Ok. That should be good enough indeed.

> I think it's reasonable to require a fixed secure OS (i.e. one that
> correctly enables any required WARs) be used with any upstream kernel,
> since running in normal world would be a new feature that we'd be
> supporting.
> 

Sure. But it would be nice if we can support existing systems which have a
secure OS we can't change.

> An SMC to read/write the diag register sounds the opposite of secure...
> 

GP OMAP devices provide this, but then again they aren't meant to be secure
even though they run linx in non-secure mode...

Cheers,

Peter.

Multi-platform, and secure-only ARM errata workarounds

[Stephen Warren]

On 03/06/2013 01:14 AM, Peter De Schrijver wrote:
...
> I was under the
> impression there is no way to know if you're running in secure mode
> or non-secure mode.

There's no ARM-architected way. Given Tegra's implementation, there are
apparently some registers that behave differently that we can use (e.g.
read value values in secure mode, 0xffffffff in non-secure etc.)

Multi-platform, and secure-only ARM errata workarounds

[Santosh Shilimkar]

On Tuesday 05 March 2013 10:30 PM, Stephen Warren wrote:
> On 03/05/2013 12:40 AM, Peter De Schrijver wrote:
>> On Mon, Mar 04, 2013 at 06:08:27PM +0100, Stephen Warren wrote:
>>> On 03/04/2013 02:16 AM, Peter De Schrijver wrote:
>>>> On Mon, Mar 04, 2013 at 07:34:36AM +0100, Peter De Schrijver wrote:
>>>>> On Fri, Mar 01, 2013 at 06:37:27PM +0100, Stephen Warren wrote:
>>>>>
>>
>> ...
>>
>>>> 1) Handle CPU0 errata WARs in the bootloader
>>>
>>> OK - there's not much choice here, and I've posted a patch for this for
>>> Tegra U-Boot already.
>>>
>>>> 2) Indicate in device tree if linux is booting in secude mode or non-secure
>>>>    mode.
>>>> 3) Use this information in the kernel to decide how to apply the WARs for
>>>>    secondary core bringup and after powerungating.
>>>
>>> Hmmm. That seems like a lot of overhead to avoid duplicating roughly 8
>>> assembly instructions per Tegra version. Also, some/all of the WARs in
>>
>> Unfortunately we can't write to the diag register if we are in non-secure
>> mode. So unless we never want to support running in non-secure mode, we will
>> need to make the distinction somehow and use a different method for non-secure
>> mode. Or assume the secure OS has applied the WARs.
> 
> Yes. The secure OS really has to have enabled the appropriate WARs
> before jumping into the kernel's reset vector. If/when we support the
> upstream kernel running on Tegra in non-secure mode, the plan was to use
> a Tegra-specific mechanism to detect secure-vs-normal mode in the Tegra
> reset vector, and skip the application of secure-only WARs based on that.
> 
>> I'm afraid existing secure
>> OS implementations for Tegra don't work that way though. They just offer an
>> SMC which allows the kernel to read and write the diag register.
> 
> I had a downstream discussion about this, and Bo Yan said someone had
> verified this was working correctly for at least for some WARs on some
> CPUs and for the one particular secure OS we're using.
> 
> I think it's reasonable to require a fixed secure OS (i.e. one that
> correctly enables any required WARs) be used with any upstream kernel,
> since running in normal world would be a new feature that we'd be
> supporting.
> 
> An SMC to read/write the diag register sounds the opposite of secure...
> 
>>> question probably need to be applied very early by assembly code, e.g.
>>> before MMU is re-enabled, so I think you'd end up parsing DT from
>>> assembly again, which would be painful. I tend to think just including
>>> the code in the kernel's SoC-specific reset handler is simplest, and
>>> even with the slight duplication, probably most maintainable. I've
>>> written a patch for this for Tegra already, which I hope to post later
>>> today, depending on testing and what other stuff I get side-tracked on.
>>
>> No. We could just set a flag in __tegra_cpu_reset_handler_data based on the
>> info in DT or use a different reset handler. DT is parsed before bringing up
>> secondary CPUs, so this approach should work I think.
> 
> Yes, that could work.
> 
It might work for few but it isn't an alternative which is maintainable.

Olof proposed to have a common code which can be executed before kernel boot
in recent Linaro connect multi-platform discussion.
Though there was no conclusion on where this file can be part of kernel source
tree. Just form errata WA version control perspective, its is best to have such
errata WA as part of kernel code instead of spreading it over boot-loader, kernel
and firmware/sleep code.

Some of these errata's needs to be enabled before MMU gets enabled and as
discussed in past, we can still get this with 1:1 mapping code once
the SOCs are detected. This can potentially avoid the difficult patching
part for boot CPU which is proposed to be handled in boot-loader. We can
then potentially let the secondary CPUs also execute the same code before
getting into secondary_startup()

And if possible also re-use this code for CPU sleep code as well though
this one isn't must have since this part is already manageable in platform
sleep code.

Regards,
Santosh

Multi-platform, and secure-only ARM errata workarounds

[Olof Johansson]

On Sun, Mar 10, 2013 at 10:25 AM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> On Tuesday 05 March 2013 10:30 PM, Stephen Warren wrote:
>> On 03/05/2013 12:40 AM, Peter De Schrijver wrote:
>>> On Mon, Mar 04, 2013 at 06:08:27PM +0100, Stephen Warren wrote:
>>>> On 03/04/2013 02:16 AM, Peter De Schrijver wrote:
>>>>> On Mon, Mar 04, 2013 at 07:34:36AM +0100, Peter De Schrijver wrote:
>>>>>> On Fri, Mar 01, 2013 at 06:37:27PM +0100, Stephen Warren wrote:
>>>>>>
>>>
>>> ...
>>>
>>>>> 1) Handle CPU0 errata WARs in the bootloader
>>>>
>>>> OK - there's not much choice here, and I've posted a patch for this for
>>>> Tegra U-Boot already.
>>>>
>>>>> 2) Indicate in device tree if linux is booting in secude mode or non-secure
>>>>>    mode.
>>>>> 3) Use this information in the kernel to decide how to apply the WARs for
>>>>>    secondary core bringup and after powerungating.
>>>>
>>>> Hmmm. That seems like a lot of overhead to avoid duplicating roughly 8
>>>> assembly instructions per Tegra version. Also, some/all of the WARs in
>>>
>>> Unfortunately we can't write to the diag register if we are in non-secure
>>> mode. So unless we never want to support running in non-secure mode, we will
>>> need to make the distinction somehow and use a different method for non-secure
>>> mode. Or assume the secure OS has applied the WARs.
>>
>> Yes. The secure OS really has to have enabled the appropriate WARs
>> before jumping into the kernel's reset vector. If/when we support the
>> upstream kernel running on Tegra in non-secure mode, the plan was to use
>> a Tegra-specific mechanism to detect secure-vs-normal mode in the Tegra
>> reset vector, and skip the application of secure-only WARs based on that.
>>
>>> I'm afraid existing secure
>>> OS implementations for Tegra don't work that way though. They just offer an
>>> SMC which allows the kernel to read and write the diag register.
>>
>> I had a downstream discussion about this, and Bo Yan said someone had
>> verified this was working correctly for at least for some WARs on some
>> CPUs and for the one particular secure OS we're using.
>>
>> I think it's reasonable to require a fixed secure OS (i.e. one that
>> correctly enables any required WARs) be used with any upstream kernel,
>> since running in normal world would be a new feature that we'd be
>> supporting.
>>
>> An SMC to read/write the diag register sounds the opposite of secure...
>>
>>>> question probably need to be applied very early by assembly code, e.g.
>>>> before MMU is re-enabled, so I think you'd end up parsing DT from
>>>> assembly again, which would be painful. I tend to think just including
>>>> the code in the kernel's SoC-specific reset handler is simplest, and
>>>> even with the slight duplication, probably most maintainable. I've
>>>> written a patch for this for Tegra already, which I hope to post later
>>>> today, depending on testing and what other stuff I get side-tracked on.
>>>
>>> No. We could just set a flag in __tegra_cpu_reset_handler_data based on the
>>> info in DT or use a different reset handler. DT is parsed before bringing up
>>> secondary CPUs, so this approach should work I think.
>>
>> Yes, that could work.
>>
> It might work for few but it isn't an alternative which is maintainable.
>
> Olof proposed to have a common code which can be executed before kernel boot
> in recent Linaro connect multi-platform discussion.
> Though there was no conclusion on where this file can be part of kernel source
> tree. Just form errata WA version control perspective, its is best to have such
> errata WA as part of kernel code instead of spreading it over boot-loader, kernel
> and firmware/sleep code.


Yeah, that was mentioned in the discussion at Linaro Connect that we
should have brought out here on the mailing list. I just got home and
haven't had time to follow up on it, but thanks for the reminder. :)

My reasoning was this:

1) Sleep/powerdown recovery code can per definition know what platform
we're running on, so we can do the right thing at runtime there
2) The real tricky one are the very early errata enablement before MMU
enablement


On platforms where we can update firmware, (2) can and should be done
from there. (1) is still possible to do in the kernel for those cases.

The tricky part is where we don't have easily updateable firmware. To
date, these are very likely to still be platforms running legacy
firmware that does not know about device tree either, so we have a
window of opportunity to take advantage of that:

If we already have to use appended DTB on these systems, then we
already have a need to wrap the kernel in a per-system image (i.e. by
appending the DTB to the kernel and booting that). If we're already
doing that, we could just as well add a wrapper around the whole thing
that enables the errata before launching the zimage.

This is not the same as putting it in the zimage boot wrapper code,
since that means it's a buildtime and not a kernel install time thing.
This needs to be done at install/wrap time. Maybe the wrap code
fragments could be kept in the kernel tree and supplied from there
though, we didn't reach closure on that.



-Olof

Multi-platform, and secure-only ARM errata workarounds

[Stephen Warren]

On 03/10/2013 12:47 PM, Olof Johansson wrote:
> On Sun, Mar 10, 2013 at 10:25 AM, Santosh Shilimkar
> <santosh.shilimkar@ti.com> wrote:
>> On Tuesday 05 March 2013 10:30 PM, Stephen Warren wrote:
>>> On 03/05/2013 12:40 AM, Peter De Schrijver wrote:
>>>> On Mon, Mar 04, 2013 at 06:08:27PM +0100, Stephen Warren wrote:
>>>>> On 03/04/2013 02:16 AM, Peter De Schrijver wrote:
>>>>>> On Mon, Mar 04, 2013 at 07:34:36AM +0100, Peter De Schrijver wrote:
>>>>>>> On Fri, Mar 01, 2013 at 06:37:27PM +0100, Stephen Warren wrote:
>>>>>>>
>>>>
>>>> ...
>>>>
>>>>>> 1) Handle CPU0 errata WARs in the bootloader
>>>>>
>>>>> OK - there's not much choice here, and I've posted a patch for this for
>>>>> Tegra U-Boot already.
>>>>>
>>>>>> 2) Indicate in device tree if linux is booting in secude mode or non-secure
>>>>>>    mode.
>>>>>> 3) Use this information in the kernel to decide how to apply the WARs for
>>>>>>    secondary core bringup and after powerungating.
>>>>>
>>>>> Hmmm. That seems like a lot of overhead to avoid duplicating roughly 8
>>>>> assembly instructions per Tegra version. Also, some/all of the WARs in
>>>>
>>>> Unfortunately we can't write to the diag register if we are in non-secure
>>>> mode. So unless we never want to support running in non-secure mode, we will
>>>> need to make the distinction somehow and use a different method for non-secure
>>>> mode. Or assume the secure OS has applied the WARs.
>>>
>>> Yes. The secure OS really has to have enabled the appropriate WARs
>>> before jumping into the kernel's reset vector. If/when we support the
>>> upstream kernel running on Tegra in non-secure mode, the plan was to use
>>> a Tegra-specific mechanism to detect secure-vs-normal mode in the Tegra
>>> reset vector, and skip the application of secure-only WARs based on that.
>>>
>>>> I'm afraid existing secure
>>>> OS implementations for Tegra don't work that way though. They just offer an
>>>> SMC which allows the kernel to read and write the diag register.
>>>
>>> I had a downstream discussion about this, and Bo Yan said someone had
>>> verified this was working correctly for at least for some WARs on some
>>> CPUs and for the one particular secure OS we're using.
>>>
>>> I think it's reasonable to require a fixed secure OS (i.e. one that
>>> correctly enables any required WARs) be used with any upstream kernel,
>>> since running in normal world would be a new feature that we'd be
>>> supporting.
>>>
>>> An SMC to read/write the diag register sounds the opposite of secure...
>>>
>>>>> question probably need to be applied very early by assembly code, e.g.
>>>>> before MMU is re-enabled, so I think you'd end up parsing DT from
>>>>> assembly again, which would be painful. I tend to think just including
>>>>> the code in the kernel's SoC-specific reset handler is simplest, and
>>>>> even with the slight duplication, probably most maintainable. I've
>>>>> written a patch for this for Tegra already, which I hope to post later
>>>>> today, depending on testing and what other stuff I get side-tracked on.
>>>>
>>>> No. We could just set a flag in __tegra_cpu_reset_handler_data based on the
>>>> info in DT or use a different reset handler. DT is parsed before bringing up
>>>> secondary CPUs, so this approach should work I think.
>>>
>>> Yes, that could work.
>>>
>> It might work for few but it isn't an alternative which is maintainable.
>>
>> Olof proposed to have a common code which can be executed before kernel boot
>> in recent Linaro connect multi-platform discussion.
>> Though there was no conclusion on where this file can be part of kernel source
>> tree. Just form errata WA version control perspective, its is best to have such
>> errata WA as part of kernel code instead of spreading it over boot-loader, kernel
>> and firmware/sleep code.
> 
> 
> Yeah, that was mentioned in the discussion at Linaro Connect that we
> should have brought out here on the mailing list. I just got home and
> haven't had time to follow up on it, but thanks for the reminder. :)
> 
> My reasoning was this:
> 
> 1) Sleep/powerdown recovery code can per definition know what platform
> we're running on, so we can do the right thing at runtime there

So this seems to be the opposite of what Santosh was saying. By
definition, if you're talking about platform-specific knowledge, surely
you are /not/ talking about common code. If you /are/ talking about
common code that enables the WARs based on some data that was created by
the kernel before it power-saved, then that's exactly what Peter was
proposing, so I don't understand Santosh's "not maintainable" comment...

Item (2) below I assume is intended only to address the
CPU0-on-first-boot issue. Any code that "wraps" (is prepended to?) the
kernel isn't part of the kernel, and hence won't be available for the
kernel to use when bringing up CPUn, or when resuming from power save,
since the whole point is the kernel doesn't even know that code exists,
right?

> 2) The real tricky one are the very early errata enablement before MMU
> enablement
> 
> On platforms where we can update firmware, (2) can and should be done
> from there. (1) is still possible to do in the kernel for those cases.
> 
> The tricky part is where we don't have easily updateable firmware. To
> date, these are very likely to still be platforms running legacy
> firmware that does not know about device tree either, so we have a
> window of opportunity to take advantage of that:
> 
> If we already have to use appended DTB on these systems, then we
> already have a need to wrap the kernel in a per-system image (i.e. by
> appending the DTB to the kernel and booting that). If we're already
> doing that, we could just as well add a wrapper around the whole thing
> that enables the errata before launching the zimage.
> 
> This is not the same as putting it in the zimage boot wrapper code,
> since that means it's a buildtime and not a kernel install time thing.
> This needs to be done at install/wrap time. Maybe the wrap code
> fragments could be kept in the kernel tree and supplied from there
> though, we didn't reach closure on that.

Multi-platform, and secure-only ARM errata workarounds

[Jason Gunthorpe]

On Sun, Mar 10, 2013 at 11:47:31AM -0700, Olof Johansson wrote:

> If we already have to use appended DTB on these systems, then we
> already have a need to wrap the kernel in a per-system image (i.e. by
> appending the DTB to the kernel and booting that). If we're already
> doing that, we could just as well add a wrapper around the whole thing
> that enables the errata before launching the zimage.

What about bundling the DTB and a block of asm code to do 'CPU
startup' together somehow? If it was easy to locate the code in the
bundle then it could be jumped to directly from the vmlinux pre-mmu
startup code, from the secondary CPU startup, and from the CPU wake up
as well.

The tool to generate this bundle could generate and compile the asm
code based on an errata list in the DTB itself..

Regards,
Jason