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.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
target/xtensa/xtensa-semi.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
index fa21b7e11fc..21d23e39de5 100644
--- a/target/xtensa/xtensa-semi.c
+++ b/target/xtensa/xtensa-semi.c
@@ -328,10 +328,17 @@ void HELPER(simcall)(CPUXtensaState *env)
struct timeval tv = {0};
if (target_tv) {
+ bool cpu_big_endian = xtensa_isa_is_big_endian(env->config->isa);
+ bool swap_needed = HOST_BIG_ENDIAN != cpu_big_endian;
+
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]);
+ if (swap_needed) {
+ bswap32s(&target_tvv[0]);
+ bswap32s(&target_tvv[1]);
+ }
+ tv.tv_sec = (int32_t)target_tvv[0];
+ tv.tv_usec = (int32_t)target_tvv[1];
}
if (fd < 3 && sim_console) {
if ((fd == 1 || fd == 2) && rq == SELECT_ONE_WRITE) {
@@ -381,6 +388,8 @@ void HELPER(simcall)(CPUXtensaState *env)
int argc = semihosting_get_argc();
int str_offset = (argc + 1) * sizeof(uint32_t);
int i;
+ bool cpu_big_endian = xtensa_isa_is_big_endian(env->config->isa);
+ bool swap_needed = HOST_BIG_ENDIAN != cpu_big_endian;
uint32_t argptr;
for (i = 0; i < argc; ++i) {
@@ -388,6 +397,9 @@ void HELPER(simcall)(CPUXtensaState *env)
int str_size = strlen(str) + 1;
argptr = tswap32(regs[3] + str_offset);
+ if (swap_needed) {
+ bswap32s(&argptr);
+ }
cpu_memory_rw_debug(cs,
regs[3] + i * sizeof(uint32_t),
--
2.45.2
On Thu, Dec 5, 2024 at 3:24 PM Philippe Mathieu-Daudé <philmd@linaro.org> 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.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> target/xtensa/xtensa-semi.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
> index fa21b7e11fc..21d23e39de5 100644
> --- a/target/xtensa/xtensa-semi.c
> +++ b/target/xtensa/xtensa-semi.c
> @@ -328,10 +328,17 @@ void HELPER(simcall)(CPUXtensaState *env)
> struct timeval tv = {0};
>
> if (target_tv) {
> + bool cpu_big_endian = xtensa_isa_is_big_endian(env->config->isa);
> + bool swap_needed = HOST_BIG_ENDIAN != cpu_big_endian;
> +
> 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]);
> + if (swap_needed) {
> + bswap32s(&target_tvv[0]);
> + bswap32s(&target_tvv[1]);
> + }
> + tv.tv_sec = (int32_t)target_tvv[0];
> + tv.tv_usec = (int32_t)target_tvv[1];
This conversion looks a bit heavy. Maybe it would be better to
give tswap*() an additional env argument and give the target
a way to deal with it?
> }
> if (fd < 3 && sim_console) {
> if ((fd == 1 || fd == 2) && rq == SELECT_ONE_WRITE) {
> @@ -381,6 +388,8 @@ void HELPER(simcall)(CPUXtensaState *env)
> int argc = semihosting_get_argc();
> int str_offset = (argc + 1) * sizeof(uint32_t);
> int i;
> + bool cpu_big_endian = xtensa_isa_is_big_endian(env->config->isa);
> + bool swap_needed = HOST_BIG_ENDIAN != cpu_big_endian;
> uint32_t argptr;
>
> for (i = 0; i < argc; ++i) {
> @@ -388,6 +397,9 @@ void HELPER(simcall)(CPUXtensaState *env)
> int str_size = strlen(str) + 1;
>
> argptr = tswap32(regs[3] + str_offset);
The tswap() is still here.
> + if (swap_needed) {
> + bswap32s(&argptr);
> + }
>
> cpu_memory_rw_debug(cs,
> regs[3] + i * sizeof(uint32_t),
--
Thanks.
-- Max
On 6/12/24 13:41, Max Filippov wrote:
> On Thu, Dec 5, 2024 at 3:24 PM Philippe Mathieu-Daudé <philmd@linaro.org> 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.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> target/xtensa/xtensa-semi.c | 16 ++++++++++++++--
>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
>> index fa21b7e11fc..21d23e39de5 100644
>> --- a/target/xtensa/xtensa-semi.c
>> +++ b/target/xtensa/xtensa-semi.c
>> @@ -328,10 +328,17 @@ void HELPER(simcall)(CPUXtensaState *env)
>> struct timeval tv = {0};
>>
>> if (target_tv) {
>> + bool cpu_big_endian = xtensa_isa_is_big_endian(env->config->isa);
>> + bool swap_needed = HOST_BIG_ENDIAN != cpu_big_endian;
>> +
>> 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]);
>> + if (swap_needed) {
>> + bswap32s(&target_tvv[0]);
>> + bswap32s(&target_tvv[1]);
>> + }
>> + tv.tv_sec = (int32_t)target_tvv[0];
>> + tv.tv_usec = (int32_t)target_tvv[1];
>
> This conversion looks a bit heavy. Maybe it would be better to
> give tswap*() an additional env argument and give the target
> a way to deal with it?
Yeah, something like that. TBH I'm not really happy with these
changes, and am having hard time figuring out a simple and
meaningful API.
>
>> }
>> if (fd < 3 && sim_console) {
>> if ((fd == 1 || fd == 2) && rq == SELECT_ONE_WRITE) {
>> @@ -381,6 +388,8 @@ void HELPER(simcall)(CPUXtensaState *env)
>> int argc = semihosting_get_argc();
>> int str_offset = (argc + 1) * sizeof(uint32_t);
>> int i;
>> + bool cpu_big_endian = xtensa_isa_is_big_endian(env->config->isa);
>> + bool swap_needed = HOST_BIG_ENDIAN != cpu_big_endian;
>> uint32_t argptr;
>>
>> for (i = 0; i < argc; ++i) {
>> @@ -388,6 +397,9 @@ void HELPER(simcall)(CPUXtensaState *env)
>> int str_size = strlen(str) + 1;
>>
>> argptr = tswap32(regs[3] + str_offset);
>
> The tswap() is still here.
Oops!
>
>> + if (swap_needed) {
>> + bswap32s(&argptr);
>> + }
>>
>> cpu_memory_rw_debug(cs,
>> regs[3] + i * sizeof(uint32_t),
>
On Fri, 6 Dec 2024 at 14:35, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 6/12/24 13:41, Max Filippov wrote:
> > On Thu, Dec 5, 2024 at 3:24 PM Philippe Mathieu-Daudé <philmd@linaro.org> 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.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> >> ---
> >> target/xtensa/xtensa-semi.c | 16 ++++++++++++++--
> >> 1 file changed, 14 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/target/xtensa/xtensa-semi.c b/target/xtensa/xtensa-semi.c
> >> index fa21b7e11fc..21d23e39de5 100644
> >> --- a/target/xtensa/xtensa-semi.c
> >> +++ b/target/xtensa/xtensa-semi.c
> >> @@ -328,10 +328,17 @@ void HELPER(simcall)(CPUXtensaState *env)
> >> struct timeval tv = {0};
> >>
> >> if (target_tv) {
> >> + bool cpu_big_endian = xtensa_isa_is_big_endian(env->config->isa);
> >> + bool swap_needed = HOST_BIG_ENDIAN != cpu_big_endian;
> >> +
> >> 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]);
> >> + if (swap_needed) {
> >> + bswap32s(&target_tvv[0]);
> >> + bswap32s(&target_tvv[1]);
> >> + }
> >> + tv.tv_sec = (int32_t)target_tvv[0];
> >> + tv.tv_usec = (int32_t)target_tvv[1];
> >
> > This conversion looks a bit heavy. Maybe it would be better to
> > give tswap*() an additional env argument and give the target
> > a way to deal with it?
>
> Yeah, something like that. TBH I'm not really happy with these
> changes, and am having hard time figuring out a simple and
> meaningful API.
You could punt the issue here by having xtensa include
semihosting/uaccess.h, and then replace the direct
cpu_memory_rw_debug / tswap calls with
get_user_u32(tv.tv_sec, target_tv);
get_user_u32(tv.tv_sec, target_tv + 4);
These are still doing cpu_memory_rw_debug + tswap under
the hood, but it makes this code in xtensa-semi.c simpler :-)
and you'll need to figure out the plan for get/put_user_*
at some point for Arm and other common-semihosting targets.
> >
> >> }
> >> if (fd < 3 && sim_console) {
> >> if ((fd == 1 || fd == 2) && rq == SELECT_ONE_WRITE) {
> >> @@ -381,6 +388,8 @@ void HELPER(simcall)(CPUXtensaState *env)
> >> int argc = semihosting_get_argc();
> >> int str_offset = (argc + 1) * sizeof(uint32_t);
> >> int i;
> >> + bool cpu_big_endian = xtensa_isa_is_big_endian(env->config->isa);
> >> + bool swap_needed = HOST_BIG_ENDIAN != cpu_big_endian;
> >> uint32_t argptr;
> >>
> >> for (i = 0; i < argc; ++i) {
> >> @@ -388,6 +397,9 @@ void HELPER(simcall)(CPUXtensaState *env)
> >> int str_size = strlen(str) + 1;
> >>
> >> argptr = tswap32(regs[3] + str_offset);
> >
> > The tswap() is still here.
>
> Oops!
Similarly here
put_user_u32(regs[3] + str_offset, regs[3] + i * sizeof(uint32_t));
-- PMM
© 2016 - 2026 Red Hat, Inc.