Re: [PATCH] Semihost SYS_READC implementation (v4)

["Keith Packard" <keithp@keithp.com>]

[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]

Peter Maydell <peter.maydell@linaro.org> writes:

> I had an idle glance at this implementation, and this:
>
>    uint32_t pre = opcode_at(&ctx->base, ctx->base.pc_next - 4);
>    uint32_t ebreak = opcode_at(&ctx->base, ctx->base.pc_next);
>    uint32_t post = opcode_at(&ctx->base, ctx->base.pc_next + 4);
>
> (where opcode_at() is a wrapper for cpu_ldl_code()) has
> some unfortunate side effects: if the previous instruction
> is in the previous MMU page, or the following instruction
> is in the next MMU page, you might incorrectly trigger
> an exception (where QEMU will just longjmp straight out of
> the cpu_ldl_code()) if that other page isn't actually mapped
> in the guest's page table. You need to be careful not to access
> code outside the page you're actually on unless you're really
> going to execute it and are OK with it faulting.

I can't even find the implementation of cpu_ldl_code; the qemu source
code is somewhat obscure in this area. But, longjmp'ing out of the
middle of that seems like a bad idea.

> Does your semihosting spec expect to have the semihosting
> call work if the sequence crosses a page boundary, the
> code is being executed by a userspace process, and one of
> the two pages has been paged out by the OS ?

You've seen the entirety of the RISC-V semihosting spec already.  For
now, perhaps we should limit RISC-V semihosting support to devices
without paging support and await a more complete spec.

As you suggest, disallowing the sequence from crossing a page boundary
would be a simple fix, but that would require wording changes to the
spec.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]