[Qemu-devel] [PATCH v5 0/3] tests/microbit-test: Add UART device test

[Qemu-devel] [PATCH v5 0/3] tests/microbit-test: Add UART device test

[Julia Suvorova]

v5:
    * Replace g_assert with g_assert_* [Thomas, Alex]
    * Increase the waiting time for an event [Thomas]
    * Remove 'nowait' [Thomas]
    * Rewrite uart_wait_for_event to use time difference instead of loop
v4:
    * Replace sprintf with g_strdup_printf [Peter]
    * Move socket connection to qtest library [Peter]
    * Use memcmp instead of strcmp [Stefan]
    * Remove using global_qtest [Thomas]
v3:
    * Fix directory leak [Stefan]

Based-on: <20190110094020.18354-1-stefanha@redhat.com>

Julia Suvorova (3):
  tests/libqtest: Introduce qtest_init_with_serial()
  tests/microbit-test: Make test independent of global_qtest
  tests/microbit-test: Check nRF51 UART functionality

 tests/libqtest.c      |  25 ++++
 tests/libqtest.h      |  11 ++
 tests/microbit-test.c | 336 +++++++++++++++++++++++++++---------------
 3 files changed, 254 insertions(+), 118 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()

[Julia Suvorova]

Run qtest with a socket that connects QEMU chardev and test code.

Signed-off-by: Julia Suvorova <jusual@mail.ru>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/libqtest.c | 25 +++++++++++++++++++++++++
 tests/libqtest.h | 11 +++++++++++
 2 files changed, 36 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 55750dd68d..6fb30855fa 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -315,6 +315,31 @@ QTestState *qtest_initf(const char *fmt, ...)
     return s;
 }
 
+QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
+{
+    int sock_fd_init;
+    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
+    QTestState *qts;
+
+    g_assert_true(mkdtemp(sock_dir) != NULL);
+    sock_path = g_strdup_printf("%s/sock", sock_dir);
+
+    sock_fd_init = init_socket(sock_path);
+
+    qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
+                      sock_path, extra_args);
+
+    *sock_fd = socket_accept(sock_fd_init);
+
+    unlink(sock_path);
+    g_free(sock_path);
+    rmdir(sock_dir);
+
+    g_assert_true(*sock_fd >= 0);
+
+    return qts;
+}
+
 void qtest_quit(QTestState *s)
 {
     g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s));
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 7ea94139b0..5937f91912 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -62,6 +62,17 @@ QTestState *qtest_init(const char *extra_args);
  */
 QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
 
+/**
+ * qtest_init_with_serial:
+ * @extra_args: other arguments to pass to QEMU.  CAUTION: these
+ * arguments are subject to word splitting and shell evaluation.
+ * @sock_fd: pointer to store the socket file descriptor for
+ * connection with serial.
+ *
+ * Returns: #QTestState instance.
+ */
+QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
+
 /**
  * qtest_quit:
  * @s: #QTestState instance to operate on.
-- 
2.17.1

Re: [Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()

[Thomas Huth]

On 2019-01-23 13:07, Julia Suvorova wrote:
> Run qtest with a socket that connects QEMU chardev and test code.
> 
> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/libqtest.c | 25 +++++++++++++++++++++++++
>  tests/libqtest.h | 11 +++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 55750dd68d..6fb30855fa 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -315,6 +315,31 @@ QTestState *qtest_initf(const char *fmt, ...)
>      return s;
>  }
>  
> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
> +{
> +    int sock_fd_init;
> +    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
> +    QTestState *qts;
> +
> +    g_assert_true(mkdtemp(sock_dir) != NULL);

Oh, nice, I wasn't aware of g_assert_true yet, that's a good idea here,
indeed.

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()

[Alex Bennée]

Julia Suvorova <jusual@mail.ru> writes:

> Run qtest with a socket that connects QEMU chardev and test code.
>
> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  tests/libqtest.c | 25 +++++++++++++++++++++++++
>  tests/libqtest.h | 11 +++++++++++
>  2 files changed, 36 insertions(+)
>
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 55750dd68d..6fb30855fa 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -315,6 +315,31 @@ QTestState *qtest_initf(const char *fmt, ...)
>      return s;
>  }
>
> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
> +{
> +    int sock_fd_init;
> +    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
> +    QTestState *qts;
> +
> +    g_assert_true(mkdtemp(sock_dir) != NULL);
> +    sock_path = g_strdup_printf("%s/sock", sock_dir);
> +
> +    sock_fd_init = init_socket(sock_path);
> +
> +    qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
> +                      sock_path, extra_args);
> +
> +    *sock_fd = socket_accept(sock_fd_init);
> +
> +    unlink(sock_path);
> +    g_free(sock_path);
> +    rmdir(sock_dir);
> +
> +    g_assert_true(*sock_fd >= 0);
> +
> +    return qts;
> +}
> +
>  void qtest_quit(QTestState *s)
>  {
>      g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, s));
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 7ea94139b0..5937f91912 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -62,6 +62,17 @@ QTestState *qtest_init(const char *extra_args);
>   */
>  QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
>
> +/**
> + * qtest_init_with_serial:
> + * @extra_args: other arguments to pass to QEMU.  CAUTION: these
> + * arguments are subject to word splitting and shell evaluation.
> + * @sock_fd: pointer to store the socket file descriptor for
> + * connection with serial.
> + *
> + * Returns: #QTestState instance.
> + */
> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);
> +
>  /**
>   * qtest_quit:
>   * @s: #QTestState instance to operate on.


--
Alex Bennée

Re: [Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1645 bytes --]

On 1/23/19 8:20 AM, Alex Bennée wrote:
> 
> Julia Suvorova <jusual@mail.ru> writes:
> 
>> Run qtest with a socket that connects QEMU chardev and test code.
>>
>> Signed-off-by: Julia Suvorova <jusual@mail.ru>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> 

>> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
>> +{
>> +    int sock_fd_init;
>> +    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
>> +    QTestState *qts;
>> +
>> +    g_assert_true(mkdtemp(sock_dir) != NULL);
>> +    sock_path = g_strdup_printf("%s/sock", sock_dir);
>> +
>> +    sock_fd_init = init_socket(sock_path);
>> +
>> +    qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
>> +                      sock_path, extra_args);

EWWWW.  Please, let's not bake in this interface.  Relying on shell
splitting is nasty.


>>
>> +/**
>> + * qtest_init_with_serial:
>> + * @extra_args: other arguments to pass to QEMU.  CAUTION: these
>> + * arguments are subject to word splitting and shell evaluation.
>> + * @sock_fd: pointer to store the socket file descriptor for
>> + * connection with serial.
>> + *
>> + * Returns: #QTestState instance.
>> + */
>> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd);

I would MUCH rather see an interface that used varargs to pass extra
arguments as a list/array of actual arguments, no shell splitting required.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH v5 1/3] tests/libqtest: Introduce qtest_init_with_serial()

[Daniel P. Berrangé]

