[PATCH v3] tests/qtest: add qtests for npcm7xx sdhci

[PATCH v3] tests/qtest: add qtests for npcm7xx sdhci

[Patrick Venture]

From: Shengtan Mao <stmao@google.com>

Reviewed-by: Hao Wu <wuhaotsh@google.com>
Reviewed-by: Chris Rauer <crauer@google.com>
Signed-off-by: Shengtan Mao <stmao@google.com>
Signed-off-by: Patrick Venture <venture@google.com>
---
v3:
 * fixup compilation from missing macro value
v2:
 * update copyright year
 * check result of open
 * use g_free instead of free
 * move declarations to the top
 * use g_file_open_tmp
---
 tests/qtest/meson.build          |   1 +
 tests/qtest/npcm7xx_sdhci-test.c | 215 +++++++++++++++++++++++++++++++
 2 files changed, 216 insertions(+)
 create mode 100644 tests/qtest/npcm7xx_sdhci-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 842b1df420..2b566999cd 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -189,6 +189,7 @@ qtests_npcm7xx = \
    'npcm7xx_gpio-test',
    'npcm7xx_pwm-test',
    'npcm7xx_rng-test',
+   'npcm7xx_sdhci-test',
    'npcm7xx_smbus-test',
    'npcm7xx_timer-test',
    'npcm7xx_watchdog_timer-test'] + \
