Re: [PATCH] memory: Make memory_region_readd_subregion() properly handle mapped aliases

[Niek Linnenbank]

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