On 26.10.21 19:00, Philippe Mathieu-Daudé wrote:
> On 10/26/21 18:06, David Hildenbrand wrote:
>> memory_region_is_mapped() currently does not return "true" when a memory
>> region is mapped via an alias.
>>
>> Assuming we have:
>> alias (A0) -> alias (A1) -> region (R0)
>> Mapping A0 would currently only make memory_region_is_mapped() succeed
>> on A0, but not on A1 and R0.
>>
>> Let's fix that by adding a "mapped_via_alias" counter to memory regions and
>> updating it accordingly when an alias gets (un)mapped.
>>
>> I am not aware of actual issues, this is rather a cleanup to make it
>> consistent.
>>
>> Signed-off-by: David Hildenbrand <da...@redhat.com>
>> ---
>> include/exec/memory.h | 1 +
>> softmmu/memory.c | 12 +++++++++++-
>> 2 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/exec/memory.h b/include/exec/memory.h
>> index a185b6dcb8..35382d9870 100644
>> --- a/include/exec/memory.h
>> +++ b/include/exec/memory.h
>> @@ -707,6 +707,7 @@ struct MemoryRegion {
>> const MemoryRegionOps *ops;
>> void *opaque;
>> MemoryRegion *container;
>> + int mapped_via_alias; /* Mapped via an alias, container might be NULL */
>> Int128 size;
>> hwaddr addr;
>> void (*destructor)(MemoryRegion *mr);
>> diff --git a/softmmu/memory.c b/softmmu/memory.c
>> index e5826faa0c..17ca896c38 100644
>> --- a/softmmu/memory.c
>> +++ b/softmmu/memory.c
>> @@ -2524,8 +2524,13 @@ static void
>> memory_region_add_subregion_common(MemoryRegion *mr,
>> hwaddr offset,
>> MemoryRegion *subregion)
>> {
>> + MemoryRegion *alias;
>> +
>> assert(!subregion->container);
>> subregion->container = mr;
>> + for (alias = subregion->alias; alias; alias = alias->alias) {
>> + alias->mapped_via_alias++;
>> + }
>> subregion->addr = offset;
>> memory_region_update_container_subregions(subregion);
>> }
>> @@ -2550,9 +2555,14 @@ void memory_region_add_subregion_overlap(MemoryRegion
>> *mr,
>> void memory_region_del_subregion(MemoryRegion *mr,
>> MemoryRegion *subregion)
>> {
>> + MemoryRegion *alias;
>> +
>> memory_region_transaction_begin();
>> assert(subregion->container == mr);
>> subregion->container = NULL;
>> + for (alias = subregion->alias; alias; alias = alias->alias) {
>> + alias->mapped_via_alias--;
>
> assert(alias->mapped_via_alias >= 0);
Makes sense, I'll respin with that -- thanks!
--
Thanks,
David / dhildenb
