[Qemu-devel] [PATCH for-1.2 v2] target-xtensa: return ENOSYS for unimplemented simcalls

[Qemu-devel] [PATCH for-1.2 v2] target-xtensa: return ENOSYS for unimplemented simcalls

[Max Filippov]

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

Re: [Qemu-devel] [PATCH for-1.2 v2] target-xtensa: return ENOSYS for unimplemented simcalls

[Max Filippov]

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?

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH for-1.2 v2] target-xtensa: return ENOSYS for unimplemented simcalls

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH for-1.2 v2] target-xtensa: return ENOSYS for unimplemented simcalls

[Max Filippov]

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?

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH for-1.2 v2] target-xtensa: return ENOSYS for unimplemented simcalls

[Peter Maydell]

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

Re: [Qemu-devel] [PATCH for-1.2 v2] target-xtensa: return ENOSYS for unimplemented simcalls

[Max Filippov]

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.
-- Max

Re: [Qemu-devel] [PATCH for-1.2 v2] target-xtensa: return ENOSYS for unimplemented simcalls

[Blue Swirl]

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
>
>