[Qemu-devel] [PATCH 0/8] SDHCI: add a qtest and fix few issues

[Qemu-devel] [PATCH 0/8] SDHCI: add a qtest and fix few issues

[Philippe Mathieu-Daudé]

This series adds a qtest for the SD Host Controller Interface.

To keep it simple, for each device using a SDHCI IP we ask the the capabilities
it supports, and verify with the public datashits availables.

So far the QEMU devices only implement the v2 and v3 Specs.

This exposes few inconsistencies, we fix them.

Regards,

Phil.

Based-on: 20171213232025.24503-27-f4bug@amsat.org
          (SDCard housekeeping)

Philippe Mathieu-Daudé (8):
  sdhci: add a "sd-spec-version" property
  sdhci: some ARM boards do support SD_HOST_SPECv3_VERS
  sdhci: add qtest to check the SD Spec version
  sdhci: fix CAPAB/MAXCURR registers, both are 64bit and read-only
  sdhci: add check_capab_readonly() qtest
  sdhci: add a check_capab_baseclock() qtest
  sdhci: add a check_capab_sdma() qtest
  sdhci: add a check_capab_v3() qtest

 include/hw/sd/sdhci.h        |  14 +++-
 hw/sd/sdhci-internal.h       |   4 +-
 hw/arm/bcm2835_peripherals.c |   7 ++
 hw/arm/fsl-imx6.c            |   6 ++
 hw/arm/xilinx_zynq.c         |   2 +
 hw/sd/sdhci.c                |  30 +++++++--
 tests/sdhci-test.c           | 152 +++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile.include       |   1 +
 8 files changed, 206 insertions(+), 10 deletions(-)
 create mode 100644 tests/sdhci-test.c

-- 
2.15.1

[Qemu-devel] [PATCH 1/8] sdhci: add a "sd-spec-version" property

[Philippe Mathieu-Daudé]

default to SDHCI v2

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
I am not sure which real VENDOR is HCVER=0x24, we probably don't care.

 hw/sd/sdhci-internal.h |  4 ++--
 include/hw/sd/sdhci.h  | 10 ++++++++++
 hw/sd/sdhci.c          |  5 ++++-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index e941bc2386..7e4a9d79d1 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -210,9 +210,9 @@
 /* Slot interrupt status */
 #define SDHC_SLOT_INT_STATUS            0xFC
 
-/* HWInit Host Controller Version Register 0x0401 */
+/* HWInit Host Controller Version Register */
 #define SDHC_HCVER                      0xFE
-#define SD_HOST_SPECv2_VERS             0x2401
+#define SDHC_HCVER_VENDOR               0x24
 
 #define SDHC_REGISTERS_MAP_SIZE         0x100
 #define SDHC_INSERTION_DELAY            (NANOSECONDS_PER_SECOND)
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index 579e0ea644..f8e91ce903 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -88,6 +88,9 @@ typedef struct SDHCIState {
     /* Force Event Auto CMD12 Error Interrupt Reg - write only */
     /* Force Event Error Interrupt Register- write only */
     /* RO Host Controller Version Register always reads as 0x2401 */
+    struct {
+        uint8_t spec_version;
+    } capabilities;
 } SDHCIState;
 
 #define TYPE_PCI_SDHCI "sdhci-pci"
@@ -97,4 +100,11 @@ typedef struct SDHCIState {
 #define SYSBUS_SDHCI(obj)                               \
      OBJECT_CHECK(SDHCIState, (obj), TYPE_SYSBUS_SDHCI)
 
+/* Host Controller Specification Version */
+enum sdhci_spec_version {
+    SD_HOST_SPECv1_VERS     = 0x00,
+    SD_HOST_SPECv2_VERS     = 0x01,
+    SD_HOST_SPECv3_VERS     = 0x02
+};
+
 #endif /* SDHCI_H */
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index d8188fdc2a..d6145342fb 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -908,7 +908,8 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)
         ret = (uint32_t)(s->admasysaddr >> 32);
         break;
     case SDHC_SLOT_INT_STATUS:
-        ret = (SD_HOST_SPECv2_VERS << 16) | sdhci_slotint(s);
+        ret = (SDHC_HCVER_VENDOR << 24) | (s->capabilities.spec_version << 16);
+        ret |= sdhci_slotint(s);
         break;
     default:
         qemu_log_mask(LOG_UNIMP, "SDHC rd_%ub @0x%02" HWADDR_PRIx " "
@@ -1263,6 +1264,8 @@ const VMStateDescription sdhci_vmstate = {
 /* Capabilities registers provide information on supported features of this
  * specific host controller implementation */
 static Property sdhci_properties[] = {
+    DEFINE_PROP_UINT8("sd-spec-version", SDHCIState,
+                      capabilities.spec_version, SD_HOST_SPECv2_VERS),
     DEFINE_PROP_UINT32("capareg", SDHCIState, capareg, UINT32_MAX),
     DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
     DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
-- 
2.15.1

[Qemu-devel] [PATCH 2/8] sdhci: some ARM boards do support SD_HOST_SPECv3_VERS

[Philippe Mathieu-Daudé]

set the property with object_property_set_uint() or qdev_prop_set_uint8().

[Zynq part based on a patch from Alistair Francis <alistair.francis@xilinx.com>
 from qemu/xilinx tag xilinx-v2015.2]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/bcm2835_peripherals.c | 7 +++++++
 hw/arm/fsl-imx6.c            | 6 ++++++
 hw/arm/xilinx_zynq.c         | 2 ++
 3 files changed, 15 insertions(+)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 60a0eec4d1..e686725015 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -264,6 +264,13 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    object_property_set_uint(OBJECT(&s->sdhci), SD_HOST_SPECv3_VERS,
+                             "sd-spec-version", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+
     object_property_set_bool(OBJECT(&s->sdhci), true, "realized", &err);
     if (err) {
         error_propagate(errp, err);
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index 59ef33efa9..c474e707af 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -348,6 +348,12 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
             { FSL_IMX6_uSDHC4_ADDR, FSL_IMX6_uSDHC4_IRQ },
         };
 
+        object_property_set_uint(OBJECT(&s->esdhc[i]), SD_HOST_SPECv3_VERS,
+                                 "sd-spec-version", &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
         object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err);
         if (err) {
             error_propagate(errp, err);
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 9275ae6daf..0292feea74 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -250,6 +250,7 @@ static void zynq_init(MachineState *machine)
 
     dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
     qdev_prop_set_string(dev, "sd-bus-name", "sd.0");
+    qdev_prop_set_uint8(dev, "sd-spec-version", SD_HOST_SPECv3_VERS);
     qdev_init_nofail(dev);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]);
@@ -262,6 +263,7 @@ static void zynq_init(MachineState *machine)
     object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal);
 
     dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
+    qdev_prop_set_uint8(dev, "sd-spec-version", SD_HOST_SPECv3_VERS);
     qdev_prop_set_string(dev, "sd-bus-name", "sd.1");
     qdev_init_nofail(dev);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
-- 
2.15.1

[Qemu-devel] [PATCH 3/8] sdhci: add qtest to check the SD Spec version

[Philippe Mathieu-Daudé]

read the HCVER register with check_specs_version()

Machines tested:
- Specs v2:
  smdkc210
- Specs v3:
  sabrelite
  raspi2
  xilinx-zynq-a9

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/sdhci-test.c     | 74 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile.include |  1 +
 2 files changed, 75 insertions(+)
 create mode 100644 tests/sdhci-test.c

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
new file mode 100644
index 0000000000..4ebe1e349b
--- /dev/null
+++ b/tests/sdhci-test.c
@@ -0,0 +1,74 @@
+/*
+ * QTest testcase for SDHCI controllers
+ *
+ * Written by Philippe Mathieu-Daudé <f4bug@amsat.org>
+ *
+ * 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 "qemu/osdep.h"
+#include "libqtest.h"
+
+#define SDHC_HCVER                      0xFE
+
+static const struct sdhci_t {
+    const char *arch;
+    const char *machine;
+    struct {
+        uintptr_t addr;
+        uint8_t version;
+    } sdhci;
+} models[] = {
+    { "arm",    "smdkc210",
+        {0x12510000, 2} },
+    { "arm",    "sabrelite",
+        {0x02190000, 3} },
+    { "arm",    "raspi2",           /* bcm2835 */
+        {0x3f300000, 3} },
+    { "arm",    "xilinx-zynq-a9",   /* exynos4210 */
+        {0xe0100000, 3} },
+};
+
+static uint32_t sdhci_readl(uintptr_t base, uint32_t reg_addr)
+{
+    QTestState *qtest = global_qtest;
+
+    return qtest_readl(qtest, base + reg_addr);
+}
+
+static void check_specs_version(uintptr_t addr, uint8_t version)
+{
+    uint32_t v;
+
+    v = sdhci_readl(addr, SDHC_HCVER);
+    v &= 0xff;
+    v += 1;
+    g_assert_cmpuint(v, ==, version);
+}
+
+static void test_machine(const void *data)
+{
+    const struct sdhci_t *test = data;
+
+    global_qtest = qtest_startf("-machine %s -d unimp", test->machine);
+
+    check_specs_version(test->sdhci.addr, test->sdhci.version);
+
+    qtest_quit(global_qtest);
+}
+
+int main(int argc, char *argv[])
+{
+    char *name;
+    int i;
+
+    g_test_init(&argc, &argv, NULL);
+
+    for (i = 0; i < ARRAY_SIZE(models); i++) {
+        name = g_strdup_printf("sdhci/%s", models[i].machine);
+        qtest_add_data_func(name, &models[i], test_machine);
+        g_free(name);
+    }
+
+    return g_test_run();
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 4a1afcb499..c6c0a91827 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -355,6 +355,7 @@ check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
 gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
 check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
 gcov-files-arm-y += hw/timer/arm_mptimer.c
+check-qtest-arm-y += tests/sdhci-test$(EXESUF)
 check-qtest-arm-y += tests/sdcard_tests.py
 
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
-- 
2.15.1

[Qemu-devel] [PATCH 4/8] sdhci: fix CAPAB/MAXCURR registers, both are 64bit and read-only

[Philippe Mathieu-Daudé]

running qtests:

  $ make check-qtest-arm
    GTESTER check-qtest-arm
  SDHC rd_4b @0x44 not implemented
  SDHC wr_4b @0x40 <- 0x89abcdef not implemented
  SDHC wr_4b @0x44 <- 0x01234567 not implemented

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/sd/sdhci.h |  4 ++--
 hw/sd/sdhci.c         | 25 ++++++++++++++++++++-----
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index f8e91ce903..d5093fe3fd 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -75,8 +75,8 @@ typedef struct SDHCIState {
     uint16_t acmd12errsts; /* Auto CMD12 error status register */
     uint64_t admasysaddr;  /* ADMA System Address Register */
 
-    uint32_t capareg;      /* Capabilities Register */
-    uint32_t maxcurr;      /* Maximum Current Capabilities Register */
+    uint64_t capareg;      /* Capabilities Register */
+    uint64_t maxcurr;      /* Maximum Current Capabilities Register */
     uint8_t  *fifo_buffer; /* SD host i/o FIFO buffer */
     uint32_t buf_maxsz;
     uint16_t data_count;   /* current element in FIFO buffer */
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index d6145342fb..4d269c7ac4 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -99,7 +99,7 @@
 
 static void sdhci_init_capareg(SDHCIState *s, Error **errp)
 {
-    if (s->capareg == UINT32_MAX) {
+    if (s->capareg == UINT64_MAX) {
         s->capareg = SDHC_CAPAB_REG_DEFAULT;
     }
 }
@@ -893,10 +893,16 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)
         ret = s->acmd12errsts;
         break;
     case SDHC_CAPAREG:
-        ret = s->capareg;
+        ret = (uint32_t)s->capareg;
+        break;
+    case SDHC_CAPAREG + 4:
+        ret = (uint32_t)(s->capareg >> 32);
         break;
     case SDHC_MAXCURR:
-        ret = s->maxcurr;
+        ret = (uint32_t)s->maxcurr;
+        break;
+    case SDHC_MAXCURR + 4:
+        ret = (uint32_t)(s->maxcurr >> 32);
         break;
     case SDHC_ADMAERR:
         ret =  s->admaerr;
@@ -1117,6 +1123,15 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
         }
         sdhci_update_irq(s);
         break;
+
+    case SDHC_CAPAREG:
+    case SDHC_CAPAREG + 4:
+    case SDHC_MAXCURR:
+    case SDHC_MAXCURR + 4:
+        qemu_log_mask(LOG_GUEST_ERROR, "SDHC wr_%ub @0x%02" HWADDR_PRIx
+                      " <- 0x%08x read-only\n", size, offset, value >> shift);
+        break;
+
     default:
         qemu_log_mask(LOG_UNIMP, "SDHC wr_%ub @0x%02" HWADDR_PRIx " <- 0x%08x "
                       "not implemented\n", size, offset, value >> shift);
@@ -1266,8 +1281,8 @@ const VMStateDescription sdhci_vmstate = {
 static Property sdhci_properties[] = {
     DEFINE_PROP_UINT8("sd-spec-version", SDHCIState,
                       capabilities.spec_version, SD_HOST_SPECv2_VERS),
-    DEFINE_PROP_UINT32("capareg", SDHCIState, capareg, UINT32_MAX),
-    DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
+    DEFINE_PROP_UINT64("capareg", SDHCIState, capareg, UINT64_MAX),
+    DEFINE_PROP_UINT64("maxcurr", SDHCIState, maxcurr, 0),
     DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
                      false),
     DEFINE_PROP_LINK("dma-memory", SDHCIState, dma_mr,
-- 
2.15.1

[Qemu-devel] [PATCH 5/8] sdhci: add check_capab_readonly() qtest

[Philippe Mathieu-Daudé]

read the CAPAB register.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/sdhci-test.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 4ebe1e349b..01373a69df 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -9,6 +9,7 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 
+#define SDHC_CAPAB                      0x40
 #define SDHC_HCVER                      0xFE
 
 static const struct sdhci_t {
@@ -36,6 +37,20 @@ static uint32_t sdhci_readl(uintptr_t base, uint32_t reg_addr)
     return qtest_readl(qtest, base + reg_addr);
 }
 
+static uint64_t sdhci_readq(uintptr_t base, uint32_t reg_addr)
+{
+    QTestState *qtest = global_qtest;
+
+    return qtest_readq(qtest, base + reg_addr);
+}
+
+static void sdhci_writeq(uintptr_t base, uint32_t reg_addr, uint64_t value)
+{
+    QTestState *qtest = global_qtest;
+
+    qtest_writeq(qtest, base + reg_addr, value);
+}
+
 static void check_specs_version(uintptr_t addr, uint8_t version)
 {
     uint32_t v;
@@ -46,6 +61,20 @@ static void check_specs_version(uintptr_t addr, uint8_t version)
     g_assert_cmpuint(v, ==, version);
 }
 
+static void check_capab_readonly(uintptr_t addr)
+{
+    const uint64_t vrand = 0x123456789abcdef;
+    uint64_t capab0, capab1;
+
+    capab0 = sdhci_readq(addr, SDHC_CAPAB);
+    g_assert_cmpuint(capab0, !=, vrand);
+
+    sdhci_writeq(addr, SDHC_CAPAB, vrand);
+    capab1 = sdhci_readq(addr, SDHC_CAPAB);
+    g_assert_cmpuint(capab1, !=, vrand);
+    g_assert_cmpuint(capab1, ==, capab0);
+}
+
 static void test_machine(const void *data)
 {
     const struct sdhci_t *test = data;
@@ -53,6 +82,7 @@ static void test_machine(const void *data)
     global_qtest = qtest_startf("-machine %s -d unimp", test->machine);
 
     check_specs_version(test->sdhci.addr, test->sdhci.version);
+    check_capab_readonly(test->sdhci.addr);
 
     qtest_quit(global_qtest);
 }
-- 
2.15.1

[Qemu-devel] [PATCH 6/8] sdhci: add a check_capab_baseclock() qtest

[Philippe Mathieu-Daudé]

So far only the bcm2835 is tested (52 MHz for the raspi2 machine).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
I didn't dig the dark web enough to get the other datashits

 tests/sdhci-test.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 01373a69df..966bd00499 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -7,9 +7,11 @@
  * See the COPYING file in the top-level directory.
  */
 #include "qemu/osdep.h"
+#include "hw/registerfields.h"
 #include "libqtest.h"
 
 #define SDHC_CAPAB                      0x40
+FIELD(SDHC_CAPAB, BASECLKFREQ,               8, 8); /* since v2 */
 #define SDHC_HCVER                      0xFE
 
 static const struct sdhci_t {
@@ -18,16 +20,17 @@ static const struct sdhci_t {
     struct {
         uintptr_t addr;
         uint8_t version;
+        uint8_t baseclock;
     } sdhci;
 } models[] = {
     { "arm",    "smdkc210",
-        {0x12510000, 2} },
+        {0x12510000, 2, 0} },
     { "arm",    "sabrelite",
-        {0x02190000, 3} },
+        {0x02190000, 3, 0} },
     { "arm",    "raspi2",           /* bcm2835 */
-        {0x3f300000, 3} },
+        {0x3f300000, 3, 52} },
     { "arm",    "xilinx-zynq-a9",   /* exynos4210 */
-        {0xe0100000, 3} },
+        {0xe0100000, 3, 0} },
 };
 
 static uint32_t sdhci_readl(uintptr_t base, uint32_t reg_addr)
@@ -75,6 +78,18 @@ static void check_capab_readonly(uintptr_t addr)
     g_assert_cmpuint(capab1, ==, capab0);
 }
 
+static void check_capab_baseclock(uintptr_t addr, uint8_t expected_freq)
+{
+    uint64_t capab, capab_freq;
+
+    if (!expected_freq) {
+        return;
+    }
+    capab = sdhci_readq(addr, SDHC_CAPAB);
+    capab_freq = FIELD_EX64(capab, SDHC_CAPAB, BASECLKFREQ);
+    g_assert_cmpuint(capab_freq, ==, expected_freq);
+}
+
 static void test_machine(const void *data)
 {
     const struct sdhci_t *test = data;
@@ -83,6 +98,7 @@ static void test_machine(const void *data)
 
     check_specs_version(test->sdhci.addr, test->sdhci.version);
     check_capab_readonly(test->sdhci.addr);
+    check_capab_baseclock(test->sdhci.addr, test->sdhci.baseclock);
 
     qtest_quit(global_qtest);
 }
-- 
2.15.1

[Qemu-devel] [RFC PATCH 7/8] sdhci: add a check_capab_sdma() qtest

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
RFC because I still need to verify with DS.

 tests/sdhci-test.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 966bd00499..b7646bccc6 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -12,6 +12,7 @@
 
 #define SDHC_CAPAB                      0x40
 FIELD(SDHC_CAPAB, BASECLKFREQ,               8, 8); /* since v2 */
+FIELD(SDHC_CAPAB, SDMA,                     22, 1);
 #define SDHC_HCVER                      0xFE
 
 static const struct sdhci_t {
@@ -21,16 +22,19 @@ static const struct sdhci_t {
         uintptr_t addr;
         uint8_t version;
         uint8_t baseclock;
+        struct {
+            bool sdma;
+        } capab;
     } sdhci;
 } models[] = {
     { "arm",    "smdkc210",
-        {0x12510000, 2, 0} },
+        {0x12510000, 2, 0, {1} } },
     { "arm",    "sabrelite",
-        {0x02190000, 3, 0} },
+        {0x02190000, 3, 0, {1} } },
     { "arm",    "raspi2",           /* bcm2835 */
-        {0x3f300000, 3, 52} },
+        {0x3f300000, 3, 52, {0} } },
     { "arm",    "xilinx-zynq-a9",   /* exynos4210 */
-        {0xe0100000, 3, 0} },
+        {0xe0100000, 3, 0, {1} } },
 };
 
 static uint32_t sdhci_readl(uintptr_t base, uint32_t reg_addr)
@@ -90,6 +94,15 @@ static void check_capab_baseclock(uintptr_t addr, uint8_t expected_freq)
     g_assert_cmpuint(capab_freq, ==, expected_freq);
 }
 
+static void check_capab_sdma(uintptr_t addr, bool supported)
+{
+    uint64_t capab, capab_sdma;
+
+    capab = sdhci_readq(addr, SDHC_CAPAB);
+    capab_sdma = FIELD_EX64(capab, SDHC_CAPAB, SDMA);
+    g_assert_cmpuint(capab_sdma, ==, supported);
+}
+
 static void test_machine(const void *data)
 {
     const struct sdhci_t *test = data;
@@ -98,6 +111,7 @@ static void test_machine(const void *data)
 
     check_specs_version(test->sdhci.addr, test->sdhci.version);
     check_capab_readonly(test->sdhci.addr);
+    check_capab_sdma(test->sdhci.addr, test->sdhci.capab.sdma);
     check_capab_baseclock(test->sdhci.addr, test->sdhci.baseclock);
 
     qtest_quit(global_qtest);
-- 
2.15.1

[Qemu-devel] [PATCH 8/8] sdhci: add a check_capab_v3() qtest

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/sdhci-test.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index b7646bccc6..60b28f27b3 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -13,6 +13,8 @@
 #define SDHC_CAPAB                      0x40
 FIELD(SDHC_CAPAB, BASECLKFREQ,               8, 8); /* since v2 */
 FIELD(SDHC_CAPAB, SDMA,                     22, 1);
+FIELD(SDHC_CAPAB, SDR,                      32, 3); /* since v3 */
+FIELD(SDHC_CAPAB, DRIVER,                   36, 3); /* since v3 */
 #define SDHC_HCVER                      0xFE
 
 static const struct sdhci_t {
@@ -103,6 +105,21 @@ static void check_capab_sdma(uintptr_t addr, bool supported)
     g_assert_cmpuint(capab_sdma, ==, supported);
 }
 
+static void check_capab_v3(uintptr_t addr, uint8_t version)
+{
+    uint64_t capab, capab_v3;
+
+    if (version >= 3) {
+        return;
+    }
+    /* before v3 those fields are RESERVED */
+    capab = sdhci_readq(addr, SDHC_CAPAB);
+    capab_v3 = FIELD_EX64(capab, SDHC_CAPAB, SDR);
+    g_assert_cmpuint(capab_v3, ==, 0);
+    capab_v3 = FIELD_EX64(capab, SDHC_CAPAB, DRIVER);
+    g_assert_cmpuint(capab_v3, ==, 0);
+}
+
 static void test_machine(const void *data)
 {
     const struct sdhci_t *test = data;
@@ -111,6 +128,7 @@ static void test_machine(const void *data)
 
     check_specs_version(test->sdhci.addr, test->sdhci.version);
     check_capab_readonly(test->sdhci.addr);
+    check_capab_v3(test->sdhci.addr, test->sdhci.version);
     check_capab_sdma(test->sdhci.addr, test->sdhci.capab.sdma);
     check_capab_baseclock(test->sdhci.addr, test->sdhci.baseclock);
 
-- 
2.15.1

Re: [Qemu-devel] [PATCH 1/8] sdhci: add a "sd-spec-version" property

[Alistair Francis]

On Wed, Dec 13, 2017 at 6:00 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> default to SDHCI v2
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
> I am not sure which real VENDOR is HCVER=0x24, we probably don't care.
>
>  hw/sd/sdhci-internal.h |  4 ++--
>  include/hw/sd/sdhci.h  | 10 ++++++++++
>  hw/sd/sdhci.c          |  5 ++++-
>  3 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
> index e941bc2386..7e4a9d79d1 100644
> --- a/hw/sd/sdhci-internal.h
> +++ b/hw/sd/sdhci-internal.h
> @@ -210,9 +210,9 @@
>  /* Slot interrupt status */
>  #define SDHC_SLOT_INT_STATUS            0xFC
>
> -/* HWInit Host Controller Version Register 0x0401 */
> +/* HWInit Host Controller Version Register */
>  #define SDHC_HCVER                      0xFE
> -#define SD_HOST_SPECv2_VERS             0x2401
> +#define SDHC_HCVER_VENDOR               0x24
>
>  #define SDHC_REGISTERS_MAP_SIZE         0x100
>  #define SDHC_INSERTION_DELAY            (NANOSECONDS_PER_SECOND)
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index 579e0ea644..f8e91ce903 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -88,6 +88,9 @@ typedef struct SDHCIState {
>      /* Force Event Auto CMD12 Error Interrupt Reg - write only */
>      /* Force Event Error Interrupt Register- write only */
>      /* RO Host Controller Version Register always reads as 0x2401 */
> +    struct {
> +        uint8_t spec_version;
> +    } capabilities;
>  } SDHCIState;
>
>  #define TYPE_PCI_SDHCI "sdhci-pci"
> @@ -97,4 +100,11 @@ typedef struct SDHCIState {
>  #define SYSBUS_SDHCI(obj)                               \
>       OBJECT_CHECK(SDHCIState, (obj), TYPE_SYSBUS_SDHCI)
>
> +/* Host Controller Specification Version */
> +enum sdhci_spec_version {
> +    SD_HOST_SPECv1_VERS     = 0x00,
> +    SD_HOST_SPECv2_VERS     = 0x01,
> +    SD_HOST_SPECv3_VERS     = 0x02
> +};
> +
>  #endif /* SDHCI_H */
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index d8188fdc2a..d6145342fb 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -908,7 +908,8 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)
>          ret = (uint32_t)(s->admasysaddr >> 32);
>          break;
>      case SDHC_SLOT_INT_STATUS:
> -        ret = (SD_HOST_SPECv2_VERS << 16) | sdhci_slotint(s);
> +        ret = (SDHC_HCVER_VENDOR << 24) | (s->capabilities.spec_version << 16);
> +        ret |= sdhci_slotint(s);
>          break;
>      default:
>          qemu_log_mask(LOG_UNIMP, "SDHC rd_%ub @0x%02" HWADDR_PRIx " "
> @@ -1263,6 +1264,8 @@ const VMStateDescription sdhci_vmstate = {
>  /* Capabilities registers provide information on supported features of this
>   * specific host controller implementation */
>  static Property sdhci_properties[] = {
> +    DEFINE_PROP_UINT8("sd-spec-version", SDHCIState,
> +                      capabilities.spec_version, SD_HOST_SPECv2_VERS),
>      DEFINE_PROP_UINT32("capareg", SDHCIState, capareg, UINT32_MAX),
>      DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
>      DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 2/8] sdhci: some ARM boards do support SD_HOST_SPECv3_VERS

[Alistair Francis]

On Wed, Dec 13, 2017 at 6:00 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> set the property with object_property_set_uint() or qdev_prop_set_uint8().
>
> [Zynq part based on a patch from Alistair Francis <alistair.francis@xilinx.com>
>  from qemu/xilinx tag xilinx-v2015.2]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/arm/bcm2835_peripherals.c | 7 +++++++
>  hw/arm/fsl-imx6.c            | 6 ++++++
>  hw/arm/xilinx_zynq.c         | 2 ++
>  3 files changed, 15 insertions(+)
>
> diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
> index 60a0eec4d1..e686725015 100644
> --- a/hw/arm/bcm2835_peripherals.c
> +++ b/hw/arm/bcm2835_peripherals.c
> @@ -264,6 +264,13 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>
> +    object_property_set_uint(OBJECT(&s->sdhci), SD_HOST_SPECv3_VERS,
> +                             "sd-spec-version", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +
>      object_property_set_bool(OBJECT(&s->sdhci), true, "realized", &err);
>      if (err) {
>          error_propagate(errp, err);
> diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
> index 59ef33efa9..c474e707af 100644
> --- a/hw/arm/fsl-imx6.c
> +++ b/hw/arm/fsl-imx6.c
> @@ -348,6 +348,12 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
>              { FSL_IMX6_uSDHC4_ADDR, FSL_IMX6_uSDHC4_IRQ },
>          };
>
> +        object_property_set_uint(OBJECT(&s->esdhc[i]), SD_HOST_SPECv3_VERS,
> +                                 "sd-spec-version", &err);
> +        if (err) {
> +            error_propagate(errp, err);
> +            return;
> +        }
>          object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err);
>          if (err) {
>              error_propagate(errp, err);
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 9275ae6daf..0292feea74 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -250,6 +250,7 @@ static void zynq_init(MachineState *machine)
>
>      dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
>      qdev_prop_set_string(dev, "sd-bus-name", "sd.0");
> +    qdev_prop_set_uint8(dev, "sd-spec-version", SD_HOST_SPECv3_VERS);
>      qdev_init_nofail(dev);
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000);
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]);
> @@ -262,6 +263,7 @@ static void zynq_init(MachineState *machine)
>      object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal);
>
>      dev = qdev_create(NULL, TYPE_SYSBUS_SDHCI);
> +    qdev_prop_set_uint8(dev, "sd-spec-version", SD_HOST_SPECv3_VERS);
>      qdev_prop_set_string(dev, "sd-bus-name", "sd.1");
>      qdev_init_nofail(dev);
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 4/8] sdhci: fix CAPAB/MAXCURR registers, both are 64bit and read-only

[Alistair Francis]

On Wed, Dec 13, 2017 at 6:00 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> running qtests:
>
>   $ make check-qtest-arm
>     GTESTER check-qtest-arm
>   SDHC rd_4b @0x44 not implemented
>   SDHC wr_4b @0x40 <- 0x89abcdef not implemented
>   SDHC wr_4b @0x44 <- 0x01234567 not implemented
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  include/hw/sd/sdhci.h |  4 ++--
>  hw/sd/sdhci.c         | 25 ++++++++++++++++++++-----
>  2 files changed, 22 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index f8e91ce903..d5093fe3fd 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -75,8 +75,8 @@ typedef struct SDHCIState {
>      uint16_t acmd12errsts; /* Auto CMD12 error status register */
>      uint64_t admasysaddr;  /* ADMA System Address Register */
>
> -    uint32_t capareg;      /* Capabilities Register */
> -    uint32_t maxcurr;      /* Maximum Current Capabilities Register */
> +    uint64_t capareg;      /* Capabilities Register */
> +    uint64_t maxcurr;      /* Maximum Current Capabilities Register */
>      uint8_t  *fifo_buffer; /* SD host i/o FIFO buffer */
>      uint32_t buf_maxsz;
>      uint16_t data_count;   /* current element in FIFO buffer */
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index d6145342fb..4d269c7ac4 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -99,7 +99,7 @@
>
>  static void sdhci_init_capareg(SDHCIState *s, Error **errp)
>  {
> -    if (s->capareg == UINT32_MAX) {
> +    if (s->capareg == UINT64_MAX) {
>          s->capareg = SDHC_CAPAB_REG_DEFAULT;
>      }
>  }
> @@ -893,10 +893,16 @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)
>          ret = s->acmd12errsts;
>          break;
>      case SDHC_CAPAREG:
> -        ret = s->capareg;
> +        ret = (uint32_t)s->capareg;
> +        break;
> +    case SDHC_CAPAREG + 4:
> +        ret = (uint32_t)(s->capareg >> 32);
>          break;
>      case SDHC_MAXCURR:
> -        ret = s->maxcurr;
> +        ret = (uint32_t)s->maxcurr;
> +        break;
> +    case SDHC_MAXCURR + 4:
> +        ret = (uint32_t)(s->maxcurr >> 32);
>          break;
>      case SDHC_ADMAERR:
>          ret =  s->admaerr;
> @@ -1117,6 +1123,15 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
>          }
>          sdhci_update_irq(s);
>          break;
> +
> +    case SDHC_CAPAREG:
> +    case SDHC_CAPAREG + 4:
> +    case SDHC_MAXCURR:
> +    case SDHC_MAXCURR + 4:
> +        qemu_log_mask(LOG_GUEST_ERROR, "SDHC wr_%ub @0x%02" HWADDR_PRIx
> +                      " <- 0x%08x read-only\n", size, offset, value >> shift);
> +        break;
> +
>      default:
>          qemu_log_mask(LOG_UNIMP, "SDHC wr_%ub @0x%02" HWADDR_PRIx " <- 0x%08x "
>                        "not implemented\n", size, offset, value >> shift);
> @@ -1266,8 +1281,8 @@ const VMStateDescription sdhci_vmstate = {
>  static Property sdhci_properties[] = {
>      DEFINE_PROP_UINT8("sd-spec-version", SDHCIState,
>                        capabilities.spec_version, SD_HOST_SPECv2_VERS),
> -    DEFINE_PROP_UINT32("capareg", SDHCIState, capareg, UINT32_MAX),
> -    DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
> +    DEFINE_PROP_UINT64("capareg", SDHCIState, capareg, UINT64_MAX),
> +    DEFINE_PROP_UINT64("maxcurr", SDHCIState, maxcurr, 0),
>      DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
>                       false),
>      DEFINE_PROP_LINK("dma-memory", SDHCIState, dma_mr,
> --
> 2.15.1
>
>