Re: [Qemu-devel] [Qemu-arm] [PATCH 3/8] cputlb: Support generating CPU exceptions on memory transaction failures

[Peter Maydell]

On 4 August 2017 at 18:20, Peter Maydell <address@hidden> wrote:
> Call the new cpu_transaction_failed() hook at the places where
> CPU generated code interacts with the memory system:
>  io_readx()
>  io_writex()
>  get_page_addr_code()

> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c

>  static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
> +                      int mmu_idx,
>                        uint64_t val, target_ulong addr,
>                        uintptr_t retaddr, int size)
>  {
> @@ -783,6 +791,7 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
> *iotlbentry,
>      hwaddr physaddr = iotlbentry->addr;
>      MemoryRegion *mr = iotlb_to_region(cpu, physaddr, iotlbentry->attrs);
>      bool locked = false;
> +    MemTxResult r;
>
>      physaddr = (physaddr & TARGET_PAGE_MASK) + addr;
>      if (mr != &io_mem_rom && mr != &io_mem_notdirty && !cpu->can_do_io) {
> @@ -795,7 +804,12 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry 
> *iotlbentry,
>          qemu_mutex_lock_iothread();
>          locked = true;
>      }
> -    memory_region_dispatch_write(mr, physaddr, val, size, iotlbentry->attrs);
> +    r = memory_region_dispatch_write(mr, physaddr,
> +                                     val, size, iotlbentry->attrs);
> +    if (r != MEMTX_OK) {
> +        cpu_transaction_failed(cpu, physaddr, addr, size, MMU_DATA_STORE,
> +                               mmu_idx, iotlbentry->attrs, r, retaddr);
> +    }

I was looking at a bug that involved stepping through this function,
and it turns out that the value in the variable "physaddr" here is
not in fact the physical address of the access. It's just the offset
into the memory region.

This doesn't matter for Arm or Alpha, which don't actually need the
physaddr for reporting the exceptions, and those are the only
current implementations of the transaction_failed hook.
But we should fix this so it doesn't bite us when we do eventually
have a cpu that needs the physaddr...

If we have a CPUIOTLBEntry how do we get back to the physaddr for it?

thanks
-- PMM