* [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases
@ 2022-02-01 10:09 David Hildenbrand
2022-02-01 10:51 ` Paolo Bonzini
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: David Hildenbrand @ 2022-02-01 10:09 UTC (permalink / raw)
To: qemu-devel
Cc: Paolo Bonzini, Niek Linnenbank, Philippe Mathieu-Daudé,
Peter Xu, David Hildenbrand
memory_region_readd_subregion() wants to readd a region by first
removing it and then readding it. For readding, it doesn't use one of
the memory_region_add_*() variants, which is why fail to re-increment the
mr->mapped_via_alias counters, resulting in the
assert(alias->mapped_via_alias >= 0) in memory_region_del_subregion()
triggering the next time we call memory_region_readd_subregion().
Fix it by using memory_region_add_subregion_common() for readding the
region.
Reported-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Fixes: 5ead62185d23 ("memory: Make memory_region_is_mapped() succeed when mapped via an alias")
Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
softmmu/memory.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/softmmu/memory.c b/softmmu/memory.c
index 678dc62f06..76bb1677fe 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -2626,8 +2626,7 @@ static void memory_region_readd_subregion(MemoryRegion *mr)
memory_region_transaction_begin();
memory_region_ref(mr);
memory_region_del_subregion(container, mr);
- mr->container = container;
- memory_region_update_container_subregions(mr);
+ memory_region_add_subregion_common(container, mr->addr, mr);
memory_region_unref(mr);
memory_region_transaction_commit();
}
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases
2022-02-01 10:09 [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases David Hildenbrand
@ 2022-02-01 10:51 ` Paolo Bonzini
2022-02-28 20:03 ` Niek Linnenbank
2022-02-01 10:53 ` Philippe Mathieu-Daudé via
2022-03-28 22:47 ` Philippe Mathieu-Daudé
2 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2022-02-01 10:51 UTC (permalink / raw)
To: David Hildenbrand, qemu-devel
Cc: Niek Linnenbank, Philippe Mathieu-Daudé, Peter Xu
On 2/1/22 11:09, David Hildenbrand wrote:
> memory_region_readd_subregion() wants to readd a region by first
> removing it and then readding it. For readding, it doesn't use one of
> the memory_region_add_*() variants, which is why fail to re-increment the
> mr->mapped_via_alias counters, resulting in the
> assert(alias->mapped_via_alias >= 0) in memory_region_del_subregion()
> triggering the next time we call memory_region_readd_subregion().
>
> Fix it by using memory_region_add_subregion_common() for readding the
> region.
>
> Reported-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> Fixes: 5ead62185d23 ("memory: Make memory_region_is_mapped() succeed when mapped via an alias")
> Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> softmmu/memory.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 678dc62f06..76bb1677fe 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -2626,8 +2626,7 @@ static void memory_region_readd_subregion(MemoryRegion *mr)
> memory_region_transaction_begin();
> memory_region_ref(mr);
> memory_region_del_subregion(container, mr);
> - mr->container = container;
> - memory_region_update_container_subregions(mr);
> + memory_region_add_subregion_common(container, mr->addr, mr);
> memory_region_unref(mr);
> memory_region_transaction_commit();
> }
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases
2022-02-01 10:09 [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases David Hildenbrand
2022-02-01 10:51 ` Paolo Bonzini
@ 2022-02-01 10:53 ` Philippe Mathieu-Daudé via
2022-03-28 22:47 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé via @ 2022-02-01 10:53 UTC (permalink / raw)
To: David Hildenbrand, qemu-devel; +Cc: Niek Linnenbank, Paolo Bonzini, Peter Xu
On 2/1/22 11:09, David Hildenbrand wrote:
> memory_region_readd_subregion() wants to readd a region by first
> removing it and then readding it. For readding, it doesn't use one of
> the memory_region_add_*() variants, which is why fail to re-increment the
> mr->mapped_via_alias counters, resulting in the
> assert(alias->mapped_via_alias >= 0) in memory_region_del_subregion()
> triggering the next time we call memory_region_readd_subregion().
>
> Fix it by using memory_region_add_subregion_common() for readding the
> region.
>
> Reported-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> Fixes: 5ead62185d23 ("memory: Make memory_region_is_mapped() succeed when mapped via an alias")
> Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> softmmu/memory.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 678dc62f06..76bb1677fe 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -2626,8 +2626,7 @@ static void memory_region_readd_subregion(MemoryRegion *mr)
> memory_region_transaction_begin();
> memory_region_ref(mr);
> memory_region_del_subregion(container, mr);
> - mr->container = container;
> - memory_region_update_container_subregions(mr);
> + memory_region_add_subregion_common(container, mr->addr, mr);
> memory_region_unref(mr);
> memory_region_transaction_commit();
> }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases
2022-02-01 10:51 ` Paolo Bonzini
@ 2022-02-28 20:03 ` Niek Linnenbank
2022-03-21 21:59 ` Niek Linnenbank
0 siblings, 1 reply; 6+ messages in thread
From: Niek Linnenbank @ 2022-02-28 20:03 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Philippe Mathieu-Daudé,
QEMU Developers, Peter Xu, David Hildenbrand
[-- Attachment #1: Type: text/plain, Size: 2039 bytes --]
Hi Paolo,
Thanks for queing this patch. I did not yet see it appear in master. Do you
know when we can expect to see it?
Regards,
Niek
On Tue, Feb 1, 2022 at 11:51 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 2/1/22 11:09, David Hildenbrand wrote:
> > memory_region_readd_subregion() wants to readd a region by first
> > removing it and then readding it. For readding, it doesn't use one of
> > the memory_region_add_*() variants, which is why fail to re-increment the
> > mr->mapped_via_alias counters, resulting in the
> > assert(alias->mapped_via_alias >= 0) in memory_region_del_subregion()
> > triggering the next time we call memory_region_readd_subregion().
> >
> > Fix it by using memory_region_add_subregion_common() for readding the
> > region.
> >
> > Reported-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> > Fixes: 5ead62185d23 ("memory: Make memory_region_is_mapped() succeed
> when mapped via an alias")
> > Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Peter Xu <peterx@redhat.com>
> > Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> > Signed-off-by: David Hildenbrand <david@redhat.com>
> > ---
> > softmmu/memory.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/softmmu/memory.c b/softmmu/memory.c
> > index 678dc62f06..76bb1677fe 100644
> > --- a/softmmu/memory.c
> > +++ b/softmmu/memory.c
> > @@ -2626,8 +2626,7 @@ static void
> memory_region_readd_subregion(MemoryRegion *mr)
> > memory_region_transaction_begin();
> > memory_region_ref(mr);
> > memory_region_del_subregion(container, mr);
> > - mr->container = container;
> > - memory_region_update_container_subregions(mr);
> > + memory_region_add_subregion_common(container, mr->addr, mr);
> > memory_region_unref(mr);
> > memory_region_transaction_commit();
> > }
>
> Queued, thanks.
>
> Paolo
>
--
Niek Linnenbank
[-- Attachment #2: Type: text/html, Size: 3131 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases
2022-02-28 20:03 ` Niek Linnenbank
@ 2022-03-21 21:59 ` Niek Linnenbank
0 siblings, 0 replies; 6+ messages in thread
From: Niek Linnenbank @ 2022-03-21 21:59 UTC (permalink / raw)
To: Paolo Bonzini, Peter Maydell
Cc: Philippe Mathieu-Daudé,
QEMU Developers, Peter Xu, David Hildenbrand
[-- Attachment #1: Type: text/plain, Size: 2803 bytes --]
Hi Paolo, Peter,
With the 7.0 release approaching, just a friendly reminder that the patch
in this thread is not yet in master
(2058fdbe81e2985c226a026851dd26b146d3395c).
It's currently preventing the orangepi-pc board to boot SD card images from
U-Boot, terminating with an assert:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg865382.html
If it helps, I did a full build with David's patch and re-ran all the
boot_linux_console.py tests and all passed OK.
Would it be possible to get this in before the upcoming release?
Thanks in advance,
Niek
On Mon, Feb 28, 2022 at 9:03 PM Niek Linnenbank <nieklinnenbank@gmail.com>
wrote:
> Hi Paolo,
>
> Thanks for queing this patch. I did not yet see it appear in master. Do
> you know when we can expect to see it?
>
> Regards,
> Niek
>
> On Tue, Feb 1, 2022 at 11:51 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>> On 2/1/22 11:09, David Hildenbrand wrote:
>> > memory_region_readd_subregion() wants to readd a region by first
>> > removing it and then readding it. For readding, it doesn't use one of
>> > the memory_region_add_*() variants, which is why fail to re-increment
>> the
>> > mr->mapped_via_alias counters, resulting in the
>> > assert(alias->mapped_via_alias >= 0) in memory_region_del_subregion()
>> > triggering the next time we call memory_region_readd_subregion().
>> >
>> > Fix it by using memory_region_add_subregion_common() for readding the
>> > region.
>> >
>> > Reported-by: Niek Linnenbank <nieklinnenbank@gmail.com>
>> > Fixes: 5ead62185d23 ("memory: Make memory_region_is_mapped() succeed
>> when mapped via an alias")
>> > Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
>> > Cc: Paolo Bonzini <pbonzini@redhat.com>
>> > Cc: Peter Xu <peterx@redhat.com>
>> > Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
>> > Signed-off-by: David Hildenbrand <david@redhat.com>
>> > ---
>> > softmmu/memory.c | 3 +--
>> > 1 file changed, 1 insertion(+), 2 deletions(-)
>> >
>> > diff --git a/softmmu/memory.c b/softmmu/memory.c
>> > index 678dc62f06..76bb1677fe 100644
>> > --- a/softmmu/memory.c
>> > +++ b/softmmu/memory.c
>> > @@ -2626,8 +2626,7 @@ static void
>> memory_region_readd_subregion(MemoryRegion *mr)
>> > memory_region_transaction_begin();
>> > memory_region_ref(mr);
>> > memory_region_del_subregion(container, mr);
>> > - mr->container = container;
>> > - memory_region_update_container_subregions(mr);
>> > + memory_region_add_subregion_common(container, mr->addr, mr);
>> > memory_region_unref(mr);
>> > memory_region_transaction_commit();
>> > }
>>
>> Queued, thanks.
>>
>> Paolo
>>
>
>
> --
> Niek Linnenbank
>
>
--
Niek Linnenbank
[-- Attachment #2: Type: text/html, Size: 4403 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases
2022-02-01 10:09 [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases David Hildenbrand
2022-02-01 10:51 ` Paolo Bonzini
2022-02-01 10:53 ` Philippe Mathieu-Daudé via
@ 2022-03-28 22:47 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-03-28 22:47 UTC (permalink / raw)
To: David Hildenbrand, qemu-devel
Cc: Paolo Bonzini, Niek Linnenbank, Philippe Mathieu-Daudé, Peter Xu
On 1/2/22 11:09, David Hildenbrand wrote:
> memory_region_readd_subregion() wants to readd a region by first
> removing it and then readding it. For readding, it doesn't use one of
> the memory_region_add_*() variants, which is why fail to re-increment the
> mr->mapped_via_alias counters, resulting in the
> assert(alias->mapped_via_alias >= 0) in memory_region_del_subregion()
> triggering the next time we call memory_region_readd_subregion().
>
> Fix it by using memory_region_add_subregion_common() for readding the
> region.
>
> Reported-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> Fixes: 5ead62185d23 ("memory: Make memory_region_is_mapped() succeed when mapped via an alias")
> Tested-by: Niek Linnenbank <nieklinnenbank@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> softmmu/memory.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Thanks, queued to mips-fixes.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-28 22:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-01 10:09 [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases David Hildenbrand
2022-02-01 10:51 ` Paolo Bonzini
2022-02-28 20:03 ` Niek Linnenbank
2022-03-21 21:59 ` Niek Linnenbank
2022-02-01 10:53 ` Philippe Mathieu-Daudé via
2022-03-28 22:47 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.