[Qemu-devel] Cleaning up handling of CPU exceptions accessing non-existent memory/devices

[Qemu-devel] Cleaning up handling of CPU exceptions accessing non-existent memory/devices

[Peter Maydell]

Hi; I've been looking at our code to handle allowing guest CPUs
to generate exceptions when they access non-existent memory or
devices (in ARM terms, external aborts; also often called bus errors).

At the moment we have a hook for this: do_unassigned_access().
Unfortunately this was designed a long time ago and has some nasty
problems. The hook is called from the unassigned_mem_write() and
unassigned_mem_read() functions if the current_cpu pointer is non-NULL.
This means that:
 * it will be called even if the access was not directly done by the
   CPU -- for instance if the CPU writes to an ethernet device enable
   register and triggers the ethernet device to try to read a packet
   out of non-existent memory, the CPU will take a bogus exception
 * it won't be called if there really is a device there and it actively
   reports a bus error by returning a failure MemTxResult code
 * it's called even when KVM is enabled even though trying to
   raise an exception then will crash because there's no setjmp
   context to longjmp back to

Since we have MemTxResult now, we don't need to try to call the CPU
hook from down at the bottom of the memory system; we can just call
it near the top when we get back a failure status.

My proposal is that we should define a new QOM CPU hook:
  void (*do_transaction_failed)(CPUState *cpu, hwaddr physaddr, vaddr addr,
                                unsigned size, MMUAccessType access_type,
                                MemTxAttrs attrs, MemTxResult response);

which is called from io_readx() and io_writex() if their calls
to memory_region_dispatch_{read,write} return something other than
MEMTX_OK. We can also call it from get_page_addr_code() as a
way to report instruction-fetch faults (where we currently call
cpu_unassigned_access), though I have a feeling there is a nicer
place to handle this (perhaps best left for later cleanup).

That will mean that:
 * memory accesses from generated code can cause exceptions due to
   bus faults
 * memory accesses by C code calling helper functions which take a
   virtual address can cause busfaults (which longjump out) -- but
   that should be already expected as those calls can cause MMU
   exceptions and alignment exceptions. (This kind of helper I think
   is just helper_{le,be,ret}_{ld,st}_*)
 * any other memory access by C code (ie by physical address) will not
   cause a bus fault exception -- C code must use one of the load/store
   functions that returns a MemTxResult and handle it appropriately
   if it needs some particular behaviour.

I think this is an easy rule of thumb to remember ('exceptions for
vaddr accesses; handle failure yourself for physaddr accesses') and
it seems likely that most physaddr accesses will want special casing
anyway.

The awkward part is the transition from the current hook to the new one;
we can put in the new hook easily enough, but the old one is used by:
 alpha, microblaze, mips, sparc, xtensa
Converting the hook code from the old API to the new will be easy,
but it's harder to know if there were places where the target is
relying on the old behaviour that an access by physical address
would generate an exception; in an ideal world we'd audit all the
loads and stores the target code did to check their behaviour...
I can have a go at converting some of these archs, but I can't
necessarily promise I'll get all the obscure corner cases.

When we've got all the archs converted we can delete the old
hook code.

Does this seem like a good plan?

thanks
-- PMM

Re: [Qemu-devel] Cleaning up handling of CPU exceptions accessing non-existent memory/devices

[Richard Henderson]

On 08/01/2017 03:50 AM, Peter Maydell wrote:
> Does this seem like a good plan?

Seems reasonable to me.

I'll warn you ahead of time that Alpha raises a machine-check exception for all 
of these, which qemu will dutifully raise.  But I never filled in those bits in 
the rom; the machine-check entry point will invoke qemu shutdown.


r~

Re: [Qemu-devel] Cleaning up handling of CPU exceptions accessing non-existent memory/devices

[Peter Maydell]

On 1 August 2017 at 11:50, Peter Maydell <peter.maydell@linaro.org> wrote:
> My proposal is that we should define a new QOM CPU hook:
>   void (*do_transaction_failed)(CPUState *cpu, hwaddr physaddr, vaddr addr,
>                                 unsigned size, MMUAccessType access_type,
>                                 MemTxAttrs attrs, MemTxResult response);

I realised that we probably also want the 'uintptr_t retaddr'
and 'int mmu_idx' that tlb_fill() and the do_unaligned_access
hook both already have.

I have some patches mostly-ready which implement the
common code changes plus the ARM mode implementation of the
hook function.

thanks
-- PMM