diff mbox series

[v2,2/4] semihosting/arm-compat-semi: unify GET/SET_ARG helpers

Message ID 20210309141727.12522-3-alex.bennee@linaro.org
State Superseded
Headers show
Series semihosting/next (SYS_HEAPINFO fix) | expand

Commit Message

Alex Bennée March 9, 2021, 2:17 p.m. UTC
From the semihosting point of view what we want to know is the current
mode of the processor. Unify this into a single helper and allow us to
use the same GET/SET_ARG helpers for the rest of the code.

Note: we aren't currently testing riscv32 due to missing toolchain for
check-tcg tests.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

---
 semihosting/arm-compat-semi.c | 54 ++++++++++++-----------------------
 1 file changed, 18 insertions(+), 36 deletions(-)

-- 
2.20.1

Comments

Peter Maydell March 9, 2021, 4:33 p.m. UTC | #1
On Tue, 9 Mar 2021 at 14:24, Alex Bennée <alex.bennee@linaro.org> wrote:
>

> From the semihosting point of view what we want to know is the current

> mode of the processor. Unify this into a single helper and allow us to

> use the same GET/SET_ARG helpers for the rest of the code.

>

> Note: we aren't currently testing riscv32 due to missing toolchain for

> check-tcg tests.

>

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

> ---

>  semihosting/arm-compat-semi.c | 54 ++++++++++++-----------------------

>  1 file changed, 18 insertions(+), 36 deletions(-)

>

> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c

> index 94950b6c56..733eea1e2d 100644

> --- a/semihosting/arm-compat-semi.c

> +++ b/semihosting/arm-compat-semi.c

> @@ -767,15 +767,28 @@ static const GuestFDFunctions guestfd_fns[] = {

>      },

>  };

>

> -/* Read the input value from the argument block; fail the semihosting

> - * call if the memory read fails.

> +/*

> + * Read the input value from the argument block; fail the semihosting

> + * call if the memory read fails. Eventually we could use a generic

> + * CPUState helper function here.

>   */

> +static inline bool is_64bit_semihosting(CPUArchState *env)

> +{

>  #ifdef TARGET_ARM

> +    return is_a64(env);

> +#elif defined(TARGET_RISCV)


can we be consistent about whether we use defined() or not, please ?

> +    return !riscv_cpu_is_32bit(env);

> +#else

> +#error un-handled architecture

> +#endif


Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


thanks
-- PMM
Xingtao Yao (Fujitsu)" via March 9, 2021, 5:02 p.m. UTC | #2
Alex Bennée <alex.bennee@linaro.org> writes:

> Note: we aren't currently testing riscv32 due to missing toolchain for

> check-tcg tests.


That's surprising -- the usual risc-v toolchain supports both 64- and
32- bit targets.

Othewise, this patch is

Reviewed-by: Keith Packard <keithp@keithp.com>


-- 
-keith
Alex Bennée March 9, 2021, 5:24 p.m. UTC | #3
Keith Packard <keithp@keithp.com> writes:

> Alex Bennée <alex.bennee@linaro.org> writes:

>

>> Note: we aren't currently testing riscv32 due to missing toolchain for

>> check-tcg tests.

>

> That's surprising -- the usual risc-v toolchain supports both 64- and

> 32- bit targets.


If you can give me the runes for it I can tweak check-tcg to generate
the 32 bit binaries.

>

> Othewise, this patch is

>

> Reviewed-by: Keith Packard <keithp@keithp.com>



-- 
Alex Bennée
diff mbox series

Patch

diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index 94950b6c56..733eea1e2d 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -767,15 +767,28 @@  static const GuestFDFunctions guestfd_fns[] = {
     },
 };
 
-/* Read the input value from the argument block; fail the semihosting
- * call if the memory read fails.
+/*
+ * Read the input value from the argument block; fail the semihosting
+ * call if the memory read fails. Eventually we could use a generic
+ * CPUState helper function here.
  */
+static inline bool is_64bit_semihosting(CPUArchState *env)
+{
 #ifdef TARGET_ARM
+    return is_a64(env);
+#elif defined(TARGET_RISCV)
+    return !riscv_cpu_is_32bit(env);
+#else
+#error un-handled architecture
+#endif
+}
+
+
 #define GET_ARG(n) do {                                 \
-    if (is_a64(env)) {                                  \
+    if (is_64bit_semihosting(env)) {                    \
         if (get_user_u64(arg ## n, args + (n) * 8)) {   \
             errno = EFAULT;                             \
-            return set_swi_errno(cs, -1);              \
+            return set_swi_errno(cs, -1);               \
         }                                               \
     } else {                                            \
         if (get_user_u32(arg ## n, args + (n) * 4)) {   \
@@ -786,41 +799,10 @@  static const GuestFDFunctions guestfd_fns[] = {
 } while (0)
 
 #define SET_ARG(n, val)                                 \
-    (is_a64(env) ?                                      \
+    (is_64bit_semihosting(env) ?                        \
      put_user_u64(val, args + (n) * 8) :                \
      put_user_u32(val, args + (n) * 4))
-#endif
 
-#ifdef TARGET_RISCV
-
-/*
- * get_user_ual is defined as get_user_u32 in softmmu-semi.h,
- * we need a macro that fetches a target_ulong
- */
-#define get_user_utl(arg, p)                    \
-    ((sizeof(target_ulong) == 8) ?              \
-     get_user_u64(arg, p) :                     \
-     get_user_u32(arg, p))
-
-/*
- * put_user_ual is defined as put_user_u32 in softmmu-semi.h,
- * we need a macro that stores a target_ulong
- */
-#define put_user_utl(arg, p)                    \
-    ((sizeof(target_ulong) == 8) ?              \
-     put_user_u64(arg, p) :                     \
-     put_user_u32(arg, p))
-
-#define GET_ARG(n) do {                                                 \
-        if (get_user_utl(arg ## n, args + (n) * sizeof(target_ulong))) { \
-            errno = EFAULT;                                             \
-            return set_swi_errno(cs, -1);                              \
-        }                                                               \
-    } while (0)
-
-#define SET_ARG(n, val)                                 \
-    put_user_utl(val, args + (n) * sizeof(target_ulong))
-#endif
 
 /*
  * Do a semihosting call.