On Wed, Jan 23, 2019 at 08:44:12AM -0600, Eric Blake wrote:
> On 1/23/19 8:20 AM, Alex Bennée wrote:
> > 
> > Julia Suvorova <jusual@mail.ru> writes:
> > 
> >> Run qtest with a socket that connects QEMU chardev and test code.
> >>
> >> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> >> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > 
> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> > 
> 
> >> +QTestState *qtest_init_with_serial(const char *extra_args, int *sock_fd)
> >> +{
> >> +    int sock_fd_init;
> >> +    char *sock_path, sock_dir[] = "/tmp/qtest-serial-XXXXXX";
> >> +    QTestState *qts;
> >> +
> >> +    g_assert_true(mkdtemp(sock_dir) != NULL);
> >> +    sock_path = g_strdup_printf("%s/sock", sock_dir);
> >> +
> >> +    sock_fd_init = init_socket(sock_path);
> >> +
> >> +    qts = qtest_initf("-chardev socket,id=s0,path=%s -serial chardev:s0 %s",
> >> +                      sock_path, extra_args);
> 
> EWWWW.  Please, let's not bake in this interface.  Relying on shell
> splitting is nasty.

qtest_initf() is long a standing interface that's relied on
shell splitting for years :-( I agree we should fix that, but
its a pre-existing problem that this patch doesn't make worse.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v5 2/3] tests/microbit-test: Make test independent of global_qtest

[Julia Suvorova]

Using of global_qtest is not required here. Let's replace functions like
readl() with the corresponding qtest_* counterparts.

Signed-off-by: Julia Suvorova <jusual@mail.ru>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/microbit-test.c | 247 ++++++++++++++++++++++--------------------
 1 file changed, 129 insertions(+), 118 deletions(-)

diff --git a/tests/microbit-test.c b/tests/microbit-test.c
index dcdc0cd41a..afeb6b082a 100644
--- a/tests/microbit-test.c
+++ b/tests/microbit-test.c
@@ -24,22 +24,22 @@
 #include "hw/i2c/microbit_i2c.h"
 
 /* Read a byte from I2C device at @addr from register @reg */
-static uint32_t i2c_read_byte(uint32_t addr, uint32_t reg)
+static uint32_t i2c_read_byte(QTestState *qts, uint32_t addr, uint32_t reg)
 {
     uint32_t val;
 
-    writel(NRF51_TWI_BASE + NRF51_TWI_REG_ADDRESS, addr);
-    writel(NRF51_TWI_BASE + NRF51_TWI_TASK_STARTTX, 1);
-    writel(NRF51_TWI_BASE + NRF51_TWI_REG_TXD, reg);
-    val = readl(NRF51_TWI_BASE + NRF51_TWI_EVENT_TXDSENT);
+    qtest_writel(qts, NRF51_TWI_BASE + NRF51_TWI_REG_ADDRESS, addr);
+    qtest_writel(qts, NRF51_TWI_BASE + NRF51_TWI_TASK_STARTTX, 1);
+    qtest_writel(qts, NRF51_TWI_BASE + NRF51_TWI_REG_TXD, reg);
+    val = qtest_readl(qts, NRF51_TWI_BASE + NRF51_TWI_EVENT_TXDSENT);
     g_assert_cmpuint(val, ==, 1);
-    writel(NRF51_TWI_BASE + NRF51_TWI_TASK_STOP, 1);
+    qtest_writel(qts, NRF51_TWI_BASE + NRF51_TWI_TASK_STOP, 1);
 
-    writel(NRF51_TWI_BASE + NRF51_TWI_TASK_STARTRX, 1);
-    val = readl(NRF51_TWI_BASE + NRF51_TWI_EVENT_RXDREADY);
+    qtest_writel(qts, NRF51_TWI_BASE + NRF51_TWI_TASK_STARTRX, 1);
+    val = qtest_readl(qts, NRF51_TWI_BASE + NRF51_TWI_EVENT_RXDREADY);
     g_assert_cmpuint(val, ==, 1);
-    val = readl(NRF51_TWI_BASE + NRF51_TWI_REG_RXD);
-    writel(NRF51_TWI_BASE + NRF51_TWI_TASK_STOP, 1);
+    val = qtest_readl(qts, NRF51_TWI_BASE + NRF51_TWI_REG_RXD);
+    qtest_writel(qts, NRF51_TWI_BASE + NRF51_TWI_TASK_STOP, 1);
 
     return val;
 }
@@ -47,22 +47,25 @@ static uint32_t i2c_read_byte(uint32_t addr, uint32_t reg)
 static void test_microbit_i2c(void)
 {
     uint32_t val;
+    QTestState *qts = qtest_init("-M microbit");
 
     /* We don't program pins/irqs but at least enable the device */
-    writel(NRF51_TWI_BASE + NRF51_TWI_REG_ENABLE, 5);
+    qtest_writel(qts, NRF51_TWI_BASE + NRF51_TWI_REG_ENABLE, 5);
 
     /* MMA8653 magnetometer detection */
-    val = i2c_read_byte(0x3A, 0x0D);
+    val = i2c_read_byte(qts, 0x3A, 0x0D);
     g_assert_cmpuint(val, ==, 0x5A);
 
-    val = i2c_read_byte(0x3A, 0x0D);
+    val = i2c_read_byte(qts, 0x3A, 0x0D);
     g_assert_cmpuint(val, ==, 0x5A);
 
     /* LSM303 accelerometer detection */
-    val = i2c_read_byte(0x3C, 0x4F);
+    val = i2c_read_byte(qts, 0x3C, 0x4F);
     g_assert_cmpuint(val, ==, 0x40);
 
-    writel(NRF51_TWI_BASE + NRF51_TWI_REG_ENABLE, 0);
+    qtest_writel(qts, NRF51_TWI_BASE + NRF51_TWI_REG_ENABLE, 0);
+
+    qtest_quit(qts);
 }
 
 static void test_nrf51_gpio(void)
@@ -80,220 +83,228 @@ static void test_nrf51_gpio(void)
         {NRF51_GPIO_REG_DIRCLR, 0x00000000}
     };
 
+    QTestState *qts = qtest_init("-M microbit");
+
     /* Check reset state */
     for (i = 0; i < ARRAY_SIZE(reset_state); i++) {
         expected = reset_state[i].expected;
-        actual = readl(NRF51_GPIO_BASE + reset_state[i].addr);
+        actual = qtest_readl(qts, NRF51_GPIO_BASE + reset_state[i].addr);
         g_assert_cmpuint(actual, ==, expected);
     }
 
     for (i = 0; i < NRF51_GPIO_PINS; i++) {
         expected = 0x00000002;
-        actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START + i * 4);
+        actual = qtest_readl(qts, NRF51_GPIO_BASE +
+                                  NRF51_GPIO_REG_CNF_START + i * 4);
         g_assert_cmpuint(actual, ==, expected);
     }
 
     /* Check dir bit consistency between dir and cnf */
     /* Check set via DIRSET */
     expected = 0x80000001;
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_DIRSET, expected);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_DIR);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_DIRSET, expected);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_DIR);
     g_assert_cmpuint(actual, ==, expected);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START) & 0x01;
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START)
+             & 0x01;
     g_assert_cmpuint(actual, ==, 0x01);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_END) & 0x01;
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_END) & 0x01;
     g_assert_cmpuint(actual, ==, 0x01);
 
     /* Check clear via DIRCLR */
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_DIRCLR, 0x80000001);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_DIR);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_DIRCLR, 0x80000001);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_DIR);
     g_assert_cmpuint(actual, ==, 0x00000000);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START) & 0x01;
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START)
+             & 0x01;
     g_assert_cmpuint(actual, ==, 0x00);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_END) & 0x01;
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_END) & 0x01;
     g_assert_cmpuint(actual, ==, 0x00);
 
     /* Check set via DIR */
     expected = 0x80000001;
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_DIR, expected);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_DIR);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_DIR, expected);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_DIR);
     g_assert_cmpuint(actual, ==, expected);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START) & 0x01;
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START)
+             & 0x01;
     g_assert_cmpuint(actual, ==, 0x01);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_END) & 0x01;
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_END) & 0x01;
     g_assert_cmpuint(actual, ==, 0x01);
 
     /* Reset DIR */
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_DIR, 0x00000000);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_DIR, 0x00000000);
 
     /* Check Input propagates */
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0x00);
-    qtest_set_irq_in(global_qtest, "/machine/nrf51", "unnamed-gpio-in", 0, 0);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0x00);
+    qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, 0);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
     g_assert_cmpuint(actual, ==, 0x00);
