qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-arm] [PATCH 2/4] qtest: Support named interrupts


From: Cédric Le Goater
Subject: Re: [Qemu-arm] [PATCH 2/4] qtest: Support named interrupts
Date: Tue, 22 Nov 2016 18:22:03 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

On 11/17/2016 05:36 AM, Alastair D'Silva wrote:
> From: Alastair D'Silva <address@hidden>
> 
> The QTest framework cannot work with named interrupts. This patch
> adds support for them, as well as the ability to manipulate them
> from within a test.
> 
> Read actions are via callbacks, which allows for pulsed interrupts
> to be read (the polled method used for the unnamed interrupts
> cannot read pulsed interrupts as the value is reverted before the
> test sees the changes).
> 
> Signed-off-by: Alastair D'Silva <address@hidden>

This looks OK to me but I am clearly not an expert in this area.
Maybe Paolo has some comments to add.

Reviewed-by: Cédric Le Goater <address@hidden>

Thanks,

C. 

> ---
>  qtest.c          | 98 
> ++++++++++++++++++++++++++++++++++++++++++--------------
>  tests/libqtest.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++---
>  tests/libqtest.h | 59 ++++++++++++++++++++++++++++++++++
>  3 files changed, 216 insertions(+), 28 deletions(-)
> 
> diff --git a/qtest.c b/qtest.c
> index 46b99ae..a56503b 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -40,7 +40,6 @@ static DeviceState *irq_intercept_dev;
>  static FILE *qtest_log_fp;
>  static CharBackend qtest_chr;
>  static GString *inbuf;
> -static int irq_levels[MAX_IRQ];
>  static qemu_timeval start_time;
>  static bool qtest_opened;
>  
> @@ -160,10 +159,16 @@ static bool qtest_opened;
>   *
>   *  IRQ raise NUM
>   *  IRQ lower NUM
> + *  IRQ_NAMED NAME NUM LEVEL
>   *
>   * 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).
> + *
> + *  > irq_set NAME NUM LEVEL
> + *  < OK
> + *
> + *  Set the named input IRQ to the level (0/1)
>   */
>  
>  static int hex2nib(char ch)
> @@ -243,17 +248,31 @@ static void GCC_FMT_ATTR(2, 3) qtest_sendf(CharBackend 
> *chr,
>      va_end(ap);
>  }
>  
> +typedef struct qtest_irq {
> +    qemu_irq old_irq;
> +    char *name;
> +    bool last_level;
> +} qtest_irq;
> +
>  static void qtest_irq_handler(void *opaque, int n, int level)
>  {
> -    qemu_irq old_irq = *(qemu_irq *)opaque;
> -    qemu_set_irq(old_irq, level);
> +    qtest_irq *data = (qtest_irq *)opaque;
> +    level = !!level;
> +
> +    qemu_set_irq(data->old_irq, level);
>  
> -    if (irq_levels[n] != level) {
> +    if (level != data->last_level) {
>          CharBackend *chr = &qtest_chr;
> -        irq_levels[n] = level;
>          qtest_send_prefix(chr);
> -        qtest_sendf(chr, "IRQ %s %d\n",
> -                    level ? "raise" : "lower", n);
> +
> +        if (data->name) {
> +            qtest_sendf(chr, "IRQ_NAMED %s %d %d\n",
> +                    data->name, n, level);
> +        } else {
> +            qtest_sendf(chr, "IRQ %s %d\n", level ? "raise" : "lower", n);
> +        }
> +
> +        data->last_level = level;
>      }
>  }
>  
> @@ -289,7 +308,7 @@ static void qtest_process_command(CharBackend *chr, gchar 
> **words)
>          if (!dev) {
>              qtest_send_prefix(chr);
>              qtest_send(chr, "FAIL Unknown device\n");
> -         return;
> +            return;
>          }
>  
>          if (irq_intercept_dev) {
> @@ -299,33 +318,69 @@ static void qtest_process_command(CharBackend *chr, 
> gchar **words)
>              } else {
>                  qtest_send(chr, "OK\n");
>              }
> -         return;
> +            return;
>          }
>  
>          QLIST_FOREACH(ngl, &dev->gpios, node) {
> -            /* We don't support intercept of named GPIOs yet */
> -            if (ngl->name) {
> -                continue;
> -            }
>              if (words[0][14] == 'o') {
>                  int i;
>                  for (i = 0; i < ngl->num_out; ++i) {
> -                    qemu_irq *disconnected = g_new0(qemu_irq, 1);
> -                    qemu_irq icpt = qemu_allocate_irq(qtest_irq_handler,
> -                                                      disconnected, i);
> +                    qtest_irq *data = g_new0(qtest_irq, 1);
> +                    data->name = ngl->name;
> +                    qemu_irq icpt = qemu_allocate_irq(qtest_irq_handler, 
> data,
> +                            i);
>  
> -                    *disconnected = qdev_intercept_gpio_out(dev, icpt,
> +                    data->old_irq = qdev_intercept_gpio_out(dev, icpt,
>                                                              ngl->name, i);
>                  }
>              } else {
> -                qemu_irq_intercept_in(ngl->in, qtest_irq_handler,
> -                                      ngl->num_in);
> +                qtest_irq *data = g_new0(qtest_irq, 1);
> +                data->old_irq = *ngl->in;
> +                data->name = ngl->name;
> +                qemu_irq_intercept_in(ngl->in, qtest_irq_handler, 
> ngl->num_in);
>              }
>          }
>          irq_intercept_dev = dev;
>          qtest_send_prefix(chr);
>          qtest_send(chr, "OK\n");
>  
> +    } else if (strcmp(words[0], "irq_set") == 0) {
> +        DeviceState *dev;
> +        NamedGPIOList *ngl;
> +        int level;
> +        qemu_irq irq = NULL;
> +        int irq_num;
> +
> +        g_assert(words[1]); /* device */
> +        g_assert(words[2]); /* gpio list */
> +        g_assert(words[3]); /* gpio line in list */
> +        g_assert(words[4]); /* level */
> +        dev = DEVICE(object_resolve_path(words[1], NULL));
> +        if (!dev) {
> +            qtest_send_prefix(chr);
> +            qtest_send(chr, "FAIL Unknown device\n");
> +            return;
> +        }
> +
> +        irq_num = atoi(words[3]);
> +        level = atoi(words[4]);
> +
> +        QLIST_FOREACH(ngl, &dev->gpios, node) {
> +            if (strcmp(words[2], ngl->name) == 0 && ngl->num_in > irq_num) {
> +                irq = ngl->in[irq_num];
> +            }
> +        }
> +
> +        if (irq == NULL) {
> +            qtest_send_prefix(chr);
> +            qtest_send(chr, "FAIL Unknown IRQ\n");
> +            return;
> +        }
> +
> +        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) {
> @@ -622,8 +677,6 @@ static int qtest_can_read(void *opaque)
>  
>  static void qtest_event(void *opaque, int event)
>  {
> -    int i;
> -
>      switch (event) {
>      case CHR_EVENT_OPENED:
>          /*
> @@ -632,9 +685,6 @@ static void qtest_event(void *opaque, int event)
>           * used.  Injects an extra reset even when it's not used, and
>           * that can mess up tests, e.g. -boot once.
>           */
> -        for (i = 0; i < ARRAY_SIZE(irq_levels); i++) {
> -            irq_levels[i] = 0;
> -        }
>          qemu_gettimeofday(&start_time);
>          qtest_opened = true;
>          if (qtest_log_fp) {
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 6f69752..43da151 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -21,12 +21,16 @@
>  #include <sys/wait.h>
>  #include <sys/un.h>
>  
> +#include <glib.h>
> +
>  #include "qapi/qmp/json-parser.h"
>  #include "qapi/qmp/json-streamer.h"
>  #include "qapi/qmp/qjson.h"
>  
> +
>  #define MAX_IRQ 256
>  #define SOCKET_TIMEOUT 50
> +#define IRQ_KEY_LENGTH 64
>  
>  QTestState *global_qtest;
>  
> @@ -34,12 +38,22 @@ struct QTestState
>  {
>      int fd;
>      int qmp_fd;
> +    GHashTable *irq_handlers;
>      bool irq_level[MAX_IRQ];
>      GString *rx;
>      pid_t qemu_pid;  /* our child QEMU process */
>      bool big_endian;
>  };
>  
> +typedef struct irq_action {
> +    void (*cb)(void *opaque, const char *name, int irq, bool level);
> +    void *opaque;
> +    const char *name;
> +    int n;
> +    bool level;
> +} irq_action;
> +
> +
>  static GHookList abrt_hooks;
>  static GList *qtest_instances;
>  static struct sigaction sigact_old;
> @@ -216,6 +230,8 @@ QTestState *qtest_init(const char *extra_args)
>  
>      s->big_endian = qtest_query_target_endianness(s);
>  
> +    s->irq_handlers = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, 
> g_free);
> +
>      return s;
>  }
>  
> @@ -224,6 +240,8 @@ void qtest_quit(QTestState *s)
>      qtest_instances = g_list_remove(qtest_instances, s);
>      g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s));
>  
> +    g_hash_table_destroy(s->irq_handlers);
> +
>      /* Uninstall SIGABRT handler on last instance */
>      if (!qtest_instances) {
>          cleanup_sigabrt_handler();
> @@ -304,11 +322,35 @@ static GString *qtest_recv_line(QTestState *s)
>      return line;
>  }
>  
> +void qtest_irq_attach(QTestState *s, const char *name, int irq,
> +        void (*irq_cb)(void *opaque, const char *name, int irq, bool level),
> +        void *opaque)
> +{
> +    char key[IRQ_KEY_LENGTH];
> +    irq_action *action = g_new0(irq_action, 1);
> +
> +    action->cb = irq_cb;
> +    action->name = name;
> +    action->n = irq;
> +    action->opaque = opaque;
> +    action->level = false;
> +
> +    g_assert_cmpint(snprintf(key, sizeof(key), "%s.%d",
> +            name, irq), <, sizeof(key));
> +
> +    g_hash_table_insert(s->irq_handlers, g_strdup(key), action);
> +}
> +
> +#define MAX_ACTIONS 256
>  static gchar **qtest_rsp(QTestState *s, int expected_args)
>  {
>      GString *line;
>      gchar **words;
>      int i;
> +    int action_index;
> +    int action_count = 0;
> +    bool action_raise[MAX_ACTIONS];
> +    irq_action *actions[MAX_ACTIONS];
>  
>  redo:
>      line = qtest_recv_line(s);
> @@ -325,10 +367,29 @@ redo:
>          g_assert_cmpint(irq, >=, 0);
>          g_assert_cmpint(irq, <, MAX_IRQ);
>  
> -        if (strcmp(words[1], "raise") == 0) {
> -            s->irq_level[irq] = true;
> -        } else {
> -            s->irq_level[irq] = false;
> +        s->irq_level[irq] = (strcmp(words[1], "raise") == 0);
> +
> +        g_strfreev(words);
> +        goto redo;
> +    } else if (strcmp(words[0], "IRQ_NAMED") == 0) {
> +        bool level;
> +        char key[IRQ_KEY_LENGTH];
> +        irq_action *action;
> +
> +        g_assert(words[1] != NULL);
> +        g_assert(words[2] != NULL);
> +        g_assert(words[3] != NULL);
> +
> +        level = (words[3][0] == '1');
> +
> +        g_assert_cmpint(snprintf(key, sizeof(key), "%s.%s",
> +                words[1], words[2]), <, sizeof(key));
> +
> +        action = g_hash_table_lookup(s->irq_handlers, key);
> +
> +        if (action) {
> +            action_raise[action_count] = level;
> +            actions[action_count++] = action;
>          }
>  
>          g_strfreev(words);
> @@ -346,6 +407,17 @@ redo:
>          g_strfreev(words);
>      }
>  
> +/* Defer processing of IRQ actions until all communications have been 
> handled,
> + * otherwise, interrupt handler that cause further communication can disrupt
> + * the communication stream
> + */
> +    for (action_index = 0; action_index < action_count; action_index++) {
> +        irq_action *action = actions[action_index];
> +        action->cb(action->opaque, action->name, action->n,
> +                action_raise[action_index]);
> +        action->level = action_raise[action_index];
> +    }
> +
>      return words;
>  }
>  
> @@ -918,3 +990,10 @@ bool qtest_big_endian(QTestState *s)
>  {
>      return s->big_endian;
>  }
> +
> +void qtest_irq_set(QTestState *s, const char *id, const char *gpiolist, int 
> n,
> +        bool level)
> +{
> +    qtest_sendf(s, "irq_set %s %s %d %d\n", id, gpiolist, n, level);
> +    qtest_rsp(s, 0);
> +}
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 90f182e..c74373c 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -176,6 +176,34 @@ void qtest_irq_intercept_in(QTestState *s, const char 
> *string);
>  void qtest_irq_intercept_out(QTestState *s, const char *string);
>  
>  /**
> + * irq_attach:
> + * @s: #QTestState instance to operate on.
> + * @name: the name of the GPIO list containing the IRQ
> + * @irq: The IRQ number within the GPIO list to attach to
> + * @irq_cb: The callback to execute when the interrupt changes
> + * @opaque: opaque info to pass to the callback
> + *
> + * Attach a callback to an intercepted interrupt
> + */
> +void qtest_irq_attach(QTestState *s, const char *name, int irq,
> +        void (*irq_cb)(void *opaque, const char *name, int irq, bool level),
> +        void *opaque);
> +
> +/**
> + * qtest_irq_set
> + * Set an interrupt level
> + * @s: #QTestState instance to operate on.
> + * @id: the device to inject interrupts for
> + * @gpiolist: the GPIO list containing the IRQ
> + * @n: the GPIO within the list
> + * @level: the IRQ level
> + *
> + * Set an interrupt to a nominated level
> + */
> +void qtest_irq_set(QTestState *s, const char *id, const char *gpiolist, int 
> n,
> +        bool level);
> +
> +/**
>   * qtest_outb:
>   * @s: #QTestState instance to operate on.
>   * @addr: I/O port to write to.
> @@ -626,6 +654,37 @@ static inline void irq_intercept_out(const char *string)
>  }
>  
>  /**
> + * irq_attach:
> + * @name: the name of the gpio list containing the IRQ
> + * @irq: The IRQ to attach to
> + * @irq_cb: The callback to execute when the interrupt changes
> + * @opaque: opaque info to pass to the callback
> + *
> + * Attach a callback to an intecepted interrupt
> + */
> +static inline void irq_attach(const char *name, int irq,
> +        void (*irq_cb)(void *opaque, const char *name, int irq, bool level),
> +        void *opaque)
> +{
> +    qtest_irq_attach(global_qtest, name, irq, irq_cb, opaque);
> +}
> +
> +/**
> + * qtest_irq_set
> + * Set an interrupt level
> + * @id: the device to inject interrupts for
> + * @gpiolist: the GPIO list containing the line to seh
> + * @n: the line to set within the list
> + * @level: the IRQ level
> + */
> +static inline void irq_set(const char *id, const char *gpiolist, int n,
> +        bool level)
> +{
> +    qtest_irq_set(global_qtest, id, gpiolist, n, level);
> +}
> +
> +
> +/**
>   * outb:
>   * @addr: I/O port to write to.
>   * @value: Value being written.
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]