On 9 July 2018 at 18:50, Richard Henderson <r...@twiddle.net> wrote: > On 07/09/2018 10:33 AM, Peter Maydell wrote: >>> Well... ok. We should document that this, surprisingly, does not include >>> actual ram. Just things that are ram with caveats. >> >> I think it must include actual RAM, or we would not be able to >> execute from actual RAM ? The only way to get to get_page_addr_code()'s >> "here's the ram_addr_t for this" exit path is if >> memory_region_is_unassigned() >> returns false. >> >>>> - return mr != &io_mem_rom && mr != &io_mem_notdirty && !mr->rom_device >>>> - && mr != &io_mem_watch; > > Huh. I don't follow how this old expression excludes ram, > and so assumed that must be checked separately earlier. > > There's clearly still something here I don't understand.
I dug a bit deeper, and the reason why this works is that the MR it's operating on is not an arbitrary MR for this chunk of the address space. It's the section->mr for the section returned by iotlb_to_section() for the iotlbentry value. That value is set up by memory_region_section_get_iotlb(), which picks either PHYS_SECTION_NOTDIRTY or PHYS_SECTION_ROM for any MR that is RAM (by the memory_region_is_ram() definition). So then the iotlb_to_section() call will return one of the dummy sections and its section->mr will be either io_mem_rom or io_mem_notdirty, not the original MR. The other thing I've noticed is that after my in-progress patches to handle execution from MMIO regions, the code that calls memory_region_is_unassigned() is just iotlbentry = &env->iotlb[mmu_idx][index]; section = iotlb_to_section(cpu, iotlbentry->addr, iotlbentry->attrs); mr = section->mr; if (memory_region_is_unassigned(mr)) { ... and there is no other use of 'mr', so perhaps it would be better to have a function that takes a MemoryRegionSection rather than a MemoryRegion that must be the one we got from section->mr... thanks -- PMM