-    qtest_set_irq_in(global_qtest, "/machine/nrf51", "unnamed-gpio-in", 0, 1);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
+    qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, 1);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
     g_assert_cmpuint(actual, ==, 0x01);
-    qtest_set_irq_in(global_qtest, "/machine/nrf51", "unnamed-gpio-in", 0, -1);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
+    qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, -1);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
     g_assert_cmpuint(actual, ==, 0x01);
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0x02);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0x02);
 
     /* Check pull-up working */
-    qtest_set_irq_in(global_qtest, "/machine/nrf51", "unnamed-gpio-in", 0, 0);
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b0000);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
+    qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, 0);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b0000);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
     g_assert_cmpuint(actual, ==, 0x00);
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b1110);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b1110);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
     g_assert_cmpuint(actual, ==, 0x01);
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0x02);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0x02);
 
     /* Check pull-down working */
-    qtest_set_irq_in(global_qtest, "/machine/nrf51", "unnamed-gpio-in", 0, 1);
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b0000);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
+    qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, 1);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b0000);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
     g_assert_cmpuint(actual, ==, 0x01);
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b0110);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b0110);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
     g_assert_cmpuint(actual, ==, 0x00);
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0x02);
-    qtest_set_irq_in(global_qtest, "/machine/nrf51", "unnamed-gpio-in", 0, -1);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0x02);
+    qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, -1);
 
     /* Check Output propagates */
