| Message ID | 20220607204557.658541-49-richard.henderson@linaro.org |
|---|---|
| State | Accepted |
| Commit | 1577eec0fca6fd67bdc0727d10de4bdc3f8afa95 |
| Headers | show |
| Series | semihosting cleanup | expand |
On 13:45 Tue 07 Jun , Richard Henderson wrote: > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Luc Michel <lmichel@kalray.eu> > --- > semihosting/arm-compat-semi.c | 27 ++++++++++++++++++--------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c > index 20e99cdcc0..4c8932ad54 100644 > --- a/semihosting/arm-compat-semi.c > +++ b/semihosting/arm-compat-semi.c > @@ -302,6 +302,22 @@ common_semi_flen_fstat_cb(CPUState *cs, uint64_t ret, int err) > common_semi_cb(cs, ret, err); > } > > +static void > +common_semi_readc_cb(CPUState *cs, uint64_t ret, int err) > +{ > + if (!err) { > + CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; > + uint8_t ch; > + > + if (get_user_u8(ch, common_semi_stack_bottom(cs) - 1)) { > + ret = -1, err = EFAULT; > + } else { > + ret = ch; > + } > + } > + common_semi_cb(cs, ret, err); > +} > + > #define SHFB_MAGIC_0 0x53 > #define SHFB_MAGIC_1 0x48 > #define SHFB_MAGIC_2 0x46 > @@ -427,15 +443,8 @@ void do_common_semihosting(CPUState *cs) > break; > > case TARGET_SYS_READC: > - { > - uint8_t ch; > - int ret = qemu_semihosting_console_read(cs, &ch, 1); > - if (ret == 1) { > - common_semi_cb(cs, ch, 0); > - } else { > - common_semi_cb(cs, -1, EIO); > - } > - } > + semihost_sys_read_gf(cs, common_semi_readc_cb, &console_in_gf, > + common_semi_stack_bottom(cs) - 1, 1); > break; > > case TARGET_SYS_ISERROR: > -- > 2.34.1 > > > > > To declare a filtering error, please use the following link : https://www.security-mail.net/reporter.php?mid=1799d.629fd6b1.98b43.0&r=lmichel%40kalrayinc.com&s=qemu-devel-bounces%2Blmichel%3Dkalrayinc.com%40nongnu.org&o=%5BPATCH+v4+48%2F53%5D+semihosting%3A+Use+console_in_gf+for+SYS_READC&verdict=C&c=b9cfba5cbbf5d995e5235734d7ab1a5181b14825 > --
Richard Henderson <richard.henderson@linaro.org> writes: > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > semihosting/arm-compat-semi.c | 27 ++++++++++++++++++--------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c > index 20e99cdcc0..4c8932ad54 100644 > --- a/semihosting/arm-compat-semi.c > +++ b/semihosting/arm-compat-semi.c > @@ -302,6 +302,22 @@ common_semi_flen_fstat_cb(CPUState *cs, uint64_t ret, int err) > common_semi_cb(cs, ret, err); > } > > +static void > +common_semi_readc_cb(CPUState *cs, uint64_t ret, int err) > +{ > + if (!err) { > + CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; Why do you even both extracting env here if it's not being used? > + uint8_t ch; > + > + if (get_user_u8(ch, common_semi_stack_bottom(cs) - 1)) { > + ret = -1, err = EFAULT; > + } else { > + ret = ch; > + } > + } > + common_semi_cb(cs, ret, err); > +} > + > #define SHFB_MAGIC_0 0x53 > #define SHFB_MAGIC_1 0x48 > #define SHFB_MAGIC_2 0x46 > @@ -427,15 +443,8 @@ void do_common_semihosting(CPUState *cs) > break; > > case TARGET_SYS_READC: > - { > - uint8_t ch; > - int ret = qemu_semihosting_console_read(cs, &ch, 1); > - if (ret == 1) { > - common_semi_cb(cs, ch, 0); > - } else { > - common_semi_cb(cs, -1, EIO); > - } > - } > + semihost_sys_read_gf(cs, common_semi_readc_cb, &console_in_gf, > + common_semi_stack_bottom(cs) - 1, 1); > break; > > case TARGET_SYS_ISERROR:
On 6/27/22 14:37, Alex Bennée wrote: > > Richard Henderson <richard.henderson@linaro.org> writes: > >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> semihosting/arm-compat-semi.c | 27 ++++++++++++++++++--------- >> 1 file changed, 18 insertions(+), 9 deletions(-) >> >> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c >> index 20e99cdcc0..4c8932ad54 100644 >> --- a/semihosting/arm-compat-semi.c >> +++ b/semihosting/arm-compat-semi.c >> @@ -302,6 +302,22 @@ common_semi_flen_fstat_cb(CPUState *cs, uint64_t ret, int err) >> common_semi_cb(cs, ret, err); >> } >> >> +static void >> +common_semi_readc_cb(CPUState *cs, uint64_t ret, int err) >> +{ >> + if (!err) { >> + CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; > > Why do you even both extracting env here if it's not being used? > >> + uint8_t ch; >> + >> + if (get_user_u8(ch, common_semi_stack_bottom(cs) - 1)) { It is used in here, for system-mode, but not user-mode. It's ugly, I know, but that's the interface we inherited. The simplest non-ifdef solution is to mark the variable unused. r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 6/27/22 14:37, Alex Bennée wrote: >> Richard Henderson <richard.henderson@linaro.org> writes: >> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> semihosting/arm-compat-semi.c | 27 ++++++++++++++++++--------- >>> 1 file changed, 18 insertions(+), 9 deletions(-) >>> >>> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c >>> index 20e99cdcc0..4c8932ad54 100644 >>> --- a/semihosting/arm-compat-semi.c >>> +++ b/semihosting/arm-compat-semi.c >>> @@ -302,6 +302,22 @@ common_semi_flen_fstat_cb(CPUState *cs, uint64_t ret, int err) >>> common_semi_cb(cs, ret, err); >>> } >>> +static void >>> +common_semi_readc_cb(CPUState *cs, uint64_t ret, int err) >>> +{ >>> + if (!err) { >>> + CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; >> Why do you even both extracting env here if it's not being used? >> >>> + uint8_t ch; >>> + >>> + if (get_user_u8(ch, common_semi_stack_bottom(cs) - 1)) { > > It is used in here, for system-mode, but not user-mode. > It's ugly, I know, but that's the interface we inherited. > The simplest non-ifdef solution is to mark the variable unused. Fair enough. > > > r~
diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c index 20e99cdcc0..4c8932ad54 100644 --- a/semihosting/arm-compat-semi.c +++ b/semihosting/arm-compat-semi.c @@ -302,6 +302,22 @@ common_semi_flen_fstat_cb(CPUState *cs, uint64_t ret, int err) common_semi_cb(cs, ret, err); } +static void +common_semi_readc_cb(CPUState *cs, uint64_t ret, int err) +{ + if (!err) { + CPUArchState *env G_GNUC_UNUSED = cs->env_ptr; + uint8_t ch; + + if (get_user_u8(ch, common_semi_stack_bottom(cs) - 1)) { + ret = -1, err = EFAULT; + } else { + ret = ch; + } + } + common_semi_cb(cs, ret, err); +} + #define SHFB_MAGIC_0 0x53 #define SHFB_MAGIC_1 0x48 #define SHFB_MAGIC_2 0x46 @@ -427,15 +443,8 @@ void do_common_semihosting(CPUState *cs) break; case TARGET_SYS_READC: - { - uint8_t ch; - int ret = qemu_semihosting_console_read(cs, &ch, 1); - if (ret == 1) { - common_semi_cb(cs, ch, 0); - } else { - common_semi_cb(cs, -1, EIO); - } - } + semihost_sys_read_gf(cs, common_semi_readc_cb, &console_in_gf, + common_semi_stack_bottom(cs) - 1, 1); break; case TARGET_SYS_ISERROR:
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- semihosting/arm-compat-semi.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)