Re: [PATCH v3 6/7] tests/qtest: Add API functions to capture IRQ toggling

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

Cc'ing maintainers:

$ ./scripts/get_maintainer.pl -f tests/qtest/libqtest.c
Fabiano Rosas <farosas@suse.de> (maintainer:qtest)
Laurent Vivier <lvivier@redhat.com> (maintainer:qtest)
Paolo Bonzini <pbonzini@redhat.com> (reviewer:qtest)

On 16/12/24 15:18, Gustavo Romero wrote:
> Currently, the QTest API does not provide a function to capture when an
> IRQ line is raised or lowered, although the QTest Protocol already
> reports such IRQ transitions. As a consequence, it is also not possible
> to capture when an IRQ line is toggled. Functions like qtest_get_irq()
> only read the current state of the intercepted IRQ lines, which is
> already high (or low) when the function is called if the IRQ line is
> toggled. Therefore, these functions miss the IRQ line state transitions.
> 
> This commit introduces two new API functions:
> qtest_get_irq_raised_counter() and qtest_get_irq_lowered_counter().
> These functions allow capturing the number of times an observed IRQ line
> transitioned from low to high state or from high to low state,
> respectively.
> 
> When used together, these new API functions then allow checking if one
> or more pulses were generated (indicating if the IRQ line was toggled).
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Acked-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/qtest/libqtest.c | 24 ++++++++++++++++++++++++
>   tests/qtest/libqtest.h | 28 ++++++++++++++++++++++++++++
>   2 files changed, 52 insertions(+)
> 
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 8de5f1fde3..dfe3139a88 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -83,6 +83,8 @@ struct QTestState
>       int expected_status;
>       bool big_endian;
>       bool irq_level[MAX_IRQ];
> +    uint64_t irq_raised_counter[MAX_IRQ];
> +    uint64_t irq_lowered_counter[MAX_IRQ];
>       GString *rx;
>       QTestTransportOps ops;
>       GList *pending_events;
> @@ -515,6 +517,8 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
>       s->rx = g_string_new("");
>       for (i = 0; i < MAX_IRQ; i++) {
>           s->irq_level[i] = false;
> +        s->irq_raised_counter[i] = 0;
> +        s->irq_lowered_counter[i] = 0;
>       }
>   
>       /*
> @@ -706,8 +710,10 @@ redo:
>           g_assert_cmpint(irq, <, MAX_IRQ);
>   
>           if (strcmp(words[1], "raise") == 0) {
> +            s->irq_raised_counter[irq]++;
>               s->irq_level[irq] = true;
>           } else {
> +            s->irq_lowered_counter[irq]++;
>               s->irq_level[irq] = false;
>           }
>   
> @@ -999,6 +1005,22 @@ bool qtest_get_irq(QTestState *s, int num)
>       return s->irq_level[num];
>   }
>   
> +uint64_t qtest_get_irq_raised_counter(QTestState *s, int num)
> +{
> +    /* dummy operation in order to make sure irq is up to date */
> +    qtest_inb(s, 0);

Isn't it better to simply call:

        qtest_rsp(s);

?

> +
> +    return s->irq_raised_counter[num];
> +}
> +
> +uint64_t qtest_get_irq_lowered_counter(QTestState *s, int num)
> +{
> +    /* dummy operation in order to make sure irq is up to date */
> +    qtest_inb(s, 0);

Ditto.

> +
> +    return s->irq_lowered_counter[num];
> +}
> +
>   void qtest_module_load(QTestState *s, const char *prefix, const char *libname)
>   {
>       qtest_sendf(s, "module_load %s %s\n", prefix, libname);
> @@ -1902,6 +1924,8 @@ QTestState *qtest_inproc_init(QTestState **s, bool log, const char* arch,
>       qts->wstatus = 0;
>       for (int i = 0; i < MAX_IRQ; i++) {
>           qts->irq_level[i] = false;
> +        qts->irq_raised_counter[i] = 0;
> +        qts->irq_lowered_counter[i] = 0;
>       }
>   
>       qtest_client_set_rx_handler(qts, qtest_client_inproc_recv_line);
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index f23d80e9e5..b73c04139e 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -389,6 +389,34 @@ void qtest_module_load(QTestState *s, const char *prefix, const char *libname);
>    */
>   bool qtest_get_irq(QTestState *s, int num);
>   
> +/**
> + * qtest_get_irq_raised_counter:
> + * @s: #QTestState instance to operate on.
> + * @num: Interrupt to observe.
> + *
> + * This function can be used in conjunction with the
> + * qtest_get_irq_lowered_counter() to check if one or more pulses where
> + * generated on the observed interrupt.

Missing to mention a device must be previously intercepted with
qtest_irq_intercept_*().

> + *
> + * Returns: The number of times IRQ @num was raised, i.e., transitioned from
> + * a low state (false) to a high state (true).
> + */
> +uint64_t qtest_get_irq_raised_counter(QTestState *s, int num);
> +
> +/**
> + * qtest_get_irq_lowered_counter:
> + * @s: #QTestState instance to operate on.
> + * @num: Interrupt to observe.
> + *
> + * This function can be used in conjunction with the
> + * qtest_get_irq_raised_counter() to check if one or more pulses where
> + * generated on the observed interrupt.

Ditto.

> + *
> + * Returns: The number of times IRQ @num was lowered, i.e., transitioned from
> + * a high state (true) to a low state (false).
> + */
> +uint64_t qtest_get_irq_lowered_counter(QTestState *s, int num);
> +
>   /**
>    * qtest_irq_intercept_in:
>    * @s: #QTestState instance to operate on.