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

Hao Wu posted 1 patch 2 years, 2 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220225174451.192304-1-wuhaotsh@google.com
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Tyrone Ting <kfting@nuvoton.com>, Havard Skinnemoen <hskinnemoen@google.com>
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
[PATCH v5] tests/qtest: add qtests for npcm7xx sdhci
Posted by Hao Wu 2 years, 2 months ago
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>
---
v5
 * use memcmp to compare whether strings are expected
v4
 * use strncmp instead of strcmp
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 f33d84d19b..721eafad12 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -190,6 +190,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..c1f496fb29
--- /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(!memcmp(rmsg, msg, len));
+
+    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(!memcmp(rmsg, msg, len));
+
+    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.1.574.g5d30c73bfb-goog


Re: [PATCH v5] tests/qtest: add qtests for npcm7xx sdhci
Posted by Peter Maydell 1 year, 11 months ago
On Fri, 25 Feb 2022 at 17:45, Hao Wu <wuhaotsh@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>

Hi; John Snow tells me that this test fails in the tests/vm/netbsd
VM (you can test this with 'make vm-build-netbsd') because the
assert() on the ftruncate() call fails:

> +    ret = ftruncate(fd, NPCM7XX_TEST_IMAGE_SIZE);
> +    g_assert_cmpint(ret, ==, 0);

> +#define NPCM7XX_TEST_IMAGE_SIZE (1 << 30)

I haven't investigated the exact cause, but this is a
gigabyte, right? That's a pretty massive file for a test case to
create -- can we make the test use a more sensible size of
sd card image ?

thanks
-- PMM
Re: [PATCH v5] tests/qtest: add qtests for npcm7xx sdhci
Posted by Patrick Venture 1 year, 11 months ago
On Thu, May 26, 2022 at 8:54 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Fri, 25 Feb 2022 at 17:45, Hao Wu <wuhaotsh@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>
>
> Hi; John Snow tells me that this test fails in the tests/vm/netbsd
> VM (you can test this with 'make vm-build-netbsd') because the
> assert() on the ftruncate() call fails:
>
> > +    ret = ftruncate(fd, NPCM7XX_TEST_IMAGE_SIZE);
> > +    g_assert_cmpint(ret, ==, 0);
>
> > +#define NPCM7XX_TEST_IMAGE_SIZE (1 << 30)
>
> I haven't investigated the exact cause, but this is a
> gigabyte, right? That's a pretty massive file for a test case to
> create -- can we make the test use a more sensible size of
> sd card image ?
>

It looks like the nuvoton part had an issue with a smaller image size, but
we can resurrect that thread and poke at it a bit and see what shakes out.


>
> thanks
> -- PMM
>
Re: [PATCH v5] tests/qtest: add qtests for npcm7xx sdhci
Posted by Hao Wu 1 year, 10 months ago
Hi,

We did some experiments on this issue. It looks like the image size
restriction is in firmware. So in qtest we can make it
much smaller (e.g. 1MB) and the test still passes. We can send a patch with
this change if necessary.

On Thu, May 26, 2022 at 9:21 AM Patrick Venture <venture@google.com> wrote:

>
>
> On Thu, May 26, 2022 at 8:54 AM Peter Maydell <peter.maydell@linaro.org>
> wrote:
>
>> On Fri, 25 Feb 2022 at 17:45, Hao Wu <wuhaotsh@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>
>>
>> Hi; John Snow tells me that this test fails in the tests/vm/netbsd
>> VM (you can test this with 'make vm-build-netbsd') because the
>> assert() on the ftruncate() call fails:
>>
>> > +    ret = ftruncate(fd, NPCM7XX_TEST_IMAGE_SIZE);
>> > +    g_assert_cmpint(ret, ==, 0);
>>
>> > +#define NPCM7XX_TEST_IMAGE_SIZE (1 << 30)
>>
>> I haven't investigated the exact cause, but this is a
>> gigabyte, right? That's a pretty massive file for a test case to
>> create -- can we make the test use a more sensible size of
>> sd card image ?
>>
>
> It looks like the nuvoton part had an issue with a smaller image size, but
> we can resurrect that thread and poke at it a bit and see what shakes out.
>
>
>>
>> thanks
>> -- PMM
>>
>
Re: [PATCH v5] tests/qtest: add qtests for npcm7xx sdhci
Posted by Peter Maydell 1 year, 10 months ago
On Thu, 9 Jun 2022 at 22:26, Hao Wu <wuhaotsh@google.com> wrote:
>
> Hi,
>
> We did some experiments on this issue. It looks like the image size restriction is in firmware. So in qtest we can make it
> much smaller (e.g. 1MB) and the test still passes. We can send a patch with this change if necessary.

Yes, please send a patch.

thanks
-- PMM
Re: [PATCH v5] tests/qtest: add qtests for npcm7xx sdhci
Posted by Philippe Mathieu-Daudé via 1 year, 11 months ago
On 26/5/22 18:21, Patrick Venture wrote:
> On Thu, May 26, 2022 at 8:54 AM Peter Maydell <peter.maydell@linaro.org 
> <mailto:peter.maydell@linaro.org>> wrote:
> 
>     On Fri, 25 Feb 2022 at 17:45, Hao Wu <wuhaotsh@google.com
>     <mailto:wuhaotsh@google.com>> wrote:
>      >
>      > From: Shengtan Mao <stmao@google.com <mailto:stmao@google.com>>
>      >
>      > Reviewed-by: Hao Wu <wuhaotsh@google.com
>     <mailto:wuhaotsh@google.com>>
>      > Reviewed-by: Chris Rauer <crauer@google.com
>     <mailto:crauer@google.com>>
>      > Signed-off-by: Shengtan Mao <stmao@google.com
>     <mailto:stmao@google.com>>
>      > Signed-off-by: Patrick Venture <venture@google.com
>     <mailto:venture@google.com>>
> 
>     Hi; John Snow tells me that this test fails in the tests/vm/netbsd
>     VM (you can test this with 'make vm-build-netbsd') because the
>     assert() on the ftruncate() call fails:
> 
>      > +    ret = ftruncate(fd, NPCM7XX_TEST_IMAGE_SIZE);
>      > +    g_assert_cmpint(ret, ==, 0);
> 
>      > +#define NPCM7XX_TEST_IMAGE_SIZE (1 << 30)
> 
>     I haven't investigated the exact cause, but this is a
>     gigabyte, right? That's a pretty massive file for a test case to
>     create -- can we make the test use a more sensible size of
>     sd card image ?
> 
> 
> It looks like the nuvoton part had an issue with a smaller image size, 
> but we can resurrect that thread and poke at it a bit and see what 
> shakes out.

Could you use the null-co block driver instead?

     -blockdev driver=null-co,size=1G,read-zeroes=on,node-name=null0
     -device sd-card,drive=null0

Re: [PATCH v5] tests/qtest: add qtests for npcm7xx sdhci
Posted by Peter Maydell 2 years, 2 months ago
On Fri, 25 Feb 2022 at 17:45, Hao Wu <wuhaotsh@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