Hi David,
While I realize my response is quite late, I wanted to report this error I
found when running the acceptance
tests for the orangepi-pc machine using avocado:
ARMBIAN_ARTIFACTS_CACHED=yes AVOCADO_ALLOW_LARGE_STORAGE=yes avocado
--show=app,console run -t machine:orangepi-pc
tests/avocado/boot_linux_console.py
...
(4/5)
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08:
-console: U-Boot SPL 2020.04-armbian (Sep 02 2020 - 10:16:13 +0200)
\console: DRAM:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'4-tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08',
'logdir': '/home/fox/avocado/job-results/job-2022-01-30T23.09-af49e...
(90.64 s)
(5/5)
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9:
/console: U-Boot SPL 2020.01+dfsg-1 (Jan 08 2020 - 08:19:44 +0000)
console: DRAM:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'5-tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9',
'logdir': '/home/fox/avocado/job-results/job-2022-01-30T23.09-af49...
(90.64 s)
RESULTS : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 2 |
CANCEL 0
JOB TIME : 221.25 s
Basically the two tests freeze during the part where the U-Boot bootloader
needs to detect the amount of memory. We model this in the
hw/misc/allwinner-h3-dramc.c file.
And when running the machine manually it shows an assert on
'alias->mapped_via_alias >= 0'. When running manually via gdb, I was able
to collect this backtrace:
$ gdb ./build/qemu-system-arm
...
gdb) run -M orangepi-pc -nographic
./Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img
...
U-Boot SPL 2020.04-armbian (Sep 02 2020 - 10:16:13 +0200)
DRAM:
qemu-system-arm: ../softmmu/memory.c:2588: memory_region_del_subregion:
Assertion `alias->mapped_via_alias >= 0' failed.
Thread 4 "qemu-system-arm" received signal SIGABRT, Aborted.
[Switching to Thread 0x7ffff5f72700 (LWP 32866)]
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007ffff7535859 in __GI_abort () at abort.c:79
#2 0x00007ffff7535729 in __assert_fail_base
(fmt=0x7ffff76cb588 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=0x55555642fd65 "alias->mapped_via_alias >= 0",
file=0x55555642f8cd "../softmmu/memory.c", line=2588, function=<optimized
out>)
at assert.c:92
#3 0x00007ffff7546f36 in __GI___assert_fail
(assertion=0x55555642fd65 "alias->mapped_via_alias >= 0",
file=0x55555642f8cd "../softmmu/memory.c", line=2588,
function=0x555556430690 <__PRETTY_FUNCTION__.8>
"memory_region_del_subregion") at assert.c:101
#4 0x0000555555e587e0 in memory_region_del_subregion (mr=0x555556f0bc00,
subregion=0x7ffff5fa1090) at ../softmmu/memory.c:2588
#5 0x0000555555e589f3 in memory_region_readd_subregion (mr=0x7ffff5fa1090)
at ../softmmu/memory.c:2630
#6 0x0000555555e58a5f in memory_region_set_address (mr=0x7ffff5fa1090,
addr=1090519040) at ../softmmu/memory.c:2642
#7 0x0000555555ac352b in allwinner_h3_dramc_map_rows (s=0x7ffff5fa0c50,
row_bits=16 '\020', bank_bits=2 '\002', page_size=512) at
../hw/misc/allwinner-h3-dramc.c:92
#8 0x0000555555ac36c2 in allwinner_h3_dramcom_write
(opaque=0x7ffff5fa0c50, offset=0, val=4396785, size=4) at
../hw/misc/allwinner-h3-dramc.c:131
#9 0x0000555555e52561 in memory_region_write_accessor (mr=0x7ffff5fa11a0,
addr=0, value=0x7ffff5f710e8, size=4, shift=0, mask=4294967295, attrs=...)
at ../softmmu/memory.c:492
#10 0x0000555555e527ad in access_with_adjusted_size (addr=0,
value=0x7ffff5f710e8, size=4, access_size_min=4, access_size_max=4,
access_fn=
0x555555e52467 <memory_region_write_accessor>, mr=0x7ffff5fa11a0,
attrs=...) at ../softmmu/memory.c:554
#11 0x0000555555e55935 in memory_region_dispatch_write (mr=0x7ffff5fa11a0,
addr=0, data=4396785, op=MO_32, attrs=...) at ../softmmu/memory.c:1514
#12 0x0000555555f891ae in io_writex (env=0x7ffff5f7ce30,
iotlbentry=0x7fffec0275f0, mmu_idx=7, val=4396785, addr=29761536,
retaddr=140734938367479, op=MO_32) at ../accel/tcg/cputlb.c:1420
#13 0x0000555555f8ba10 in store_helper (env=0x7ffff5f7ce30, addr=29761536,
val=4396785, oi=3623, retaddr=140734938367479, op=MO_32) at
../accel/tcg/cputlb.c:2355
#14 0x0000555555f8bdda in full_le_stl_mmu (env=0x7ffff5f7ce30,
addr=29761536, val=4396785, oi=3623, retaddr=140734938367479) at
../accel/tcg/cputlb.c:2443
#15 0x0000555555f8be16 in helper_le_stl_mmu (env=0x7ffff5f7ce30,
addr=29761536, val=4396785, oi=3623, retaddr=140734938367479) at
../accel/tcg/cputlb.c:2449
#16 0x00007fff680245f7 in code_gen_buffer ()
#17 0x0000555555f754cb in cpu_tb_exec (cpu=0x7ffff5f730a0,
itb=0x7fffa802b400, tb_exit=0x7ffff5f7182c) at ../accel/tcg/cpu-exec.c:357
#18 0x0000555555f76366 in cpu_loop_exec_tb (cpu=0x7ffff5f730a0,
tb=0x7fffa802b400, last_tb=0x7ffff5f71840, tb_exit=0x7ffff5f7182c) at
../accel/tcg/cpu-exec.c:842
#19 0x0000555555f76720 in cpu_exec (cpu=0x7ffff5f730a0) at
../accel/tcg/cpu-exec.c:1001
#20 0x0000555555f993dd in tcg_cpus_exec (cpu=0x7ffff5f730a0) at
../accel/tcg/tcg-accel-ops.c:67
#21 0x0000555555f9976d in mttcg_cpu_thread_fn (arg=0x7ffff5f730a0) at
../accel/tcg/tcg-accel-ops-mttcg.c:95
#22 0x000055555624bf4d in qemu_thread_start (args=0x5555572b6780) at
../util/qemu-thread-posix.c:556
#23 0x00007ffff770b609 in start_thread (arg=<optimized out>) at
pthread_create.c:477
#24 0x00007ffff7632293 in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:95
So it seems that the hw/misc/allwinner-h3-dramc.c file is using the call
memory_region_set_address, where internally we are calling
memory_region_del_subregion.
The allwinner-h3-dramc.c file does use memory_region_add_subregion_overlap
once in the realize function, but might use the memory_region_set_address
multiple times.
It looks to me this is the path where the assert comes in. If I revert this
patch on current master, the machine boots without the assertion.
Would you be able to help out how we can best resolve this? Ofcourse, if
there is anything needed to be changed on the allwinner-h3-dramc.c file, I
would be happy to prepare a patch for that.
Kind regards,
Niek Linnenbank
On Tue, Nov 2, 2021 at 5:46 PM David Hildenbrand <da...@redhat.com> 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.
>
> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
> Reviewed-by: Peter Xu <pet...@redhat.com>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
> ---
> include/exec/memory.h | 1 +
> softmmu/memory.c | 13 ++++++++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 20f1b27377..fea1a493b9 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -738,6 +738,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 7340e19ff5..b52b6a2f66 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++;
> + }
> subregion->addr = offset;
> memory_region_update_container_subregions(subregion);
> }
> @@ -2561,9 +2566,15 @@ 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);
> + }
> QTAILQ_REMOVE(&mr->subregions, subregion, subregions_link);
> memory_region_unref(subregion);
> memory_region_update_pending |= mr->enabled && subregion->enabled;
> @@ -2660,7 +2671,7 @@ static FlatRange *flatview_lookup(FlatView *view,
> AddrRange addr)
>
> bool memory_region_is_mapped(MemoryRegion *mr)
> {
> - return mr->container ? true : false;
> + return !!mr->container || mr->mapped_via_alias;
> }
>
> /* Same as memory_region_find, but it does not add a reference to the
> --
> 2.31.1
>
>
>
--
Niek Linnenbank
