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",
>
