Re: [PATCH-v3] [SPARC] Add a workaround for the LEON3FT store-store errata

[Daniel Cederman]

On 2017-07-07 18:04, Eric Botcazou wrote:

Great! Would you mind to apply the patch for us? The only person here
with write access just went on vacation. I have submitted a new version
(v4) with the change that applies to both main and 7.


OK, will do.



Thanks!

--
Daniel Cederman
Cobham Gaisler

Re: [PATCH-v3] [SPARC] Add a workaround for the LEON3FT store-store errata

[Eric Botcazou]

> Great! Would you mind to apply the patch for us? The only person here
> with write access just went on vacation. I have submitted a new version
> (v4) with the change that applies to both main and 7.

OK, will do.

-- 
Eric Botcazou

Re: [PATCH-v3] [SPARC] Add a workaround for the LEON3FT store-store errata

[Daniel Cederman]

On 2017-07-07 12:01, Eric Botcazou wrote:

We can drop the define if necessary, but we would like to keep the two
flags. Would that be OK to apply?


Yes, OK to apply on mainline and 7 branch with this change, thanks.



Great! Would you mind to apply the patch for us? The only person here 
with write access just went on vacation. I have submitted a new version 
(v4) with the change that applies to both main and 7.


--
Daniel Cederman
Cobham Gaisler

Re: [PATCH-v3] [SPARC] Add a workaround for the LEON3FT store-store errata

[Eric Botcazou]

> We can drop the define if necessary, but we would like to keep the two
> flags. Would that be OK to apply?

Yes, OK to apply on mainline and 7 branch with this change, thanks.

-- 
Eric Botcazou

Re: [PATCH-v3] [SPARC] Add a workaround for the LEON3FT store-store errata

[Sebastian Huber]

On 04/07/17 15:38, Daniel Cederman wrote:



On 2017-06-30 07:11, Sebastian Huber wrote:

On 29/06/17 18:05, David Miller wrote:


From: Daniel Cederman 
Date: Thu, 29 Jun 2017 17:15:43 +0200


I'm not thrilled with this, it's undocumented, the other workaround
don't have
it and I don't think that we really need it.

The B2BST errata workaround requires more changes to assembler
routines commonly used by operating systems, such as for example
register window handling, than what the UT699 workaround needed. It
would be nice to have a way to only enable these modification when the
-mfix- flag is used. The alternative would be to provide a define
directly on the compiler command line in conjunction with -mfix
flag. But if more changes are required later on it would be good to
have the define more closely tied to the flag to minimize the number
of changes to Makefiles and etc.

Personally, I have never seen compiler based CPP defines as ever being
useful for tailoring OS assembler code.  Ever.

In most cases you will want to support several families of CPUs and
therefore sort out the individual cpu support assembler routines
internally in the kernel sources.


This depends on the operating system you use. For some  embedded 
systems where the application and the operating system are one 
executable it is quite common to use compiler provided defines in 
assembly code.


For example:

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/machine/arm/memcpy-armv7a.S;h=cd7962e075a30cb90ec073d77b177c3536429b9b;hb=HEAD 



For a software development kit, the run-time libraries are built for 
a set of multilibs. Each assembler file may use multilib specific 
compiler defines, e.g. floating point unit present or not, errata XYZ 
present or not, etc.




We can drop the define if necessary, but we would like to keep the two 
flags. Would that be OK to apply?




If I read the GRLIB-TN-0009 correctly, then we have to adjust the 
context switch, interrupt processing and window management code in 
RTEMS. So, we definitely need this define.


Since this errata affects actually the GRLIB, which is used in many 
products, should we really start adding -mfix-some-processor options? 
The GRLIB affected by this errata may be used in custom designs as well. 
I suggest to simply add a


-mfix-leon3ft-b2bst

option which enables the workaround and adds a builtin define

#define __FIX_LEON3FT_B2BST

The documentation for this option should mention this and also reference 
the GRLIB-TN-0009 and maybe the affected known processors.


--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: [PATCH-v3] [SPARC] Add a workaround for the LEON3FT store-store errata

[Daniel Cederman]

On 2017-06-30 07:11, Sebastian Huber wrote:

On 29/06/17 18:05, David Miller wrote:


From: Daniel Cederman 
Date: Thu, 29 Jun 2017 17:15:43 +0200


I'm not thrilled with this, it's undocumented, the other workaround
don't have
it and I don't think that we really need it.

The B2BST errata workaround requires more changes to assembler
routines commonly used by operating systems, such as for example
register window handling, than what the UT699 workaround needed. It
would be nice to have a way to only enable these modification when the
-mfix- flag is used. The alternative would be to provide a define
directly on the compiler command line in conjunction with -mfix
flag. But if more changes are required later on it would be good to
have the define more closely tied to the flag to minimize the number
of changes to Makefiles and etc.

Personally, I have never seen compiler based CPP defines as ever being
useful for tailoring OS assembler code.  Ever.

In most cases you will want to support several families of CPUs and
therefore sort out the individual cpu support assembler routines
internally in the kernel sources.


This depends on the operating system you use. For some  embedded systems 
where the application and the operating system are one executable it is 
quite common to use compiler provided defines in assembly code.


For example:

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/machine/arm/memcpy-armv7a.S;h=cd7962e075a30cb90ec073d77b177c3536429b9b;hb=HEAD 



For a software development kit, the run-time libraries are built for a 
set of multilibs. Each assembler file may use multilib specific compiler 
defines, e.g. floating point unit present or not, errata XYZ present or 
not, etc.




We can drop the define if necessary, but we would like to keep the two 
flags. Would that be OK to apply?


