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 <r...@adacore.com>
> Olivier Hainque <hain...@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
>
