On 8/2/25 12:58 PM, Peter Maydell wrote:
On Sat, 2 Aug 2025 at 00:26, Pierrick Bouvier
<[email protected]> wrote:
This allows to get rid of sizeof(target_ulong) for riscv, without
changing the semantic.
Reviewed-by: Richard Henderson <[email protected]>
Signed-off-by: Pierrick Bouvier <[email protected]>
---
target/arm/common-semi-target.h | 5 -----
target/riscv/common-semi-target.h | 5 -----
semihosting/arm-compat-semi.c | 4 +++-
3 files changed, 3 insertions(+), 11 deletions(-)
diff --git a/target/arm/common-semi-target.h b/target/arm/common-semi-target.h
index da51f2d7f54..7ebd2136d93 100644
--- a/target/arm/common-semi-target.h
+++ b/target/arm/common-semi-target.h
@@ -34,11 +34,6 @@ static inline void common_semi_set_ret(CPUState *cs,
target_ulong ret)
}
}
-static inline bool common_semi_sys_exit_extended(CPUState *cs, int nr)
-{
- return nr == TARGET_SYS_EXIT_EXTENDED || is_a64(cpu_env(cs));
-}
-
static inline bool is_64bit_semihosting(CPUArchState *env)
{
return is_a64(env);
diff --git a/target/riscv/common-semi-target.h
b/target/riscv/common-semi-target.h
index 7c8a59e0cc3..63bbcd2d5fa 100644
--- a/target/riscv/common-semi-target.h
+++ b/target/riscv/common-semi-target.h
@@ -25,11 +25,6 @@ static inline void common_semi_set_ret(CPUState *cs,
target_ulong ret)
env->gpr[xA0] = ret;
}
-static inline bool common_semi_sys_exit_extended(CPUState *cs, int nr)
-{
- return (nr == TARGET_SYS_EXIT_EXTENDED || sizeof(target_ulong) == 8);
-}
-
static inline bool is_64bit_semihosting(CPUArchState *env)
{
return riscv_cpu_mxl(env) != MXL_RV32;
diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index 3f653c6e7a9..ef57d7205c8 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -755,7 +755,9 @@ void do_common_semihosting(CPUState *cs)
{
uint32_t ret;
- if (common_semi_sys_exit_extended(cs, nr)) {
+ bool extended = (nr == TARGET_SYS_EXIT_EXTENDED ||
+ is_64bit_semihosting(cpu_env(cs)));
This doesn't look right. Whether a target supports the sensible
(extended) semantics for SYS_EXIT is a different question from
whether it's 32 bit or not. For Arm, it happens that the point
where we defined the newer semantics coincided with the A64
architecture. For riscv I don't know -- if they made the 32-bit
riscv not have SYS_EXIT_EXTENDED that was an unfortunate choice
but they're probably stuck with it now. For any future architecture
that decides to adopt Arm-compatible semihosting the right
choice will be to provide the extended semantics regardless of
bit width.
This is why there's a common_semi_sys_exit_extended() function
that each target architecture needs to implement.
It happens to coincide for now, so I felt it was useless to leave this
function given both architecture have the exact same implementation:
nr == TARGET_SYS_EXIT_EXTENDED ||
is_64bit_semihosting(cpu_env(cs))
As you prefer we keep it for a potential new architecture, I will remove
this patch and keep it.
thanks
-- PMM
Regards,
Pierrick
