Re: [PATCH v7 0/6] memory: prevent dma-reentracy issues

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 13/3/23 15:52, Alexander Bulekov wrote:
> On 230313 1502, Thomas Huth wrote:
>> On 13/03/2023 09.24, Alexander Bulekov wrote:
>>> v6 -> v7:
>>>       - Fix bad qemu_bh_new_guarded calls found by Thomas (Patch 4)
>>>       - Add an MR-specific flag to disable reentrancy (Patch 5)
>>>       - Disable reentrancy checks for lsi53c895a's RAM-like MR (Patch 6)
>>>       Patches 5 and 6 need review. I left the review-tags for Patch 4,
>>>       however a few of the qemu_bh_new_guarded calls have changed.
>>
>>   Hi Alexander,
>>
>> there seems to be another issue with one of the avocado tests:
>>
>>   make -j8 qemu-system-aarch64
>>   make check-venv
>>   ./tests/venv/bin/avocado run \
>>     tests/avocado/boot_linux_console.py:BootLinuxConsole.test_aarch64_raspi3_atf
>>
>> ... works fine for me with the master branch, but it fails
>> for me after applying your patch series.
>> Can you reproduce that failure?
> 
> #0  __GI_exit (status=0x1) at ./stdlib/exit.c:143
> #1  0x0000555555f05819 in access_with_adjusted_size (addr=0x0, addr@entry=0x7ffff3b609d0, value=0x7ffff3b609d0, size=size@entry=0x4, access_size_min=0x1, access_size_max=0x4, access_fn=0x555555f0b4b0 <memory_region_read_accessor>, mr=0x7
> #2  0x0000555555f05380 in memory_region_dispatch_read1 (mr=0x7ffff3e34990, addr=0x1, pval=<optimized out>, size=0x4, attrs=...) at ../softmmu/memory.c:1442
> #3  memory_region_dispatch_read (mr=<optimized out>, mr@entry=0x7ffff3e34990, addr=0x1, pval=<optimized out>, pval@entry=0x7ffff3b609d0, op=<optimized out>, attrs=..., attrs@entry=...) at ../softmmu/memory.c:1476
> #4  0x0000555555f1278f in address_space_ldl_internal (as=<optimized out>, addr=<optimized out>, attrs=..., result=0x0, endian=DEVICE_LITTLE_ENDIAN) at ../memory_ldst.c.inc:41
> #5  0x00005555559ebb5d in ldl_le_phys (as=0x7ffff3e35258, addr=0x80) at /home/alxndr/Development/qemu-demo/qemu/include/exec/memory_ldst_phys.h.inc:79
> #6  bcm2835_mbox_update (s=0x7ffff3e34f20) at ../hw/misc/bcm2835_mbox.c:109
> #7  0x00005555559ecd5d in bcm2835_property_write (opaque=0x7ffff3e34600, offset=<optimized out>, value=<optimized out>, size=<optimized out>) at ../hw/misc/bcm2835_property.c:349
> #8  0x0000555555f05903 in memory_region_write_accessor (mr=0x7ffff3e34990, addr=0x0, value=<optimized out>, size=0x4, shift=<optimized out>, mask=<optimized out>, attrs=...) at ../softmmu/memory.c:493
> #9  0x0000555555f0576b in access_with_adjusted_size (addr=addr@entry=0x0, value=0x7ffff3b60c38, value@entry=0x7ffff3b60c28, size=size@entry=0x4, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x555555f05820 <
> attrs=...) at ../softmmu/memory.c:570
> #10 0x0000555555f055c6 in memory_region_dispatch_write (mr=<optimized out>, mr@entry=0x7ffff3e34990, addr=0x0, data=<optimized out>, data@entry=0x2f2228, op=<optimized out>, attrs=..., attrs@entry=...) at ../softmmu/memory.c:1532
> #11 0x0000555555f132ec in address_space_stl_internal (as=<optimized out>, addr=<optimized out>, val=0x2f2228, attrs=..., result=0x0, endian=DEVICE_LITTLE_ENDIAN) at ../memory_ldst.c.inc:319
> #12 0x00005555559eb9a4 in stl_le_phys (as=<optimized out>, addr=0x80, val=0x2f2228) at /home/alxndr/Development/qemu-demo/qemu/include/exec/memory_ldst_phys.h.inc:121
> #13 bcm2835_mbox_write (opaque=0x7ffff3e34f20, offset=<optimized out>, value=0x2f2228, size=<optimized out>) at ../hw/misc/bcm2835_mbox.c:227
> #14 0x0000555555f05903 in memory_region_write_accessor (mr=0x7ffff3e352b0, addr=0xa0, value=<optimized out>, size=0x4, shift=<optimized out>, mask=<optimized out>, attrs=...) at ../softmmu/memory.c:493
> #15 0x0000555555f0576b in access_with_adjusted_size (addr=addr@entry=0xa0, value=0x7ffff3b60e48, value@entry=0x7ffff3b60e38, size=size@entry=0x4, access_size_min=<optimized out>, access_size_max=<optimized out>, access_fn=0x555555f05820
>   attrs=...) at ../softmmu/memory.c:570
> #16 0x0000555555f055c6 in memory_region_dispatch_write (mr=<optimized out>, mr@entry=0x2, addr=addr@entry=0xa0, data=<optimized out>, data@entry=0x2f2228, op=<optimized out>, op@entry=MO_32, attrs=...) at ../softmmu/memory.c:1532
> #17 0x0000555555f9b3ae in io_writex (env=0x7ffff3dd60e0, full=0x55555790c710, mmu_idx=0x7, val=0x4, val@entry=0x2f2228, addr=0x3f00b8a0, retaddr=retaddr@entry=0x7fffac01f9dd, op=MO_32) at ../accel/tcg/cputlb.c:1430
> #18 0x0000555555f90062 in store_helper (env=<optimized out>, addr=<optimized out>, val=0x2f2228, oi=<optimized out>, retaddr=0x7ffff3b609d0, op=MO_32) at ../accel/tcg/cputlb.c:2454
> #19 full_le_stl_mmu (env=<optimized out>, addr=<optimized out>, val=0x2f2228, oi=<optimized out>, retaddr=0x7ffff3b609d0) at ../accel/tcg/cputlb.c:2542
> #20 0x00007fffac01f9dd in code_gen_buffer ()
> #21 0x0000555555f7367e in cpu_tb_exec (cpu=cpu@entry=0x7ffff3dd4210, itb=itb@entry=0x7fffac01f8c0 <code_gen_buffer+129171>, tb_exit=tb_exit@entry=0x7ffff3b6148c) at ../accel/tcg/cpu-exec.c:460
> #22 0x0000555555f744f9 in cpu_loop_exec_tb (cpu=0x7ffff3dd4210, tb=<optimized out>, pc=<optimized out>, tb_exit=0x7ffff3b6148c, last_tb=<optimized out>) at ../accel/tcg/cpu-exec.c:894
> #23 cpu_exec_loop (cpu=cpu@entry=0x7ffff3dd4210, sc=sc@entry=0x7ffff3b61510) at ../accel/tcg/cpu-exec.c:1005
> #24 0x0000555555f73c27 in cpu_exec_setjmp (cpu=cpu@entry=0x7ffff3dd4210, sc=sc@entry=0x7ffff3b61510) at ../accel/tcg/cpu-exec.c:1037
> #25 0x0000555555f73aee in cpu_exec (cpu=cpu@entry=0x7ffff3dd4210) at ../accel/tcg/cpu-exec.c:1063
> #26 0x0000555555f9da4f in tcg_cpus_exec (cpu=cpu@entry=0x7ffff3dd4210) at ../accel/tcg/tcg-accel-ops.c:81
> #27 0x0000555555f9e019 in mttcg_cpu_thread_fn (arg=arg@entry=0x7ffff3dd4210) at ../accel/tcg/tcg-accel-ops-mttcg.c:95
> #28 0x000055555611d0c5 in qemu_thread_start (args=0x555557923bf0) at ../util/qemu-thread-posix.c:541
> #29 0x00007ffff6960fd4 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
> #30 0x00007ffff69e166c in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
> 
> Some sort of relationship between bcm2835_property and
> bcm2835_mbox
> 
> bcm2835_property calls directly into bcm2835_mbox which reads from
> bcm2835_property..

These models emulate (synchronously within vCPU) a vDSP firmware:
another core accessing the address space asynchronously.

> Guess bcm2835_property s->iomem needs to be marked as reentrancy-safe
> as-well.

Right.

Now I wonder again if this is a good time to merge this change set.
Hopefully users will test the RC before the final release.