Re: [RFC PATCH] tests/qtest/stm32l4x5_usart: Avoid accessing NVIC via MMIO

[Fabiano Rosas <farosas@suse.de>]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> The STM32L4x5 SoC family use a ARM Cortex-M core. Such
> core is architecturally tied with a NVIC (interrupt controller),
> having the NVIC MMIO mapped in the core address space.
>
> When using the QTest accelerator, we don't emulate vCPU, only
> a dummy is created. For now, QTest is only supposed to access
> MMIO devices mapped on the main 'SysBus'. Thus it shouldn't
> be able to access a NVIC MMIO region, because such region is
> specific to a vCPU address space, which isn't available under
> QTest.
>
> In order to avoid NVIC MMIO accesses, rather than checking
> UART IRQs on the NVIC, intercept the UART IRQ and check for
> raised/lowered events.
>
> The sysbus USART1 IRQ output is wired to EXTI #26 input.
> Use qtest_irq_intercept_out_named() to intercept it, count
> the events with qtest_get_irq_lowered_counter() and
> qtest_get_irq_raised_counter().
>
> Remove the then unused check/clear_nvic_pending() methods.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Based-on: <20241216141818.111255-7-gustavo.romero@linaro.org>
> "tests/qtest: Add API functions to capture IRQ toggling"
>
> This patch is to fix the problem reported by Fabiano when
> removing the &first_cpu global in qtest, see analysis in:
> https://lore.kernel.org/qemu-devel/05820c9b-a683-4eb4-a836-e97aa708d5e5@linaro.org/
>
> Note, while writing that patch I noticed a problem with the
> b-l475e-iot01a machine. In bl475e_init() somes output GPIOs
> are wired twice. The EXTI outputs are passed to the SoC with
> qdev_pass_gpios(), and few are re-wired to the various output
> IRQ splitters. I'll open a GitLab issue so it can be cleared
> later.
> ---
>  tests/qtest/stm32l4x5_usart-test.c | 33 +++++++-----------------------

What about these other ones?
stm32l4x5_exti-test.c
stm32l4x5_rcc-test.c

>  1 file changed, 7 insertions(+), 26 deletions(-)
>
> diff --git a/tests/qtest/stm32l4x5_usart-test.c b/tests/qtest/stm32l4x5_usart-test.c
> index 927bab63614..35622e9434d 100644
> --- a/tests/qtest/stm32l4x5_usart-test.c
> +++ b/tests/qtest/stm32l4x5_usart-test.c
> @@ -46,26 +46,7 @@ REG32(ICR, 0x20)
>  REG32(RDR, 0x24)
>  REG32(TDR, 0x28)
>  
> -#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);
> -}
> -
> -static bool clear_nvic_pending(QTestState *qts, unsigned int n)
> -{
> -    /* No USART interrupts are less than 32 */
> -    assert(n > 32);
> -    n -= 32;
> -    qtest_writel(qts, NVIC_ICPR1, (1 << n));
> -    return true;
> -}
> +#define USART1_EXTI_IRQ 26
>  
>  /*
>   * Wait indefinitely for the flag to be updated.
> @@ -195,6 +176,8 @@ static void init_uart(QTestState *qts)
>      /* Enable the transmitter, the receiver and the USART. */
>      qtest_writel(qts, (USART1_BASE_ADDR + A_CR1),
>          cr1 | R_CR1_UE_MASK | R_CR1_RE_MASK | R_CR1_TE_MASK);
> +
> +    qtest_irq_intercept_out_named(qts, "machine/soc/exti", "sysbus-irq");
>  }
>  
>  static void test_write_read(void)
> @@ -221,7 +204,7 @@ static void test_receive_char(void)
>      g_assert_true(send(sock_fd, "a", 1, 0) == 1);
>      usart_wait_for_flag(qts, USART1_BASE_ADDR + A_ISR, R_ISR_RXNE_MASK);
>      g_assert_cmphex(qtest_readl(qts, USART1_BASE_ADDR + A_RDR), ==, 'a');
> -    g_assert_false(check_nvic_pending(qts, USART1_IRQ));
> +    g_assert_cmpuint(qtest_get_irq_lowered_counter(qts, USART1_EXTI_IRQ), ==, 0);
>  
>      /* Now with the IRQ */
>      cr1 = qtest_readl(qts, (USART1_BASE_ADDR + A_CR1));
> @@ -230,8 +213,7 @@ static void test_receive_char(void)
>      g_assert_true(send(sock_fd, "b", 1, 0) == 1);
>      usart_wait_for_flag(qts, USART1_BASE_ADDR + A_ISR, R_ISR_RXNE_MASK);
>      g_assert_cmphex(qtest_readl(qts, USART1_BASE_ADDR + A_RDR), ==, 'b');
> -    g_assert_true(check_nvic_pending(qts, USART1_IRQ));
> -    clear_nvic_pending(qts, USART1_IRQ);
> +    g_assert_cmpuint(qtest_get_irq_lowered_counter(qts, USART1_EXTI_IRQ), >, 0);
>  
>      close(sock_fd);
>  
> @@ -251,7 +233,7 @@ static void test_send_char(void)
>      qtest_writel(qts, USART1_BASE_ADDR + A_TDR, 'c');
>      g_assert_true(recv(sock_fd, s, 1, 0) == 1);
>      g_assert_cmphex(s[0], ==, 'c');
> -    g_assert_false(check_nvic_pending(qts, USART1_IRQ));
> +    g_assert_cmpuint(qtest_get_irq_lowered_counter(qts, USART1_EXTI_IRQ), ==, 0);
>  
>      /* Now with the IRQ */
>      cr1 = qtest_readl(qts, (USART1_BASE_ADDR + A_CR1));
> @@ -260,8 +242,7 @@ static void test_send_char(void)
>      qtest_writel(qts, USART1_BASE_ADDR + A_TDR, 'd');
>      g_assert_true(recv(sock_fd, s, 1, 0) == 1);
>      g_assert_cmphex(s[0], ==, 'd');
> -    g_assert_true(check_nvic_pending(qts, USART1_IRQ));
> -    clear_nvic_pending(qts, USART1_IRQ);
> +    g_assert_cmpuint(qtest_get_irq_raised_counter(qts, USART1_EXTI_IRQ), >, 0);
>  
>      close(sock_fd);