On 12/09/17 01:30, Paolo Bonzini wrote: > On 11/09/2017 14:08, Alexey Kardashevskiy wrote: >>> Ok, this makes sense. Maybe it should be a flatview rather than an >>> AddressSpaceDispatch (a FlatView is essentially a list of >>> MemoryRegionSections; attaching the ASD to the FlatView is more or less >>> an implementation detail). >> The helpers I converted from AddressSpace to AddressSpaceDispatch do use >> dispatch structure only and do not use FlatView so it seemed logical. > > Understood, but from a design POV FlatView makes more sense. > >> btw this address_space in MemoryRegionSection - it does not seem to make >> much sense in the PhysPageMap::sections array, it only makes sense when >> MemoryRegionSection uses as a temporary object when calling listeners. Will >> it make sense if we enforce MemoryRegionSection::address_space to be NULL >> in the array and not NULL when used temporary? > > memory_region_section_get_iotlb needs to access the AddressSpaceDispatch > for sections stored in the PhysPageMap array, because > memory_region_section_get_iotlb uses the ASD to compute the section index.
Ohhh, not extremely trivial, out of curiosity - is that iotlb encoding described anywhere? Anyway, this can be simplified (or rather made more straightforward?) - tlb_set_page_with_attrs() can calculate the section index and pass it to memory_region_section_get_iotlb(). Still does not make much sense? It just looks quite useless to keep that address_space pointer alive just for one case which can easily avoid using this pointer. -- Alexey