--
Daniel Cederman
Cobham Gaisler

Re: [PATCH-v3] [SPARC] Add a workaround for the LEON3FT store-store errata

[Sebastian Huber]

On 29/06/17 18:05, David Miller wrote:


From: Daniel Cederman 
Date: Thu, 29 Jun 2017 17:15:43 +0200


I'm not thrilled with this, it's undocumented, the other workaround
don't have
it and I don't think that we really need it.

The B2BST errata workaround requires more changes to assembler
routines commonly used by operating systems, such as for example
register window handling, than what the UT699 workaround needed. It
would be nice to have a way to only enable these modification when the
-mfix- flag is used. The alternative would be to provide a define
directly on the compiler command line in conjunction with -mfix
flag. But if more changes are required later on it would be good to
have the define more closely tied to the flag to minimize the number
of changes to Makefiles and etc.

Personally, I have never seen compiler based CPP defines as ever being
useful for tailoring OS assembler code.  Ever.

In most cases you will want to support several families of CPUs and
therefore sort out the individual cpu support assembler routines
internally in the kernel sources.


This depends on the operating system you use. For some  embedded systems 
where the application and the operating system are one executable it is 
quite common to use compiler provided defines in assembly code.


For example:

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;a=blob;f=newlib/libc/machine/arm/memcpy-armv7a.S;h=cd7962e075a30cb90ec073d77b177c3536429b9b;hb=HEAD

For a software development kit, the run-time libraries are built for a 
set of multilibs. Each assembler file may use multilib specific compiler 
defines, e.g. floating point unit present or not, errata XYZ present or 
not, etc.


--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

Re: [PATCH-v3] [SPARC] Add a workaround for the LEON3FT store-store errata

[David Miller]

From: Daniel Cederman 
Date: Thu, 29 Jun 2017 17:15:43 +0200

>> I'm not thrilled with this, it's undocumented, the other workaround
>> don't have
>> it and I don't think that we really need it.
> 
> The B2BST errata workaround requires more changes to assembler
> routines commonly used by operating systems, such as for example
> register window handling, than what the UT699 workaround needed. It
> would be nice to have a way to only enable these modification when the
> -mfix- flag is used. The alternative would be to provide a define
> directly on the compiler command line in conjunction with -mfix
> flag. But if more changes are required later on it would be good to
> have the define more closely tied to the flag to minimize the number
> of changes to Makefiles and etc.

Personally, I have never seen compiler based CPP defines as ever being
useful for tailoring OS assembler code.  Ever.

In most cases you will want to support several families of CPUs and
therefore sort out the individual cpu support assembler routines
internally in the kernel sources.

Re: [PATCH-v3] [SPARC] Add a workaround for the LEON3FT store-store errata

[Daniel Cederman]

Hello Eric,

Thank you for reviewing the patch.


Let's forget -mfix-gr712rc for now, -mfix-ut700 is enough I think.


I think it would be confusing to use the -mfix-ut700 flag when compiling 
for the GR712RC. Now when we are not using a generic name for the errata 
workaround we should at least have unique flags for the two major CPUs 
that are afflicted by this errata.



I'm not thrilled with this, it's undocumented, the other workaround don't have
it and I don't think that we really need it.


The B2BST errata workaround requires more changes to assembler routines 
commonly used by operating systems, such as for example register window 
handling, than what the UT699 workaround needed. It would be nice to 
have a way to only enable these modification when the -mfix- flag is 
used. The alternative would be to provide a define directly on the 
compiler command line in conjunction with -mfix flag. But if more 
changes are required later on it would be good to have the define more 
closely tied to the flag to minimize the number of changes to Makefiles 
and etc.


Would it be OK to add if we document it properly?

--
Daniel Cederman
Cobham Gaisler

Re: [PATCH-v3] [SPARC] Add a workaround for the LEON3FT store-store errata

[Eric Botcazou]

> This patch adds a workaround to the Sparc backend for the LEON3FT
> store-store errata. It is enabled when using the -mfix-ut699,
> -mfix-ut700, or -mfix-gr712rc flag.

Let's forget -mfix-gr712rc for now, -mfix-ut700 is enough I think.

> The workaround inserts NOP instructions to prevent the following two
> instruction sequences from being generated:
> 
> std -> stb/sth/st/std
> stb/sth/st -> any single non-store/load instruction -> stb/sth/st/std
> 
> The __FIX_B2BST define can be used to only enable workarounds in assembly
> code when the flag is used.

I'm not thrilled with this, it's undocumented, the other workaround don't have 
it and I don't think that we really need it.

> See GRLIB-TN-0009, "LEON3FT Stale Cache Entry After Store with Data Tag
> Parity Error", for more information.
> 
> gcc/ChangeLog:
> 
> 2017-06-21  Daniel Cederman  
> 
>   * config/sparc/sparc.c (sparc_do_work_around_errata): Insert NOP
>   instructions to prevent sequences that can trigger the store-store
>   errata for certain LEON3FT processors.
>   (sparc_option_override): -mfix-ut699, -mfix-ut700, and
>   -mfix-gr712rc enables the errata workaround.
>   * config/sparc/sparc-c.c (sparc_target_macros): Define __FIX_B2BST
>   when errata workaround is enabled.
>   * config/sparc/sparc.md: Prevent stores in delay slot.
>   * config/sparc/sparc.opt: Add -mfix-ut700 and -mfix-gr712rc flag.
>   * doc/invoke.texi: Document -mfix-ut700 and -mfix-gr712rc flag.

OK for mainline and 7 branch modulo the above two remarks.

-- 
Eric Botcazou