-    irq_intercept_out("/machine/nrf51");
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b0011);
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_OUTSET, 0x01);
-    g_assert_true(get_irq(0));
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_OUTCLR, 0x01);
-    g_assert_false(get_irq(0));
+    qtest_irq_intercept_out(qts, "/machine/nrf51");
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b0011);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_OUTSET, 0x01);
+    g_assert_true(qtest_get_irq(qts, 0));
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_OUTCLR, 0x01);
+    g_assert_false(qtest_get_irq(qts, 0));
 
     /* Check self-stimulation */
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b01);
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_OUTSET, 0x01);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b01);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_OUTSET, 0x01);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
     g_assert_cmpuint(actual, ==, 0x01);
 
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_OUTCLR, 0x01);
-    actual = readl(NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_OUTCLR, 0x01);
+    actual = qtest_readl(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_IN) & 0x01;
     g_assert_cmpuint(actual, ==, 0x00);
 
     /*
      * Check short-circuit - generates an guest_error which must be checked
      * manually as long as qtest can not scan qemu_log messages
      */
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b01);
-    writel(NRF51_GPIO_BASE + NRF51_GPIO_REG_OUTSET, 0x01);
-    qtest_set_irq_in(global_qtest, "/machine/nrf51", "unnamed-gpio-in", 0, 0);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_CNF_START, 0b01);
+    qtest_writel(qts, NRF51_GPIO_BASE + NRF51_GPIO_REG_OUTSET, 0x01);
+    qtest_set_irq_in(qts, "/machine/nrf51", "unnamed-gpio-in", 0, 0);
+
+    qtest_quit(qts);
 }
 
-static void timer_task(hwaddr task)
+static void timer_task(QTestState *qts, hwaddr task)
 {
-    writel(NRF51_TIMER_BASE + task, NRF51_TRIGGER_TASK);
+    qtest_writel(qts, NRF51_TIMER_BASE + task, NRF51_TRIGGER_TASK);
 }
 
-static void timer_clear_event(hwaddr event)
+static void timer_clear_event(QTestState *qts, hwaddr event)
 {
-    writel(NRF51_TIMER_BASE + event, NRF51_EVENT_CLEAR);
+    qtest_writel(qts, NRF51_TIMER_BASE + event, NRF51_EVENT_CLEAR);
 }
 
-static void timer_set_bitmode(uint8_t mode)
+static void timer_set_bitmode(QTestState *qts, uint8_t mode)
 {
-    writel(NRF51_TIMER_BASE + NRF51_TIMER_REG_BITMODE, mode);
+    qtest_writel(qts, NRF51_TIMER_BASE + NRF51_TIMER_REG_BITMODE, mode);
 }
 
-static void timer_set_prescaler(uint8_t prescaler)
+static void timer_set_prescaler(QTestState *qts, uint8_t prescaler)
 {
-    writel(NRF51_TIMER_BASE + NRF51_TIMER_REG_PRESCALER, prescaler);
+    qtest_writel(qts, NRF51_TIMER_BASE + NRF51_TIMER_REG_PRESCALER, prescaler);
 }
 
-static void timer_set_cc(size_t idx, uint32_t value)
+static void timer_set_cc(QTestState *qts, size_t idx, uint32_t value)
 {
-    writel(NRF51_TIMER_BASE + NRF51_TIMER_REG_CC0 + idx * 4, value);
+    qtest_writel(qts, NRF51_TIMER_BASE + NRF51_TIMER_REG_CC0 + idx * 4, value);
 }
 
