Re: [Qemu-devel] [PULL v2 6/7] exec: allow to get a pointer for some mmio memory region

[Peter Maydell]

On 27 June 2017 at 16:37, Edgar E. Iglesias <address@hidden> wrote:
> From: KONRAD Frederic <address@hidden>
>
> This introduces a special callback which allows to run code from some MMIO
> devices.
>
> SysBusDevice with a MemoryRegion which implements the request_ptr callback 
> will
> be notified when the guest try to execute code from their offset. Then it will
> be able to eg: pre-load some code from an SPI device or ask a pointer from an
> external simulator, etc..
>
> When the pointer or the data in it are no longer valid the device has to
> invalidate it.

> +static void memory_region_do_invalidate_mmio_ptr(CPUState *cpu,
> +                                                 run_on_cpu_data data)
> +{
> +    MMIOPtrInvalidate *invalidate_data = (MMIOPtrInvalidate *)data.host_ptr;
> +    MemoryRegion *mr = invalidate_data->mr;
> +    hwaddr offset = invalidate_data->offset;
> +    unsigned size = invalidate_data->size;
> +    MemoryRegionSection section = memory_region_find(mr, offset, size);
> +
> +    qemu_mutex_lock_iothread();
> +
> +    /* Reset dirty so this doesn't happen later. */
> +    cpu_physical_memory_test_and_clear_dirty(offset, size, 1);
> +
> +    if (section.mr != mr) {
> +        /* memory_region_find add a ref on section.mr */
> +        memory_region_unref(section.mr);
> +        if (MMIO_INTERFACE(section.mr->owner)) {

Could somebody explain why it's OK to unref section.mr here before
we go on to do things with it, rather than only unrefing it after
we've finished using it?

Also, by my reading memory_region_find() will always ref
ret.mr (if it's not NULL), whereas this code only unrefs it
if section.mr == mr. Does this leak a reference in the case
where section.mr != mr, or am I missing something ?

> +            /* We found the interface just drop it. */
> +            object_property_set_bool(section.mr->owner, false, "realized",
> +                                     NULL);
> +            object_unref(section.mr->owner);
> +            object_unparent(section.mr->owner);
> +        }
> +    }
> +
> +    qemu_mutex_unlock_iothread();
> +
> +    if (invalidate_data->allocated) {
> +        g_free(invalidate_data);
> +    } else {
> +        invalidate_data->busy = 0;
> +    }
> +}

thanks
-- PMM