On 10 August 2017 at 14:08, KONRAD Frederic <[email protected]> wrote: > > > On 08/10/2017 02:43 PM, Peter Maydell wrote: >> >> On 10 August 2017 at 13:21, KONRAD Frederic <[email protected]> >> wrote: >>>
>> Looking at the implementation it seems like this will work in >> practice (because the invalidate code in memory.c checks that >> the MR it's about to drop really is an MMIO_INTERFACE), but >> if so we should document this. However, I'm not totally convinced >> that it really will work in complicated cases like where you >> have device A part of whose memory range is a container which >> holds a device B which is also using the mmio_pointer API: >> in that case if device A calls invalidate_mmio_ptr with an >> address that's in the part of A's memory region that is the >> device B container it could end up invalidating an mmio-interface >> that's actually inside device B. So it would be safer to say >> "the caller may only invalidate pointers it's actually told >> the memory system about". >> > > I see what you mean but I'm not sure this will happen because the > mmio-interface is mapped on @mr which is passed to invalidate. > > So if device A doesn't have any mmio-interface mapped it can't > find the device B mmio-interface as we pass device A > MemoryRegion. I think what you're saying is (1) you can't create an mmio-ptr for a container region (2) when you call invalidate_mmio_ptr you have to pass exactly the MR that has the memory-region-ops that the have the request_ptr set. This isn't entirely clear from the docs, but OTOH it seems reasonable and I agree that in that case you can't get the situation I suggested above. > But I agree the doc is a little confusing about that. > >> (Conversely if we're convinced that passing bogus pointers >> to memory_region_invalidate_mmio_ptr() is fine then we >> don't need to add the q->mmio_execution_enabled flag check.) > > > True but this will trigger an async_safe_work with all the > overhead. If we care about that then we should have a "have we used the cached region as an mmio_ptr" flag separately anyway, so that we don't trigger async_safe_work for the case of "not executing from the memory, but we had to call lqspi_load_cache() because the guest did a read from some address that was outside the cached section". thanks -- PMM
