Re: [U-Boot] [PATCH 2/2] arm: omap3: Bring back ARM errata workaround 725233

[Tom Rini]

On Fri, Mar 10, 2017 at 05:55:37PM +0200, Siarhei Siamashka wrote:
> On Wed, 8 Mar 2017 09:30:07 -0500
> Tom Rini  wrote:
> 
> > On Mon, Mar 06, 2017 at 03:16:53AM +0200, Siarhei Siamashka wrote:
> > 
> > > The workaround for ARM errata 725233 had been lost since
> > > commit 45bf05854bc94e (armv7: adapt omap3 to the new cache
> > > maintenance framework). Bring it back in order to avoid
> > > very difficult to reproduce, but actually encountered in
> > > the wild CPU deadlocks when running software rendered
> > > X11 desktop on OMAP3530 hardware.
> > > 
> > > This patch adds the new errata define to the whitelist instead
> > > of introducing a new Kconfig option. It's probably best to
> > > convert all ARM errata to Kconfig in one go via a separate
> > > patch.
> > > 
> > > Signed-off-by: Siarhei Siamashka   
> > 
> > In concept:
> > Reviewed-by: Tom Rini 
> 
> Thanks!
>  
> > Do you want to v2 on top of my patch that migrates the current ARM
> > errata or would you rather I v2 it and apply?  Thanks again!
>  
> Sure, either way is fine for me.

OK.  I'm building the conversion now.

> There is just one thing that is not very good yet. Setting the
> L2 Auxiliary Control Register is currently a no-op in my patch
> for the HS variants of OMAP3430/3530 because they are using a
> different secure monitor API. And I don't know anything
> about it and don't even have such hardware to run any tests.
> 
> And there is not much that can be done about it there. We have
> no serial console yet to print any diagnostic warnings.
> 
> So I wonder if it makes sense to add one more errata related
> patch, responsible for verifying correctness of the applied
> errata workarounds at a much later stage. Basically, the difference
> is the following. When applying errata workarounds:
>   * Writing to coprocessor registers is restricted and may
> require the use of SoC-specific secure monitor API.
>   * Errata workarounds need to be applied as early as possible
> and we don't have the serial console for any debugging
> or diagnostic messages yet.
> And when validating errata workarounds:
>   * Reading coprocessor registers is not a problem and can be
> done via a simple MRC instruction.
>   * The validation can be done at any time, even postponed
> until just before loading the OS kernel. We can print
> warnings if something is not right or even abort booting
> (after printing a comprehensive error message).
> 
> So if I implement such additional errata validation patch, then
> anyone with a HS OMAP3430 device can notice that there is a
> problem and implement the missing L2 Auxiliary Control Register
> setup code. What do you think about this?

Adding an optional verification stage (or command) could be interesting.
I don't see a problem in concept with adding something, again so long as
it's optional.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/2] arm: omap3: Bring back ARM errata workaround 725233

[Siarhei Siamashka]

On Wed, 8 Mar 2017 09:30:07 -0500
Tom Rini  wrote:

> On Mon, Mar 06, 2017 at 03:16:53AM +0200, Siarhei Siamashka wrote:
> 
> > The workaround for ARM errata 725233 had been lost since
> > commit 45bf05854bc94e (armv7: adapt omap3 to the new cache
> > maintenance framework). Bring it back in order to avoid
> > very difficult to reproduce, but actually encountered in
> > the wild CPU deadlocks when running software rendered
> > X11 desktop on OMAP3530 hardware.
> > 
> > This patch adds the new errata define to the whitelist instead
> > of introducing a new Kconfig option. It's probably best to
> > convert all ARM errata to Kconfig in one go via a separate
> > patch.
> > 
> > Signed-off-by: Siarhei Siamashka   
> 
> In concept:
> Reviewed-by: Tom Rini 

Thanks!
 
> Do you want to v2 on top of my patch that migrates the current ARM
> errata or would you rather I v2 it and apply?  Thanks again!
 
Sure, either way is fine for me.

There is just one thing that is not very good yet. Setting the
L2 Auxiliary Control Register is currently a no-op in my patch
for the HS variants of OMAP3430/3530 because they are using a
different secure monitor API. And I don't know anything
about it and don't even have such hardware to run any tests.

And there is not much that can be done about it there. We have
no serial console yet to print any diagnostic warnings.

So I wonder if it makes sense to add one more errata related
patch, responsible for verifying correctness of the applied
errata workarounds at a much later stage. Basically, the difference
is the following. When applying errata workarounds:
  * Writing to coprocessor registers is restricted and may
require the use of SoC-specific secure monitor API.
  * Errata workarounds need to be applied as early as possible
and we don't have the serial console for any debugging
or diagnostic messages yet.
And when validating errata workarounds:
  * Reading coprocessor registers is not a problem and can be
done via a simple MRC instruction.
  * The validation can be done at any time, even postponed
until just before loading the OS kernel. We can print
warnings if something is not right or even abort booting
(after printing a comprehensive error message).

So if I implement such additional errata validation patch, then
anyone with a HS OMAP3430 device can notice that there is a
problem and implement the missing L2 Auxiliary Control Register
setup code. What do you think about this?

-- 
Best regards,
Siarhei Siamashka
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/2] arm: omap3: Bring back ARM errata workaround 725233

[Tom Rini]

On Mon, Mar 06, 2017 at 03:16:53AM +0200, Siarhei Siamashka wrote:

> The workaround for ARM errata 725233 had been lost since
> commit 45bf05854bc94e (armv7: adapt omap3 to the new cache
> maintenance framework). Bring it back in order to avoid
> very difficult to reproduce, but actually encountered in
> the wild CPU deadlocks when running software rendered
> X11 desktop on OMAP3530 hardware.
> 
> This patch adds the new errata define to the whitelist instead
> of introducing a new Kconfig option. It's probably best to
> convert all ARM errata to Kconfig in one go via a separate
> patch.
> 
> Signed-off-by: Siarhei Siamashka 

In concept:
Reviewed-by: Tom Rini 

Do you want to v2 on top of my patch that migrates the current ARM
errata or would you rather I v2 it and apply?  Thanks again!

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot