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

["Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>]

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
>