[Qemu-devel] [PATCH v4 0/4] semihosting at translate time fixes

Alex Bennée posted 4 patches 4 years, 7 months ago
Test docker-clang@ubuntu failed
Test FreeBSD passed
Test checkpatch passed
Test docker-mingw@fedora passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190906202636.28642-1-alex.bennee@linaro.org
accel/tcg/atomic_template.h |  2 +-
target/arm/helper.c         | 96 +++++++++----------------------------
target/arm/m_helper.c       | 18 +++----
target/arm/translate.c      | 24 ++++++++--
4 files changed, 48 insertions(+), 92 deletions(-)
[Qemu-devel] [PATCH v4 0/4] semihosting at translate time fixes
Posted by Alex Bennée 4 years, 7 months ago
Hi Peter,

Hopefully this is the final version of the semihosting at translate
time patches. I've applied Richard's IS_USER changes and gated the SVN
for !M profile.

Alex Bennée (3):
  target/arm: handle M-profile semihosting at translate time
  target/arm: handle A-profile semihosting at translate time
  target/arm: remove run time semihosting checks

Emilio G. Cota (1):
  atomic_template: fix indentation in GEN_ATOMIC_HELPER

 accel/tcg/atomic_template.h |  2 +-
 target/arm/helper.c         | 96 +++++++++----------------------------
 target/arm/m_helper.c       | 18 +++----
 target/arm/translate.c      | 24 ++++++++--
 4 files changed, 48 insertions(+), 92 deletions(-)

-- 
2.20.1


Re: [Qemu-devel] [PATCH v4 0/4] semihosting at translate time fixes
Posted by Peter Maydell 4 years, 7 months ago
On Fri, 6 Sep 2019 at 21:26, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Hi Peter,
>
> Hopefully this is the final version of the semihosting at translate
> time patches. I've applied Richard's IS_USER changes and gated the SVN
> for !M profile.
>
> Alex Bennée (3):
>   target/arm: handle M-profile semihosting at translate time
>   target/arm: handle A-profile semihosting at translate time
>   target/arm: remove run time semihosting checks

Hi. I've just been looking at these, and I noticed that
they seem to accidentally extend the "no semihosting
in user mode" check that is currently for softmmu only
to also cover linux-user mode (where it would amount
to "never provide semihosting"). This is because we used
to do the check in the helper.c code which is only used
by softmmu, and not in the linux-user/arm/cpu_loop.c
equivalent that linux-user uses. But now we do the check
in translate.c, which is common to both.

There's also some missed cleanup in that the linux-user
code can also have the "maybe EXCP_BKPT/EXCP_SWI is a semihosting
call" checks deleted.

> Emilio G. Cota (1):
>   atomic_template: fix indentation in GEN_ATOMIC_HELPER

I've taken the atomic_template fix into target-arm.next,
since it's unrelated.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 0/4] semihosting at translate time fixes
Posted by Alex Bennée 4 years, 7 months ago
Peter Maydell <peter.maydell@linaro.org> writes:

> On Fri, 6 Sep 2019 at 21:26, Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>> Hi Peter,
>>
>> Hopefully this is the final version of the semihosting at translate
>> time patches. I've applied Richard's IS_USER changes and gated the SVN
>> for !M profile.
>>
>> Alex Bennée (3):
>>   target/arm: handle M-profile semihosting at translate time
>>   target/arm: handle A-profile semihosting at translate time
>>   target/arm: remove run time semihosting checks
>
> Hi. I've just been looking at these, and I noticed that
> they seem to accidentally extend the "no semihosting
> in user mode" check that is currently for softmmu only
> to also cover linux-user mode (where it would amount
> to "never provide semihosting").

I misread Richard's comments - he only actually said to drop the #ifndef
CONFIG_USER while using !IS_USER for M profile. I'll return the #ifndef
CONFIG_USER for A-profile.

It does seem a bit weird that userspace linux-user does do semihosting
whereas EL0 in softmmu doesn't. Is that because we are effectively
short-circuiting what a real ARM kernel would be doing for EL0?

> This is because we used
> to do the check in the helper.c code which is only used
> by softmmu, and not in the linux-user/arm/cpu_loop.c
> equivalent that linux-user uses. But now we do the check
> in translate.c, which is common to both.
>
> There's also some missed cleanup in that the linux-user
> code can also have the "maybe EXCP_BKPT/EXCP_SWI is a semihosting
> call" checks deleted.

I'll have a look at that.

>
>> Emilio G. Cota (1):
>>   atomic_template: fix indentation in GEN_ATOMIC_HELPER
>
> I've taken the atomic_template fix into target-arm.next,
> since it's unrelated.
>
> thanks
> -- PMM


--
Alex Bennée

Re: [Qemu-devel] [PATCH v4 0/4] semihosting at translate time fixes
Posted by Peter Maydell 4 years, 7 months ago
On Wed, 11 Sep 2019 at 14:14, Alex Bennée <alex.bennee@linaro.org> wrote:
> It does seem a bit weird that userspace linux-user does do semihosting
> whereas EL0 in softmmu doesn't. Is that because we are effectively
> short-circuiting what a real ARM kernel would be doing for EL0?

It's because the "not for EL0" is a rather bogus attempt
at 'security' by not allowing userspace in a system emulator
to access the semihosting API, reserving it instead for
the guest OS (its EL1). This concept doesn't apply for
linux-user mode, where there is no guest EL1, and where in any
case the semihosting API doesn't allow the guest code to do
anything it couldn't do by directly making host OS syscalls.

I suspect this "not for EL0" thing is not something anybody
else's semihosting implementation does (though I haven't checked).

One idea I've vaguely thought about is an idea of a more
'safe' semihosting mode, where we only provide the calls
which we think are reasonable for a not-really-trusted
guest: so you could write to stdout but not read/write
arbitrary files on the filesystem, for instance.

thanks
-- PMM