Re: [Qemu-devel] [RFC PATCH] mmio-exec: Make device return MemoryRegion rather than host pointer

[KONRAD Frederic]

On 04/26/2018 05:09 PM, Peter Maydell wrote:
> Our current interface for allowing a device to support execution from
> MMIO regions has the device return a pointer into host memory
> containing the contents to be used for execution.  Unfortunately the
> obvious implementation this encourages (and which our only in-tree
> user uses) is to have a single buffer which is reused for each new
> request_ptr call.  This doesn't work, because RCU means that removal
> of the old RAMBlock based on the host pointer may be deferred until a
> point after a new RAMBlock with the same pointer has been added to
> the list.  The RAMBlock APIs assume that the host-pointer to ram_addr
> mapping is unique, and mmio-exec is breaking that assumption.
>
> Instead, change the API so that the device is responsible for
> allocating a new RAM MemoryRegion and populating it.  The
> MemoryRegion will be g_free()d when the region is eventually
> invalidated or otherwise unneeded.
>
> HACKS:
>   * Note that in order for the refcounting to work correctly without
>     having to manufacture a device purely to be the owner of the
>     MemoryRegion, we must pass OBJECT(newmr) as the owner (so the MR is
>     its own owner!) and NULL as the name (or the property created on the
>     owner causes the refcount to go negative during finalization)...
>
> QUESTIONS:
>   * do we really need to postpone things from
>     memory_region_invalidate_mmio_ptr() to
>     memory_region_do_invalidate_mmio_ptr(), given that the deletion of
>     the memory region and ramblock is RCU-deferred anyway?
>   * should we add the subregion at elevated prio so it definitely hits
>     first?  I've left it the way the existing code does for the
>     moment...
>   * is there any way to avoid the weird self-owning MemoryRegion
>     and corresponding need to pass a NULL name pointer?

About refcounting and self-owning MemoryRegion:
I put the MemoryRegion in the mmio_interface to avoid this issues
and to passes the base address, size, etc.

Fred

> NOTES:
>   * This change means that hw/misc/mmio_interface.c is no longer
>     used; if this gets beyond RFC stage then another patch to
>     delete it can follow.
>   * The "request_mmio_exec mustn't fail after it has called
>     memory_region_mmio_ptr_alloc()" requirement is a bit ugly, but
>     could probably be removed.