Re: [Qemu-devel] [PATCH 2/2] tests: Add tmp105 unit test

[Alex Horn]

Thanks so much for taking the initiative on creating functional tests
for the TMP105 hardware model. I am exciting to see these changes and
I am wondering if your QTests could be complemented with the following
standalone unit tests:

  
https://github.com/ahorn/benchmarks/blob/965e571d92e677e8e64ad5faa0107f5dbd451981/qemu-hw/tmp105/tmp105-test.c

Their purpose is similar to yours except that unit tests focus on
checking the internal state of a hardware model without requiring a
bus implementation or a socket connection for the QTest client-server
infrastructure. That is, unit tests do not seek to replace QTests but
strengthen them (see also below).

In fact, I have tried including the above unit tests in QEMU. For
this, the following lines were added to tests/Makefile:

  check-unit-y += tests/test-tmp105$(EXESUF)
  tests/test-tmp105$(EXESUF): tests/test-tmp105.o hw/tmp105.o
$(qobject-obj-y) $(common-obj-y) $(qom-obj-y)

The last three $(...) variables are required to compile. However,
"make check" results in a NSS linking error:

/usr/bin/ld: libcacard/vcard_emul_nss.o: undefined reference to symbol
'CERT_GetDefaultCertDB@@NSS_3.2'
/usr/bin/ld: note: 'CERT_GetDefaultCertDB@@NSS_3.2' is defined in DSO
/usr/lib64/libnss3.so so try adding it to the linker command line
/usr/lib64/libnss3.so: could not read symbols: Invalid operation
collect2: ld returned 1 exit status
make: *** [tests/test-tmp105] Error 1

If these type of issues could be resolved, it would be possible to
encourage others to write unit tests for their hardware models.

The utility of these unit tests is manifold. Noteworthy, a certain
level of confidence can be achieved before integrating a hardware
model into a bus implementation (e.g. OMAP or SMBus for I2C). This
could also prove useful for overall regression testing and debugging.
Finally, unit tests encourage modular designs (e.g. [1]). Such
modularity could also be a catalyst for new QEMU use cases and a
potentially broader developer base.

With kind regards,
Alex

[1] http://en.wikipedia.org/wiki/Dependency_injection

