All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christopher Covington <cov@codeaurora.org>
To: Peter Maydell <peter.maydell@linaro.org>,
	QEMU Developers <qemu-devel@nongnu.org>
Cc: Christopher Covington <christopher.covington@linaro.org>,
	Patch Tracking <patches@linaro.org>
Subject: Re: [Qemu-devel] [PATCH 9/9] target-arm: Wire up HLT 0xf000 as the A64 semihosting instruction
Date: Mon, 14 Sep 2015 14:36:03 -0400	[thread overview]
Message-ID: <55F71393.5000703@codeaurora.org> (raw)
In-Reply-To: <CAFEAcA9w+OpFqLDF7fBkm1Qk-4pE346+ayUq8amVvoz1EXV5EA@mail.gmail.com>

Hi Peter,

On 08/27/2015 02:35 PM, Peter Maydell wrote:
> On 13 August 2015 at 17:35, Peter Maydell <peter.maydell@linaro.org> wrote:
>> For the A64 instruction set, the semihosting call instruction
>> is 'HLT 0xf000'. Wire this up to call do_arm_semihosting()
>> if semihosting is enabled.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
> 
>> @@ -1553,8 +1554,17 @@ static void disas_exc(DisasContext *s, uint32_t insn)
>>              unallocated_encoding(s);
>>              break;
>>          }
>> -        /* HLT */
>> -        unsupported_encoding(s, insn);
>> +        /* HLT. This has two purposes.
>> +         * Architecturally, it is an external halting debug instruction.
>> +         * Since QEMU doesn't implement external debug, we treat this as
>> +         * it is required for halting debug disabled: it will UNDEF.
>> +         * Secondly, "HLT 0xf000" is the A64 semihosting syscall instruction.
>> +         */
>> +        if (semihosting_enabled() && imm16 == 0xf000) {
>> +            gen_exception_internal_insn(s, 0, EXCP_SEMIHOST);
>> +        } else {
>> +            unsupported_encoding(s, insn);
>> +        }
> 
> Christopher pointed out to me at KVM Forum that this isn't
> consistent with how we do 32-bit ARM semihosting, which has a
> check to prevent its use from userspace in system emulation.
> (The idea is that semihosting is basically a "guest can pwn
> your host" API, so giving access to it to guest userspace is
> kind of brave.)

> There is a usecase for allowing unfettered access to semihosting
> in system emulation mode (basically, running bare metal test
> binaries). I think we should deal with that by having a separate
> command line option for "userspace semihosting access is OK",
> which changes the behaviour for both 32-bit and 64-bit semihosting
> APIs. Alternatively, we could instead allow userspace to use
> "safe" parts of the semihosting API, like "print to stdout",
> but not the less safe parts like "open and write to arbitrary
> host files". Or we could decide that this safety check isn't
> actually very useful (no other model/debug environment has it
> that I know of) and drop it entirely; but that makes me a little
> nervous.

I find allowing trusted guests to access host files to be a very useful
feature. To me it is very similar to passing through / (root) via VirtIO-9P.
Perhaps a useful way of making sure the user knows what files their guest is
gaining access to would be to have a semihosting path prefix option. That way
access could be allowed nowhere; clearly allow everywhere (/); or clearly be
restricted to, and relative to, a certain sysroot directory
(/home/user/my-sysroot).

Christopher Covington

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

  reply	other threads:[~2015-09-14 18:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-13 16:35 [Qemu-devel] [PATCH 0/9] target-arm: Implement A64 semihosting Peter Maydell
2015-08-13 16:35 ` [Qemu-devel] [PATCH 1/9] target-arm/arm-semi.c: Fix broken SYS_WRITE0 via gdb Peter Maydell
2015-08-13 16:35 ` [Qemu-devel] [PATCH 2/9] target-arm: Improve semihosting debug prints Peter Maydell
2015-08-13 16:35 ` [Qemu-devel] [PATCH 3/9] gdbstub: Implement gdb_do_syscallv() Peter Maydell
2015-08-13 16:35 ` [Qemu-devel] [PATCH 4/9] target-arm/arm-semi.c: Factor out repeated 'return env->regs[0]' Peter Maydell
2015-08-19 15:52   ` Christopher Covington
2015-08-13 16:35 ` [Qemu-devel] [PATCH 5/9] include/exec/softmmu-semi.h: Add support for 64-bit values Peter Maydell
2015-08-13 16:35 ` [Qemu-devel] [PATCH 6/9] target-arm/arm-semi.c: Support widening APIs to 64 bits Peter Maydell
2015-08-19 20:59   ` Christopher Covington
2015-08-13 16:35 ` [Qemu-devel] [PATCH 7/9] target-arm/arm-semi.c: Implement A64 specific SyncCacheRange call Peter Maydell
2015-08-19 21:01   ` Christopher Covington
2015-08-13 16:35 ` [Qemu-devel] [PATCH 8/9] target-arm/arm-semi.c: SYS_EXIT on A64 takes a parameter block Peter Maydell
2015-08-13 16:35 ` [Qemu-devel] [PATCH 9/9] target-arm: Wire up HLT 0xf000 as the A64 semihosting instruction Peter Maydell
2015-08-19 16:19   ` Christopher Covington
2015-08-27 18:35   ` Peter Maydell
2015-09-14 18:36     ` Christopher Covington [this message]
2015-08-25 20:40 ` [Qemu-devel] [PATCH 0/9] target-arm: Implement A64 semihosting Christopher Covington

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55F71393.5000703@codeaurora.org \
    --to=cov@codeaurora.org \
    --cc=christopher.covington@linaro.org \
    --cc=patches@linaro.org \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.