[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] exec.c: Replace memory_region_is_unassigned() w
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH] exec.c: Replace memory_region_is_unassigned() with memory_region_is_ram_backed() |
Date: |
Tue, 10 Jul 2018 11:56:28 +0100 |
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