Re: [Qemu-devel] [PATCH v3 01/13] qtest: Add set_irq_in command to set IRQ/GPIO level

[Laurent Vivier]

On 31/10/2018 01:25, Steffen Görtz wrote:
> Adds a new qtest command "set_irq_in" which allows
> to set qemu gpio lines to a given level.
> 
> Based on https://lists.gnu.org/archive/html/qemu-devel/2012-12/msg02363.html
> which never got merged.
> 
> Signed-off-by: Steffen Görtz <address@hidden>
> Originally-by: Matthew Ogilvie <address@hidden>
> ---
>  qtest.c          | 40 ++++++++++++++++++++++++++++++++++++++++
>  tests/libqtest.c | 10 ++++++++++
>  tests/libqtest.h | 28 ++++++++++++++++++++++++++++
>  3 files changed, 78 insertions(+)
> 
> diff --git a/qtest.c b/qtest.c
> index 69b9e9962b..214f42a6c5 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -164,6 +164,14 @@ static bool qtest_opened;
>   * where NUM is an IRQ number.  For the PC, interrupts can be intercepted
>   * simply with "irq_intercept_in ioapic" (note that IRQ0 comes out with
>   * NUM=0 even though it is remapped to GSI 2).
> + *
> + * Setting interrupt level:
> + *
> + *  > set_irq_in QOM-PATH IRQ LEVEL

This seems not correct according to:

    qtest_sendf(s, "set_irq_in %s %s %d %d\n", qom_path, name, num,
                level);

> + *  < OK
> + *
> + * Forcibly set the given interrupt pin to the given level.
> + *
>   */
>  
>  static int hex2nib(char ch)
> @@ -326,7 +334,39 @@ static void qtest_process_command(CharBackend *chr, 
> gchar **words)
>          irq_intercept_dev = dev;
>          qtest_send_prefix(chr);
>          qtest_send(chr, "OK\n");
> +    } else if (strcmp(words[0], "set_irq_in") == 0) {
> +        DeviceState *dev;
> +        qemu_irq irq;
> +        char *name;
> +        int ret;
> +        long num;

qdev_get_gpio_in_named() and qemu_set_irq() take an "int" for the "num"
and "level" parameters, perhaps we can match it here?

> +        long level;
>  
> +        g_assert(words[1] && words[2] && words[3] && words[4]);
> +
> +        dev = DEVICE(object_resolve_path(words[1], NULL));
> +        if (!dev) {
> +            qtest_send_prefix(chr);
> +            qtest_send(chr, "FAIL Unknown device\n");
> +            return;
> +        }
> +
> +        if (strcmp(words[2], "unnamed-gpio-in") == 0) {
> +            name = NULL;
> +        } else {
> +            name = words[2];
> +        }
> +
> +        ret = qemu_strtol(words[3], NULL, 10, &num);
> +        g_assert(!ret);
> +        ret = qemu_strtol(words[4], NULL, 10, &level);
> +        g_assert(!ret);

Why do you force base 10?

> +
> +        irq = qdev_get_gpio_in_named(dev, name, num);
> +
> +        qemu_set_irq(irq, level);
> +        qtest_send_prefix(chr);
> +        qtest_send(chr, "OK\n");
>      } else if (strcmp(words[0], "outb") == 0 ||
>                 strcmp(words[0], "outw") == 0 ||
>                 strcmp(words[0], "outl") == 0) {
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 44ce118cfc..1cbde0d91a 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -732,6 +732,16 @@ void qtest_irq_intercept_in(QTestState *s, const char 
> *qom_path)
>      qtest_rsp(s, 0);
>  }
>  
> +void qtest_set_irq_in(QTestState *s, const char *qom_path, const char *name,
> +                      int num, int level)
> +{
> +    if (!name) {
> +        name = "unnamed-gpio-in";
> +    }
> +    qtest_sendf(s, "set_irq_in %s %s %d %d\n", qom_path, name, num, level);

And here "num" and "level" are "int", so you should match this in qtest.c

Thanks,
Laurent