Re: [PATCH] test/qtest: Add an API function to capture IRQ toggling

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

On 13/11/23 07:59, Thomas Huth wrote:
> On 12/11/2023 02.38, Gustavo Romero wrote:
>> Currently the QTest API does not provide a function to allow capturing
>> when an IRQ line is toggled (raised then lowered). Functions like
>> qtest_get_irq() read the current state of the intercepted IRQ lines,
>> which is already low when the function is called, since the line is
>> toggled.
>>
>> This commit introduces a new function, qtest_get_irq_trigger_counter(),
>> which returns the number of times a given intercepted IRQ line was
>> triggered (raised), hence allowing to capture when an IRQ line was
>> toggled.
>>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> ---
>>   tests/qtest/libqtest.c | 12 ++++++++++++
>>   tests/qtest/libqtest.h |  9 +++++++++
>>   2 files changed, 21 insertions(+)
>>
>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>> index f33a210861..21891b52f1 100644
>> --- a/tests/qtest/libqtest.c
>> +++ b/tests/qtest/libqtest.c
>> @@ -82,6 +82,7 @@ struct QTestState
>>       int expected_status;
>>       bool big_endian;
>>       bool irq_level[MAX_IRQ];
>> +    uint64_t irq_trigger_counter[MAX_IRQ];
>>       GString *rx;
>>       QTestTransportOps ops;
>>       GList *pending_events;
>> @@ -498,6 +499,7 @@ 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_trigger_counter[i] = 0;
>>       }
>>       /*
>> @@ -690,6 +692,7 @@ redo:
>>           if (strcmp(words[1], "raise") == 0) {
>>               s->irq_level[irq] = true;
>> +            s->irq_trigger_counter[irq]++;

This is 'irq_raised_counter',

> Not sure whether you can get some "raise" events in a row without some 
> "lower" events in between ... but just in case, I wonder whether it 
> would make sense to check whether it is really a rising edge, i.e.:
> 
>             if (strcmp(words[1], "raise") == 0) {
>                 if (!s->irq_level[irq]) {
>                     s->irq_trigger_counter[irq]++;
>                 }
>                 s->irq_level[irq] = true;
> 
> What do you think?

This is 'irq_pulsed_counter'. 'irq_lowered_counter' could also be
useful (at least for completeness).

Per Gustavo's description, he indeed wants irq_pulsed_counter (or
irq_toggled_counter'.

> 
>>           } else {
>>               s->irq_level[irq] = false;
>>           }
> 
> Anyway:
> Acked-by: Thomas Huth <thuth@redhat.com>
>