diff mbox series

[v4,03/11] target/mips: Create report_fault for semihosting

Message ID 20220608051945.802339-4-richard.henderson@linaro.org
State Superseded
Headers show
Series target/mips: semihosting cleanup | expand

Commit Message

Richard Henderson June 8, 2022, 5:19 a.m. UTC 
The UHI specification does not have an EFAULT value,
and further specifies that "undefined UHI operations
should not return control to the target".

So, log the error and abort.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/mips/tcg/sysemu/mips-semi.c | 33 ++++++++++++++----------------
 1 file changed, 15 insertions(+), 18 deletions(-)

Comments

Philippe Mathieu-Daudé June 10, 2022, 3:05 p.m. UTC | #1 
Hi Richard,

On 8/6/22 07:19, Richard Henderson wrote:
> The UHI specification does not have an EFAULT value,
> and further specifies that "undefined UHI operations
> should not return control to the target".
> 
> So, log the error and abort.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   target/mips/tcg/sysemu/mips-semi.c | 33 ++++++++++++++----------------
>   1 file changed, 15 insertions(+), 18 deletions(-)
> 
> diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
> index 2a039baf4c..33221444e1 100644
> --- a/target/mips/tcg/sysemu/mips-semi.c
> +++ b/target/mips/tcg/sysemu/mips-semi.c
> @@ -114,6 +114,13 @@ enum UHIErrno {
>       UHI_EXDEV           = 18,
>   };
>   
> +static void report_fault(CPUMIPSState *env)
> +{
> +    int op = env->active_tc.gpr[25];
> +    error_report("Fault during UHI operation %d", op);
> +    abort();

This is a guest error, no need to debug QEMU internals...
Can we simply exit(1) instead?
Richard Henderson June 11, 2022, 3:53 p.m. UTC | #2 
On 6/10/22 08:05, Philippe Mathieu-Daudé wrote:
>> +static void report_fault(CPUMIPSState *env)
>> +{
>> +    int op = env->active_tc.gpr[25];
>> +    error_report("Fault during UHI operation %d", op);
>> +    abort();
> 
> This is a guest error, no need to debug QEMU internals...
> Can we simply exit(1) instead?

How does this debug qemu internals?
It exits with SIGABRT.

I suppose we could exit(1), but we'd want to change the other existing uses of abort too.


r~
diff mbox series

Patch

diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 2a039baf4c..33221444e1 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -114,6 +114,13 @@  enum UHIErrno {
     UHI_EXDEV           = 18,
 };
 
+static void report_fault(CPUMIPSState *env)
+{
+    int op = env->active_tc.gpr[25];
+    error_report("Fault during UHI operation %d", op);
+    abort();
+}
+
 static int errno_mips(int host_errno)
 {
     /* Errno values taken from asm-mips/errno.h */
@@ -136,8 +143,7 @@  static int copy_stat_to_target(CPUMIPSState *env, const struct stat *src,
     hwaddr len = sizeof(struct UHIStat);
     UHIStat *dst = lock_user(VERIFY_WRITE, vaddr, len, 0);
     if (!dst) {
-        errno = EFAULT;
-        return -1;
+        report_fault(env);
     }
 
     dst->uhi_st_dev = tswap16(src->st_dev);
@@ -188,8 +194,7 @@  static int write_to_file(CPUMIPSState *env, target_ulong fd, target_ulong vaddr,
     int num_of_bytes;
     void *dst = lock_user(VERIFY_READ, vaddr, len, 1);
     if (!dst) {
-        errno = EFAULT;
-        return -1;
+        report_fault(env);
     }
 
     if (offset) {
@@ -213,8 +218,7 @@  static int read_from_file(CPUMIPSState *env, target_ulong fd,
     int num_of_bytes;
     void *dst = lock_user(VERIFY_WRITE, vaddr, len, 0);
     if (!dst) {
-        errno = EFAULT;
-        return -1;
+        report_fault(env);
     }
 
     if (offset) {
@@ -237,7 +241,7 @@  static int copy_argn_to_target(CPUMIPSState *env, int arg_num,
     int strsize = strlen(semihosting_get_arg(arg_num)) + 1;
     char *dst = lock_user(VERIFY_WRITE, vaddr, strsize, 0);
     if (!dst) {
-        return -1;
+        report_fault(env);
     }
 
     strcpy(dst, semihosting_get_arg(arg_num));
@@ -250,9 +254,7 @@  static int copy_argn_to_target(CPUMIPSState *env, int arg_num,
     do {                                        \
         p = lock_user_string(addr);             \
         if (!p) {                               \
-            gpr[2] = -1;                        \
-            gpr[3] = EFAULT;                    \
-            return;                             \
+            report_fault(env);                  \
         }                                       \
     } while (0)
 
@@ -260,16 +262,11 @@  static int copy_argn_to_target(CPUMIPSState *env, int arg_num,
     do {                                                \
         p = lock_user_string(addr);                     \
         if (!p) {                                       \
-            gpr[2] = -1;                                \
-            gpr[3] = EFAULT;                            \
-            return;                                     \
+            report_fault(env);                          \
         }                                               \
         p2 = lock_user_string(addr2);                   \
         if (!p2) {                                      \
-            unlock_user(p, addr, 0);                    \
-            gpr[2] = -1;                                \
-            gpr[3] = EFAULT;                            \
-            return;                                     \
+            report_fault(env);                          \
         }                                               \
     } while (0)
 
@@ -400,7 +397,7 @@  void mips_semihosting(CPUMIPSState *env)
         break;
 #endif
     default:
-        fprintf(stderr, "Unknown UHI operation %d\n", op);
+        error_report("Unknown UHI operation %d", op);
         abort();
     }
     return;