| Message ID | 20180718104836.18488-2-michael@walle.cc (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 2018-07-18 12:48, schrieb Michael Walle: > It is only possible to retrieve the current state of an interrupt line. > But > there are devices which just pulses the interrupt line. Introduce a > latch > which is set by qtest and which can be cleared by the test case. > > Signed-off-by: Michael Walle <michael@walle.cc> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Andreas Färber <afaerber@suse.de> Hi, unfortunately, there was never any feedback. Are you ok with this patch in general? There is still one patch for an additional test case in my queue, which depends on this. -michael > --- > tests/libqtest.c | 19 +++++++++++++++++++ > tests/libqtest.h | 40 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 59 insertions(+) > > diff --git a/tests/libqtest.c b/tests/libqtest.c > index 098af6aec4..85e1f6f92a 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -40,6 +40,7 @@ struct QTestState > int fd; > int qmp_fd; > bool irq_level[MAX_IRQ]; > + bool irq_latch[MAX_IRQ]; > GString *rx; > pid_t qemu_pid; /* our child QEMU process */ > bool big_endian; > @@ -233,6 +234,7 @@ QTestState *qtest_init_without_qmp_handshake(bool > use_oob, > s->rx = g_string_new(""); > for (i = 0; i < MAX_IRQ; i++) { > s->irq_level[i] = false; > + s->irq_latch[i] = false; > } > > if (getenv("QTEST_STOP")) { > @@ -386,6 +388,7 @@ redo: > > if (strcmp(words[1], "raise") == 0) { > s->irq_level[irq] = true; > + s->irq_latch[irq] = true; > } else { > s->irq_level[irq] = false; > } > @@ -678,6 +681,22 @@ bool qtest_get_irq(QTestState *s, int num) > return s->irq_level[num]; > } > > +bool qtest_get_irq_latched(QTestState *s, int num) > +{ > + g_assert_cmpint(num, <, MAX_IRQ); > + > + /* dummy operation in order to make sure irq is up to date */ > + qtest_inb(s, 0); > + > + return s->irq_latch[num]; > +} > + > +void qtest_clear_irq_latch(QTestState *s, int num) > +{ > + g_assert_cmpint(num, <, MAX_IRQ); > + s->irq_latch[num] = false; > +} > + > static int64_t qtest_clock_rsp(QTestState *s) > { > gchar **words; > diff --git a/tests/libqtest.h b/tests/libqtest.h > index ac52872cbe..721dd50863 100644 > --- a/tests/libqtest.h > +++ b/tests/libqtest.h > @@ -192,6 +192,24 @@ char *qtest_hmpv(QTestState *s, const char *fmt, > va_list ap); > bool qtest_get_irq(QTestState *s, int num); > > /** > + * qtest_get_irq_latched: > + * @s: #QTestState instance to operate on. > + * @num: Interrupt to observe. > + * > + * Returns: The latched state of the @num interrupt. > + */ > +bool qtest_get_irq_latched(QTestState *s, int num); > + > +/** > + * qtest_clear_irq_latch: > + * @s: #QTestState instance to operate on. > + * @num: Interrupt to operate on. > + * > + * Clears the latch of the @num interrupt. > + */ > +void qtest_clear_irq_latch(QTestState *s, int num); > + > +/** > * qtest_irq_intercept_in: > * @s: #QTestState instance to operate on. > * @string: QOM path of a device. > @@ -638,6 +656,28 @@ static inline bool get_irq(int num) > } > > /** > + * get_irq_latched: > + * @num: Interrupt to observe. > + * > + * Returns: The latched level of the @num interrupt. > + */ > +static inline bool get_irq_latched(int num) > +{ > + return qtest_get_irq_latched(global_qtest, num); > +} > + > +/** > + * clear_irq_latch: > + * @num: Interrupt to operate on. > + * > + * Clears the latch of the @num interrupt. > + */ > +static inline void clear_irq_latch(int num) > +{ > + return qtest_clear_irq_latch(global_qtest, num); > +} > + > +/** > * irq_intercept_in: > * @string: QOM path of a device. > *
Am 31.10.18 um 09:31 schrieb Michael Walle: > Am 2018-07-18 12:48, schrieb Michael Walle: >> It is only possible to retrieve the current state of an interrupt >> line. But >> there are devices which just pulses the interrupt line. Introduce a latch >> which is set by qtest and which can be cleared by the test case. >> >> Signed-off-by: Michael Walle <michael@walle.cc> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Andreas Färber <afaerber@suse.de> > > Hi, > > unfortunately, there was never any feedback. Are you ok with this patch > in general? There is still one patch for an additional test case in my > queue, which depends on this. No objection from my side, but Paolo may be a better reviewer for irq. Regards, Andreas > > -michael > >> --- >> tests/libqtest.c | 19 +++++++++++++++++++ >> tests/libqtest.h | 40 ++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 59 insertions(+) >> >> diff --git a/tests/libqtest.c b/tests/libqtest.c >> index 098af6aec4..85e1f6f92a 100644 >> --- a/tests/libqtest.c >> +++ b/tests/libqtest.c >> @@ -40,6 +40,7 @@ struct QTestState >> int fd; >> int qmp_fd; >> bool irq_level[MAX_IRQ]; >> + bool irq_latch[MAX_IRQ]; >> GString *rx; >> pid_t qemu_pid; /* our child QEMU process */ >> bool big_endian; >> @@ -233,6 +234,7 @@ QTestState *qtest_init_without_qmp_handshake(bool >> use_oob, >> s->rx = g_string_new(""); >> for (i = 0; i < MAX_IRQ; i++) { >> s->irq_level[i] = false; >> + s->irq_latch[i] = false; >> } >> >> if (getenv("QTEST_STOP")) { >> @@ -386,6 +388,7 @@ redo: >> >> if (strcmp(words[1], "raise") == 0) { >> s->irq_level[irq] = true; >> + s->irq_latch[irq] = true; >> } else { >> s->irq_level[irq] = false; >> } >> @@ -678,6 +681,22 @@ bool qtest_get_irq(QTestState *s, int num) >> return s->irq_level[num]; >> } >> >> +bool qtest_get_irq_latched(QTestState *s, int num) >> +{ >> + g_assert_cmpint(num, <, MAX_IRQ); >> + >> + /* dummy operation in order to make sure irq is up to date */ >> + qtest_inb(s, 0); >> + >> + return s->irq_latch[num]; >> +} >> + >> +void qtest_clear_irq_latch(QTestState *s, int num) >> +{ >> + g_assert_cmpint(num, <, MAX_IRQ); >> + s->irq_latch[num] = false; >> +} >> + >> static int64_t qtest_clock_rsp(QTestState *s) >> { >> gchar **words; >> diff --git a/tests/libqtest.h b/tests/libqtest.h >> index ac52872cbe..721dd50863 100644 >> --- a/tests/libqtest.h >> +++ b/tests/libqtest.h >> @@ -192,6 +192,24 @@ char *qtest_hmpv(QTestState *s, const char *fmt, >> va_list ap); >> bool qtest_get_irq(QTestState *s, int num); >> >> /** >> + * qtest_get_irq_latched: >> + * @s: #QTestState instance to operate on. >> + * @num: Interrupt to observe. >> + * >> + * Returns: The latched state of the @num interrupt. >> + */ >> +bool qtest_get_irq_latched(QTestState *s, int num); >> + >> +/** >> + * qtest_clear_irq_latch: >> + * @s: #QTestState instance to operate on. >> + * @num: Interrupt to operate on. >> + * >> + * Clears the latch of the @num interrupt. >> + */ >> +void qtest_clear_irq_latch(QTestState *s, int num); >> + >> +/** >> * qtest_irq_intercept_in: >> * @s: #QTestState instance to operate on. >> * @string: QOM path of a device. >> @@ -638,6 +656,28 @@ static inline bool get_irq(int num) >> } >> >> /** >> + * get_irq_latched: >> + * @num: Interrupt to observe. >> + * >> + * Returns: The latched level of the @num interrupt. >> + */ >> +static inline bool get_irq_latched(int num) >> +{ >> + return qtest_get_irq_latched(global_qtest, num); >> +} >> + >> +/** >> + * clear_irq_latch: >> + * @num: Interrupt to operate on. >> + * >> + * Clears the latch of the @num interrupt. >> + */ >> +static inline void clear_irq_latch(int num) >> +{ >> + return qtest_clear_irq_latch(global_qtest, num); >> +} >> + >> +/** >> * irq_intercept_in: >> * @string: QOM path of a device. >> *
On 18/07/2018 12:48, Michael Walle wrote: > /** > + * get_irq_latched: > + * @num: Interrupt to observe. > + * > + * Returns: The latched level of the @num interrupt. > + */ > +static inline bool get_irq_latched(int num) > +{ > + return qtest_get_irq_latched(global_qtest, num); > +} > + > +/** > + * clear_irq_latch: > + * @num: Interrupt to operate on. > + * > + * Clears the latch of the @num interrupt. > + */ > +static inline void clear_irq_latch(int num) > +{ > + return qtest_clear_irq_latch(global_qtest, num); > +} > + > +/** > * irq_intercept_in: > * @string: QOM path of a device. > * > Just one thing, I think an atomic test-and-clear is a better API. I understand that what you have works, because get_irq_latched+clear_irq_latch don't have any intervening access to the qtest socket. Paolo
Am 2018-10-31 11:19, schrieb Paolo Bonzini: > On 18/07/2018 12:48, Michael Walle wrote: >> /** >> + * get_irq_latched: >> + * @num: Interrupt to observe. >> + * >> + * Returns: The latched level of the @num interrupt. >> + */ >> +static inline bool get_irq_latched(int num) >> +{ >> + return qtest_get_irq_latched(global_qtest, num); >> +} >> + >> +/** >> + * clear_irq_latch: >> + * @num: Interrupt to operate on. >> + * >> + * Clears the latch of the @num interrupt. >> + */ >> +static inline void clear_irq_latch(int num) >> +{ >> + return qtest_clear_irq_latch(global_qtest, num); >> +} >> + >> +/** >> * irq_intercept_in: >> * @string: QOM path of a device. >> * >> > > Just one thing, I think an atomic test-and-clear is a better API. I > understand that what you have works, because > get_irq_latched+clear_irq_latch don't have any intervening access to > the > qtest socket. Ok thanks, like get_and_clear_irq_latched()? Do you think of we still need the get_irq_latched() to get the state without actually clearing it? I mean my simple test case won't need it, but there might be some cases where you may want peek at the state. -michael
On 31/10/2018 15:30, Michael Walle wrote: >> >> Just one thing, I think an atomic test-and-clear is a better API. I >> understand that what you have works, because >> get_irq_latched+clear_irq_latch don't have any intervening access to the >> qtest socket. > > Ok thanks, like get_and_clear_irq_latched()? Do you think of we still > need the get_irq_latched() to get the state without actually clearing > it? I mean my simple test case won't need it, but there might be some > cases where you may want peek at the state. Maybe - it's easy enough to add it, so if your test doesn't need it you can certainly leave it out. Paolo
diff --git a/tests/libqtest.c b/tests/libqtest.c index 098af6aec4..85e1f6f92a 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -40,6 +40,7 @@ struct QTestState int fd; int qmp_fd; bool irq_level[MAX_IRQ]; + bool irq_latch[MAX_IRQ]; GString *rx; pid_t qemu_pid; /* our child QEMU process */ bool big_endian; @@ -233,6 +234,7 @@ QTestState *qtest_init_without_qmp_handshake(bool use_oob, s->rx = g_string_new(""); for (i = 0; i < MAX_IRQ; i++) { s->irq_level[i] = false; + s->irq_latch[i] = false; } if (getenv("QTEST_STOP")) { @@ -386,6 +388,7 @@ redo: if (strcmp(words[1], "raise") == 0) { s->irq_level[irq] = true; + s->irq_latch[irq] = true; } else { s->irq_level[irq] = false; } @@ -678,6 +681,22 @@ bool qtest_get_irq(QTestState *s, int num) return s->irq_level[num]; } +bool qtest_get_irq_latched(QTestState *s, int num) +{ + g_assert_cmpint(num, <, MAX_IRQ); + + /* dummy operation in order to make sure irq is up to date */ + qtest_inb(s, 0); + + return s->irq_latch[num]; +} + +void qtest_clear_irq_latch(QTestState *s, int num) +{ + g_assert_cmpint(num, <, MAX_IRQ); + s->irq_latch[num] = false; +} + static int64_t qtest_clock_rsp(QTestState *s) { gchar **words; diff --git a/tests/libqtest.h b/tests/libqtest.h index ac52872cbe..721dd50863 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -192,6 +192,24 @@ char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap); bool qtest_get_irq(QTestState *s, int num); /** + * qtest_get_irq_latched: + * @s: #QTestState instance to operate on. + * @num: Interrupt to observe. + * + * Returns: The latched state of the @num interrupt. + */ +bool qtest_get_irq_latched(QTestState *s, int num); + +/** + * qtest_clear_irq_latch: + * @s: #QTestState instance to operate on. + * @num: Interrupt to operate on. + * + * Clears the latch of the @num interrupt. + */ +void qtest_clear_irq_latch(QTestState *s, int num); + +/** * qtest_irq_intercept_in: * @s: #QTestState instance to operate on. * @string: QOM path of a device. @@ -638,6 +656,28 @@ static inline bool get_irq(int num) } /** + * get_irq_latched: + * @num: Interrupt to observe. + * + * Returns: The latched level of the @num interrupt. + */ +static inline bool get_irq_latched(int num) +{ + return qtest_get_irq_latched(global_qtest, num); +} + +/** + * clear_irq_latch: + * @num: Interrupt to operate on. + * + * Clears the latch of the @num interrupt. + */ +static inline void clear_irq_latch(int num) +{ + return qtest_clear_irq_latch(global_qtest, num); +} + +/** * irq_intercept_in: * @string: QOM path of a device. *
It is only possible to retrieve the current state of an interrupt line. But there are devices which just pulses the interrupt line. Introduce a latch which is set by qtest and which can be cleared by the test case. Signed-off-by: Michael Walle <michael@walle.cc> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Andreas Färber <afaerber@suse.de> --- tests/libqtest.c | 19 +++++++++++++++++++ tests/libqtest.h | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+)