Re: [PATCH v2 2/3] memory: Make memory_region_is_mapped() succeed when mapped via an alias

[David Hildenbrand]

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 <david@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