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

[Peter Maydell]

On Thu, 9 Jun 2022 at 22:26, Hao Wu  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

[Hao Wu]

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  wrote:

>
>
> On Thu, May 26, 2022 at 8:54 AM Peter Maydell 
> wrote:
>
>> On Fri, 25 Feb 2022 at 17:45, Hao Wu  wrote:
>> >
>> > From: Shengtan Mao 
>> >
>> > Reviewed-by: Hao Wu 
>> > Reviewed-by: Chris Rauer 
>> > Signed-off-by: Shengtan Mao 
>> > Signed-off-by: Patrick Venture 
>>
>> 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

[Philippe Mathieu-Daudé via]

On 26/5/22 18:21, Patrick Venture wrote:
On Thu, May 26, 2022 at 8:54 AM Peter Maydell > wrote:


On Fri, 25 Feb 2022 at 17:45, Hao Wu mailto:wuhao...@google.com>> wrote:
 >
 > From: Shengtan Mao mailto:st...@google.com>>
 >
 > Reviewed-by: Hao Wu mailto:wuhao...@google.com>>
 > Reviewed-by: Chris Rauer mailto:cra...@google.com>>
 > Signed-off-by: Shengtan Mao mailto:st...@google.com>>
 > Signed-off-by: Patrick Venture mailto:vent...@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

[Patrick Venture]

On Thu, May 26, 2022 at 8:54 AM Peter Maydell 
wrote:

> On Fri, 25 Feb 2022 at 17:45, Hao Wu  wrote:
> >
> > From: Shengtan Mao 
> >
> > Reviewed-by: Hao Wu 
> > Reviewed-by: Chris Rauer 
> > Signed-off-by: Shengtan Mao 
> > Signed-off-by: Patrick Venture 
>
> 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

[Peter Maydell]

On Fri, 25 Feb 2022 at 17:45, Hao Wu  wrote:
>
> From: Shengtan Mao 
>
> Reviewed-by: Hao Wu 
> Reviewed-by: Chris Rauer 
> Signed-off-by: Shengtan Mao 
> Signed-off-by: Patrick Venture 

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

[Peter Maydell]

On Fri, 25 Feb 2022 at 17:45, Hao Wu  wrote:
>
> From: Shengtan Mao 
>
> Reviewed-by: Hao Wu 
> Reviewed-by: Chris Rauer 
> Signed-off-by: Shengtan Mao 
> Signed-off-by: Patrick Venture 



Applied to target-arm.next, thanks.

-- PMM

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

[Hao Wu]

From: Shengtan Mao 

Reviewed-by: Hao Wu 
Reviewed-by: Chris Rauer 
Signed-off-by: Shengtan Mao 
Signed-off-by: Patrick Venture 
---
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 00..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, 0x4120, 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, 0x4567, 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,
+