| Message ID | 1345658615-5005-1-git-send-email-jcmvbkbc@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Aug 22, 2012 at 10:03 PM, Max Filippov <jcmvbkbc@gmail.com> wrote: > This prevents guest from proceeding with uninitialised garbage returned > from unimplemented simcalls. > > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> > --- > target-xtensa/xtensa-semi.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) ping?
On 22 August 2012 19:03, Max Filippov <jcmvbkbc@gmail.com> wrote: > --- a/target-xtensa/xtensa-semi.c > +++ b/target-xtensa/xtensa-semi.c > @@ -218,6 +218,8 @@ void HELPER(simcall)(CPUXtensaState *env) > > default: > qemu_log("%s(%d): not implemented\n", __func__, regs[2]); > + regs[2] = -1; > + regs[3] = ENOSYS; > break; > } This doesn't look right -- ENOSYS is a host errno, and may vary between host OSes and CPU architectures. I would have thought you'd want to return a value defined by whatever guest ABI we're emulating here. -- PMM
On Wed, Aug 29, 2012 at 1:38 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 22 August 2012 19:03, Max Filippov <jcmvbkbc@gmail.com> wrote: >> --- a/target-xtensa/xtensa-semi.c >> +++ b/target-xtensa/xtensa-semi.c >> @@ -218,6 +218,8 @@ void HELPER(simcall)(CPUXtensaState *env) >> >> default: >> qemu_log("%s(%d): not implemented\n", __func__, regs[2]); >> + regs[2] = -1; >> + regs[3] = ENOSYS; >> break; >> } > > This doesn't look right -- ENOSYS is a host errno, and may vary > between host OSes and CPU architectures. I would have thought you'd > want to return a value defined by whatever guest ABI we're > emulating here. That means also converting errno after open/close/read/write... Is there a way to reuse linux-user errno convertor in the softmmu target?
On 29 August 2012 11:13, Max Filippov <jcmvbkbc@gmail.com> wrote: > On Wed, Aug 29, 2012 at 1:38 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 22 August 2012 19:03, Max Filippov <jcmvbkbc@gmail.com> wrote: >>> --- a/target-xtensa/xtensa-semi.c >>> +++ b/target-xtensa/xtensa-semi.c >>> @@ -218,6 +218,8 @@ void HELPER(simcall)(CPUXtensaState *env) >>> >>> default: >>> qemu_log("%s(%d): not implemented\n", __func__, regs[2]); >>> + regs[2] = -1; >>> + regs[3] = ENOSYS; >>> break; >>> } >> >> This doesn't look right -- ENOSYS is a host errno, and may vary >> between host OSes and CPU architectures. I would have thought you'd >> want to return a value defined by whatever guest ABI we're >> emulating here. > > That means also converting errno after open/close/read/write... > Is there a way to reuse linux-user errno convertor in the softmmu target? I don't think so, no. I've just looked at the ARM semihosting code, and we also return host errnos, though in the ARM case we can sort of justify this because the definition of the SYS_ERRNO semihosting call says "Whether errno is set or not, and to what value, is entirely host-specific, except where the ANSI C standard defines the behavior" ...so although returning host errnos is not very useful in some ways it's not in breach of the semihosting spec. Since xtensa-semi.c currently works by returning host errnos elsewhere I'm happy for this patch to go into 1.2 if you think it merits it, and maybe look at the errno issue more generally after that. -- PMM
On Wed, Aug 29, 2012 at 2:34 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 29 August 2012 11:13, Max Filippov <jcmvbkbc@gmail.com> wrote: >> On Wed, Aug 29, 2012 at 1:38 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> On 22 August 2012 19:03, Max Filippov <jcmvbkbc@gmail.com> wrote: >>>> --- a/target-xtensa/xtensa-semi.c >>>> +++ b/target-xtensa/xtensa-semi.c >>>> @@ -218,6 +218,8 @@ void HELPER(simcall)(CPUXtensaState *env) >>>> >>>> default: >>>> qemu_log("%s(%d): not implemented\n", __func__, regs[2]); >>>> + regs[2] = -1; >>>> + regs[3] = ENOSYS; >>>> break; >>>> } >>> >>> This doesn't look right -- ENOSYS is a host errno, and may vary >>> between host OSes and CPU architectures. I would have thought you'd >>> want to return a value defined by whatever guest ABI we're >>> emulating here. >> >> That means also converting errno after open/close/read/write... >> Is there a way to reuse linux-user errno convertor in the softmmu target? > > I don't think so, no. > > I've just looked at the ARM semihosting code, and we also return > host errnos, though in the ARM case we can sort of justify this > because the definition of the SYS_ERRNO semihosting call says > "Whether errno is set or not, and to what value, is entirely > host-specific, except where the ANSI C standard defines the behavior" > > ...so although returning host errnos is not very useful in some > ways it's not in breach of the semihosting spec. > > Since xtensa-semi.c currently works by returning host errnos > elsewhere I'm happy for this patch to go into 1.2 if you think > it merits it, and maybe look at the errno issue more generally > after that. Thanks for the review, Peter. Blue, please apply it as is, I will post errno convertor for 1.3.
Thanks, applied. On Wed, Aug 22, 2012 at 6:03 PM, Max Filippov <jcmvbkbc@gmail.com> wrote: > This prevents guest from proceeding with uninitialised garbage returned > from unimplemented simcalls. > > Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> > --- > target-xtensa/xtensa-semi.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/target-xtensa/xtensa-semi.c b/target-xtensa/xtensa-semi.c > index 1c8a19e..6d001c2 100644 > --- a/target-xtensa/xtensa-semi.c > +++ b/target-xtensa/xtensa-semi.c > @@ -218,6 +218,8 @@ void HELPER(simcall)(CPUXtensaState *env) > > default: > qemu_log("%s(%d): not implemented\n", __func__, regs[2]); > + regs[2] = -1; > + regs[3] = ENOSYS; > break; > } > } > -- > 1.7.7.6 > >
diff --git a/target-xtensa/xtensa-semi.c b/target-xtensa/xtensa-semi.c index 1c8a19e..6d001c2 100644 --- a/target-xtensa/xtensa-semi.c +++ b/target-xtensa/xtensa-semi.c @@ -218,6 +218,8 @@ void HELPER(simcall)(CPUXtensaState *env) default: qemu_log("%s(%d): not implemented\n", __func__, regs[2]); + regs[2] = -1; + regs[3] = ENOSYS; break; } }
This prevents guest from proceeding with uninitialised garbage returned from unimplemented simcalls. Signed-off-by: Max Filippov <jcmvbkbc@gmail.com> --- target-xtensa/xtensa-semi.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)