On 11/17/2016 05:36 AM, Alastair D'Silva wrote: > From: Alastair D'Silva <alast...@d-silva.org> > > 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 <alast...@d-silva.org>
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 <c...@kaod.org> 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. >