diff mbox series

[v2,1/9] target/xtensa: Remove tswap() calls in semihosting simcall() helper

Message ID 20241211230357.97036-2-philmd@linaro.org
State New
Headers show
Series misc: Reduce 'exec/tswap.h' inclusions | expand

Commit Message

Philippe Mathieu-Daudé Dec. 11, 2024, 11:03 p.m. UTC
In preparation of heterogeneous emulation where cores with
different endianness can run concurrently, we need to remove
the tswap() calls -- which use a fixed per-binary endianness.

Get the endianness of the CPU accessed using the libisa
xtensa_isa_is_big_endian() call and replace the tswap() calls
by bswap() ones when necessary.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/xtensa/xtensa-semi.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

Comments

Philippe Mathieu-Daudé Dec. 11, 2024, 11:08 p.m. UTC | #1
On 12/12/24 00:03, Philippe Mathieu-Daudé wrote:
> In preparation of heterogeneous emulation where cores with
> different endianness can run concurrently, we need to remove
> the tswap() calls -- which use a fixed per-binary endianness.
> 
> Get the endianness of the CPU accessed using the libisa
> xtensa_isa_is_big_endian() call and replace the tswap() calls
> by bswap() ones when necessary.

Instead read here:

   In preparation of heterogeneous emulation where cores with
   different endianness can run concurrently, replace the pair
   of cpu_memory_rw_debug() + tswap() calls by put/get_user_u32()
   ones, which still do the same under the hood, but simplify the
   code maintenance (having less sites to do endianness code
   conversion).

> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/xtensa/xtensa-semi.c | 15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
> index fa21b7e11fc..2ded8e5634e 100644
> --- a/target/xtensa/xtensa-semi.c
> +++ b/target/xtensa/xtensa-semi.c
> @@ -30,6 +30,7 @@
>   #include "chardev/char-fe.h"
>   #include "exec/helper-proto.h"
>   #include "semihosting/semihost.h"
> +#include "semihosting/uaccess.h"
>   #include "qapi/error.h"
>   #include "qemu/log.h"
>   
> @@ -323,15 +324,12 @@ void HELPER(simcall)(CPUXtensaState *env)
>               uint32_t fd = regs[3];
>               uint32_t rq = regs[4];
>               uint32_t target_tv = regs[5];
> -            uint32_t target_tvv[2];
>   
>               struct timeval tv = {0};
>   
>               if (target_tv) {
> -                cpu_memory_rw_debug(cs, target_tv,
> -                        (uint8_t *)target_tvv, sizeof(target_tvv), 0);
> -                tv.tv_sec = (int32_t)tswap32(target_tvv[0]);
> -                tv.tv_usec = (int32_t)tswap32(target_tvv[1]);
> +                get_user_u32(tv.tv_sec, target_tv);
> +                get_user_u32(tv.tv_sec, target_tv + 4);
>               }
>               if (fd < 3 && sim_console) {
>                   if ((fd == 1 || fd == 2) && rq == SELECT_ONE_WRITE) {
> @@ -387,11 +385,8 @@ void HELPER(simcall)(CPUXtensaState *env)
>                   const char *str = semihosting_get_arg(i);
>                   int str_size = strlen(str) + 1;
>   
> -                argptr = tswap32(regs[3] + str_offset);
> -
> -                cpu_memory_rw_debug(cs,
> -                                    regs[3] + i * sizeof(uint32_t),
> -                                    (uint8_t *)&argptr, sizeof(argptr), 1);
> +                put_user_u32(regs[3] + str_offset,
> +                             regs[3] + i * sizeof(uint32_t));
>                   cpu_memory_rw_debug(cs,
>                                       regs[3] + str_offset,
>                                       (uint8_t *)str, str_size, 1);
Richard Henderson Dec. 11, 2024, 11:44 p.m. UTC | #2
On 12/11/24 17:08, Philippe Mathieu-Daudé wrote:
> On 12/12/24 00:03, Philippe Mathieu-Daudé wrote:
>> In preparation of heterogeneous emulation where cores with
>> different endianness can run concurrently, we need to remove
>> the tswap() calls -- which use a fixed per-binary endianness.
>>
>> Get the endianness of the CPU accessed using the libisa
>> xtensa_isa_is_big_endian() call and replace the tswap() calls
>> by bswap() ones when necessary.
> 
> Instead read here:
> 
>    In preparation of heterogeneous emulation where cores with
>    different endianness can run concurrently, replace the pair
>    of cpu_memory_rw_debug() + tswap() calls by put/get_user_u32()
>    ones, which still do the same under the hood, but simplify the
>    code maintenance (having less sites to do endianness code
>    conversion).
> 
>> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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


r~
diff mbox series

Patch

diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
index fa21b7e11fc..2ded8e5634e 100644
--- a/target/xtensa/xtensa-semi.c
+++ b/target/xtensa/xtensa-semi.c
@@ -30,6 +30,7 @@ 
 #include "chardev/char-fe.h"
 #include "exec/helper-proto.h"
 #include "semihosting/semihost.h"
+#include "semihosting/uaccess.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
 
@@ -323,15 +324,12 @@  void HELPER(simcall)(CPUXtensaState *env)
             uint32_t fd = regs[3];
             uint32_t rq = regs[4];
             uint32_t target_tv = regs[5];
-            uint32_t target_tvv[2];
 
             struct timeval tv = {0};
 
             if (target_tv) {
-                cpu_memory_rw_debug(cs, target_tv,
-                        (uint8_t *)target_tvv, sizeof(target_tvv), 0);
-                tv.tv_sec = (int32_t)tswap32(target_tvv[0]);
-                tv.tv_usec = (int32_t)tswap32(target_tvv[1]);
+                get_user_u32(tv.tv_sec, target_tv);
+                get_user_u32(tv.tv_sec, target_tv + 4);
             }
             if (fd < 3 && sim_console) {
                 if ((fd == 1 || fd == 2) && rq == SELECT_ONE_WRITE) {
@@ -387,11 +385,8 @@  void HELPER(simcall)(CPUXtensaState *env)
                 const char *str = semihosting_get_arg(i);
                 int str_size = strlen(str) + 1;
 
-                argptr = tswap32(regs[3] + str_offset);
-
-                cpu_memory_rw_debug(cs,
-                                    regs[3] + i * sizeof(uint32_t),
-                                    (uint8_t *)&argptr, sizeof(argptr), 1);
+                put_user_u32(regs[3] + str_offset,
+                             regs[3] + i * sizeof(uint32_t));
                 cpu_memory_rw_debug(cs,
                                     regs[3] + str_offset,
                                     (uint8_t *)str, str_size, 1);