diff --git a/tests/qtest/npcm7xx_sdhci-test.c b/tests/qtest/npcm7xx_sdhci-test.c
new file mode 100644
index 0000000000..7de28f900b
--- /dev/null
+++ b/tests/qtest/npcm7xx_sdhci-test.c
@@ -0,0 +1,215 @@
+/*
+ * QTests for NPCM7xx SD-3.0 / MMC-4.51 Host Controller
+ *
+ * Copyright (c) 2022 Google LLC
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/sd/npcm7xx_sdhci.h"
+
+#include "libqos/libqtest.h"
+#include "libqtest-single.h"
+#include "libqos/sdhci-cmd.h"
+
+#define NPCM7XX_REG_SIZE 0x100
+#define NPCM7XX_MMC_BA 0xF0842000
+#define NPCM7XX_BLK_SIZE 512
+#define NPCM7XX_TEST_IMAGE_SIZE (1 << 30)
+
+char *sd_path;
+
+static QTestState *setup_sd_card(void)
+{
+    QTestState *qts = qtest_initf(
+        "-machine kudo-bmc "
+        "-device sd-card,drive=drive0 "
+        "-drive id=drive0,if=none,file=%s,format=raw,auto-read-only=off",
+        sd_path);
+
+    qtest_writew(qts, NPCM7XX_MMC_BA + SDHC_SWRST, SDHC_RESET_ALL);
+    qtest_writew(qts, NPCM7XX_MMC_BA + SDHC_CLKCON,
+                 SDHC_CLOCK_SDCLK_EN | SDHC_CLOCK_INT_STABLE |
+                     SDHC_CLOCK_INT_EN);
+    sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_APP_CMD);
+    sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x41200000, 0, (41 << 8));
+    sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_ALL_SEND_CID);
+    sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_SEND_RELATIVE_ADDR);
+    sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x45670000, 0,
+                   SDHC_SELECT_DESELECT_CARD);
+
+    return qts;
+}
+
+static void write_sdread(QTestState *qts, const char *msg)
+{
+    int fd, ret;
+    size_t len = strlen(msg);
+    char *rmsg = g_malloc(len);
+
+    /* write message to sd */
+    fd = open(sd_path, O_WRONLY);
+    g_assert(fd >= 0);
+    ret = write(fd, msg, len);
+    close(fd);
+    g_assert(ret == len);
+
+    /* read message using sdhci */
+    ret = sdhci_read_cmd(qts, NPCM7XX_MMC_BA, rmsg, len);
+    g_assert(ret == len);
+    g_assert(!strcmp(rmsg, msg));
+
+    g_free(rmsg);
+}
+
+/* Check MMC can read values from sd */
+static void test_read_sd(void)
+{
+    QTestState *qts = setup_sd_card();
+
+    write_sdread(qts, "hello world");
+    write_sdread(qts, "goodbye");
+
+    qtest_quit(qts);
+}
+
+static void sdwrite_read(QTestState *qts, const char *msg)
+{
+    int fd, ret;
+    size_t len = strlen(msg);
+    char *rmsg = g_malloc(len);
+
+    /* write message using sdhci */
+    sdhci_write_cmd(qts, NPCM7XX_MMC_BA, msg, len, NPCM7XX_BLK_SIZE);
+
+    /* read message from sd */
+    fd = open(sd_path, O_RDONLY);
+    g_assert(fd >= 0);
+    ret = read(fd, rmsg, len);
+    close(fd);
+    g_assert(ret == len);
+
+    g_assert(!strcmp(rmsg, msg));
+
+    g_free(rmsg);
+}
+
+/* Check MMC can write values to sd */
+static void test_write_sd(void)
+{
+    QTestState *qts = setup_sd_card();
+
+    sdwrite_read(qts, "hello world");
+    sdwrite_read(qts, "goodbye");
+
+    qtest_quit(qts);
+}
+
+/* Check SDHCI has correct default values. */
+static void test_reset(void)
+{
+    QTestState *qts = qtest_init("-machine kudo-bmc");
+    uint64_t addr = NPCM7XX_MMC_BA;
+    uint64_t end_addr = addr + NPCM7XX_REG_SIZE;
+    uint16_t prstvals_resets[] = {NPCM7XX_PRSTVALS_0_RESET,
+                                  NPCM7XX_PRSTVALS_1_RESET,
+                                  0,
+                                  NPCM7XX_PRSTVALS_3_RESET,
+                                  0,
+                                  0};
+    int i;
+    uint32_t mask;
+
+    while (addr < end_addr) {
+        switch (addr - NPCM7XX_MMC_BA) {
+        case SDHC_PRNSTS:
+            /*
+             * ignores bits 20 to 24: they are changed when reading registers
+             */
+            mask = 0x1f00000;
+            g_assert_cmphex(qtest_readl(qts, addr) | mask, ==,
+                            NPCM7XX_PRSNTS_RESET | mask);
+            addr += 4;
+            break;
+        case SDHC_BLKGAP:
+            g_assert_cmphex(qtest_readb(qts, addr), ==, NPCM7XX_BLKGAP_RESET);
+            addr += 1;
+            break;
+        case SDHC_CAPAB:
+            g_assert_cmphex(qtest_readq(qts, addr), ==, NPCM7XX_CAPAB_RESET);
+            addr += 8;
+            break;
+        case SDHC_MAXCURR:
+            g_assert_cmphex(qtest_readq(qts, addr), ==, NPCM7XX_MAXCURR_RESET);
+            addr += 8;
+            break;
+        case SDHC_HCVER:
+            g_assert_cmphex(qtest_readw(qts, addr), ==, NPCM7XX_HCVER_RESET);
+            addr += 2;
+            break;
+        case NPCM7XX_PRSTVALS:
+            for (i = 0; i < NPCM7XX_PRSTVALS_SIZE; ++i) {
+                g_assert_cmphex(qtest_readw(qts, addr + 2 * i), ==,
+                                prstvals_resets[i]);
+            }
+            addr += NPCM7XX_PRSTVALS_SIZE * 2;
+            break;
+        default:
+            g_assert_cmphex(qtest_readb(qts, addr), ==, 0);
+            addr += 1;
+        }
+    }
+
+    qtest_quit(qts);
+}
+
+static void drive_destroy(void)
+{
+    unlink(sd_path);
+    g_free(sd_path);
+}
+
+static void drive_create(void)
+{
+    int fd, ret;
+    GError *error = NULL;
+
+    /* Create a temporary raw image */
+    fd = g_file_open_tmp("sdhci_XXXXXX", &sd_path, &error);
+    if (fd == -1) {
+        fprintf(stderr, "unable to create sdhci file: %s\n", error->message);
+        g_error_free(error);
+    }
+    g_assert(sd_path != NULL);
+
+    ret = ftruncate(fd, NPCM7XX_TEST_IMAGE_SIZE);
+    g_assert_cmpint(ret, ==, 0);
+    g_message("%s", sd_path);
+    close(fd);
+}
+
+int main(int argc, char **argv)
+{
+    int ret;
+
+    drive_create();
+
+    g_test_init(&argc, &argv, NULL);
+
+    qtest_add_func("npcm7xx_sdhci/reset", test_reset);
+    qtest_add_func("npcm7xx_sdhci/write_sd", test_write_sd);
+    qtest_add_func("npcm7xx_sdhci/read_sd", test_read_sd);
+
+    ret = g_test_run();
+    drive_destroy();
+    return ret;
+}
-- 
2.35.0.263.gb82422642f-goog

Re: [PATCH v3] tests/qtest: add qtests for npcm7xx sdhci

[Peter Maydell]

On Tue, 8 Feb 2022 at 18:18, Patrick Venture <venture@google.com> wrote:
>
> From: Shengtan Mao <stmao@google.com>
>
> Reviewed-by: Hao Wu <wuhaotsh@google.com>
> Reviewed-by: Chris Rauer <crauer@google.com>
> Signed-off-by: Shengtan Mao <stmao@google.com>
> Signed-off-by: Patrick Venture <venture@google.com>
> ---



Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH v3] tests/qtest: add qtests for npcm7xx sdhci

[Peter Maydell]

On Wed, 16 Feb 2022 at 17:30, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 8 Feb 2022 at 18:18, Patrick Venture <venture@google.com> wrote:
> >
> > From: Shengtan Mao <stmao@google.com>
> >
> > Reviewed-by: Hao Wu <wuhaotsh@google.com>
> > Reviewed-by: Chris Rauer <crauer@google.com>
> > Signed-off-by: Shengtan Mao <stmao@google.com>
> > Signed-off-by: Patrick Venture <venture@google.com>
> > ---
>
>
>
> Applied to target-arm.next, thanks.

