qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 2/2] memory: Update description of memory_region_is_mapped


From: David Hildenbrand
Subject: Re: [PATCH v1 2/2] memory: Update description of memory_region_is_mapped()
Date: Tue, 12 Oct 2021 12:09:31 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0


The less confusing would be one where check works for any memory region
involved.

Exactly, so for any alias, even in-between another alias and the target.


       I am not aware of actual issues, this is rather a cleanup.
Signed-off-by: David Hildenbrand <david@redhat.com>

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 75b4f600e3..93d0190202 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -728,6 +728,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 3bcfc3899b..1168a00819 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -2535,8 +2535,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++;

it it necessary to update mapped_via_alias for intermediate aliases?
Why not just update on counter only on leaf (aliased region)?

Assume we have alias0 -> alias1 -> region and map alias0.

Once alias0 is mapped it will have ->container set and
memory_region_is_mapped(alias0) will return "true".

With my patch, both, "alias1" and the region will be marked
"mapped_via_alias" and memory_region_is_mapped() will succeed on both of
them. With what you propose, memory_region_is_mapped() would only
succeed on the region (well, and on alias 0) but not on alias1.

as long as add_subregion increments counter on leaf it doesn't matter
how many intermediate aliases are there. Check on every one of them
should end up at the leaf counter (at expense of traversing
chain on every check but less state to track/think about).


Sure, we could also let memory_region_is_mapped() walk all aliases to the leaf. Not sure though, if it really simplifies things. It merely adds another loop and doesn't get rid of the others :) But I don't particularly care.

--
Thanks,

David / dhildenb




reply via email to

[Prev in Thread] Current Thread [Next in Thread]