diff mbox series

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

Message ID 20220225174451.192304-1-wuhaotsh@google.com (mailing list archive)
State New, archived
Headers show
Series [v5] tests/qtest: add qtests for npcm7xx sdhci | expand

Commit Message

Hao Wu Feb. 25, 2022, 5:44 p.m. UTC
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

Comments

Peter Maydell Feb. 27, 2022, 2:54 p.m. UTC | #1
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
Peter Maydell May 26, 2022, 3:53 p.m. UTC | #2
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
Patrick Leis May 26, 2022, 4:21 p.m. UTC | #3
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
>
Philippe Mathieu-Daudé May 31, 2022, 8:26 a.m. UTC | #4
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
Hao Wu June 9, 2022, 9:25 p.m. UTC | #5
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
>>
>
Peter Maydell June 10, 2022, 8:59 a.m. UTC | #6
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
diff mbox series

Patch

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;
+}