diff mbox series

[v2,1/4] util/cacheflush: fix cache on windows-arm64

Message ID 20230216134911.6803-2-pierrick.bouvier@linaro.org
State Superseded
Headers show
Series Adds support for running QEMU natively on windows-arm64 | expand

Commit Message

Pierrick Bouvier Feb. 16, 2023, 1:49 p.m. UTC
ctr_el0 access is privileged on this platform and fails as an illegal
instruction.

Windows does not offer a way to flush data cache from userspace, and
only FlushInstructionCache is available in Windows API.

The generic implementation of flush_idcache_range uses,
__builtin___clear_cache, which already use the FlushInstructionCache
function. So we rely on that.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 util/cacheflush.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Richard Henderson Feb. 16, 2023, 7:42 p.m. UTC | #1
On 2/16/23 03:49, Pierrick Bouvier wrote:
> ctr_el0 access is privileged on this platform and fails as an illegal
> instruction.
> 
> Windows does not offer a way to flush data cache from userspace, and
> only FlushInstructionCache is available in Windows API.
> 
> The generic implementation of flush_idcache_range uses,
> __builtin___clear_cache, which already use the FlushInstructionCache
> function. So we rely on that.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

This is reasonable for now.  I'll note that gcc does not yet support windows for aarch64, 
and I would guess this would be fixed for libgcc at such time as.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~


> ---
>   util/cacheflush.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/util/cacheflush.c b/util/cacheflush.c
> index 2c2c73e085..0a0acd70fa 100644
> --- a/util/cacheflush.c
> +++ b/util/cacheflush.c
> @@ -121,8 +121,10 @@ static void sys_cache_info(int *isize, int *dsize)
>   static bool have_coherent_icache;
>   #endif
>   
> -#if defined(__aarch64__) && !defined(CONFIG_DARWIN)
> -/* Apple does not expose CTR_EL0, so we must use system interfaces. */
> +#if defined(__aarch64__) && !defined(CONFIG_DARWIN) && !defined(CONFIG_WIN32)
> +/* Apple does not expose CTR_EL0, so we must use system interfaces.
> + * Windows neither, but we use a generic implementation of flush_idcache_range
> + * in this case. */
>   static uint64_t save_ctr_el0;
>   static void arch_cache_info(int *isize, int *dsize)
>   {
> @@ -225,7 +227,9 @@ 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)
> +/* For Windows, we use generic implementation of flush_idcache_range, that
> + * performs a call to FlushInstructionCache, through __builtin___clear_cache */
>   
>   #ifdef CONFIG_DARWIN
>   /* Apple does not expose CTR_EL0, so we must use system interfaces. */
Peter Maydell Feb. 17, 2023, 3:32 p.m. UTC | #2
On Thu, 16 Feb 2023 at 13:49, Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> ctr_el0 access is privileged on this platform and fails as an illegal
> instruction.
>
> Windows does not offer a way to flush data cache from userspace, and
> only FlushInstructionCache is available in Windows API.
>
> The generic implementation of flush_idcache_range uses,
> __builtin___clear_cache, which already use the FlushInstructionCache
> function. So we rely on that.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  util/cacheflush.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/util/cacheflush.c b/util/cacheflush.c
> index 2c2c73e085..0a0acd70fa 100644
> --- a/util/cacheflush.c
> +++ b/util/cacheflush.c
> @@ -121,8 +121,10 @@ static void sys_cache_info(int *isize, int *dsize)
>  static bool have_coherent_icache;
>  #endif
>
> -#if defined(__aarch64__) && !defined(CONFIG_DARWIN)
> -/* Apple does not expose CTR_EL0, so we must use system interfaces. */
> +#if defined(__aarch64__) && !defined(CONFIG_DARWIN) && !defined(CONFIG_WIN32)
> +/* Apple does not expose CTR_EL0, so we must use system interfaces.
> + * Windows neither, but we use a generic implementation of flush_idcache_range
> + * in this case. */

QEMU multiline comment syntax requires the /* and */ to be
on lines of their own (here and in your other comment).

