Re: [PATCH v2 0/2] qtest: Remove uses of 'first_cpu'

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

On 4/1/25 00:48, Philippe Mathieu-Daudé wrote:
> On 3/1/25 13:26, Fabiano Rosas wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>
>>> Hi Fabiano,
>>>
>>> On 12/12/24 00:37, Philippe Mathieu-Daudé wrote:
>>>> Replace first_cpu->as by address_space_memory.
>>>>
>>>> Philippe Mathieu-Daudé (2):
>>>>     system/qtest: Remove uses of 'first_cpu'
>>>>     qtest/fuzz: Remove uses of 'first_cpu'
> 
>>> Ping :)
>>
>> Hi!
>>
>> Good that you pinged, I thought you were looking into the test failures
>> from v1. I copied you in this other thread that mentioned them as well:
>>
>> https://lore.kernel.org/r/87y10jctbd.fsf@suse.de
>>
>> Applying this series on top of master just now:
>>
>> Summary of Failures:
>>
>>   10/519 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_usart- 
>> test               ERROR            1.40s   exit status 1
>> 165/519 qemu:qtest+qtest-arm / qtest-arm/sse-timer- 
>> test                     ERROR            0.40s   killed by signal 6 
>> SIGABRT
>> 185/519 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_exti- 
>> test                ERROR            0.19s   exit status 1
>> 187/519 qemu:qtest+qtest-arm / qtest-arm/stm32l4x5_rcc- 
>> test                 ERROR            0.19s   exit status 1
>> 515/519 qemu:qtest+qtest-arm / qtest-arm/microbit- 
>> test                      TIMEOUT         60.01s   killed by signal 15 
>> SIGTERM
> 
> Hmm interesting, I have a "quick-before-posting-series" set
> of tests, and a "full-before-posting-pullreq" one. The quick
> is quite extensive, but only include Aarch64. I thought, since
> it contains the ARM targets, they would also be tested, but
> no...
> 
> All these tests use ARM Cortex-M cores, which have a particularity,
> the ARM cores are tied to architectural specific HW (NVIC). We
> model cores in target/arm/cpu, and HW part in hw/intc/armv7m_nvic.c.
> 
> NVIC is only 'visible' from vCPU address space, not the "main sysbus".
> 
> See this flatview diff of tcg/qtest:
> 
> @@ -1,11 +1,12 @@
>   FlatView #1
> - AS "cpu-memory-0", root: armv7m-container
> + AS "memory", root: system
> - Root memory region: armv7m-container
> + Root memory region: system
>     0000000000000000-00000000000fffff (prio 0, rom): flash
>     0000000008000000-00000000080fffff (prio 0, rom): flash
>     0000000010000000-0000000010007fff (prio 0, ram): SRAM2
>     0000000020000000-0000000020017fff (prio 0, ram): SRAM1
> -  0000000022000000-0000000023ffffff (prio 0, i/o): bitband
>     0000000040000000-00000000400003ff (prio -1000, i/o): TIM2
>     0000000040000400-00000000400007ff (prio -1000, i/o): TIM3
>     0000000040000800-0000000040000bff (prio -1000, i/o): TIM4
> @@ -52,7 +53,6 @@
>     0000000040022000-00000000400223ff (prio -1000, i/o): FLASH
>     0000000040023000-00000000400233ff (prio -1000, i/o): CRC
>     0000000040024000-00000000400243ff (prio -1000, i/o): TSC
> -  0000000042000000-0000000043ffffff (prio 0, i/o): bitband
>     0000000048000000-00000000480003ff (prio 0, i/o): stm32l4x5-gpio
>     0000000048000400-00000000480007ff (prio 0, i/o): stm32l4x5-gpio
>     0000000048000800-0000000048000bff (prio 0, i/o): stm32l4x5-gpio
> @@ -66,9 +66,4 @@
>     0000000050060800-0000000050060bff (prio -1000, i/o): RNG
>     00000000a0000000-00000000a0000fff (prio -1000, i/o): FMC
>     00000000a0001000-00000000a00013ff (prio -1000, i/o): QUADSPI
> -  00000000e0000000-00000000e000dfff (prio -1, i/o): nvic-default
> -  00000000e000e000-00000000e000e00f (prio 0, i/o): nvic_sysregs
> -  00000000e000e010-00000000e000e0ef (prio 1, i/o): v7m_systick
> -  00000000e000e0f0-00000000e000efff (prio 0, i/o): nvic_sysregs 
> @00000000000000f0
> -  00000000e000f000-00000000e00fffff (prio -1, i/o): nvic-default 
> @000000000000f000
> 
> So under qtest with no vcpu, the nvic is not accessible without
> specifying a non-global address space.
> 
> And qtests access NVIC, see this function:
> 
>      #define NVIC_ISPR1 0XE000E204
>      #define NVIC_ICPR1 0xE000E284
>      #define USART1_IRQ 37
> 
>      static bool check_nvic_pending(QTestState *qts, unsigned int n)
>      {
>          /* No USART interrupts are less than 32 */
>          assert(n > 32);
>          n -= 32;
>          return qtest_readl(qts, NVIC_ISPR1) & (1 << n);
>      }
> 
> I tend to think the current situation works by luck, and this series
> is yet another example of sysbus abuses.
> 
> I'll give it some thoughts. Maybe we can discuss it at the next
> community call.

Tentative fix after today's community call:
https://lore.kernel.org/qemu-devel/20250107192637.67683-1-philmd@linaro.org/