On 8/2/19 6:04 PM, Peter Maydell wrote: > The MIPS Jazz ('magnum' and 'pica61') boards have some code which > overrides the CPU's do_unassigned_access hook, so they can intercept > it and not raise exceptions on data accesses to invalid addresses, > only for instruction fetches. > > We want to switch MIPS over to using the do_transaction_failed > hook instead, so add an intercept for that as well, and make > the board code install whichever hook the CPU is actually using. > Once we've changed the CPU implementation we can remove the > redundant code for the old hook. > > Note: I am suspicious that the behaviour as implemented here may not > be what the hardware really does. It was added in commit > 54e755588cf1e90f0b14 to restore the behaviour that was broken by > commit c658b94f6e8c206c59d. But prior to commit c658b94f6e8c206c59d > every MIPS board generated exceptions for instruction access to > invalid addresses but not for data accesses; and other boards, > notably Malta, were fixed by making all invalid accesses behave as > reads-as-zero (see the call to empty_slot_init() in > mips_malta_init()). Hardware that raises exceptions for instruction > access and not data access seems to me to be an unlikely design, and > it's possible that the right way to emulate this is to make the Jazz > boards do what we did with Malta (or some variation of that). > Nonetheless, since I don't have access to real hardware to test > against I have taken the approach of "make QEMU continue to behave > the same way it did before this commit". I have updated the comment > to correct the parts that are no longer accurate and note that > the hardware might behave differently. > > The test case for the need for the hook-hijacking is in > https://bugs.launchpad.net/qemu/+bug/1245924 That BIOS will boot OK > either with this overriding of both hooks, or with a simple "global > memory region to ignore bad accesses of all types", so it doesn't > provide evidence either way, unfortunately. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org> > --- > hw/mips/mips_jazz.c | 54 ++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 46 insertions(+), 8 deletions(-) > > diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c > index fa8775d4284..c64b4c78809 100644 > --- a/hw/mips/mips_jazz.c > +++ b/hw/mips/mips_jazz.c > @@ -123,6 +123,28 @@ static void mips_jazz_do_unassigned_access(CPUState > *cpu, hwaddr addr, > (*real_do_unassigned_access)(cpu, addr, is_write, is_exec, opaque, size); > } > > +static void (*real_do_transaction_failed)(CPUState *cpu, hwaddr physaddr, > + vaddr addr, unsigned size, > + MMUAccessType access_type, > + int mmu_idx, MemTxAttrs attrs, > + MemTxResult response, > + uintptr_t retaddr); > + > +static void mips_jazz_do_transaction_failed(CPUState *cs, hwaddr physaddr, > + vaddr addr, unsigned size, > + MMUAccessType access_type, > + int mmu_idx, MemTxAttrs attrs, > + MemTxResult response, > + uintptr_t retaddr) > +{ > + if (access_type != MMU_INST_FETCH) { > + /* ignore invalid access (ie do not raise exception) */ > + return; > + } > + (*real_do_transaction_failed)(cs, physaddr, addr, size, access_type, > + mmu_idx, attrs, response, retaddr); > +} > + > static void mips_jazz_init(MachineState *machine, > enum jazz_model_e jazz_model) > { > @@ -157,16 +179,32 @@ static void mips_jazz_init(MachineState *machine, > env = &cpu->env; > qemu_register_reset(main_cpu_reset, cpu); > > - /* Chipset returns 0 in invalid reads and do not raise data exceptions. > + /* > + * Chipset returns 0 in invalid reads and do not raise data exceptions. > * However, we can't simply add a global memory region to catch > - * everything, as memory core directly call unassigned_mem_read/write > - * on some invalid accesses, which call do_unassigned_access on the > - * CPU, which raise an exception. > - * Handle that case by hijacking the do_unassigned_access method on > - * the CPU, and do not raise exceptions for data access. */ > + * everything, as this would make all accesses including instruction > + * accesses be ignored and not raise exceptions. > + * So instead we hijack either the do_unassigned_access method or > + * the do_transaction_failed method on the CPU, and do not raise > exceptions > + * for data access. > + * > + * NOTE: this behaviour of raising exceptions for bad instruction > + * fetches but not bad data accesses was added in commit 54e755588cf1e9 > + * to restore behaviour broken by c658b94f6e8c206, but it is not clear > + * whether the real hardware behaves this way. It is possible that > + * real hardware ignores bad instruction fetches as well -- if so then > + * we could replace this hijacking of CPU methods with a simple global > + * memory region that catches all memory accesses, as we do on Malta.
Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> > + */ > cc = CPU_GET_CLASS(cpu); > - real_do_unassigned_access = cc->do_unassigned_access; > - cc->do_unassigned_access = mips_jazz_do_unassigned_access; > + if (cc->do_unassigned_access) { > + real_do_unassigned_access = cc->do_unassigned_access; > + cc->do_unassigned_access = mips_jazz_do_unassigned_access; > + } > + if (cc->do_transaction_failed) { > + real_do_transaction_failed = cc->do_transaction_failed; > + cc->do_transaction_failed = mips_jazz_do_transaction_failed; > + } > > /* allocate RAM */ > memory_region_allocate_system_memory(ram, NULL, "mips_jazz.ram", >