On 18 December 2014 at 00:26, David Morrison <dmorri...@invlim.com> wrote: > This patch fixes two bugs in Qemu for OpenRISC, and enables more > functionality from or1k-elf-gdb: > > 1) Fixed the decoding of "system" instructions (starting with 0x2) > in dec_sys() in translate.c. In particular, the l.trap instruction > is now correctly decoded, which enables for singlestepping and > breakpoints to be set in GDB. > > 2) Fixed a memory read error when debugging kernels inside Qemu and > the OpenRISC MMU is enabled
Thanks for this patch; comments below. > Signed-off-by: David R. Morrison <dmorri...@invlim.com> > --- > target-openrisc/cpu.h | 1 + > target-openrisc/mmu.c | 2 +- > target-openrisc/translate.c | 2 +- > 3 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h > index 69b96c6..6b08af6 100644 > --- a/target-openrisc/cpu.h > +++ b/target-openrisc/cpu.h > @@ -20,6 +20,7 @@ > #ifndef CPU_OPENRISC_H > #define CPU_OPENRISC_H > > +#define TARGET_HAS_ICE > #define TARGET_LONG_BITS 32 > #define ELF_MACHINE EM_OPENRISC This looks like a correct change, but it should be in its own patch. (The general principle is that each unrelated bug fix should get a patch and thus a git commit of its own.) > diff --git a/target-openrisc/mmu.c b/target-openrisc/mmu.c > index 750a936..bbd05f1 100644 > --- a/target-openrisc/mmu.c > +++ b/target-openrisc/mmu.c > @@ -219,7 +219,7 @@ hwaddr openrisc_cpu_get_phys_page_debug(CPUState *cs, > vaddr addr) > hwaddr phys_addr; > int prot; > > - if (cpu_openrisc_get_phys_addr(cpu, &phys_addr, &prot, addr, 0)) { > + if (cpu_openrisc_get_phys_nommu(cpu, &phys_addr, &prot, addr, 0)) { This looks wrong -- we won't do the virtual-to-physical translation on the addresses provided by the debugger if we use the _nommu() function. You definitely need to be doing a v-to-p translation here somehow. > return -1; > } > > diff --git a/target-openrisc/translate.c b/target-openrisc/translate.c > index 407bd97..d36278f 100644 > --- a/target-openrisc/translate.c > +++ b/target-openrisc/translate.c > @@ -1320,7 +1320,7 @@ static void dec_sys(DisasContext *dc, uint32_t insn) > #ifdef OPENRISC_DISAS > uint32_t K16; > #endif > - op0 = extract32(insn, 16, 8); > + op0 = extract32(insn, 16, 10); > #ifdef OPENRISC_DISAS > K16 = extract32(insn, 0, 16); > #endif This change should also go in a patch of its own, since it's not related to either the HAS_ICE fix or the change to get_phys_page_debug(). thanks -- PMM