On 12 December 2012 06:29, Andreas Färber <address@hidden> wrote:
> Exercise all four commands of the TMP105, testing for an issue in the
> I2C TX path. The test case is specific to the n800's OMAP I2C for now
> and is the first test case for arm.
>
> Signed-off-by: Andreas Färber <address@hidden>
> ---
>  tests/Makefile      |    2 +
>  tests/tmp105-test.c |  205 
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 Dateien geändert, 207 Zeilen hinzugefügt(+)
>  create mode 100644 tests/tmp105-test.c
>
> diff --git a/tests/Makefile b/tests/Makefile
> index b60f0fb..69eff45 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -28,6 +28,7 @@ check-qtest-i386-y += tests/rtc-test$(EXESUF)
>  check-qtest-x86_64-y = $(check-qtest-i386-y)
>  check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
>  check-qtest-sparc64-y = tests/m48t59-test$(EXESUF)
> +check-qtest-arm-y = tests/tmp105-test$(EXESUF)
>
>  GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h 
> tests/test-qmp-commands.h
>
> @@ -78,6 +79,7 @@ tests/rtc-test$(EXESUF): tests/rtc-test.o $(trace-obj-y)
>  tests/m48t59-test$(EXESUF): tests/m48t59-test.o $(trace-obj-y)
>  tests/fdc-test$(EXESUF): tests/fdc-test.o tests/libqtest.o $(trace-obj-y)
>  tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o tests/libqtest.o 
> $(trace-obj-y)
> +tests/tmp105-test$(EXESUF): tests/tmp105-test.o $(trace-obj-y)
>
>  # QTest rules
>
> diff --git a/tests/tmp105-test.c b/tests/tmp105-test.c
> new file mode 100644
> index 0000000..26d51f7
> --- /dev/null
> +++ b/tests/tmp105-test.c
> @@ -0,0 +1,205 @@
> +/*
> + * QTest testcase for the TMP105 temperature sensor
> + *
> + * Copyright (c) 2012 Andreas Färber
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#include "libqtest.h"
> +
> +#include <glib.h>
> +#include <string.h>
> +
> +enum OMAPI2CRegisters {
> +    OMAP_I2C_REV  = 0x00,
> +    OMAP_I2C_STAT = 0x08,
> +    OMAP_I2C_CNT  = 0x18,
> +    OMAP_I2C_DATA = 0x1c,
> +    OMAP_I2C_CON  = 0x24,
> +    OMAP_I2C_SA   = 0x2c,
> +};
> +
> +enum OMAPI2CSTATBits {
> +    OMAP_I2C_STAT_NACK = 1 << 1,
> +    OMAP_I2C_STAT_ARDY = 1 << 2,
> +    OMAP_I2C_STAT_RRDY = 1 << 3,
> +    OMAP_I2C_STAT_XRDY = 1 << 4,
> +    OMAP_I2C_STAT_ROVR = 1 << 11,
> +    OMAP_I2C_STAT_SBD  = 1 << 15,
> +};
> +
> +enum OMAPI2CCONBits {
> +    OMAP_I2C_CON_STT    = 1 << 0,
> +    OMAP_I2C_CON_STP    = 1 << 1,
> +    OMAP_I2C_CON_TRX    = 1 << 9,
> +    OMAP_I2C_CON_MST    = 1 << 10,
> +    OMAP_I2C_CON_BE     = 1 << 14,
> +    OMAP_I2C_CON_I2C_EN = 1 << 15,
> +};
> +
> +#define OMAP2_I2C_1_BASE 0x48070000
> +
> +#define N8X0_ADDR 0x48
> +
> +static const uint32_t i2c_base = OMAP2_I2C_1_BASE;
> +
> +static void omap_i2c_init(void)
> +{
> +    uint16_t data;
> +
> +    /* verify the mmio address by looking for a known signature */
> +    memread(i2c_base + OMAP_I2C_REV, &data, 2);
> +    g_assert_cmphex(data, ==, 0x34);
> +}
> +
> +static void omap_i2c_set_slave_addr(uint8_t addr)
> +{
> +    uint16_t data = addr;
> +
> +    memwrite(i2c_base + OMAP_I2C_SA, &data, 2);
> +    memread(i2c_base + OMAP_I2C_SA, &data, 2);
> +    g_assert_cmphex(data, ==, addr);
> +}
> +
> +static void omap_i2c_send(uint8_t addr, const void *buf, uint16_t len)
> +{
> +    uint16_t data;
> +
> +    omap_i2c_set_slave_addr(addr);
> +
> +    data = len;
> +    memwrite(i2c_base + OMAP_I2C_CNT, &data, 2);
> +
> +    data = OMAP_I2C_CON_I2C_EN |
> +           OMAP_I2C_CON_TRX |
> +           OMAP_I2C_CON_MST |
> +           OMAP_I2C_CON_STT |
> +           OMAP_I2C_CON_STP;
> +    memwrite(i2c_base + OMAP_I2C_CON, &data, 2);
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) != 0);
> +
> +    memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +    g_assert((data & OMAP_I2C_STAT_NACK) == 0);
> +
> +    while (len > 1) {
> +        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +        g_assert((data & OMAP_I2C_STAT_XRDY) != 0);
> +
> +        memwrite(i2c_base + OMAP_I2C_DATA, buf, 2);
> +        buf = (uint8_t *)buf + 2;
> +        len -= 2;
> +    }
> +    if (len == 1) {
> +        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +        g_assert((data & OMAP_I2C_STAT_XRDY) != 0);
> +
> +        memwrite(i2c_base + OMAP_I2C_DATA, buf, 1);
> +    }
> +
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) == 0);
> +}
> +
> +static void omap_i2c_recv(uint8_t addr, void *buf, uint16_t len)
> +{
> +    uint16_t data, stat;
> +
> +    omap_i2c_set_slave_addr(addr);
> +
> +    data = len;
> +    memwrite(i2c_base + OMAP_I2C_CNT, &data, 2);
> +
> +    data = OMAP_I2C_CON_I2C_EN |
> +           OMAP_I2C_CON_MST |
> +           OMAP_I2C_CON_STT |
> +           OMAP_I2C_CON_STP;
> +    memwrite(i2c_base + OMAP_I2C_CON, &data, 2);
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) == 0);
> +
> +    memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +    g_assert((data & OMAP_I2C_STAT_NACK) == 0);
> +
> +    memread(i2c_base + OMAP_I2C_CNT, &data, 2);
> +    g_assert_cmpuint(data, ==, len);
> +
> +    while (len > 0) {
> +        memread(i2c_base + OMAP_I2C_STAT, &data, 2);
> +        g_assert((data & OMAP_I2C_STAT_RRDY) != 0);
> +        g_assert((data & OMAP_I2C_STAT_ROVR) == 0);
> +
> +        memread(i2c_base + OMAP_I2C_DATA, &data, 2);
> +
> +        memread(i2c_base + OMAP_I2C_STAT, &stat, 2);
> +        if ((stat & OMAP_I2C_STAT_SBD) != 0) {
> +            *(uint8_t *)buf = data & 0xf;
> +            buf = (uint8_t *)buf + 1;
> +            len--;
> +        } else {
> +            memcpy(buf, &data, 2);
> +            buf = (uint8_t *)buf + 2;
> +            len -= 2;
> +        }
> +    }
> +
> +    memread(i2c_base + OMAP_I2C_CON, &data, 2);
> +    g_assert((data & OMAP_I2C_CON_STP) == 0);
> +}
> +
> +static void send_and_receive(void)
> +{
> +    const uint8_t addr = N8X0_ADDR;
> +    uint8_t cmd[3];
> +    uint8_t resp[2];
> +
> +    omap_i2c_init();
> +
> +    cmd[0] = 0;
> +    omap_i2c_send(addr, cmd, 1);
> +    omap_i2c_recv(addr, resp, 2);
> +    g_assert_cmpuint(((uint16_t)resp[0] << 8) | resp[1], ==, 0);
> +
> +    cmd[0] = 1;
> +    cmd[1] = 0x0;
> +    omap_i2c_send(addr, cmd, 2);
> +    omap_i2c_recv(addr, resp, 1);
> +    g_assert_cmphex(resp[0], ==, cmd[1]);
> +
> +    cmd[0] = 2;
> +    cmd[1] = 0x12;
> +    cmd[2] = 0x34;
> +    omap_i2c_send(addr, cmd, 3);
> +    omap_i2c_recv(addr, resp, 2);
> +    g_assert_cmphex(resp[0], ==, cmd[1]);
> +    g_assert_cmphex(resp[1], ==, cmd[2]);
> +
> +    cmd[0] = 3;
> +    cmd[1] = 0x42;
> +    cmd[2] = 0x31;
> +    omap_i2c_send(addr, cmd, 3);
> +    omap_i2c_recv(addr, resp, 2);
> +    g_assert_cmphex(resp[0], ==, cmd[1]);
> +    g_assert_cmphex(resp[1], ==, cmd[2]);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    QTestState *s = NULL;
> +    int ret;
> +
> +    g_test_init(&argc, &argv, NULL);
> +
> +    s = qtest_start("-display none -machine n800");
> +
> +    qtest_add_func("/tmp105/tx-rx", send_and_receive);
> +
> +    ret = g_test_run();
> +
> +    if (s) {
> +        qtest_quit(s);
> +    }
> +
> +    return ret;
> +}
> --
> 1.7.10.4
>