| Message ID | 20190910144428.32597-5-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Implement semihosting v2.0 | expand |
Peter Maydell <peter.maydell@linaro.org> writes: > The semihosting code has to build for both user-only and softmmu; > for user-only it needs access to the TaskState struct that holds > per-thread information. For softmmu we don't need it. > > Currently the softmmu set_swi_errno() takes a CPUARMState *, > which it doesn't use, and the 'ts' variable in do_arm_semihosting() > is set to either be a TaskState* or a CPUARMState* depending on > whether CONFIG_USER_ONLY is set, so that the callsite always > passes 'ts'. Since we don't actually need the CPUARMState *, > we can instead make set_swi_errno() always take a TaskState*, > by providing a local-to-this-file dummy typedef for the softmmu > case and setting ts to NULL for softmmu. > > This will make it easier to have other functions which pass > through the TaskState*, because now they can have the same > prototype regardless of whether they're CONFIG_USER_ONLY or not. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target/arm/arm-semi.c | 48 ++++++++++++++++++++++++------------------- > 1 file changed, 27 insertions(+), 21 deletions(-) > > diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c > index 05491bf5248..ce3ba554bef 100644 > --- a/target/arm/arm-semi.c > +++ b/target/arm/arm-semi.c > @@ -36,6 +36,13 @@ > #else > #include "exec/gdbstub.h" > #include "qemu/cutils.h" > + > +/* > + * Dummy typedef so that we can have functions that take > + * a TaskState* even if we're building for softmmu; in that > + * case the argument will always be NULL. > + */ > +typedef void TaskState; > #endif > > #define TARGET_SYS_OPEN 0x01 > @@ -213,27 +220,24 @@ static GuestFD *get_guestfd(int guestfd) > return gf; > } > > -#ifdef CONFIG_USER_ONLY > -static inline uint32_t set_swi_errno(TaskState *ts, uint32_t code) > -{ > - if (code == (uint32_t)-1) > - ts->swi_errno = errno; > - return code; > -} > -#else > +#ifndef CONFIG_USER_ONLY > static target_ulong syscall_err; > > -static inline uint32_t set_swi_errno(CPUARMState *env, uint32_t code) > -{ > - if (code == (uint32_t)-1) { > - syscall_err = errno; > - } > - return code; > -} > - > #include "exec/softmmu-semi.h" > #endif > > +static inline uint32_t set_swi_errno(TaskState *ts, uint32_t code) > +{ > + if (code == (uint32_t)-1) { > +#ifdef CONFIG_USER_ONLY > + ts->swi_errno = errno; > +#else > + syscall_err = errno; > +#endif > + } > + return code; > +} > + > static target_ulong arm_semi_syscall_len; > > static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err) > @@ -374,13 +378,15 @@ target_ulong do_arm_semihosting(CPUARMState *env) > int nr; > uint32_t ret; > uint32_t len; > -#ifdef CONFIG_USER_ONLY > - TaskState *ts = cs->opaque; > -#else > - CPUARMState *ts = env; > -#endif > + TaskState *ts; > GuestFD *gf; > > +#ifdef CONFIG_USER_ONLY > + ts = cs->opaque; > +#else > + ts = NULL; > +#endif Why not pass cs to set_swi_errno and deal with all the differences in the helper? > + > if (is_a64(env)) { > /* Note that the syscall number is in W0, not X0 */ > nr = env->xregs[0] & 0xffffffffU; -- Alex Bennée
On Thu, 12 Sep 2019 at 12:44, Alex Bennée <alex.bennee@linaro.org> wrote: > Why not pass cs to set_swi_errno and deal with all the differences in > the helper? Mmm, that might be better. I think I was going for not changing the existing use of TaskState in the code paths that use it. thanks -- PMM
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c index 05491bf5248..ce3ba554bef 100644 --- a/target/arm/arm-semi.c +++ b/target/arm/arm-semi.c @@ -36,6 +36,13 @@ #else #include "exec/gdbstub.h" #include "qemu/cutils.h" + +/* + * Dummy typedef so that we can have functions that take + * a TaskState* even if we're building for softmmu; in that + * case the argument will always be NULL. + */ +typedef void TaskState; #endif #define TARGET_SYS_OPEN 0x01 @@ -213,27 +220,24 @@ static GuestFD *get_guestfd(int guestfd) return gf; } -#ifdef CONFIG_USER_ONLY -static inline uint32_t set_swi_errno(TaskState *ts, uint32_t code) -{ - if (code == (uint32_t)-1) - ts->swi_errno = errno; - return code; -} -#else +#ifndef CONFIG_USER_ONLY static target_ulong syscall_err; -static inline uint32_t set_swi_errno(CPUARMState *env, uint32_t code) -{ - if (code == (uint32_t)-1) { - syscall_err = errno; - } - return code; -} - #include "exec/softmmu-semi.h" #endif +static inline uint32_t set_swi_errno(TaskState *ts, uint32_t code) +{ + if (code == (uint32_t)-1) { +#ifdef CONFIG_USER_ONLY + ts->swi_errno = errno; +#else + syscall_err = errno; +#endif + } + return code; +} + static target_ulong arm_semi_syscall_len; static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err) @@ -374,13 +378,15 @@ target_ulong do_arm_semihosting(CPUARMState *env) int nr; uint32_t ret; uint32_t len; -#ifdef CONFIG_USER_ONLY - TaskState *ts = cs->opaque; -#else - CPUARMState *ts = env; -#endif + TaskState *ts; GuestFD *gf; +#ifdef CONFIG_USER_ONLY + ts = cs->opaque; +#else + ts = NULL; +#endif + if (is_a64(env)) { /* Note that the syscall number is in W0, not X0 */ nr = env->xregs[0] & 0xffffffffU;
The semihosting code has to build for both user-only and softmmu; for user-only it needs access to the TaskState struct that holds per-thread information. For softmmu we don't need it. Currently the softmmu set_swi_errno() takes a CPUARMState *, which it doesn't use, and the 'ts' variable in do_arm_semihosting() is set to either be a TaskState* or a CPUARMState* depending on whether CONFIG_USER_ONLY is set, so that the callsite always passes 'ts'. Since we don't actually need the CPUARMState *, we can instead make set_swi_errno() always take a TaskState*, by providing a local-to-this-file dummy typedef for the softmmu case and setting ts to NULL for softmmu. This will make it easier to have other functions which pass through the TaskState*, because now they can have the same prototype regardless of whether they're CONFIG_USER_ONLY or not. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/arm-semi.c | 48 ++++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 21 deletions(-) -- 2.20.1