-static void timer_assert_events(uint32_t ev0, uint32_t ev1, uint32_t ev2,
-                                uint32_t ev3)
+static void timer_assert_events(QTestState *qts, uint32_t ev0, uint32_t ev1,
+                                uint32_t ev2, uint32_t ev3)
 {
-    g_assert(readl(NRF51_TIMER_BASE + NRF51_TIMER_EVENT_COMPARE_0) == ev0);
-    g_assert(readl(NRF51_TIMER_BASE + NRF51_TIMER_EVENT_COMPARE_1) == ev1);
-    g_assert(readl(NRF51_TIMER_BASE + NRF51_TIMER_EVENT_COMPARE_2) == ev2);
-    g_assert(readl(NRF51_TIMER_BASE + NRF51_TIMER_EVENT_COMPARE_3) == ev3);
+    g_assert(qtest_readl(qts, NRF51_TIMER_BASE + NRF51_TIMER_EVENT_COMPARE_0)
+             == ev0);
+    g_assert(qtest_readl(qts, NRF51_TIMER_BASE + NRF51_TIMER_EVENT_COMPARE_1)
+             == ev1);
+    g_assert(qtest_readl(qts, NRF51_TIMER_BASE + NRF51_TIMER_EVENT_COMPARE_2)
+             == ev2);
+    g_assert(qtest_readl(qts, NRF51_TIMER_BASE + NRF51_TIMER_EVENT_COMPARE_3)
+             == ev3);
 }
 
 static void test_nrf51_timer(void)
 {
     uint32_t steps_to_overflow = 408;
+    QTestState *qts = qtest_init("-M microbit");
 
     /* Compare Match */
-    timer_task(NRF51_TIMER_TASK_STOP);
-    timer_task(NRF51_TIMER_TASK_CLEAR);
+    timer_task(qts, NRF51_TIMER_TASK_STOP);
+    timer_task(qts, NRF51_TIMER_TASK_CLEAR);
 
-    timer_clear_event(NRF51_TIMER_EVENT_COMPARE_0);
-    timer_clear_event(NRF51_TIMER_EVENT_COMPARE_1);
-    timer_clear_event(NRF51_TIMER_EVENT_COMPARE_2);
-    timer_clear_event(NRF51_TIMER_EVENT_COMPARE_3);
+    timer_clear_event(qts, NRF51_TIMER_EVENT_COMPARE_0);
+    timer_clear_event(qts, NRF51_TIMER_EVENT_COMPARE_1);
+    timer_clear_event(qts, NRF51_TIMER_EVENT_COMPARE_2);
+    timer_clear_event(qts, NRF51_TIMER_EVENT_COMPARE_3);
 
-    timer_set_bitmode(NRF51_TIMER_WIDTH_16); /* 16 MHz Timer */
-    timer_set_prescaler(0);
+    timer_set_bitmode(qts, NRF51_TIMER_WIDTH_16); /* 16 MHz Timer */
+    timer_set_prescaler(qts, 0);
     /* Swept over in first step */
-    timer_set_cc(0, 2);
+    timer_set_cc(qts, 0, 2);
     /* Barely miss on first step */
-    timer_set_cc(1, 162);
+    timer_set_cc(qts, 1, 162);
     /* Spot on on third step */
-    timer_set_cc(2, 480);
+    timer_set_cc(qts, 2, 480);
 
-    timer_assert_events(0, 0, 0, 0);
+    timer_assert_events(qts, 0, 0, 0, 0);
 
-    timer_task(NRF51_TIMER_TASK_START);
-    clock_step(10000);
-    timer_assert_events(1, 0, 0, 0);
+    timer_task(qts, NRF51_TIMER_TASK_START);
+    qtest_clock_step(qts, 10000);
+    timer_assert_events(qts, 1, 0, 0, 0);
 
     /* Swept over on first overflow */
-    timer_set_cc(3, 114);
+    timer_set_cc(qts, 3, 114);
 
-    clock_step(10000);
-    timer_assert_events(1, 1, 0, 0);
+    qtest_clock_step(qts, 10000);
+    timer_assert_events(qts, 1, 1, 0, 0);
 
-    clock_step(10000);
-    timer_assert_events(1, 1, 1, 0);
+    qtest_clock_step(qts, 10000);
+    timer_assert_events(qts, 1, 1, 1, 0);
 
     /* Wrap time until internal counter overflows */
     while (steps_to_overflow--) {
-        timer_assert_events(1, 1, 1, 0);
-        clock_step(10000);
+        timer_assert_events(qts, 1, 1, 1, 0);
+        qtest_clock_step(qts, 10000);
     }
 
-    timer_assert_events(1, 1, 1, 1);
+    timer_assert_events(qts, 1, 1, 1, 1);
 
-    timer_clear_event(NRF51_TIMER_EVENT_COMPARE_0);
-    timer_clear_event(NRF51_TIMER_EVENT_COMPARE_1);
-    timer_clear_event(NRF51_TIMER_EVENT_COMPARE_2);
-    timer_clear_event(NRF51_TIMER_EVENT_COMPARE_3);
-    timer_assert_events(0, 0, 0, 0);
+    timer_clear_event(qts, NRF51_TIMER_EVENT_COMPARE_0);
+    timer_clear_event(qts, NRF51_TIMER_EVENT_COMPARE_1);
+    timer_clear_event(qts, NRF51_TIMER_EVENT_COMPARE_2);
+    timer_clear_event(qts, NRF51_TIMER_EVENT_COMPARE_3);
+    timer_assert_events(qts, 0, 0, 0, 0);
 
-    timer_task(NRF51_TIMER_TASK_STOP);
+    timer_task(qts, NRF51_TIMER_TASK_STOP);
 
     /* Test Proposal: Stop/Shutdown */
     /* Test Proposal: Shortcut Compare -> Clear */
     /* Test Proposal: Shortcut Compare -> Stop */
     /* Test Proposal: Counter Mode */
+
+    qtest_quit(qts);
 }
 
 int main(int argc, char **argv)
 {
-    int ret;
-
     g_test_init(&argc, &argv, NULL);
 
-    global_qtest = qtest_initf("-machine microbit");
-
     qtest_add_func("/microbit/nrf51/gpio", test_nrf51_gpio);
     qtest_add_func("/microbit/nrf51/timer", test_nrf51_timer);
     qtest_add_func("/microbit/microbit/i2c", test_microbit_i2c);
 
-    ret = g_test_run();
-
-    qtest_quit(global_qtest);
-    return ret;
+    return g_test_run();
 }
-- 
2.17.1

[Qemu-devel] [PATCH v5 3/3] tests/microbit-test: Check nRF51 UART functionality

[Julia Suvorova]

Some functional tests for:
    Basic reception/transmittion
    Suspending
    INTEN* registers

Signed-off-by: Julia Suvorova <jusual@mail.ru>
---
 tests/microbit-test.c | 89 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/tests/microbit-test.c b/tests/microbit-test.c
index afeb6b082a..fe0f2979f8 100644
--- a/tests/microbit-test.c
+++ b/tests/microbit-test.c
@@ -19,10 +19,98 @@
 #include "libqtest.h"
 
 #include "hw/arm/nrf51.h"
+#include "hw/char/nrf51_uart.h"
 #include "hw/gpio/nrf51_gpio.h"
 #include "hw/timer/nrf51_timer.h"
 #include "hw/i2c/microbit_i2c.h"
 
+static bool uart_wait_for_event(QTestState *qts, uint32_t event_addr)
+{
+    time_t now, start = time(NULL);
+
+    while (true) {
+        if (qtest_readl(qts, event_addr) == 1) {
+            qtest_writel(qts, event_addr, 0x00);
+            return true;
+        }
+
+        /* Wait at most 10 minutes */
+        now = time(NULL);
+        if (now - start > 600) {
+            break;
+        }
+        g_usleep(10000);
+    }
+
+    return false;
+}
+
+static void uart_rw_to_rxd(QTestState *qts, int sock_fd, const char *in,
+                           char *out)
+{
+    int i, in_len = strlen(in);
+
+    g_assert_true(write(sock_fd, in, in_len) == in_len);
+    for (i = 0; i < in_len; i++) {
+        g_assert_true(uart_wait_for_event(qts, NRF51_UART_BASE +
+                                               A_UART_RXDRDY));
+        out[i] = qtest_readl(qts, NRF51_UART_BASE + A_UART_RXD);
+    }
+    out[i] = '\0';
+}
+
+static void uart_w_to_txd(QTestState *qts, const char *in)
+{
+    int i, in_len = strlen(in);
+
+    for (i = 0; i < in_len; i++) {
+        qtest_writel(qts, NRF51_UART_BASE + A_UART_TXD, in[i]);
+        g_assert_true(uart_wait_for_event(qts, NRF51_UART_BASE +
+                                               A_UART_TXDRDY));
+    }
+}
+
+static void test_nrf51_uart(void)
+{
+    int sock_fd;
+    char s[10];
+    QTestState *qts = qtest_init_with_serial("-M microbit", &sock_fd);
+
+    g_assert_true(write(sock_fd, "c", 1) == 1);
+    g_assert_cmphex(qtest_readl(qts, NRF51_UART_BASE + A_UART_RXD), ==, 0x00);
+
+    qtest_writel(qts, NRF51_UART_BASE + A_UART_ENABLE, 0x04);
+    qtest_writel(qts, NRF51_UART_BASE + A_UART_STARTRX, 0x01);
+
+    g_assert_true(uart_wait_for_event(qts, NRF51_UART_BASE + A_UART_RXDRDY));
+    qtest_writel(qts, NRF51_UART_BASE + A_UART_RXDRDY, 0x00);
+    g_assert_cmphex(qtest_readl(qts, NRF51_UART_BASE + A_UART_RXD), ==, 'c');
+
+    qtest_writel(qts, NRF51_UART_BASE + A_UART_INTENSET, 0x04);
+    g_assert_cmphex(qtest_readl(qts, NRF51_UART_BASE + A_UART_INTEN), ==, 0x04);
+    qtest_writel(qts, NRF51_UART_BASE + A_UART_INTENCLR, 0x04);
+    g_assert_cmphex(qtest_readl(qts, NRF51_UART_BASE + A_UART_INTEN), ==, 0x00);
+
+    uart_rw_to_rxd(qts, sock_fd, "hello", s);
+    g_assert_true(memcmp(s, "hello", 5) == 0);
+
+    qtest_writel(qts, NRF51_UART_BASE + A_UART_STARTTX, 0x01);
+    uart_w_to_txd(qts, "d");
+    g_assert_true(read(sock_fd, s, 10) == 1);
+    g_assert_cmphex(s[0], ==, 'd');
+
+    qtest_writel(qts, NRF51_UART_BASE + A_UART_SUSPEND, 0x01);
+    qtest_writel(qts, NRF51_UART_BASE + A_UART_TXD, 'h');
+    qtest_writel(qts, NRF51_UART_BASE + A_UART_STARTTX, 0x01);
+    uart_w_to_txd(qts, "world");
+    g_assert_true(read(sock_fd, s, 10) == 5);
+    g_assert_true(memcmp(s, "world", 5) == 0);
+
+    close(sock_fd);
+
+    qtest_quit(qts);
+}
+
 /* Read a byte from I2C device at @addr from register @reg */
 static uint32_t i2c_read_byte(QTestState *qts, uint32_t addr, uint32_t reg)
 {
@@ -302,6 +390,7 @@ int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
 
+    qtest_add_func("/microbit/nrf51/uart", test_nrf51_uart);
     qtest_add_func("/microbit/nrf51/gpio", test_nrf51_gpio);
     qtest_add_func("/microbit/nrf51/timer", test_nrf51_timer);
     qtest_add_func("/microbit/microbit/i2c", test_microbit_i2c);
-- 
2.17.1

Re: [Qemu-devel] [PATCH v5 3/3] tests/microbit-test: Check nRF51 UART functionality

[Thomas Huth]

On 2019-01-23 13:07, Julia Suvorova wrote:
> Some functional tests for:
>     Basic reception/transmittion
>     Suspending
>     INTEN* registers
> 
> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> ---
>  tests/microbit-test.c | 89 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)

Acked-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v5 3/3] tests/microbit-test: Check nRF51 UART functionality

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

On Wed, Jan 23, 2019 at 03:07:59PM +0300, Julia Suvorova wrote:
> Some functional tests for:
>     Basic reception/transmittion
>     Suspending
>     INTEN* registers
> 
> Signed-off-by: Julia Suvorova <jusual@mail.ru>
> ---
>  tests/microbit-test.c | 89 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 89 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

Re: [Qemu-devel] [PATCH v5 0/3] tests/microbit-test: Add UART device test

[Peter Maydell]

On Wed, 23 Jan 2019 at 12:08, Julia Suvorova <jusual@mail.ru> wrote:
>
> v5:
>     * Replace g_assert with g_assert_* [Thomas, Alex]
>     * Increase the waiting time for an event [Thomas]
>     * Remove 'nowait' [Thomas]
>     * Rewrite uart_wait_for_event to use time difference instead of loop
> v4:
>     * Replace sprintf with g_strdup_printf [Peter]
>     * Move socket connection to qtest library [Peter]
>     * Use memcmp instead of strcmp [Stefan]
>     * Remove using global_qtest [Thomas]
> v3:
>     * Fix directory leak [Stefan]
>
> Based-on: <20190110094020.18354-1-stefanha@redhat.com>



Applied to target-arm.next, thanks.

-- PMM