thanks
-- PMM
Pierrick Bouvier Feb. 20, 2023, 9:58 a.m. UTC | #3
On 2/17/23 16:32, Peter Maydell wrote:
> On Thu, 16 Feb 2023 at 13:49, Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
>>
>> ctr_el0 access is privileged on this platform and fails as an illegal
>> instruction.
>>
>> Windows does not offer a way to flush data cache from userspace, and
>> only FlushInstructionCache is available in Windows API.
>>
>> The generic implementation of flush_idcache_range uses,
>> __builtin___clear_cache, which already use the FlushInstructionCache
>> function. So we rely on that.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   util/cacheflush.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/util/cacheflush.c b/util/cacheflush.c
>> index 2c2c73e085..0a0acd70fa 100644
>> --- a/util/cacheflush.c
>> +++ b/util/cacheflush.c
>> @@ -121,8 +121,10 @@ static void sys_cache_info(int *isize, int *dsize)
>>   static bool have_coherent_icache;
>>   #endif
>>
>> -#if defined(__aarch64__) && !defined(CONFIG_DARWIN)
>> -/* Apple does not expose CTR_EL0, so we must use system interfaces. */
>> +#if defined(__aarch64__) && !defined(CONFIG_DARWIN) && !defined(CONFIG_WIN32)
>> +/* Apple does not expose CTR_EL0, so we must use system interfaces.
>> + * Windows neither, but we use a generic implementation of flush_idcache_range
>> + * in this case. */
> 
> QEMU multiline comment syntax requires the /* and */ to be
> on lines of their own (here and in your other comment).
> 

Sorry, I missed that point in QEMU coding style.
Will update this 👍

> thanks
> -- PMM
Pierrick Bouvier March 1, 2023, 5:27 p.m. UTC | #4
On 2/16/23 20:42, Richard Henderson wrote:
> On 2/16/23 03:49, Pierrick Bouvier wrote:
>> ctr_el0 access is privileged on this platform and fails as an illegal
>> instruction.
>>
>> Windows does not offer a way to flush data cache from userspace, and
>> only FlushInstructionCache is available in Windows API.
>>
>> The generic implementation of flush_idcache_range uses,
>> __builtin___clear_cache, which already use the FlushInstructionCache
>> function. So we rely on that.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> This is reasonable for now.  I'll note that gcc does not yet support windows for aarch64,
> and I would guess this would be fixed for libgcc at such time as.
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> 

For completeness, and after asking to Microsoft,
FlushInstructionCache does DCache-Clean-to-PoU + 
ICache-Invalidate-to-PoU, which is equivalent to calling dccvau + icivau.

Thus, it's doing the right thing.

Pierrick

> 
> r~
> 
> 
>> ---
>>    util/cacheflush.c | 10 +++++++---
>>    1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/util/cacheflush.c b/util/cacheflush.c
>> index 2c2c73e085..0a0acd70fa 100644
>> --- a/util/cacheflush.c
>> +++ b/util/cacheflush.c
>> @@ -121,8 +121,10 @@ static void sys_cache_info(int *isize, int *dsize)
>>    static bool have_coherent_icache;
>>    #endif
>>    
>> -#if defined(__aarch64__) && !defined(CONFIG_DARWIN)
>> -/* Apple does not expose CTR_EL0, so we must use system interfaces. */
>> +#if defined(__aarch64__) && !defined(CONFIG_DARWIN) && !defined(CONFIG_WIN32)
>> +/* Apple does not expose CTR_EL0, so we must use system interfaces.
>> + * Windows neither, but we use a generic implementation of flush_idcache_range
>> + * in this case. */
>>    static uint64_t save_ctr_el0;
>>    static void arch_cache_info(int *isize, int *dsize)
>>    {
>> @@ -225,7 +227,9 @@ 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)
>> +/* For Windows, we use generic implementation of flush_idcache_range, that
>> + * performs a call to FlushInstructionCache, through __builtin___clear_cache */
>>    
>>    #ifdef CONFIG_DARWIN
>>    /* Apple does not expose CTR_EL0, so we must use system interfaces. */
>
diff mbox series

Patch

diff --git a/util/cacheflush.c b/util/cacheflush.c
index 2c2c73e085..0a0acd70fa 100644
--- a/util/cacheflush.c
+++ b/util/cacheflush.c
@@ -121,8 +121,10 @@  static void sys_cache_info(int *isize, int *dsize)
 static bool have_coherent_icache;
 #endif
 
-#if defined(__aarch64__) && !defined(CONFIG_DARWIN)
-/* Apple does not expose CTR_EL0, so we must use system interfaces. */
+#if defined(__aarch64__) && !defined(CONFIG_DARWIN) && !defined(CONFIG_WIN32)
+/* Apple does not expose CTR_EL0, so we must use system interfaces.
+ * Windows neither, but we use a generic implementation of flush_idcache_range
+ * in this case. */
 static uint64_t save_ctr_el0;
 static void arch_cache_info(int *isize, int *dsize)
 {
@@ -225,7 +227,9 @@  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)
+/* For Windows, we use generic implementation of flush_idcache_range, that
+ * performs a call to FlushInstructionCache, through __builtin___clear_cache */
 
 #ifdef CONFIG_DARWIN
 /* Apple does not expose CTR_EL0, so we must use system interfaces. */