This hits assertions in some of the CI jobs, eg:
https://gitlab.com/qemu-project/qemu/-/jobs/2116932769

258/717 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_sdhci-test INTERRUPT
643.16s killed by signal 6 SIGABRT
――――――――――――――――――――――――――――――――――――― ✀ ―――――――――――――――――――――――――――――――――――――
stderr:
** Message: 06:06:50.205: /tmp/sdhci_F7ETH1
**
ERROR:../tests/qtest/npcm7xx_sdhci-test.c:101:sdwrite_read: assertion
failed: (!strcmp(rmsg, msg))
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
...terminated.

so I've dropped it again.

thanks
-- PMM

Re: [PATCH v3] tests/qtest: add qtests for npcm7xx sdhci

[Patrick Venture]

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

On Mon, Feb 21, 2022 at 5:30 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Wed, 16 Feb 2022 at 17:30, Peter Maydell <peter.maydell@linaro.org>
> wrote:
> >
> > On Tue, 8 Feb 2022 at 18:18, Patrick Venture <venture@google.com> wrote:
> > >
> > > From: Shengtan Mao <stmao@google.com>
> > >
> > > Reviewed-by: Hao Wu <wuhaotsh@google.com>
> > > Reviewed-by: Chris Rauer <crauer@google.com>
> > > Signed-off-by: Shengtan Mao <stmao@google.com>
> > > Signed-off-by: Patrick Venture <venture@google.com>
> > > ---
> >
> >
> >
> > Applied to target-arm.next, thanks.
>
> This hits assertions in some of the CI jobs, eg:
> https://gitlab.com/qemu-project/qemu/-/jobs/2116932769
>
> 258/717 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_sdhci-test INTERRUPT
> 643.16s killed by signal 6 SIGABRT
> ――――――――――――――――――――――――――――――――――――― ✀
> ―――――――――――――――――――――――――――――――――――――
> stderr:
> ** Message: 06:06:50.205: /tmp/sdhci_F7ETH1
> **
> ERROR:../tests/qtest/npcm7xx_sdhci-test.c:101:sdwrite_read: assertion
> failed: (!strcmp(rmsg, msg))
>
> ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
> ...terminated.
>
> so I've dropped it again.
>

I'm sorry to hear that, I'll have to pick up some cycles in a week or so
and see if I can reproduce the issue.


>
> thanks
> -- PMM
>

[-- Attachment #2: Type: text/html, Size: 2805 bytes --]

Re: [PATCH v3] tests/qtest: add qtests for npcm7xx sdhci

[Hao Wu]

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

The problem is probably because we read it in using "strcmp". strcmp
compares two strings that end with "\0". But one of the string is read in
using read() so it didn't read in the ending '\0' character.

We should use strncmp to compare the two strings. It probably avoids the
issue.

On Tue, Feb 22, 2022 at 5:28 PM Patrick Venture <venture@google.com> wrote:

>
>
> On Mon, Feb 21, 2022 at 5:30 AM Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
>> On Wed, 16 Feb 2022 at 17:30, Peter Maydell <peter.maydell@linaro.org>
>> wrote:
>> >
>> > On Tue, 8 Feb 2022 at 18:18, Patrick Venture <venture@google.com>
>> wrote:
>> > >
>> > > From: Shengtan Mao <stmao@google.com>
>> > >
>> > > Reviewed-by: Hao Wu <wuhaotsh@google.com>
>> > > Reviewed-by: Chris Rauer <crauer@google.com>
>> > > Signed-off-by: Shengtan Mao <stmao@google.com>
>> > > Signed-off-by: Patrick Venture <venture@google.com>
>> > > ---
>> >
>> >
>> >
>> > Applied to target-arm.next, thanks.
>>
>> This hits assertions in some of the CI jobs, eg:
>> https://gitlab.com/qemu-project/qemu/-/jobs/2116932769
>>
>> 258/717 qemu:qtest+qtest-arm / qtest-arm/npcm7xx_sdhci-test INTERRUPT
>> 643.16s killed by signal 6 SIGABRT
>> ――――――――――――――――――――――――――――――――――――― ✀
>> ―――――――――――――――――――――――――――――――――――――
>> stderr:
>> ** Message: 06:06:50.205: /tmp/sdhci_F7ETH1
>> **
>> ERROR:../tests/qtest/npcm7xx_sdhci-test.c:101:sdwrite_read: assertion
>> failed: (!strcmp(rmsg, msg))
>>
>> ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
>> ...terminated.
>>
>> so I've dropped it again.
>>
>
> I'm sorry to hear that, I'll have to pick up some cycles in a week or so
> and see if I can reproduce the issue.
>
>
>>
>> thanks
>> -- PMM
>>
>

[-- Attachment #2: Type: text/html, Size: 3845 bytes --]