Re: [Qemu-devel] [PATCH] linux-user: Handle new ARM breakpoint instruction

[Peter Maydell]

On 25 June 2014 17:11, Hunter Laux <address@hidden> wrote:
> Isn't the instruction decoder really the wrong place to do that since that
> exception is handled differently in user space and kernel space. It's my
> understanding that the instruction decoder is also shared by machine
> emulation. If you're doing machine emulation you'll still want to throw
> EXCP_UDEF because that's what Linux is expecting.

Actually the BKPT instruction generates a Prefetch Abort, not
an Undefined Instruction.

They really are different in some senses because if your kernel
is 64 bit then they generate different syndrome register values.
If your kernel is 32 bit then we will eventually generate a Prefetch
Abort from EXCP_BKPT. The key point here is that the EXCP_*
values are for the benefit of QEMU; they are not architectural,
they're just ways we use to provide information about what
happened; the code which deals with them will then use that
to do the right thing, which may involve generating one of the
architecturally defined exceptions. (Another example here is
EXCP_STREX, which is purely a way to say "hey, the linux-user
main loop needs to do some work for us now".)

In fact looking at the kernel code I was confused by your
subject line. These:
ARM:   xxxx 0111 1111 xxxx xxxx xxxx 1111 xxxx
Thumb: 1101 1110 xxxx xxxx

are not the "ARM breakpoint instruction" (which would be BKPT).
They're just some random UNDEF patterns the kernel has
decided to treat as causing SIGTRAP. So your code is OK
but your commit message could be clearer :-)

I think also I would factor the check out into its own function:

/* Return true if the instruction matches one of the patterns
 * in the "permanently undefined" encoding space which
 * Linux treats as if they were breakpoint instructions.
 */
static bool is_kernel_bkpt(CPUARMState *env, uint32_t opcode)
{
    ...
}

Then the main cpu_loop() calling code is a little simpler.

thanks
-- PMM