* [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 @ 2018-12-13 11:55 Alex Bennée 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware Alex Bennée ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Alex Bennée @ 2018-12-13 11:55 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-arm, mark.rutland, ard.biesheuvel, Alex Bennée Hi, This is an attempt to fix debugging of AArch32 binaries when running under KVM on AArch64 hardware. There are two parts to this, the first is a handling the possibility of AArch32 software breakpoints with a heuristic based on the current execution mode. The second part is delaying the setup of aarch64 debugging until the shared arm_cpu_realize function is run by which point we have parsed and decoded the actual execution mode of the guest. This doesn't solve the problem of split mode guests which switch between an AA64 EL1 and an AA32 EL0 though. I still ran into a problem with single-step. Even with Mark's single-step fixup series: To: linux-arm-kernel@lists.infradead.org Cc: kvmarm@lists.cs.columbia.edu, Subject: [PATCH 0/2] kvm/arm: make singlestep behaviour consistent Date: Fri, 9 Nov 2018 15:07:09 +0000 Message-Id: <20181109150711.45864-1-mark.rutland@arm.com> some instructions do single-step but sometimes the single-step doesn't return leading to a runaway until it hits a breakpoint. I'm not sure why this is the case because the SS state machine shouldn't be instruction sensitive. However these two patches at least make it possible to debug an AArch32 guest. Alex Bennée (2): target/arm: kvm64 make guest debug AA32 break point aware target/arm: defer setting up of aarch64 gdb until arm_cpu_realize include/hw/arm/arm.h | 2 ++ target/arm/cpu.c | 4 ++++ target/arm/cpu64.c | 20 +++++++++++++++----- target/arm/kvm64.c | 13 ++++++++++--- 4 files changed, 31 insertions(+), 8 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware 2018-12-13 11:55 [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 Alex Bennée @ 2018-12-13 11:55 ` Alex Bennée 2018-12-13 12:36 ` Ard Biesheuvel ` (2 more replies) 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 2/2] target/arm: defer setting up of aarch64 gdb until arm_cpu_realize Alex Bennée 2018-12-13 11:57 ` [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 Mark Rutland 2 siblings, 3 replies; 16+ messages in thread From: Alex Bennée @ 2018-12-13 11:55 UTC (permalink / raw) To: qemu-devel Cc: qemu-arm, mark.rutland, ard.biesheuvel, Alex Bennée, Omair Javaid, Peter Maydell When supported by the hardware we can run AA32 guests or even AA64 EL1 code with AA32 EL0 mode code. Inserting a AA64 break point into AA32 code tends to break things. This is especially acute with gdb as it inserts temporary breakpoints when stepping through code. The heuristic of checking the current mode works but it's not perfect. A user could be placing a break point in code after a mode switch and that will still fail. However there doesn't seem to be a way to force a hbreak by default. According to "set breakpoint auto-hw on": This is the default behavior. When GDB sets a breakpoint, it will try to use the target memory map to decide if software or hardware breakpoint must be used. Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Omair Javaid <omair.javaid@linaro.org> --- target/arm/kvm64.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index 0a502091e7..dd564a59b7 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -989,14 +989,20 @@ int kvm_arch_get_registers(CPUState *cs) return ret; } -/* C6.6.29 BRK instruction */ +/* BRK (A64) and BKPT (A32) instructions */ static const uint32_t brk_insn = 0xd4200000; +static const uint32_t bkpt_insn = 0xe1200070; int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) { + CPUARMState *env = &ARM_CPU(cs)->env; + int el = arm_current_el(env); + bool is_aa64 = arm_el_is_aa64(env, el); + const uint32_t *bpi = is_aa64 ? &brk_insn : &bkpt_insn; + if (have_guest_debug) { if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) || - cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) { + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)bpi, 4, 1)) { return -EINVAL; } return 0; @@ -1012,7 +1018,7 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) if (have_guest_debug) { if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk, 4, 0) || - brk != brk_insn || + !(brk == brk_insn || brk == bkpt_insn) || cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) { return -EINVAL; } @@ -1055,6 +1061,7 @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit) return false; } break; + case EC_AA32_BKPT: case EC_AA64_BKPT: if (kvm_find_sw_breakpoint(cs, env->pc)) { return true; -- 2.17.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware Alex Bennée @ 2018-12-13 12:36 ` Ard Biesheuvel 2018-12-13 14:55 ` Alex Bennée 2018-12-13 22:21 ` Richard Henderson 2018-12-14 8:37 ` Omair Javaid 2 siblings, 1 reply; 16+ messages in thread From: Ard Biesheuvel @ 2018-12-13 12:36 UTC (permalink / raw) To: Alex Bennée Cc: QEMU Developers, qemu-arm, Mark Rutland, omair.javaid, Peter Maydell Hi Alex, Thanks again for looking into this. On Thu, 13 Dec 2018 at 12:55, Alex Bennée <alex.bennee@linaro.org> wrote: > > When supported by the hardware we can run AA32 guests or even AA64 EL1 > code with AA32 EL0 mode code. Inserting a AA64 break point into AA32 > code tends to break things. This is especially acute with gdb as it > inserts temporary breakpoints when stepping through code. > > The heuristic of checking the current mode works but it's not perfect. > A user could be placing a break point in code after a mode switch and > that will still fail. However there doesn't seem to be a way to force > a hbreak by default. According to "set breakpoint auto-hw on": > > This is the default behavior. When GDB sets a breakpoint, it will try > to use the target memory map to decide if software or hardware > breakpoint must be used. > > Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Omair Javaid <omair.javaid@linaro.org> > --- > target/arm/kvm64.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index 0a502091e7..dd564a59b7 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -989,14 +989,20 @@ int kvm_arch_get_registers(CPUState *cs) > return ret; > } > > -/* C6.6.29 BRK instruction */ > +/* BRK (A64) and BKPT (A32) instructions */ > static const uint32_t brk_insn = 0xd4200000; > +static const uint32_t bkpt_insn = 0xe1200070; We'll need to cheat here, and use an opcode that is a BKPT instruction in both ARM and Thumb modes, i.e., 0xe120be70 (where the lower 16 bits constitute a T1 bkpt opcode with imm=0x70) > > int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > { > + CPUARMState *env = &ARM_CPU(cs)->env; > + int el = arm_current_el(env); > + bool is_aa64 = arm_el_is_aa64(env, el); > + const uint32_t *bpi = is_aa64 ? &brk_insn : &bkpt_insn; > + > if (have_guest_debug) { > if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) || > - cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) { > + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)bpi, 4, 1)) { Should we be dealing with endianness here? > return -EINVAL; > } > return 0; > @@ -1012,7 +1018,7 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > > if (have_guest_debug) { > if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk, 4, 0) || > - brk != brk_insn || > + !(brk == brk_insn || brk == bkpt_insn) || > cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) { > return -EINVAL; > } > @@ -1055,6 +1061,7 @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit) > return false; > } > break; > + case EC_AA32_BKPT: > case EC_AA64_BKPT: > if (kvm_find_sw_breakpoint(cs, env->pc)) { > return true; > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware 2018-12-13 12:36 ` Ard Biesheuvel @ 2018-12-13 14:55 ` Alex Bennée 2018-12-13 22:25 ` Richard Henderson 0 siblings, 1 reply; 16+ messages in thread From: Alex Bennée @ 2018-12-13 14:55 UTC (permalink / raw) To: Ard Biesheuvel Cc: QEMU Developers, qemu-arm, Mark Rutland, omair.javaid, Peter Maydell Ard Biesheuvel <ard.biesheuvel@linaro.org> writes: > Hi Alex, > > Thanks again for looking into this. > > On Thu, 13 Dec 2018 at 12:55, Alex Bennée <alex.bennee@linaro.org> wrote: <snip> > >> >> int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) >> { >> + CPUARMState *env = &ARM_CPU(cs)->env; >> + int el = arm_current_el(env); >> + bool is_aa64 = arm_el_is_aa64(env, el); >> + const uint32_t *bpi = is_aa64 ? &brk_insn : &bkpt_insn; >> + >> if (have_guest_debug) { >> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) || >> - cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) { >> + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)bpi, 4, 1)) { > > Should we be dealing with endianness here? > <snip> I don't think so - everything eventually ends up (ld|st)n_p which deals with the endianness details. -- Alex Bennée ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware 2018-12-13 14:55 ` Alex Bennée @ 2018-12-13 22:25 ` Richard Henderson 2018-12-14 16:26 ` Alex Bennée 0 siblings, 1 reply; 16+ messages in thread From: Richard Henderson @ 2018-12-13 22:25 UTC (permalink / raw) To: Alex Bennée, Ard Biesheuvel Cc: Mark Rutland, Peter Maydell, qemu-arm, QEMU Developers, omair.javaid On 12/13/18 8:55 AM, Alex Bennée wrote: > > Ard Biesheuvel <ard.biesheuvel@linaro.org> writes: > >> Hi Alex, >> >> Thanks again for looking into this. >> >> On Thu, 13 Dec 2018 at 12:55, Alex Bennée <alex.bennee@linaro.org> wrote: > <snip> >> >>> >>> int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) >>> { >>> + CPUARMState *env = &ARM_CPU(cs)->env; >>> + int el = arm_current_el(env); >>> + bool is_aa64 = arm_el_is_aa64(env, el); >>> + const uint32_t *bpi = is_aa64 ? &brk_insn : &bkpt_insn; >>> + >>> if (have_guest_debug) { >>> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) || >>> - cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) { >>> + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)bpi, 4, 1)) { >> >> Should we be dealing with endianness here? >> > <snip> > > I don't think so - everything eventually ends up (ld|st)n_p which deals > with the endianness details. I think Ard is right. You need to consider dynamic endianness with bswap_code(arm_sctlr_b(env)) r~ r~ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware 2018-12-13 22:25 ` Richard Henderson @ 2018-12-14 16:26 ` Alex Bennée 2018-12-14 16:40 ` Ard Biesheuvel 0 siblings, 1 reply; 16+ messages in thread From: Alex Bennée @ 2018-12-14 16:26 UTC (permalink / raw) To: Richard Henderson Cc: Ard Biesheuvel, Mark Rutland, Peter Maydell, qemu-arm, QEMU Developers, omair.javaid Richard Henderson <richard.henderson@linaro.org> writes: > On 12/13/18 8:55 AM, Alex Bennée wrote: >> >> Ard Biesheuvel <ard.biesheuvel@linaro.org> writes: >> >>> Hi Alex, >>> >>> Thanks again for looking into this. >>> >>> On Thu, 13 Dec 2018 at 12:55, Alex Bennée <alex.bennee@linaro.org> wrote: >> <snip> >>> >>>> >>>> int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) >>>> { >>>> + CPUARMState *env = &ARM_CPU(cs)->env; >>>> + int el = arm_current_el(env); >>>> + bool is_aa64 = arm_el_is_aa64(env, el); >>>> + const uint32_t *bpi = is_aa64 ? &brk_insn : &bkpt_insn; >>>> + >>>> if (have_guest_debug) { >>>> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) || >>>> - cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) { >>>> + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)bpi, 4, 1)) { >>> >>> Should we be dealing with endianness here? >>> >> <snip> >> >> I don't think so - everything eventually ends up (ld|st)n_p which deals >> with the endianness details. > > I think Ard is right. You need to consider dynamic endianness with > > bswap_code(arm_sctlr_b(env)) *sigh* I guess. It of course still a heuristic that can break because we don't know if the system will have switched mode by the time it gets to the breakpoint. -- Alex Bennée ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware 2018-12-14 16:26 ` Alex Bennée @ 2018-12-14 16:40 ` Ard Biesheuvel 0 siblings, 0 replies; 16+ messages in thread From: Ard Biesheuvel @ 2018-12-14 16:40 UTC (permalink / raw) To: Alex Bennée Cc: Richard Henderson, Mark Rutland, Peter Maydell, qemu-arm, QEMU Developers, Omair Javaid On Fri, 14 Dec 2018 at 17:26, Alex Bennée <alex.bennee@linaro.org> wrote: > > > Richard Henderson <richard.henderson@linaro.org> writes: > > > On 12/13/18 8:55 AM, Alex Bennée wrote: > >> > >> Ard Biesheuvel <ard.biesheuvel@linaro.org> writes: > >> > >>> Hi Alex, > >>> > >>> Thanks again for looking into this. > >>> > >>> On Thu, 13 Dec 2018 at 12:55, Alex Bennée <alex.bennee@linaro.org> wrote: > >> <snip> > >>> > >>>> > >>>> int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > >>>> { > >>>> + CPUARMState *env = &ARM_CPU(cs)->env; > >>>> + int el = arm_current_el(env); > >>>> + bool is_aa64 = arm_el_is_aa64(env, el); > >>>> + const uint32_t *bpi = is_aa64 ? &brk_insn : &bkpt_insn; > >>>> + > >>>> if (have_guest_debug) { > >>>> if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) || > >>>> - cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) { > >>>> + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)bpi, 4, 1)) { > >>> > >>> Should we be dealing with endianness here? > >>> > >> <snip> > >> > >> I don't think so - everything eventually ends up (ld|st)n_p which deals > >> with the endianness details. > > > > I think Ard is right. You need to consider dynamic endianness with > > > > bswap_code(arm_sctlr_b(env)) > > *sigh* I guess. It of course still a heuristic that can break because we > don't know if the system will have switched mode by the time it gets to > the breakpoint. > Actually, I was referring to the QEMU/host side. Instruction encodings are always little endian in ARMv7 and v8 (which is all KVM cares about in any case), but I guess it is [theoretically?] possible that we are running a BE QEMU? ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware Alex Bennée 2018-12-13 12:36 ` Ard Biesheuvel @ 2018-12-13 22:21 ` Richard Henderson 2018-12-14 8:37 ` Omair Javaid 2 siblings, 0 replies; 16+ messages in thread From: Richard Henderson @ 2018-12-13 22:21 UTC (permalink / raw) To: Alex Bennée, qemu-devel Cc: mark.rutland, Peter Maydell, Omair Javaid, ard.biesheuvel, qemu-arm On 12/13/18 5:55 AM, Alex Bennée wrote: > int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > { > + CPUARMState *env = &ARM_CPU(cs)->env; > + int el = arm_current_el(env); > + bool is_aa64 = arm_el_is_aa64(env, el); This will assert for el == 0; for that you need is_a64(env). r~ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware Alex Bennée 2018-12-13 12:36 ` Ard Biesheuvel 2018-12-13 22:21 ` Richard Henderson @ 2018-12-14 8:37 ` Omair Javaid 2018-12-14 13:53 ` Richard Henderson 2 siblings, 1 reply; 16+ messages in thread From: Omair Javaid @ 2018-12-14 8:37 UTC (permalink / raw) To: Alex Bennée Cc: qemu-devel, qemu-arm, mark.rutland, ard.biesheuvel, Peter Maydell On Thu, 13 Dec 2018 at 16:55, Alex Bennée <alex.bennee@linaro.org> wrote: > > When supported by the hardware we can run AA32 guests or even AA64 EL1 > code with AA32 EL0 mode code. Inserting a AA64 break point into AA32 > code tends to break things. This is especially acute with gdb as it > inserts temporary breakpoints when stepping through code. Hi Alex, Are you expecting GDB to switch targets (from AArch64 to AArch32 or vice versa) when there is mode switch for example a AArch64 binary calling a AArch32 library function. I think GDB will behave nicely when doing 1 type of debug but seamless switching between modes is still tricky for GDB. There has been some work in last few months on this but not sure where it has reached. > > The heuristic of checking the current mode works but it's not perfect. > A user could be placing a break point in code after a mode switch and > that will still fail. However there doesn't seem to be a way to force > a hbreak by default. According to "set breakpoint auto-hw on": On mode switch we ll have to switch target architecture on GDB side and GDB may not be able to do that seamlessly. Have you tried debugging such an example? > > This is the default behavior. When GDB sets a breakpoint, it will try > to use the target memory map to decide if software or hardware > breakpoint must be used. > > Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Omair Javaid <omair.javaid@linaro.org> > --- > target/arm/kvm64.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index 0a502091e7..dd564a59b7 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -989,14 +989,20 @@ int kvm_arch_get_registers(CPUState *cs) > return ret; > } > > -/* C6.6.29 BRK instruction */ > +/* BRK (A64) and BKPT (A32) instructions */ > static const uint32_t brk_insn = 0xd4200000; > +static const uint32_t bkpt_insn = 0xe1200070; > > int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > { > + CPUARMState *env = &ARM_CPU(cs)->env; > + int el = arm_current_el(env); > + bool is_aa64 = arm_el_is_aa64(env, el); > + const uint32_t *bpi = is_aa64 ? &brk_insn : &bkpt_insn; > + > if (have_guest_debug) { > if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 0) || > - cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk_insn, 4, 1)) { > + cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)bpi, 4, 1)) { > return -EINVAL; > } > return 0; > @@ -1012,7 +1018,7 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp) > > if (have_guest_debug) { > if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&brk, 4, 0) || > - brk != brk_insn || > + !(brk == brk_insn || brk == bkpt_insn) || > cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn, 4, 1)) { > return -EINVAL; > } > @@ -1055,6 +1061,7 @@ bool kvm_arm_handle_debug(CPUState *cs, struct kvm_debug_exit_arch *debug_exit) > return false; > } > break; > + case EC_AA32_BKPT: > case EC_AA64_BKPT: > if (kvm_find_sw_breakpoint(cs, env->pc)) { > return true; > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware 2018-12-14 8:37 ` Omair Javaid @ 2018-12-14 13:53 ` Richard Henderson 0 siblings, 0 replies; 16+ messages in thread From: Richard Henderson @ 2018-12-14 13:53 UTC (permalink / raw) To: Omair Javaid, Alex Bennée Cc: mark.rutland, Peter Maydell, qemu-arm, qemu-devel, ard.biesheuvel On 12/14/18 2:37 AM, Omair Javaid wrote: > Are you expecting GDB to switch targets (from AArch64 to AArch32 or > vice versa) when there is mode switch for example a AArch64 binary > calling a AArch32 library function. Mode changes happen only at privilege level changes. E.g. AArch32 binary makes a system call to an AArch64 kernel, or an AArch32 guest kernel takes an i/o emulation trap to an AArch64 hypervisor. But for this sort of system debugging, it would be nice to be able to single-step through the whole operation. r~ ^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v1 2/2] target/arm: defer setting up of aarch64 gdb until arm_cpu_realize 2018-12-13 11:55 [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 Alex Bennée 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware Alex Bennée @ 2018-12-13 11:55 ` Alex Bennée 2018-12-13 23:10 ` Richard Henderson 2019-01-04 15:35 ` Peter Maydell 2018-12-13 11:57 ` [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 Mark Rutland 2 siblings, 2 replies; 16+ messages in thread From: Alex Bennée @ 2018-12-13 11:55 UTC (permalink / raw) To: qemu-devel Cc: qemu-arm, mark.rutland, ard.biesheuvel, Alex Bennée, Omair Javaid, Peter Maydell If we setup earlier we miss the parsing of the aarch64 state of the CPU. If the user has booted up with: qemu-system-aarch64 -cpu host,aarch64=off -enable-kvm we end up presenting an aarch64 view of the world via the gdbstub and hilarity ensues. Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Omair Javaid <omair.javaid@linaro.org> --- include/hw/arm/arm.h | 2 ++ target/arm/cpu.c | 4 ++++ target/arm/cpu64.c | 20 +++++++++++++++----- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h index ffed39252d..f9a7a6e2fb 100644 --- a/include/hw/arm/arm.h +++ b/include/hw/arm/arm.h @@ -171,4 +171,6 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu, ticks. */ extern int system_clock_scale; +void arm_cpu_enable_aarch64_gdbstub(CPUClass *cc); + #endif /* HW_ARM_H */ diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 60411f6bfe..100a72ff81 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -890,9 +890,13 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) * queries ID_ISAR0_EL1 on such a host, the value is UNKNOWN. * Similarly, we cannot check ID_AA64PFR0 without AArch64 support. */ +#ifdef TARGET_AARCH64 if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { + CPUClass *cc = CPU_GET_CLASS(cs); no_aa32 = !cpu_isar_feature(aa64_aa32, cpu); + arm_cpu_enable_aarch64_gdbstub(cc); } +#endif if (arm_feature(env, ARM_FEATURE_V7VE)) { /* v7 Virtualization Extensions. In real hardware this implies diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 873f059bf2..53cde60557 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -434,12 +434,14 @@ static gchar *aarch64_gdb_arch_name(CPUState *cs) return g_strdup("aarch64"); } -static void aarch64_cpu_class_init(ObjectClass *oc, void *data) -{ - CPUClass *cc = CPU_CLASS(oc); +/* + * We can only setup aarch64 gdb support once we realize the CPU + * object and know what mode it has been booted in. This is called + * from arm_cpu_realize. + */ - cc->cpu_exec_interrupt = arm_cpu_exec_interrupt; - cc->set_pc = aarch64_cpu_set_pc; +void arm_cpu_enable_aarch64_gdbstub(CPUClass *cc) +{ cc->gdb_read_register = aarch64_cpu_gdb_read_register; cc->gdb_write_register = aarch64_cpu_gdb_write_register; cc->gdb_num_core_regs = 34; @@ -447,6 +449,14 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void *data) cc->gdb_arch_name = aarch64_gdb_arch_name; } +static void aarch64_cpu_class_init(ObjectClass *oc, void *data) +{ + CPUClass *cc = CPU_CLASS(oc); + + cc->cpu_exec_interrupt = arm_cpu_exec_interrupt; + cc->set_pc = aarch64_cpu_set_pc; +} + static void aarch64_cpu_register(const ARMCPUInfo *info) { TypeInfo type_info = { -- 2.17.1 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 2/2] target/arm: defer setting up of aarch64 gdb until arm_cpu_realize 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 2/2] target/arm: defer setting up of aarch64 gdb until arm_cpu_realize Alex Bennée @ 2018-12-13 23:10 ` Richard Henderson 2019-01-04 15:35 ` Peter Maydell 1 sibling, 0 replies; 16+ messages in thread From: Richard Henderson @ 2018-12-13 23:10 UTC (permalink / raw) To: Alex Bennée, qemu-devel Cc: mark.rutland, Peter Maydell, Omair Javaid, ard.biesheuvel, qemu-arm On 12/13/18 5:55 AM, Alex Bennée wrote: > If we setup earlier we miss the parsing of the aarch64 state of the > CPU. If the user has booted up with: > > qemu-system-aarch64 -cpu host,aarch64=off -enable-kvm > > we end up presenting an aarch64 view of the world via the gdbstub and > hilarity ensues. > > Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Omair Javaid <omair.javaid@linaro.org> > --- > include/hw/arm/arm.h | 2 ++ > target/arm/cpu.c | 4 ++++ > target/arm/cpu64.c | 20 +++++++++++++++----- > 3 files changed, 21 insertions(+), 5 deletions(-) Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 2/2] target/arm: defer setting up of aarch64 gdb until arm_cpu_realize 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 2/2] target/arm: defer setting up of aarch64 gdb until arm_cpu_realize Alex Bennée 2018-12-13 23:10 ` Richard Henderson @ 2019-01-04 15:35 ` Peter Maydell 2019-01-07 8:49 ` Alex Bennée 1 sibling, 1 reply; 16+ messages in thread From: Peter Maydell @ 2019-01-04 15:35 UTC (permalink / raw) To: Alex Bennée Cc: QEMU Developers, qemu-arm, Mark Rutland, Ard Biesheuvel, Omair Javaid On Thu, 13 Dec 2018 at 11:55, Alex Bennée <alex.bennee@linaro.org> wrote: > > If we setup earlier we miss the parsing of the aarch64 state of the > CPU. If the user has booted up with: > > qemu-system-aarch64 -cpu host,aarch64=off -enable-kvm > > we end up presenting an aarch64 view of the world via the gdbstub and > hilarity ensues. > > Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Cc: Omair Javaid <omair.javaid@linaro.org> > --- > include/hw/arm/arm.h | 2 ++ > target/arm/cpu.c | 4 ++++ > target/arm/cpu64.c | 20 +++++++++++++++----- > 3 files changed, 21 insertions(+), 5 deletions(-) > > diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h > index ffed39252d..f9a7a6e2fb 100644 > --- a/include/hw/arm/arm.h > +++ b/include/hw/arm/arm.h > @@ -171,4 +171,6 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu, > ticks. */ > extern int system_clock_scale; > > +void arm_cpu_enable_aarch64_gdbstub(CPUClass *cc); > + > #endif /* HW_ARM_H */ > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index 60411f6bfe..100a72ff81 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -890,9 +890,13 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) > * queries ID_ISAR0_EL1 on such a host, the value is UNKNOWN. > * Similarly, we cannot check ID_AA64PFR0 without AArch64 support. > */ > +#ifdef TARGET_AARCH64 > if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { > + CPUClass *cc = CPU_GET_CLASS(cs); > no_aa32 = !cpu_isar_feature(aa64_aa32, cpu); > + arm_cpu_enable_aarch64_gdbstub(cc); > } > +#endif This seems weird, because the fields in cc are common to all CPUs in the class, and so setting them on realize of a specific instance based on properties of the instance looks wrong... At least in theory there's no reason we couldn't have one -cpu host CPU with aarch64=off and one with aarch64=on, though I'm not sure our UI allows the user to actually set the properties per-cpu like that. thanks -- PMM ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 2/2] target/arm: defer setting up of aarch64 gdb until arm_cpu_realize 2019-01-04 15:35 ` Peter Maydell @ 2019-01-07 8:49 ` Alex Bennée 0 siblings, 0 replies; 16+ messages in thread From: Alex Bennée @ 2019-01-07 8:49 UTC (permalink / raw) To: Peter Maydell Cc: QEMU Developers, qemu-arm, Mark Rutland, Ard Biesheuvel, Omair Javaid Peter Maydell <peter.maydell@linaro.org> writes: > On Thu, 13 Dec 2018 at 11:55, Alex Bennée <alex.bennee@linaro.org> wrote: >> >> If we setup earlier we miss the parsing of the aarch64 state of the >> CPU. If the user has booted up with: >> >> qemu-system-aarch64 -cpu host,aarch64=off -enable-kvm >> >> we end up presenting an aarch64 view of the world via the gdbstub and >> hilarity ensues. >> >> Reported-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Cc: Omair Javaid <omair.javaid@linaro.org> >> --- >> include/hw/arm/arm.h | 2 ++ >> target/arm/cpu.c | 4 ++++ >> target/arm/cpu64.c | 20 +++++++++++++++----- >> 3 files changed, 21 insertions(+), 5 deletions(-) >> >> diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h >> index ffed39252d..f9a7a6e2fb 100644 >> --- a/include/hw/arm/arm.h >> +++ b/include/hw/arm/arm.h >> @@ -171,4 +171,6 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu, >> ticks. */ >> extern int system_clock_scale; >> >> +void arm_cpu_enable_aarch64_gdbstub(CPUClass *cc); >> + >> #endif /* HW_ARM_H */ >> diff --git a/target/arm/cpu.c b/target/arm/cpu.c >> index 60411f6bfe..100a72ff81 100644 >> --- a/target/arm/cpu.c >> +++ b/target/arm/cpu.c >> @@ -890,9 +890,13 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) >> * queries ID_ISAR0_EL1 on such a host, the value is UNKNOWN. >> * Similarly, we cannot check ID_AA64PFR0 without AArch64 support. >> */ >> +#ifdef TARGET_AARCH64 >> if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { >> + CPUClass *cc = CPU_GET_CLASS(cs); >> no_aa32 = !cpu_isar_feature(aa64_aa32, cpu); >> + arm_cpu_enable_aarch64_gdbstub(cc); >> } >> +#endif > > This seems weird, because the fields in cc are common > to all CPUs in the class, and so setting them on > realize of a specific instance based on properties of > the instance looks wrong... At least in theory there's > no reason we couldn't have one -cpu host CPU with > aarch64=off and one with aarch64=on, though I'm not > sure our UI allows the user to actually set the > properties per-cpu like that. So we should really move these to be object instance methods? Or possibly have object instance methods that override the class methods if they are not NULL? > > thanks > -- PMM -- Alex Bennée ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 2018-12-13 11:55 [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 Alex Bennée 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware Alex Bennée 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 2/2] target/arm: defer setting up of aarch64 gdb until arm_cpu_realize Alex Bennée @ 2018-12-13 11:57 ` Mark Rutland 2018-12-13 15:28 ` Alex Bennée 2 siblings, 1 reply; 16+ messages in thread From: Mark Rutland @ 2018-12-13 11:57 UTC (permalink / raw) To: Alex Bennée; +Cc: qemu-devel, qemu-arm, ard.biesheuvel, marc.zyngier Hi Alex, On Thu, Dec 13, 2018 at 11:55:01AM +0000, Alex Bennée wrote: > Hi, > > This is an attempt to fix debugging of AArch32 binaries when running > under KVM on AArch64 hardware. There are two parts to this, the first is > a handling the possibility of AArch32 software breakpoints with a > heuristic based on the current execution mode. The second part is > delaying the setup of aarch64 debugging until the shared arm_cpu_realize > function is run by which point we have parsed and decoded the actual > execution mode of the guest. This doesn't solve the problem of split > mode guests which switch between an AA64 EL1 and an AA32 EL0 though. > > I still ran into a problem with single-step. Even with Mark's > single-step fixup series: > > To: linux-arm-kernel@lists.infradead.org > Cc: kvmarm@lists.cs.columbia.edu, > Subject: [PATCH 0/2] kvm/arm: make singlestep behaviour consistent > Date: Fri, 9 Nov 2018 15:07:09 +0000 > Message-Id: <20181109150711.45864-1-mark.rutland@arm.com> > > some instructions do single-step but sometimes the single-step doesn't > return leading to a runaway until it hits a breakpoint. I'm not sure why > this is the case because the SS state machine shouldn't be instruction > sensitive. Could you please give an example sequence where this occurs? I'd be happy to take a look. Thanks, Mark. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 2018-12-13 11:57 ` [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 Mark Rutland @ 2018-12-13 15:28 ` Alex Bennée 0 siblings, 0 replies; 16+ messages in thread From: Alex Bennée @ 2018-12-13 15:28 UTC (permalink / raw) To: Mark Rutland; +Cc: qemu-devel, qemu-arm, ard.biesheuvel, marc.zyngier Mark Rutland <mark.rutland@arm.com> writes: > Hi Alex, > > On Thu, Dec 13, 2018 at 11:55:01AM +0000, Alex Bennée wrote: >> Hi, >> >> This is an attempt to fix debugging of AArch32 binaries when running >> under KVM on AArch64 hardware. There are two parts to this, the first is >> a handling the possibility of AArch32 software breakpoints with a >> heuristic based on the current execution mode. The second part is >> delaying the setup of aarch64 debugging until the shared arm_cpu_realize >> function is run by which point we have parsed and decoded the actual >> execution mode of the guest. This doesn't solve the problem of split >> mode guests which switch between an AA64 EL1 and an AA32 EL0 though. >> >> I still ran into a problem with single-step. Even with Mark's >> single-step fixup series: >> >> To: linux-arm-kernel@lists.infradead.org >> Cc: kvmarm@lists.cs.columbia.edu, >> Subject: [PATCH 0/2] kvm/arm: make singlestep behaviour consistent >> Date: Fri, 9 Nov 2018 15:07:09 +0000 >> Message-Id: <20181109150711.45864-1-mark.rutland@arm.com> >> >> some instructions do single-step but sometimes the single-step doesn't >> return leading to a runaway until it hits a breakpoint. I'm not sure why >> this is the case because the SS state machine shouldn't be instruction >> sensitive. > > Could you please give an example sequence where this occurs? I'd be > happy to take a look. Here is a trace in gdb: => 0x0: b 0x1000 0x4: ; <UNDEFINED> instruction: 0xffffffff 0x8: ; <UNDEFINED> instruction: 0xffffffff 0xc: ; <UNDEFINED> instruction: 0xffffffff 0x10: ; <UNDEFINED> instruction: 0xffffffff 0x1000: bl 0x372c 0x1004: andeq r12, r0, r1, asr r12 0x1008: rors pc, lr, r0 ; <UNPREDICTABLE> 0x100c: andeq r0, r0, r0 0x1010: cfstr32hi mvfx14, [r12], {120} ; 0x78 0x372c: bl 0x39d4 0x3730: bl 0x39cc 0x3734: mov r5, r0 0x3738: bl 0x39e8 0x373c: movw r1, #0 0x39d4: bx lr 0x39d8: and r1, r0, #255 ; 0xff 0x39dc: and r0, r0, #65280 ; 0xff00 0x39e0: add r0, r1, r0, lsr #7 0x39e4: bx lr Hardware assisted breakpoint 1 at 0x39d4 0x00001000 in ?? () => 0x1000: bl 0x372c 0x1004: andeq r12, r0, r1, asr r12 0x1008: rors pc, lr, r0 ; <UNPREDICTABLE> Thread 1 hit Breakpoint 1, 0x000039d4 in ?? () => 0x39d4: bx lr 0x39d8: and r1, r0, #255 ; 0xff 0x39dc: and r0, r0, #65280 ; 0xff00 0x1000: 0xeb0009c9 0x0000cc51 The second instruction (bl 0x372c) didn't single-step and we eventually hit the hbreak I set at 0x39d4. This is from ard's QEMU_EFI.fd build: http://snapshots.linaro.org/components/kernel/leg-virt-tianocore-edk2-upstream/3373/QEMU-ARM/DEBUG_GCC5/QEMU_EFI.img.gz Running with: ./aarch64-softmmu/qemu-system-aarch64 -M virt -cpu host,aarch64=off -enable-kvm -net none -nographic -bios ~/QEMU_EFI_aarch32.img -smp 2 -s -S And: gdb -ex "target remote localhost:1234" > > Thanks, > Mark. -- Alex Bennée ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-01-07 8:49 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-13 11:55 [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 Alex Bennée 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 1/2] target/arm: kvm64 make guest debug AA32 break point aware Alex Bennée 2018-12-13 12:36 ` Ard Biesheuvel 2018-12-13 14:55 ` Alex Bennée 2018-12-13 22:25 ` Richard Henderson 2018-12-14 16:26 ` Alex Bennée 2018-12-14 16:40 ` Ard Biesheuvel 2018-12-13 22:21 ` Richard Henderson 2018-12-14 8:37 ` Omair Javaid 2018-12-14 13:53 ` Richard Henderson 2018-12-13 11:55 ` [Qemu-devel] [PATCH v1 2/2] target/arm: defer setting up of aarch64 gdb until arm_cpu_realize Alex Bennée 2018-12-13 23:10 ` Richard Henderson 2019-01-04 15:35 ` Peter Maydell 2019-01-07 8:49 ` Alex Bennée 2018-12-13 11:57 ` [Qemu-devel] [PATCH v1 0/2] Fix kvm guest debugging of AA32 guests on AA64 Mark Rutland 2018-12-13 15:28 ` Alex Bennée
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.