On 03/02/2017 09:06, fred.kon...@greensocs.com wrote:
> + host = mr->ops->request_ptr(mr->opaque, addr - mr->addr, &size, &offset);
> +
> + if (!host || !size) {
> + memory_region_transaction_commit();
> + return false;
> + }
> +
> + sub = g_new(MemoryRegion, 1);
> + memory_region_init_ram_ptr(sub, OBJECT(mr), "mmio-map", size, host);
> + memory_region_add_subregion(mr, offset, sub);
> + memory_region_transaction_commit();
> + return true;
> +}
> +
> +void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
> + unsigned size)
> +{
> + MemoryRegionSection section = memory_region_find(mr, offset, size);
> +
> + if (section.mr != mr) {
> + memory_region_del_subregion(mr, section.mr);
> + /* memory_region_find add a ref on section.mr */
> + memory_region_unref(section.mr);
> + object_unparent(OBJECT(section.mr));
I think this would cause a use-after-free when using MTTCG. In general,
creating and dropping MemoryRegions dynamically can cause bugs that are
nondeterministic and hard to fix without rewriting everything.
An alternative design could be:
- memory_region_request_mmio_ptr returns a MemoryRegionCache instead of
a pointer, so that the device can map a subset of the device (e.g. a
single page)
- memory_region_request_mmio_ptr and MemoryRegionOps.request_ptr accept
a Notifier
- the device adds the Notifier to a NotifierList. Before invalidating,
it invokes the Notifier and empties the NotifierList.
- for the TLB case, the Notifier calls tlb_flush_page.
I like the general idea though!
Paolo
> + }
> +}
