[PATCH] Add libgcc support for cache clearing on ARM VxWorks

[PATCH] Add libgcc support for cache clearing on ARM VxWorks

[Olivier Hainque]

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

Hello,

On top of previous changes reworking the arm-vxworks support

  https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00085.html
  https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00075.html
  https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00078.html

This patch adds a variant implementation of _clear_cache
for arm-vxworks*, needed for proper functioning of trampolines
on targets with separate instruction/data caches.

This allows, among other things, Ada tasking programs to
run on variety of configurations which we are exercising with
at least ACATS nightly with gcc-6 based compilers.

As this is touching a common ARM implementation file (lib1funcs.S),
this presumably requires approval from an ARM port maintainer. CC
Richard for this, as he participated in recent discussions regarding
VxWorks for ARM.

OK to commit ?

Thanks much in advance for your feedback,

With Kind Regards,

Olivier

2017-08-01  Doug Rupp  <rupp@adacore.com>
            Olivier Hainque  <hainque@adacore.com>

   	libgcc/
    	* config/arm/lib1funcs.S (_clear_cache): Add variant for __vxworks.
    	* config/arm/t-vxworks: New file, add _clear_cache to LIB1ASMFUNCS.
    	* config.host (arm-wrs-vxworks, arm-wrs-vxworks7): Add arm/t-vxworks
    	to tmake_file.
    
    	gcc/
    	* config/arm/vxworks.h (CLEAR_INSN_CACHE): Define.


[-- Attachment #2: 0004-Add-libgcc-support-for-insn-cache-clearing-on-ARM-Vx.patch --]
[-- Type: application/octet-stream, Size: 2461 bytes --]

--- a/gcc/config/arm/vxworks.h
+++ b/gcc/config/arm/vxworks.h
@@ -154,6 +154,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #undef TARGET_DEFAULT_WORD_RELOCATIONS
 #define TARGET_DEFAULT_WORD_RELOCATIONS 1
 
+/* Clear the instruction cache from `beg' to `end'.  This is
+   implemented in lib1funcs.S, so ensure an error if this definition
+   is used.  */
+#undef  CLEAR_INSN_CACHE
+#define CLEAR_INSN_CACHE(BEG, END) not_used
+
 /* Define this to be nonzero if static stack checking is supported.  */
 #define STACK_CHECK_STATIC_BUILTIN 1
 
diff --git a/libgcc/config.host b/libgcc/config.host
index 9556c77..a35c8fb7 100644
--- a/libgcc/config.host
+++ b/libgcc/config.host
@@ -389,7 +389,7 @@ arc*-*-linux*)
 	extra_parts="$extra_parts crttls.o"
 	;;
 arm-wrs-vxworks|arm-wrs-vxworks7)
-	tmake_file="$tmake_file arm/t-arm arm/t-elf t-softfp-sfdf t-softfp-excl arm/t-softfp t-softfp"
+	tmake_file="$tmake_file arm/t-arm arm/t-elf t-softfp-sfdf t-softfp-excl arm/t-softfp t-softfp arm/t-vxworks"
 	extra_parts="$extra_parts crti.o crtn.o"
 	case ${host} in
 	*-*-vxworks7)
diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
index 8d8c3ce..4b21c02 100644
--- a/libgcc/config/arm/lib1funcs.S
+++ b/libgcc/config/arm/lib1funcs.S
@@ -1572,8 +1572,27 @@ LSYM(Lover12):
 	do_pop	{r7}
 	RET
 	FUNC_END clear_cache
+#elif defined __vxworks
+	ARM_FUNC_START clear_cache
+.L1:
+	mcr	p15, 0, r0, c7, c11, 1	@ Clean data cache by MVA to PoU
+	mcr	p15, 0, r0, c7, c10, 4	@ Ensure visibility of the data
+					@ cleaned from the cache
+	mcr	p15, 0, r0, c7, c5, 1	@ Invalidate instruction cache by
+					@ MVA to PoU
+	mcr	p15, 0, r0, c7, c5, 7	@ Invalidate branch predictor by
+					@ MVA to PoU
+	mcr	p15, 0, r0, c7, c10, 4	@ Ensure completion of the
+					@ invalidations
+	mcr	p15, 0, r0, c7, c5, 4	@ Synchronize fetched instruction
+					@ stream
+	add	r0, r0, #4		@ Increment MVA
+	cmp	r0, r1
+	ble	.L1			@ Loop if MVA <= End Address
+	RET
+	FUNC_END clear_cache
 #else
-#error "This is only for ARM EABI GNU/Linux"
+#error "This is only for ARM EABI GNU/Linux or ARM VxWorks"
 #endif
 #endif /* L_clear_cache */
 /* ------------------------------------------------------------------------ */
diff --git a/libgcc/config/arm/t-vxworks b/libgcc/config/arm/t-vxworks
new file mode 100644
index 0000000..ba0c109
--- /dev/null
+++ b/libgcc/config/arm/t-vxworks
@@ -0,0 +1 @@
+LIB1ASMFUNCS += _clear_cache
-- 
1.7.10.4

Re: [PATCH] Add libgcc support for cache clearing on ARM VxWorks

[Olivier Hainque]

> On Aug 1, 2017, at 16:31 , Olivier Hainque <hainque@adacore.com> wrote:
> 
> This patch adds a variant implementation of _clear_cache
> for arm-vxworks*, needed for proper functioning of trampolines
> on targets with separate instruction/data caches.

Forgot to mention:

Tested by verifying success of an in-house build and proper execution of ACATS
tests with a gcc-7 based compiler for arm-vxworks and arm-vxworks7, and checking
that a build for arm-wrs-vxworks proceeds to completion on mainline.

Re: [PATCH] Add libgcc support for cache clearing on ARM VxWorks

[Richard Earnshaw (lists)]

On 01/08/17 15:31, Olivier Hainque wrote:
> Hello,
> 
> On top of previous changes reworking the arm-vxworks support
> 
>   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00085.html
>   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00075.html
>   https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00078.html
> 
> This patch adds a variant implementation of _clear_cache
> for arm-vxworks*, needed for proper functioning of trampolines
> on targets with separate instruction/data caches.
> 
> This allows, among other things, Ada tasking programs to
> run on variety of configurations which we are exercising with
> at least ACATS nightly with gcc-6 based compilers.
> 
> As this is touching a common ARM implementation file (lib1funcs.S),
> this presumably requires approval from an ARM port maintainer. CC
> Richard for this, as he participated in recent discussions regarding
> VxWorks for ARM.
> 
> OK to commit ?
> 

I'm not going to object to this patch - it's ok at a GCC level; but I'm
not sure that architecturally this is going to work.  The implementation
of cache clearing is very specific to the implementation and I don't
think it is possible to write a portable single implementation.

I must admit, that not being a kernel expert, I'm not completely
familiar with all the details of cache maintenance, but I don't think a
single sequence like this will work in general.

Your code seems to focus only on the Data cache clearing.  Documentation
in extend.texi seems to suggest that __builtin___clear_cache is supposed
to clear the _instruction_ cache (which may of course involve also
synchronizing the Dcache to memory first).  Similar comments in md.texi
refer to the clear_cache instruction pattern flushing the Icache.  Note
that arm processors of any architecture revision do not automatically
synchronize I and D caches.

Before ARMv7 cache maintenance operations were not available in user
mode (I don't know if vxworks is expected to run only in a privileged mode).

On a system without caches the instructions will just abort

I believe that on v7 you also need ISB and DMB operations at the end of
the sequence.  But such instructions aren't available on older
architectures.

I will not object to this being committed (it's your port); but I'd
strongly recommend that you don't at this stage.

R.

> Thanks much in advance for your feedback,
> 
> With Kind Regards,
> 
> Olivier
> 
> 2017-08-01  Doug Rupp  <rupp@adacore.com>
>             Olivier Hainque  <hainque@adacore.com>
> 
>    	libgcc/
>     	* config/arm/lib1funcs.S (_clear_cache): Add variant for __vxworks.
>     	* config/arm/t-vxworks: New file, add _clear_cache to LIB1ASMFUNCS.
>     	* config.host (arm-wrs-vxworks, arm-wrs-vxworks7): Add arm/t-vxworks
>     	to tmake_file.
>     
>     	gcc/
>     	* config/arm/vxworks.h (CLEAR_INSN_CACHE): Define.
> 
> 
> 0004-Add-libgcc-support-for-insn-cache-clearing-on-ARM-Vx.patch
> 
> 
> --- a/gcc/config/arm/vxworks.h
> +++ b/gcc/config/arm/vxworks.h
> @@ -154,6 +154,12 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>  #undef TARGET_DEFAULT_WORD_RELOCATIONS
>  #define TARGET_DEFAULT_WORD_RELOCATIONS 1
>  
> +/* Clear the instruction cache from `beg' to `end'.  This is
> +   implemented in lib1funcs.S, so ensure an error if this definition
> +   is used.  */
> +#undef  CLEAR_INSN_CACHE
> +#define CLEAR_INSN_CACHE(BEG, END) not_used
> +
>  /* Define this to be nonzero if static stack checking is supported.  */
>  #define STACK_CHECK_STATIC_BUILTIN 1
>  
> diff --git a/libgcc/config.host b/libgcc/config.host
> index 9556c77..a35c8fb7 100644
> --- a/libgcc/config.host
> +++ b/libgcc/config.host
> @@ -389,7 +389,7 @@ arc*-*-linux*)
>  	extra_parts="$extra_parts crttls.o"
>  	;;
>  arm-wrs-vxworks|arm-wrs-vxworks7)
> -	tmake_file="$tmake_file arm/t-arm arm/t-elf t-softfp-sfdf t-softfp-excl arm/t-softfp t-softfp"
> +	tmake_file="$tmake_file arm/t-arm arm/t-elf t-softfp-sfdf t-softfp-excl arm/t-softfp t-softfp arm/t-vxworks"
>  	extra_parts="$extra_parts crti.o crtn.o"
>  	case ${host} in
>  	*-*-vxworks7)
> diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
> index 8d8c3ce..4b21c02 100644
> --- a/libgcc/config/arm/lib1funcs.S
> +++ b/libgcc/config/arm/lib1funcs.S
> @@ -1572,8 +1572,27 @@ LSYM(Lover12):
>  	do_pop	{r7}
>  	RET
>  	FUNC_END clear_cache
> +#elif defined __vxworks
> +	ARM_FUNC_START clear_cache
> +.L1:
> +	mcr	p15, 0, r0, c7, c11, 1	@ Clean data cache by MVA to PoU
> +	mcr	p15, 0, r0, c7, c10, 4	@ Ensure visibility of the data
> +					@ cleaned from the cache
> +	mcr	p15, 0, r0, c7, c5, 1	@ Invalidate instruction cache by
> +					@ MVA to PoU
> +	mcr	p15, 0, r0, c7, c5, 7	@ Invalidate branch predictor by
> +					@ MVA to PoU
> +	mcr	p15, 0, r0, c7, c10, 4	@ Ensure completion of the
> +					@ invalidations
> +	mcr	p15, 0, r0, c7, c5, 4	@ Synchronize fetched instruction
> +					@ stream
> +	add	r0, r0, #4		@ Increment MVA
> +	cmp	r0, r1
> +	ble	.L1			@ Loop if MVA <= End Address
> +	RET
> +	FUNC_END clear_cache
>  #else
> -#error "This is only for ARM EABI GNU/Linux"
> +#error "This is only for ARM EABI GNU/Linux or ARM VxWorks"
>  #endif
>  #endif /* L_clear_cache */
>  /* ------------------------------------------------------------------------ */
> diff --git a/libgcc/config/arm/t-vxworks b/libgcc/config/arm/t-vxworks
> new file mode 100644
> index 0000000..ba0c109
> --- /dev/null
> +++ b/libgcc/config/arm/t-vxworks
> @@ -0,0 +1 @@
> +LIB1ASMFUNCS += _clear_cache
> 

Re: [PATCH] Add libgcc support for cache clearing on ARM VxWorks

[Olivier Hainque]

Hi Richard,

> I'm not going to object to this patch - it's ok at a GCC level; but I'm
> not sure that architecturally this is going to work.  The implementation
> of cache clearing is very specific to the implementation and I don't
> think it is possible to write a portable single implementation.
> 
> I must admit, that not being a kernel expert, I'm not completely
> familiar with all the details of cache maintenance, but I don't think a
> single sequence like this will work in general.
> 
> Your code seems to focus only on the Data cache clearing.  Documentation
> in extend.texi seems to suggest that __builtin___clear_cache is supposed
> to clear the _instruction_ cache (which may of course involve also
> synchronizing the Dcache to memory first).  Similar comments in md.texi
> refer to the clear_cache instruction pattern flushing the Icache.  Note
> that arm processors of any architecture revision do not automatically
> synchronize I and D caches.
> 
> Before ARMv7 cache maintenance operations were not available in user
> mode (I don't know if vxworks is expected to run only in a privileged mode).
> 
> On a system without caches the instructions will just abort
> 
> I believe that on v7 you also need ISB and DMB operations at the end of
> the sequence.  But such instructions aren't available on older
> architectures.
> 
> I will not object to this being committed (it's your port); but I'd
> strongly recommend that you don't at this stage.

Thanks a lot for your comments, always very appreciated.

I'll discuss the details with the persons who wrote the
function body. It is quite possible that some aspects were
considered and not implemented eventually just because 
we had something that appeared to work after a number of
experiments with the particular hardware+os configurations
we have at hand. I'm sure that the change observably "fixed"
several problems in these configurations. It's a progress
from this perspective, but I can see that maybe some programs
that would have worked in some configurations before now would
abort. It's quite possible that VxWorks doesn't support such
configurations - to be double checked.

I'll see what matter we have or can gather to go at least a
bit further, and I'll get back to you then :)

Thanks again,

Olivier

Re: [PATCH] Add libgcc support for cache clearing on ARM VxWorks

[Olivier Hainque]

Hi Richard,

I discussed with the original author, have looked into the ARM cache matters
in greater detail and certainly agree with your general concerns.

There are pieces related to the insn cache clearing in the code, but I can
see how architecture specific everything is here, indeed not appropriate as
a single non-specialized entry point in libgcc.

Thanks again for your arm-expert input on the topic.

Still working on this. 

In the meantime, opinion on the two patches below ?

  https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00278.html
  (Handle DWARF2_UNWIND_INFO in arm_except_unwind_info)

  https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00271.html
  (Fix .cfi inconsistency out of builtin_eh_return)

Maybe the dwarf2 unwind support could go away when support for the old ABI
gets removed. From the VxWorks perspective, dwarf2 is only needed by versions
of VxWorks which rely on the old ABI.

Thanks in advance for your feedback,

With Kind Regards,

Olivier