On 12/09/2017 07:55, Alexey Kardashevskiy wrote: > 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?
No, I don't think so. > 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. Hmm I suppose address_space_translate_for_iotlb knows the ASD and could also return the index, basically combining it and memory_region_section_get_iotlb() into one function. Paolo