Re: [Qemu-devel] [PATCH] exec.c: Replace memory_region_is_unassigned() with memory_region_is_ram_backed()

[Peter Maydell]

On 9 July 2018 at 18:50, Richard Henderson <address@hidden> 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