Re: [PATCH 1/4] util/cacheflush: fix illegal instruction on windows-arm64

[Peter Maydell]

On Mon, 13 Feb 2023 at 20:50, Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> mrs instruction fails as an illegal instruction.
> For now, no cache information is retrieved for this platform.
> It could be specialized later, using Windows API.

Unless I'm misreading the code, there's a sys_cache_info()
implementation that's only guarded by if defined(_WIN32), so
presumably we're using that on AArch64 also. Does it return
sensible values ?

> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  util/cacheflush.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/util/cacheflush.c b/util/cacheflush.c
> index 2c2c73e085..149f103d32 100644
> --- a/util/cacheflush.c
> +++ b/util/cacheflush.c
> @@ -121,8 +121,9 @@ static void sys_cache_info(int *isize, int *dsize)
>  static bool have_coherent_icache;
>  #endif
>
> -#if defined(__aarch64__) && !defined(CONFIG_DARWIN)
> +#if defined(__aarch64__) && !defined(CONFIG_DARWIN) && !defined(CONFIG_WIN32)
>  /* Apple does not expose CTR_EL0, so we must use system interfaces. */
> +/* Does not work on windows-arm64, illegal instruction using mrs */

This comment should be better integrated with the previous, because
the reason for the illegal instruction exception on Windows is the
same as for macos -- the OS doesn't expose CTR_EL0 to userspace.

>  static uint64_t save_ctr_el0;
>  static void arch_cache_info(int *isize, int *dsize)
>  {
> @@ -225,7 +226,7 @@ static void __attribute__((constructor)) 
> init_cache_info(void)
>
>  /* Caches are coherent and do not require flushing; symbol inline. */
>
> -#elif defined(__aarch64__)
> +#elif defined(__aarch64__) && !defined(CONFIG_WIN32)

This will cause us to not use the generic aarch64 flush_idcache_range(),
which uses DC CVAU and IC IVAU. Does that not work on Windows?

If it doesn't then I think the ifdeffery would be more clearly
structured as

#elif defined(__aarch64__)

ifdef CONFIG_DARWIN
[macos implementation of flush_idcache_range]
#elif defined(CONFIG_WIN32)
/* Explanation here of why the generic version doesn't work */
#else
/* generic version */
#endif

#elif defined(__mips__)

etc

thanks
-- PMM