| Message ID | 20190910144428.32597-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Implement semihosting v2.0 | expand |
Peter Maydell <peter.maydell@linaro.org> writes: > The set_swi_errno() function is called to capture the errno > from a host system call, so that we can return -1 from the > semihosting function and later allow the guest to get a more > specific error code with the SYS_ERRNO function. It comes in > two versions, one for user-only and one for softmmu. We forgot > to capture the errno in the softmmu version; fix the error. > > (Semihosting calls directed to gdb are unaffected because > they go through a different code path that captures the > error return from the gdbstub call in arm_semi_cb() or > arm_semi_flen_cb().) > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > NB that a later commit will put in some cleanup of TaskState > that will let us reduce the duplication between the two > implementations of this function. > --- > target/arm/arm-semi.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c > index 90423a35deb..03e60105c05 100644 > --- a/target/arm/arm-semi.c > +++ b/target/arm/arm-semi.c > @@ -114,8 +114,13 @@ static inline uint32_t set_swi_errno(TaskState *ts, uint32_t code) > return code; > } > #else > +static target_ulong syscall_err; > + I appreciate that this is just moving things around but this will be broken for -smp > 1 if two vCPUs make a syscall at the same time. For linux-user this information is kept in ts->swi_errno which is per-thread. Either we need a __thread version or find somewhere in CPUARMState to store it. > static inline uint32_t set_swi_errno(CPUARMState *env, uint32_t code) > { > + if (code == (uint32_t)-1) { > + syscall_err = errno; > + } > return code; > } > > @@ -124,10 +129,6 @@ static inline uint32_t set_swi_errno(CPUARMState *env, uint32_t code) > > static target_ulong arm_semi_syscall_len; > > -#if !defined(CONFIG_USER_ONLY) > -static target_ulong syscall_err; > -#endif > - > static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err) > { > ARMCPU *cpu = ARM_CPU(cs); -- Alex Bennée
On Thu, 12 Sep 2019 at 11:40, Alex Bennée <alex.bennee@linaro.org> wrote: > > > Peter Maydell <peter.maydell@linaro.org> writes: > > > The set_swi_errno() function is called to capture the errno > > from a host system call, so that we can return -1 from the > > semihosting function and later allow the guest to get a more > > specific error code with the SYS_ERRNO function. It comes in > > two versions, one for user-only and one for softmmu. We forgot > > to capture the errno in the softmmu version; fix the error. > > > > (Semihosting calls directed to gdb are unaffected because > > they go through a different code path that captures the > > error return from the gdbstub call in arm_semi_cb() or > > arm_semi_flen_cb().) > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > NB that a later commit will put in some cleanup of TaskState > > that will let us reduce the duplication between the two > > implementations of this function. > > --- > > target/arm/arm-semi.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c > > index 90423a35deb..03e60105c05 100644 > > --- a/target/arm/arm-semi.c > > +++ b/target/arm/arm-semi.c > > @@ -114,8 +114,13 @@ static inline uint32_t set_swi_errno(TaskState *ts, uint32_t code) > > return code; > > } > > #else > > +static target_ulong syscall_err; > > + > > I appreciate that this is just moving things around but this will be > broken for -smp > 1 if two vCPUs make a syscall at the same time. For > linux-user this information is kept in ts->swi_errno which is > per-thread. Either we need a __thread version or find somewhere in > CPUARMState to store it. The semihosting API has no concept of errno being per-CPU at all. (It really predates SMP entirely, so nobody was thinking about that, and in any case it's a debug API, not a serious-work one). The assumption is that the guest is handling this somehow. The errno interface is not very useful anyway since it doesn't actually define any errno values, and it's even worse in our implementation because we use the host errno values rather than defining some clean target-specific set of values... thanks -- PMM
diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c index 90423a35deb..03e60105c05 100644 --- a/target/arm/arm-semi.c +++ b/target/arm/arm-semi.c @@ -114,8 +114,13 @@ static inline uint32_t set_swi_errno(TaskState *ts, uint32_t code) return code; } #else +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; } @@ -124,10 +129,6 @@ static inline uint32_t set_swi_errno(CPUARMState *env, uint32_t code) static target_ulong arm_semi_syscall_len; -#if !defined(CONFIG_USER_ONLY) -static target_ulong syscall_err; -#endif - static void arm_semi_cb(CPUState *cs, target_ulong ret, target_ulong err) { ARMCPU *cpu = ARM_CPU(cs);
The set_swi_errno() function is called to capture the errno from a host system call, so that we can return -1 from the semihosting function and later allow the guest to get a more specific error code with the SYS_ERRNO function. It comes in two versions, one for user-only and one for softmmu. We forgot to capture the errno in the softmmu version; fix the error. (Semihosting calls directed to gdb are unaffected because they go through a different code path that captures the error return from the gdbstub call in arm_semi_cb() or arm_semi_flen_cb().) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- NB that a later commit will put in some cleanup of TaskState that will let us reduce the duplication between the two implementations of this function. --- target/arm/arm-semi.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) -- 2.20.1