* [PATCH v4 01/11] target/mips: Use an exception for semihosting
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-10 15:07 ` Philippe Mathieu-Daudé via
2022-06-08 5:19 ` [PATCH v4 02/11] target/mips: Add UHI errno values Richard Henderson
` (9 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
Within do_interrupt, we hold the iothread lock, which
is required for Chardev access for the console, and for
the round trip for use_gdb_syscalls().
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/mips/cpu.h | 3 ++-
target/mips/tcg/tcg-internal.h | 2 ++
target/mips/tcg/sysemu_helper.h.inc | 2 --
target/mips/tcg/exception.c | 1 +
target/mips/tcg/sysemu/mips-semi.c | 4 ++--
target/mips/tcg/sysemu/tlb_helper.c | 4 ++++
target/mips/tcg/translate.c | 12 ++----------
target/mips/tcg/micromips_translate.c.inc | 6 +++---
target/mips/tcg/mips16e_translate.c.inc | 2 +-
target/mips/tcg/nanomips_translate.c.inc | 4 ++--
10 files changed, 19 insertions(+), 21 deletions(-)
diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 5335ac10a3..f56a5a95c4 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -1251,8 +1251,9 @@ enum {
EXCP_MSAFPE,
EXCP_TLBXI,
EXCP_TLBRI,
+ EXCP_SEMIHOST,
- EXCP_LAST = EXCP_TLBRI,
+ EXCP_LAST = EXCP_SEMIHOST,
};
/*
diff --git a/target/mips/tcg/tcg-internal.h b/target/mips/tcg/tcg-internal.h
index 993720b00c..1d27fa2ff9 100644
--- a/target/mips/tcg/tcg-internal.h
+++ b/target/mips/tcg/tcg-internal.h
@@ -62,6 +62,8 @@ bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
MMUAccessType access_type, int mmu_idx,
bool probe, uintptr_t retaddr);
+void mips_semihosting(CPUMIPSState *env);
+
#endif /* !CONFIG_USER_ONLY */
#endif
diff --git a/target/mips/tcg/sysemu_helper.h.inc b/target/mips/tcg/sysemu_helper.h.inc
index 4353a966f9..af585b5d9c 100644
--- a/target/mips/tcg/sysemu_helper.h.inc
+++ b/target/mips/tcg/sysemu_helper.h.inc
@@ -9,8 +9,6 @@
* SPDX-License-Identifier: LGPL-2.1-or-later
*/
-DEF_HELPER_1(do_semihosting, void, env)
-
/* CP0 helpers */
DEF_HELPER_1(mfc0_mvpcontrol, tl, env)
DEF_HELPER_1(mfc0_mvpconf0, tl, env)
diff --git a/target/mips/tcg/exception.c b/target/mips/tcg/exception.c
index 0b21e0872b..2bd77a61de 100644
--- a/target/mips/tcg/exception.c
+++ b/target/mips/tcg/exception.c
@@ -125,6 +125,7 @@ static const char * const excp_names[EXCP_LAST + 1] = {
[EXCP_TLBRI] = "TLB read-inhibit",
[EXCP_MSADIS] = "MSA disabled",
[EXCP_MSAFPE] = "MSA floating point",
+ [EXCP_SEMIHOST] = "Semihosting",
};
const char *mips_exception_name(int32_t exception)
diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 6d6296e709..ac12c802a3 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -20,10 +20,10 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "qemu/log.h"
-#include "exec/helper-proto.h"
#include "semihosting/softmmu-uaccess.h"
#include "semihosting/semihost.h"
#include "semihosting/console.h"
+#include "internal.h"
typedef enum UHIOp {
UHI_exit = 1,
@@ -238,7 +238,7 @@ static int copy_argn_to_target(CPUMIPSState *env, int arg_num,
unlock_user(p, gpr, 0); \
} while (0)
-void helper_do_semihosting(CPUMIPSState *env)
+void mips_semihosting(CPUMIPSState *env)
{
target_ulong *gpr = env->active_tc.gpr;
const UHIOp op = gpr[25];
diff --git a/target/mips/tcg/sysemu/tlb_helper.c b/target/mips/tcg/sysemu/tlb_helper.c
index 73254d1929..57ffad2902 100644
--- a/target/mips/tcg/sysemu/tlb_helper.c
+++ b/target/mips/tcg/sysemu/tlb_helper.c
@@ -1053,6 +1053,10 @@ void mips_cpu_do_interrupt(CPUState *cs)
}
offset = 0x180;
switch (cs->exception_index) {
+ case EXCP_SEMIHOST:
+ cs->exception_index = EXCP_NONE;
+ mips_semihosting(env);
+ return;
case EXCP_DSS:
env->CP0_Debug |= 1 << CP0DB_DSS;
/*
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 6de5b66650..e554b3adcc 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -12094,14 +12094,6 @@ static inline bool is_uhi(int sdbbp_code)
#endif
}
-#ifdef CONFIG_USER_ONLY
-/* The above should dead-code away any calls to this..*/
-static inline void gen_helper_do_semihosting(void *env)
-{
- g_assert_not_reached();
-}
-#endif
-
void gen_ldxs(DisasContext *ctx, int base, int index, int rd)
{
TCGv t0 = tcg_temp_new();
@@ -13910,7 +13902,7 @@ static void decode_opc_special_r6(CPUMIPSState *env, DisasContext *ctx)
break;
case R6_OPC_SDBBP:
if (is_uhi(extract32(ctx->opcode, 6, 20))) {
- gen_helper_do_semihosting(cpu_env);
+ generate_exception_end(ctx, EXCP_SEMIHOST);
} else {
if (ctx->hflags & MIPS_HFLAG_SBRI) {
gen_reserved_instruction(ctx);
@@ -14322,7 +14314,7 @@ static void decode_opc_special2_legacy(CPUMIPSState *env, DisasContext *ctx)
break;
case OPC_SDBBP:
if (is_uhi(extract32(ctx->opcode, 6, 20))) {
- gen_helper_do_semihosting(cpu_env);
+ generate_exception_end(ctx, EXCP_SEMIHOST);
} else {
/*
* XXX: not clear which exception should be raised
diff --git a/target/mips/tcg/micromips_translate.c.inc b/target/mips/tcg/micromips_translate.c.inc
index fc6ede75b8..274caf2c3c 100644
--- a/target/mips/tcg/micromips_translate.c.inc
+++ b/target/mips/tcg/micromips_translate.c.inc
@@ -826,7 +826,7 @@ static void gen_pool16c_insn(DisasContext *ctx)
break;
case SDBBP16:
if (is_uhi(extract32(ctx->opcode, 0, 4))) {
- gen_helper_do_semihosting(cpu_env);
+ generate_exception_end(ctx, EXCP_SEMIHOST);
} else {
/*
* XXX: not clear which exception should be raised
@@ -942,7 +942,7 @@ static void gen_pool16c_r6_insn(DisasContext *ctx)
case R6_SDBBP16:
/* SDBBP16 */
if (is_uhi(extract32(ctx->opcode, 6, 4))) {
- gen_helper_do_semihosting(cpu_env);
+ generate_exception_end(ctx, EXCP_SEMIHOST);
} else {
if (ctx->hflags & MIPS_HFLAG_SBRI) {
generate_exception(ctx, EXCP_RI);
@@ -1311,7 +1311,7 @@ static void gen_pool32axf(CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
break;
case SDBBP:
if (is_uhi(extract32(ctx->opcode, 16, 10))) {
- gen_helper_do_semihosting(cpu_env);
+ generate_exception_end(ctx, EXCP_SEMIHOST);
} else {
check_insn(ctx, ISA_MIPS_R1);
if (ctx->hflags & MIPS_HFLAG_SBRI) {
diff --git a/target/mips/tcg/mips16e_translate.c.inc b/target/mips/tcg/mips16e_translate.c.inc
index f57e0a5f2a..0a3ba252e4 100644
--- a/target/mips/tcg/mips16e_translate.c.inc
+++ b/target/mips/tcg/mips16e_translate.c.inc
@@ -952,7 +952,7 @@ static int decode_ase_mips16e(CPUMIPSState *env, DisasContext *ctx)
break;
case RR_SDBBP:
if (is_uhi(extract32(ctx->opcode, 5, 6))) {
- gen_helper_do_semihosting(cpu_env);
+ generate_exception_end(ctx, EXCP_SEMIHOST);
} else {
/*
* XXX: not clear which exception should be raised
diff --git a/target/mips/tcg/nanomips_translate.c.inc b/target/mips/tcg/nanomips_translate.c.inc
index 916cece4d2..5b0e4683a7 100644
--- a/target/mips/tcg/nanomips_translate.c.inc
+++ b/target/mips/tcg/nanomips_translate.c.inc
@@ -3691,7 +3691,7 @@ static int decode_nanomips_32_48_opc(CPUMIPSState *env, DisasContext *ctx)
break;
case NM_SDBBP:
if (is_uhi(extract32(ctx->opcode, 0, 19))) {
- gen_helper_do_semihosting(cpu_env);
+ generate_exception_end(ctx, EXCP_SEMIHOST);
} else {
if (ctx->hflags & MIPS_HFLAG_SBRI) {
gen_reserved_instruction(ctx);
@@ -4609,7 +4609,7 @@ static int decode_isa_nanomips(CPUMIPSState *env, DisasContext *ctx)
break;
case NM_SDBBP16:
if (is_uhi(extract32(ctx->opcode, 0, 3))) {
- gen_helper_do_semihosting(cpu_env);
+ generate_exception_end(ctx, EXCP_SEMIHOST);
} else {
if (ctx->hflags & MIPS_HFLAG_SBRI) {
gen_reserved_instruction(ctx);
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 01/11] target/mips: Use an exception for semihosting
2022-06-08 5:19 ` [PATCH v4 01/11] target/mips: Use an exception for semihosting Richard Henderson
@ 2022-06-10 15:07 ` Philippe Mathieu-Daudé via
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-06-10 15:07 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 8/6/22 07:19, Richard Henderson wrote:
> Within do_interrupt, we hold the iothread lock, which
> is required for Chardev access for the console, and for
> the round trip for use_gdb_syscalls().
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/mips/cpu.h | 3 ++-
> target/mips/tcg/tcg-internal.h | 2 ++
> target/mips/tcg/sysemu_helper.h.inc | 2 --
> target/mips/tcg/exception.c | 1 +
> target/mips/tcg/sysemu/mips-semi.c | 4 ++--
> target/mips/tcg/sysemu/tlb_helper.c | 4 ++++
> target/mips/tcg/translate.c | 12 ++----------
> target/mips/tcg/micromips_translate.c.inc | 6 +++---
> target/mips/tcg/mips16e_translate.c.inc | 2 +-
> target/mips/tcg/nanomips_translate.c.inc | 4 ++--
> 10 files changed, 19 insertions(+), 21 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 02/11] target/mips: Add UHI errno values
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
2022-06-08 5:19 ` [PATCH v4 01/11] target/mips: Use an exception for semihosting Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-10 15:07 ` Philippe Mathieu-Daudé via
2022-06-08 5:19 ` [PATCH v4 03/11] target/mips: Create report_fault for semihosting Richard Henderson
` (8 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
From the Unified Hosting Interface, MD01069 Reference Manual,
version 1.1.6, 06 July 2015.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/mips/tcg/sysemu/mips-semi.c | 40 ++++++++++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index ac12c802a3..2a039baf4c 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -74,6 +74,46 @@ enum UHIOpenFlags {
UHIOpen_EXCL = 0x800
};
+enum UHIErrno {
+ UHI_EACCESS = 13,
+ UHI_EAGAIN = 11,
+ UHI_EBADF = 9,
+ UHI_EBADMSG = 77,
+ UHI_EBUSY = 16,
+ UHI_ECONNRESET = 104,
+ UHI_EEXIST = 17,
+ UHI_EFBIG = 27,
+ UHI_EINTR = 4,
+ UHI_EINVAL = 22,
+ UHI_EIO = 5,
+ UHI_EISDIR = 21,
+ UHI_ELOOP = 92,
+ UHI_EMFILE = 24,
+ UHI_EMLINK = 31,
+ UHI_ENAMETOOLONG = 91,
+ UHI_ENETDOWN = 115,
+ UHI_ENETUNREACH = 114,
+ UHI_ENFILE = 23,
+ UHI_ENOBUFS = 105,
+ UHI_ENOENT = 2,
+ UHI_ENOMEM = 12,
+ UHI_ENOSPC = 28,
+ UHI_ENOSR = 63,
+ UHI_ENOTCONN = 128,
+ UHI_ENOTDIR = 20,
+ UHI_ENXIO = 6,
+ UHI_EOVERFLOW = 139,
+ UHI_EPERM = 1,
+ UHI_EPIPE = 32,
+ UHI_ERANGE = 34,
+ UHI_EROFS = 30,
+ UHI_ESPIPE = 29,
+ UHI_ETIMEDOUT = 116,
+ UHI_ETXTBSY = 26,
+ UHI_EWOULDBLOCK = 11,
+ UHI_EXDEV = 18,
+};
+
static int errno_mips(int host_errno)
{
/* Errno values taken from asm-mips/errno.h */
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 03/11] target/mips: Create report_fault for semihosting
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
2022-06-08 5:19 ` [PATCH v4 01/11] target/mips: Use an exception for semihosting Richard Henderson
2022-06-08 5:19 ` [PATCH v4 02/11] target/mips: Add UHI errno values Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-10 15:05 ` Philippe Mathieu-Daudé via
2022-06-08 5:19 ` [PATCH v4 04/11] target/mips: Drop link syscall from semihosting Richard Henderson
` (7 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
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();
+}
+
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;
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 03/11] target/mips: Create report_fault for semihosting
2022-06-08 5:19 ` [PATCH v4 03/11] target/mips: Create report_fault for semihosting Richard Henderson
@ 2022-06-10 15:05 ` Philippe Mathieu-Daudé via
2022-06-11 15:53 ` Richard Henderson
0 siblings, 1 reply; 20+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-06-10 15:05 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
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?
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH v4 03/11] target/mips: Create report_fault for semihosting
2022-06-10 15:05 ` Philippe Mathieu-Daudé via
@ 2022-06-11 15:53 ` Richard Henderson
0 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2022-06-11 15:53 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
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~
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 04/11] target/mips: Drop link syscall from semihosting
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
` (2 preceding siblings ...)
2022-06-08 5:19 ` [PATCH v4 03/11] target/mips: Create report_fault for semihosting Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-08 5:19 ` [PATCH v4 05/11] target/mips: Drop pread and pwrite syscalls " Richard Henderson
` (6 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
We don't implement it with _WIN32 hosts, and the syscall
is missing from the gdb remote file i/o interface.
Since we can't implement it universally, drop it.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/mips/tcg/sysemu/mips-semi.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 33221444e1..254c7fad9a 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -387,15 +387,6 @@ void mips_semihosting(CPUMIPSState *env)
gpr[2] = write_to_file(env, gpr[4], gpr[5], gpr[6], gpr[7]);
gpr[3] = errno_mips(errno);
break;
-#ifndef _WIN32
- case UHI_link:
- GET_TARGET_STRINGS_2(p, gpr[4], p2, gpr[5]);
- gpr[2] = link(p, p2);
- gpr[3] = errno_mips(errno);
- FREE_TARGET_STRING(p2, gpr[5]);
- FREE_TARGET_STRING(p, gpr[4]);
- break;
-#endif
default:
error_report("Unknown UHI operation %d", op);
abort();
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 05/11] target/mips: Drop pread and pwrite syscalls from semihosting
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
` (3 preceding siblings ...)
2022-06-08 5:19 ` [PATCH v4 04/11] target/mips: Drop link syscall from semihosting Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-10 15:07 ` Philippe Mathieu-Daudé via
2022-06-08 5:19 ` [PATCH v4 06/11] target/mips: Use semihosting/syscalls.h Richard Henderson
` (5 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
We don't implement it with _WIN32 hosts, and the syscalls
are missing from the gdb remote file i/o interface.
Since we can't implement them universally, drop them.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/mips/tcg/sysemu/mips-semi.c | 39 ++++++------------------------
1 file changed, 7 insertions(+), 32 deletions(-)
diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 254c7fad9a..93c9d3d0b3 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -188,8 +188,8 @@ static int get_open_flags(target_ulong target_flags)
return open_flags;
}
-static int write_to_file(CPUMIPSState *env, target_ulong fd, target_ulong vaddr,
- target_ulong len, target_ulong offset)
+static int write_to_file(CPUMIPSState *env, target_ulong fd,
+ target_ulong vaddr, target_ulong len)
{
int num_of_bytes;
void *dst = lock_user(VERIFY_READ, vaddr, len, 1);
@@ -197,23 +197,14 @@ static int write_to_file(CPUMIPSState *env, target_ulong fd, target_ulong vaddr,
report_fault(env);
}
- if (offset) {
-#ifdef _WIN32
- num_of_bytes = 0;
-#else
- num_of_bytes = pwrite(fd, dst, len, offset);
-#endif
- } else {
- num_of_bytes = write(fd, dst, len);
- }
+ num_of_bytes = write(fd, dst, len);
unlock_user(dst, vaddr, 0);
return num_of_bytes;
}
static int read_from_file(CPUMIPSState *env, target_ulong fd,
- target_ulong vaddr, target_ulong len,
- target_ulong offset)
+ target_ulong vaddr, target_ulong len)
{
int num_of_bytes;
void *dst = lock_user(VERIFY_WRITE, vaddr, len, 0);
@@ -221,15 +212,7 @@ static int read_from_file(CPUMIPSState *env, target_ulong fd,
report_fault(env);
}
- if (offset) {
-#ifdef _WIN32
- num_of_bytes = 0;
-#else
- num_of_bytes = pread(fd, dst, len, offset);
-#endif
- } else {
- num_of_bytes = read(fd, dst, len);
- }
+ num_of_bytes = read(fd, dst, len);
unlock_user(dst, vaddr, len);
return num_of_bytes;
@@ -309,11 +292,11 @@ void mips_semihosting(CPUMIPSState *env)
gpr[3] = errno_mips(errno);
break;
case UHI_read:
- gpr[2] = read_from_file(env, gpr[4], gpr[5], gpr[6], 0);
+ gpr[2] = read_from_file(env, gpr[4], gpr[5], gpr[6]);
gpr[3] = errno_mips(errno);
break;
case UHI_write:
- gpr[2] = write_to_file(env, gpr[4], gpr[5], gpr[6], 0);
+ gpr[2] = write_to_file(env, gpr[4], gpr[5], gpr[6]);
gpr[3] = errno_mips(errno);
break;
case UHI_lseek:
@@ -379,14 +362,6 @@ void mips_semihosting(CPUMIPSState *env)
FREE_TARGET_STRING(p, gpr[4]);
abort();
break;
- case UHI_pread:
- gpr[2] = read_from_file(env, gpr[4], gpr[5], gpr[6], gpr[7]);
- gpr[3] = errno_mips(errno);
- break;
- case UHI_pwrite:
- gpr[2] = write_to_file(env, gpr[4], gpr[5], gpr[6], gpr[7]);
- gpr[3] = errno_mips(errno);
- break;
default:
error_report("Unknown UHI operation %d", op);
abort();
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 05/11] target/mips: Drop pread and pwrite syscalls from semihosting
2022-06-08 5:19 ` [PATCH v4 05/11] target/mips: Drop pread and pwrite syscalls " Richard Henderson
@ 2022-06-10 15:07 ` Philippe Mathieu-Daudé via
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-06-10 15:07 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 8/6/22 07:19, Richard Henderson wrote:
> We don't implement it with _WIN32 hosts, and the syscalls
> are missing from the gdb remote file i/o interface.
> Since we can't implement them universally, drop them.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/mips/tcg/sysemu/mips-semi.c | 39 ++++++------------------------
> 1 file changed, 7 insertions(+), 32 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 06/11] target/mips: Use semihosting/syscalls.h
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
` (4 preceding siblings ...)
2022-06-08 5:19 ` [PATCH v4 05/11] target/mips: Drop pread and pwrite syscalls " Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-08 5:19 ` [PATCH v4 07/11] target/mips: Avoid qemu_semihosting_log_out for UHI_plog Richard Henderson
` (4 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
This separates guest file descriptors from host file descriptors,
and utilizes shared infrastructure for integration with gdbstub.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/mips/tcg/sysemu/mips-semi.c | 219 +++++++++++++----------------
1 file changed, 95 insertions(+), 124 deletions(-)
diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 93c9d3d0b3..5b78cf21a7 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -20,9 +20,11 @@
#include "qemu/osdep.h"
#include "cpu.h"
#include "qemu/log.h"
+#include "exec/gdbstub.h"
#include "semihosting/softmmu-uaccess.h"
#include "semihosting/semihost.h"
#include "semihosting/console.h"
+#include "semihosting/syscalls.h"
#include "internal.h"
typedef enum UHIOp {
@@ -121,101 +123,79 @@ static void report_fault(CPUMIPSState *env)
abort();
}
-static int errno_mips(int host_errno)
+static void uhi_cb(CPUState *cs, uint64_t ret, int err)
{
- /* Errno values taken from asm-mips/errno.h */
- switch (host_errno) {
- case 0: return 0;
- case ENAMETOOLONG: return 78;
-#ifdef EOVERFLOW
- case EOVERFLOW: return 79;
-#endif
-#ifdef ELOOP
- case ELOOP: return 90;
-#endif
- default: return EINVAL;
- }
-}
+ CPUMIPSState *env = cs->env_ptr;
-static int copy_stat_to_target(CPUMIPSState *env, const struct stat *src,
- target_ulong vaddr)
-{
- hwaddr len = sizeof(struct UHIStat);
- UHIStat *dst = lock_user(VERIFY_WRITE, vaddr, len, 0);
- if (!dst) {
+#define E(N) case E##N: err = UHI_E##N; break
+
+ switch (err) {
+ case 0:
+ break;
+ E(PERM);
+ E(NOENT);
+ E(INTR);
+ E(BADF);
+ E(BUSY);
+ E(EXIST);
+ E(NOTDIR);
+ E(ISDIR);
+ E(INVAL);
+ E(NFILE);
+ E(MFILE);
+ E(FBIG);
+ E(NOSPC);
+ E(SPIPE);
+ E(ROFS);
+ E(NAMETOOLONG);
+ default:
+ err = UHI_EINVAL;
+ break;
+ case EFAULT:
report_fault(env);
}
- dst->uhi_st_dev = tswap16(src->st_dev);
- dst->uhi_st_ino = tswap16(src->st_ino);
- dst->uhi_st_mode = tswap32(src->st_mode);
- dst->uhi_st_nlink = tswap16(src->st_nlink);
- dst->uhi_st_uid = tswap16(src->st_uid);
- dst->uhi_st_gid = tswap16(src->st_gid);
- dst->uhi_st_rdev = tswap16(src->st_rdev);
- dst->uhi_st_size = tswap64(src->st_size);
- dst->uhi_st_atime = tswap64(src->st_atime);
- dst->uhi_st_mtime = tswap64(src->st_mtime);
- dst->uhi_st_ctime = tswap64(src->st_ctime);
-#ifdef _WIN32
- dst->uhi_st_blksize = 0;
- dst->uhi_st_blocks = 0;
-#else
- dst->uhi_st_blksize = tswap64(src->st_blksize);
- dst->uhi_st_blocks = tswap64(src->st_blocks);
-#endif
- unlock_user(dst, vaddr, len);
- return 0;
+#undef E
+
+ env->active_tc.gpr[2] = ret;
+ env->active_tc.gpr[3] = err;
}
-static int get_open_flags(target_ulong target_flags)
+static void uhi_fstat_cb(CPUState *cs, uint64_t ret, int err)
{
- int open_flags = 0;
+ QEMU_BUILD_BUG_ON(sizeof(UHIStat) < sizeof(struct gdb_stat));
- if (target_flags & UHIOpen_RDWR) {
- open_flags |= O_RDWR;
- } else if (target_flags & UHIOpen_WRONLY) {
- open_flags |= O_WRONLY;
- } else {
- open_flags |= O_RDONLY;
+ if (!err) {
+ CPUMIPSState *env = cs->env_ptr;
+ target_ulong addr = env->active_tc.gpr[5];
+ UHIStat *dst = lock_user(VERIFY_WRITE, addr, sizeof(UHIStat), 1);
+ struct gdb_stat s;
+
+ if (!dst) {
+ report_fault(env);
+ }
+
+ memcpy(&s, dst, sizeof(struct gdb_stat));
+ memset(dst, 0, sizeof(UHIStat));
+
+ dst->uhi_st_dev = tswap16(be32_to_cpu(s.gdb_st_dev));
+ dst->uhi_st_ino = tswap16(be32_to_cpu(s.gdb_st_ino));
+ dst->uhi_st_mode = tswap32(be32_to_cpu(s.gdb_st_mode));
+ dst->uhi_st_nlink = tswap16(be32_to_cpu(s.gdb_st_nlink));
+ dst->uhi_st_uid = tswap16(be32_to_cpu(s.gdb_st_uid));
+ dst->uhi_st_gid = tswap16(be32_to_cpu(s.gdb_st_gid));
+ dst->uhi_st_rdev = tswap16(be32_to_cpu(s.gdb_st_rdev));
+ dst->uhi_st_size = tswap64(be64_to_cpu(s.gdb_st_size));
+ dst->uhi_st_atime = tswap64(be32_to_cpu(s.gdb_st_atime));
+ dst->uhi_st_mtime = tswap64(be32_to_cpu(s.gdb_st_mtime));
+ dst->uhi_st_ctime = tswap64(be32_to_cpu(s.gdb_st_ctime));
+ dst->uhi_st_blksize = tswap64(be64_to_cpu(s.gdb_st_blksize));
+ dst->uhi_st_blocks = tswap64(be64_to_cpu(s.gdb_st_blocks));
+
+ unlock_user(dst, addr, sizeof(UHIStat));
}
- open_flags |= (target_flags & UHIOpen_APPEND) ? O_APPEND : 0;
- open_flags |= (target_flags & UHIOpen_CREAT) ? O_CREAT : 0;
- open_flags |= (target_flags & UHIOpen_TRUNC) ? O_TRUNC : 0;
- open_flags |= (target_flags & UHIOpen_EXCL) ? O_EXCL : 0;
-
- return open_flags;
-}
-
-static int write_to_file(CPUMIPSState *env, target_ulong fd,
- target_ulong vaddr, target_ulong len)
-{
- int num_of_bytes;
- void *dst = lock_user(VERIFY_READ, vaddr, len, 1);
- if (!dst) {
- report_fault(env);
- }
-
- num_of_bytes = write(fd, dst, len);
-
- unlock_user(dst, vaddr, 0);
- return num_of_bytes;
-}
-
-static int read_from_file(CPUMIPSState *env, target_ulong fd,
- target_ulong vaddr, target_ulong len)
-{
- int num_of_bytes;
- void *dst = lock_user(VERIFY_WRITE, vaddr, len, 0);
- if (!dst) {
- report_fault(env);
- }
-
- num_of_bytes = read(fd, dst, len);
-
- unlock_user(dst, vaddr, len);
- return num_of_bytes;
+ uhi_cb(cs, ret, err);
}
static int copy_argn_to_target(CPUMIPSState *env, int arg_num,
@@ -260,68 +240,59 @@ static int copy_argn_to_target(CPUMIPSState *env, int arg_num,
void mips_semihosting(CPUMIPSState *env)
{
+ CPUState *cs = env_cpu(env);
target_ulong *gpr = env->active_tc.gpr;
const UHIOp op = gpr[25];
char *p, *p2;
switch (op) {
case UHI_exit:
- qemu_log("UHI(%d): exit(%d)\n", op, (int)gpr[4]);
+ gdb_exit(gpr[4]);
exit(gpr[4]);
+
case UHI_open:
- GET_TARGET_STRING(p, gpr[4]);
- if (!strcmp("/dev/stdin", p)) {
- gpr[2] = 0;
- } else if (!strcmp("/dev/stdout", p)) {
- gpr[2] = 1;
- } else if (!strcmp("/dev/stderr", p)) {
- gpr[2] = 2;
- } else {
- gpr[2] = open(p, get_open_flags(gpr[5]), gpr[6]);
- gpr[3] = errno_mips(errno);
+ {
+ int ret = -1;
+
+ GET_TARGET_STRING(p, gpr[4]);
+ if (!strcmp("/dev/stdin", p)) {
+ ret = 0;
+ } else if (!strcmp("/dev/stdout", p)) {
+ ret = 1;
+ } else if (!strcmp("/dev/stderr", p)) {
+ ret = 2;
+ }
+ FREE_TARGET_STRING(p, gpr[4]);
+
+ /* FIXME: reusing a guest fd doesn't seem correct. */
+ if (ret >= 0) {
+ gpr[2] = ret;
+ break;
+ }
+
+ semihost_sys_open(cs, uhi_cb, gpr[4], 0, gpr[5], gpr[6]);
}
- FREE_TARGET_STRING(p, gpr[4]);
break;
+
case UHI_close:
- if (gpr[4] < 3) {
- /* ignore closing stdin/stdout/stderr */
- gpr[2] = 0;
- return;
- }
- gpr[2] = close(gpr[4]);
- gpr[3] = errno_mips(errno);
+ semihost_sys_close(cs, uhi_cb, gpr[4]);
break;
case UHI_read:
- gpr[2] = read_from_file(env, gpr[4], gpr[5], gpr[6]);
- gpr[3] = errno_mips(errno);
+ semihost_sys_read(cs, uhi_cb, gpr[4], gpr[5], gpr[6]);
break;
case UHI_write:
- gpr[2] = write_to_file(env, gpr[4], gpr[5], gpr[6]);
- gpr[3] = errno_mips(errno);
+ semihost_sys_write(cs, uhi_cb, gpr[4], gpr[5], gpr[6]);
break;
case UHI_lseek:
- gpr[2] = lseek(gpr[4], gpr[5], gpr[6]);
- gpr[3] = errno_mips(errno);
+ semihost_sys_lseek(cs, uhi_cb, gpr[4], gpr[5], gpr[6]);
break;
case UHI_unlink:
- GET_TARGET_STRING(p, gpr[4]);
- gpr[2] = remove(p);
- gpr[3] = errno_mips(errno);
- FREE_TARGET_STRING(p, gpr[4]);
+ semihost_sys_remove(cs, uhi_cb, gpr[4], 0);
break;
case UHI_fstat:
- {
- struct stat sbuf;
- memset(&sbuf, 0, sizeof(sbuf));
- gpr[2] = fstat(gpr[4], &sbuf);
- gpr[3] = errno_mips(errno);
- if (gpr[2]) {
- return;
- }
- gpr[2] = copy_stat_to_target(env, &sbuf, gpr[5]);
- gpr[3] = errno_mips(errno);
- }
+ semihost_sys_fstat(cs, uhi_fstat_cb, gpr[4], gpr[5]);
break;
+
case UHI_argc:
gpr[2] = semihosting_get_argc();
break;
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 07/11] target/mips: Avoid qemu_semihosting_log_out for UHI_plog
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
` (5 preceding siblings ...)
2022-06-08 5:19 ` [PATCH v4 06/11] target/mips: Use semihosting/syscalls.h Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-08 5:19 ` [PATCH v4 08/11] target/mips: Use error_report for UHI_assert Richard Henderson
` (3 subsequent siblings)
10 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
Use semihost_sys_write and/or qemu_semihosting_console_write
for implementing plog. When using gdbstub, copy the temp
string below the stack so that gdb has a guest address from
which to perform the log.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/mips/tcg/sysemu/mips-semi.c | 52 +++++++++++++++++++++++-------
1 file changed, 41 insertions(+), 11 deletions(-)
diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index 5b78cf21a7..ad11a46820 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -310,20 +310,50 @@ void mips_semihosting(CPUMIPSState *env)
}
gpr[2] = copy_argn_to_target(env, gpr[4], gpr[5]);
break;
+
case UHI_plog:
- GET_TARGET_STRING(p, gpr[4]);
- p2 = strstr(p, "%d");
- if (p2) {
- int char_num = p2 - p;
- GString *s = g_string_new_len(p, char_num);
- g_string_append_printf(s, "%d%s", (int)gpr[5], p2 + 2);
- gpr[2] = qemu_semihosting_log_out(s->str, s->len);
- g_string_free(s, true);
- } else {
- gpr[2] = qemu_semihosting_log_out(p, strlen(p));
+ {
+ target_ulong addr = gpr[4];
+ ssize_t len = target_strlen(addr);
+ GString *str;
+ char *pct_d;
+
+ if (len < 0) {
+ report_fault(env);
+ }
+ p = lock_user(VERIFY_READ, addr, len, 1);
+ if (!p) {
+ report_fault(env);
+ }
+
+ pct_d = strstr(p, "%d");
+ if (!pct_d) {
+ FREE_TARGET_STRING(p, addr);
+ semihost_sys_write(cs, uhi_cb, 2, addr, len);
+ break;
+ }
+
+ str = g_string_new_len(p, pct_d - p);
+ g_string_append_printf(str, "%d%s", (int)gpr[5], pct_d + 2);
+ FREE_TARGET_STRING(p, addr);
+
+ /*
+ * When we're using gdb, we need a guest address, so
+ * drop the string onto the stack below the stack pointer.
+ */
+ if (use_gdb_syscalls()) {
+ addr = gpr[29] - str->len;
+ p = lock_user(VERIFY_WRITE, addr, str->len, 0);
+ memcpy(p, str->str, str->len);
+ unlock_user(p, addr, str->len);
+ semihost_sys_write(cs, uhi_cb, 2, addr, str->len);
+ } else {
+ gpr[2] = qemu_semihosting_console_write(str->str, str->len);
+ }
+ g_string_free(str, true);
}
- FREE_TARGET_STRING(p, gpr[4]);
break;
+
case UHI_assert:
GET_TARGET_STRINGS_2(p, gpr[4], p2, gpr[5]);
printf("assertion '");
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 08/11] target/mips: Use error_report for UHI_assert
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
` (6 preceding siblings ...)
2022-06-08 5:19 ` [PATCH v4 07/11] target/mips: Avoid qemu_semihosting_log_out for UHI_plog Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-10 15:10 ` Philippe Mathieu-Daudé via
2022-06-08 5:19 ` [PATCH v4 09/11] semihosting: Remove qemu_semihosting_log_out Richard Henderson
` (2 subsequent siblings)
10 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
Always log the assert locally. Do not report_fault, but
instead include the fact of the fault in the assertion.
Don't bother freeing allocated strings before the abort().
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/mips/tcg/sysemu/mips-semi.c | 39 ++++++++++++++----------------
1 file changed, 18 insertions(+), 21 deletions(-)
diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index ad11a46820..ae4b8849b1 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -221,18 +221,6 @@ static int copy_argn_to_target(CPUMIPSState *env, int arg_num,
} \
} while (0)
-#define GET_TARGET_STRINGS_2(p, addr, p2, addr2) \
- do { \
- p = lock_user_string(addr); \
- if (!p) { \
- report_fault(env); \
- } \
- p2 = lock_user_string(addr2); \
- if (!p2) { \
- report_fault(env); \
- } \
- } while (0)
-
#define FREE_TARGET_STRING(p, gpr) \
do { \
unlock_user(p, gpr, 0); \
@@ -243,7 +231,7 @@ void mips_semihosting(CPUMIPSState *env)
CPUState *cs = env_cpu(env);
target_ulong *gpr = env->active_tc.gpr;
const UHIOp op = gpr[25];
- char *p, *p2;
+ char *p;
switch (op) {
case UHI_exit:
@@ -355,14 +343,23 @@ void mips_semihosting(CPUMIPSState *env)
break;
case UHI_assert:
- GET_TARGET_STRINGS_2(p, gpr[4], p2, gpr[5]);
- printf("assertion '");
- printf("\"%s\"", p);
- printf("': file \"%s\", line %d\n", p2, (int)gpr[6]);
- FREE_TARGET_STRING(p2, gpr[5]);
- FREE_TARGET_STRING(p, gpr[4]);
- abort();
- break;
+ {
+ const char *msg, *file;
+
+ msg = lock_user_string(gpr[4]);
+ if (!msg) {
+ msg = "<EFAULT>";
+ }
+ file = lock_user_string(gpr[5]);
+ if (!file) {
+ file = "<EFAULT>";
+ }
+
+ error_report("UHI assertion \"%s\": file \"%s\", line %d",
+ msg, file, (int)gpr[6]);
+ abort();
+ }
+
default:
error_report("Unknown UHI operation %d", op);
abort();
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 08/11] target/mips: Use error_report for UHI_assert
2022-06-08 5:19 ` [PATCH v4 08/11] target/mips: Use error_report for UHI_assert Richard Henderson
@ 2022-06-10 15:10 ` Philippe Mathieu-Daudé via
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-06-10 15:10 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 8/6/22 07:19, Richard Henderson wrote:
> Always log the assert locally. Do not report_fault, but
> instead include the fact of the fault in the assertion.
> Don't bother freeing allocated strings before the abort().
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/mips/tcg/sysemu/mips-semi.c | 39 ++++++++++++++----------------
> 1 file changed, 18 insertions(+), 21 deletions(-)
> case UHI_assert:
> - GET_TARGET_STRINGS_2(p, gpr[4], p2, gpr[5]);
> - printf("assertion '");
> - printf("\"%s\"", p);
> - printf("': file \"%s\", line %d\n", p2, (int)gpr[6]);
> - FREE_TARGET_STRING(p2, gpr[5]);
> - FREE_TARGET_STRING(p, gpr[4]);
> - abort();
> - break;
> + {
> + const char *msg, *file;
> +
> + msg = lock_user_string(gpr[4]);
> + if (!msg) {
> + msg = "<EFAULT>";
> + }
> + file = lock_user_string(gpr[5]);
> + if (!file) {
> + file = "<EFAULT>";
> + }
> +
> + error_report("UHI assertion \"%s\": file \"%s\", line %d",
> + msg, file, (int)gpr[6]);
> + abort();
> + }
> +
> default:
> error_report("Unknown UHI operation %d", op);
> abort();
Pre-existing, so:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
But since this is a guest error, I'd prefer we exit(1) instead.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 09/11] semihosting: Remove qemu_semihosting_log_out
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
` (7 preceding siblings ...)
2022-06-08 5:19 ` [PATCH v4 08/11] target/mips: Use error_report for UHI_assert Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-10 15:11 ` Philippe Mathieu-Daudé via
2022-06-08 5:19 ` [PATCH v4 10/11] target/mips: Simplify UHI_argnlen and UHI_argn Richard Henderson
2022-06-08 5:19 ` [PATCH v4 11/11] target/mips: Remove GET_TARGET_STRING and FREE_TARGET_STRING Richard Henderson
10 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
The function is no longer used.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
include/semihosting/console.h | 13 -------------
semihosting/console.c | 9 ---------
2 files changed, 22 deletions(-)
diff --git a/include/semihosting/console.h b/include/semihosting/console.h
index 61b0cb3a94..bd78e5f03f 100644
--- a/include/semihosting/console.h
+++ b/include/semihosting/console.h
@@ -40,19 +40,6 @@ int qemu_semihosting_console_read(CPUState *cs, void *buf, int len);
*/
int qemu_semihosting_console_write(void *buf, int len);
-/**
- * qemu_semihosting_log_out:
- * @s: pointer to string
- * @len: length of string
- *
- * Send a string to the debug output. Unlike console_out these strings
- * can't be sent to a remote gdb instance as they don't exist in guest
- * memory.
- *
- * Returns: number of bytes written
- */
-int qemu_semihosting_log_out(const char *s, int len);
-
/*
* qemu_semihosting_console_block_until_ready:
* @cs: CPUState
diff --git a/semihosting/console.c b/semihosting/console.c
index cda7cf1905..5b1ec0a1c3 100644
--- a/semihosting/console.c
+++ b/semihosting/console.c
@@ -38,15 +38,6 @@ typedef struct SemihostingConsole {
static SemihostingConsole console;
-int qemu_semihosting_log_out(const char *s, int len)
-{
- if (console.chr) {
- return qemu_chr_write_all(console.chr, (uint8_t *) s, len);
- } else {
- return write(STDERR_FILENO, s, len);
- }
-}
-
#define FIFO_SIZE 1024
static int console_can_read(void *opaque)
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH v4 09/11] semihosting: Remove qemu_semihosting_log_out
2022-06-08 5:19 ` [PATCH v4 09/11] semihosting: Remove qemu_semihosting_log_out Richard Henderson
@ 2022-06-10 15:11 ` Philippe Mathieu-Daudé via
0 siblings, 0 replies; 20+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-06-10 15:11 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: Alex Bennée
On 8/6/22 07:19, Richard Henderson wrote:
> The function is no longer used.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> include/semihosting/console.h | 13 -------------
> semihosting/console.c | 9 ---------
> 2 files changed, 22 deletions(-)
>
> diff --git a/include/semihosting/console.h b/include/semihosting/console.h
> index 61b0cb3a94..bd78e5f03f 100644
> --- a/include/semihosting/console.h
> +++ b/include/semihosting/console.h
> @@ -40,19 +40,6 @@ int qemu_semihosting_console_read(CPUState *cs, void *buf, int len);
> */
> int qemu_semihosting_console_write(void *buf, int len);
>
> -/**
> - * qemu_semihosting_log_out:
> - * @s: pointer to string
> - * @len: length of string
> - *
> - * Send a string to the debug output. Unlike console_out these strings
> - * can't be sent to a remote gdb instance as they don't exist in guest
> - * memory.
> - *
> - * Returns: number of bytes written
> - */
> -int qemu_semihosting_log_out(const char *s, int len);
> -
> /*
> * qemu_semihosting_console_block_until_ready:
> * @cs: CPUState
> diff --git a/semihosting/console.c b/semihosting/console.c
> index cda7cf1905..5b1ec0a1c3 100644
> --- a/semihosting/console.c
> +++ b/semihosting/console.c
> @@ -38,15 +38,6 @@ typedef struct SemihostingConsole {
>
> static SemihostingConsole console;
>
> -int qemu_semihosting_log_out(const char *s, int len)
> -{
> - if (console.chr) {
> - return qemu_chr_write_all(console.chr, (uint8_t *) s, len);
> - } else {
> - return write(STDERR_FILENO, s, len);
> - }
> -}
> -
> #define FIFO_SIZE 1024
>
> static int console_can_read(void *opaque)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 10/11] target/mips: Simplify UHI_argnlen and UHI_argn
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
` (8 preceding siblings ...)
2022-06-08 5:19 ` [PATCH v4 09/11] semihosting: Remove qemu_semihosting_log_out Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-08 5:19 ` [PATCH v4 11/11] target/mips: Remove GET_TARGET_STRING and FREE_TARGET_STRING Richard Henderson
10 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
With semihosting_get_arg, we already have a check vs argc, so
there's no point replicating it -- just check the result vs NULL.
Merge copy_argn_to_target into its caller.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/mips/tcg/sysemu/mips-semi.c | 44 ++++++++++++++----------------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index ae4b8849b1..b54267681e 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -198,21 +198,6 @@ static void uhi_fstat_cb(CPUState *cs, uint64_t ret, int err)
uhi_cb(cs, ret, err);
}
-static int copy_argn_to_target(CPUMIPSState *env, int arg_num,
- target_ulong vaddr)
-{
- int strsize = strlen(semihosting_get_arg(arg_num)) + 1;
- char *dst = lock_user(VERIFY_WRITE, vaddr, strsize, 0);
- if (!dst) {
- report_fault(env);
- }
-
- strcpy(dst, semihosting_get_arg(arg_num));
-
- unlock_user(dst, vaddr, strsize);
- return 0;
-}
-
#define GET_TARGET_STRING(p, addr) \
do { \
p = lock_user_string(addr); \
@@ -285,18 +270,31 @@ void mips_semihosting(CPUMIPSState *env)
gpr[2] = semihosting_get_argc();
break;
case UHI_argnlen:
- if (gpr[4] >= semihosting_get_argc()) {
- gpr[2] = -1;
- return;
+ {
+ const char *s = semihosting_get_arg(gpr[4]);
+ gpr[2] = s ? strlen(s) : -1;
}
- gpr[2] = strlen(semihosting_get_arg(gpr[4]));
break;
case UHI_argn:
- if (gpr[4] >= semihosting_get_argc()) {
- gpr[2] = -1;
- return;
+ {
+ const char *s = semihosting_get_arg(gpr[4]);
+ target_ulong addr;
+ size_t len;
+
+ if (!s) {
+ gpr[2] = -1;
+ break;
+ }
+ len = strlen(s) + 1;
+ addr = gpr[5];
+ p = lock_user(VERIFY_WRITE, addr, len, 0);
+ if (!p) {
+ report_fault(env);
+ }
+ memcpy(p, s, len);
+ unlock_user(p, addr, len);
+ gpr[2] = 0;
}
- gpr[2] = copy_argn_to_target(env, gpr[4], gpr[5]);
break;
case UHI_plog:
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH v4 11/11] target/mips: Remove GET_TARGET_STRING and FREE_TARGET_STRING
2022-06-08 5:19 [PATCH v4 00/11] target/mips: semihosting cleanup Richard Henderson
` (9 preceding siblings ...)
2022-06-08 5:19 ` [PATCH v4 10/11] target/mips: Simplify UHI_argnlen and UHI_argn Richard Henderson
@ 2022-06-08 5:19 ` Richard Henderson
2022-06-10 15:12 ` Philippe Mathieu-Daudé via
10 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2022-06-08 5:19 UTC (permalink / raw)
To: qemu-devel; +Cc: f4bug
Inline these macros into the only two callers.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/mips/tcg/sysemu/mips-semi.c | 27 +++++++++------------------
1 file changed, 9 insertions(+), 18 deletions(-)
diff --git a/target/mips/tcg/sysemu/mips-semi.c b/target/mips/tcg/sysemu/mips-semi.c
index b54267681e..5fb1ad9092 100644
--- a/target/mips/tcg/sysemu/mips-semi.c
+++ b/target/mips/tcg/sysemu/mips-semi.c
@@ -198,19 +198,6 @@ static void uhi_fstat_cb(CPUState *cs, uint64_t ret, int err)
uhi_cb(cs, ret, err);
}
-#define GET_TARGET_STRING(p, addr) \
- do { \
- p = lock_user_string(addr); \
- if (!p) { \
- report_fault(env); \
- } \
- } while (0)
-
-#define FREE_TARGET_STRING(p, gpr) \
- do { \
- unlock_user(p, gpr, 0); \
- } while (0)
-
void mips_semihosting(CPUMIPSState *env)
{
CPUState *cs = env_cpu(env);
@@ -225,9 +212,13 @@ void mips_semihosting(CPUMIPSState *env)
case UHI_open:
{
+ target_ulong fname = gpr[4];
int ret = -1;
- GET_TARGET_STRING(p, gpr[4]);
+ p = lock_user_string(fname);
+ if (!p) {
+ report_fault(env);
+ }
if (!strcmp("/dev/stdin", p)) {
ret = 0;
} else if (!strcmp("/dev/stdout", p)) {
@@ -235,7 +226,7 @@ void mips_semihosting(CPUMIPSState *env)
} else if (!strcmp("/dev/stderr", p)) {
ret = 2;
}
- FREE_TARGET_STRING(p, gpr[4]);
+ unlock_user(p, fname, 0);
/* FIXME: reusing a guest fd doesn't seem correct. */
if (ret >= 0) {
@@ -243,7 +234,7 @@ void mips_semihosting(CPUMIPSState *env)
break;
}
- semihost_sys_open(cs, uhi_cb, gpr[4], 0, gpr[5], gpr[6]);
+ semihost_sys_open(cs, uhi_cb, fname, 0, gpr[5], gpr[6]);
}
break;
@@ -314,14 +305,14 @@ void mips_semihosting(CPUMIPSState *env)
pct_d = strstr(p, "%d");
if (!pct_d) {
- FREE_TARGET_STRING(p, addr);
+ unlock_user(p, addr, 0);
semihost_sys_write(cs, uhi_cb, 2, addr, len);
break;
}
str = g_string_new_len(p, pct_d - p);
g_string_append_printf(str, "%d%s", (int)gpr[5], pct_d + 2);
- FREE_TARGET_STRING(p, addr);
+ unlock_user(p, addr, 0);
/*
* When we're using gdb, we need a guest address, so
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread