[PATCH V6 5/6] hw/block/nvme: add NMIC enum value for Identify Namespace

[Minwoo Im]

Added Namespace Multi-path I/O and Namespace Sharing Capabilities (NMIC)
field to support shared namespace from controller(s).

This field is in Identify Namespace data structure in [30].

Signed-off-by: Minwoo Im 
---
 include/block/nvme.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index d6415a869c1c..ad68cdc2b92d 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -1203,6 +1203,10 @@ enum NvmeNsIdentifierType {
 NVME_NIDT_CSI   = 0x04,
 };
 
+enum NvmeIdNsNmic {
+NVME_NMIC_NS_SHARED = 1 << 0,
+};
+
 enum NvmeCsi {
 NVME_CSI_NVM= 0x00,
 NVME_CSI_ZONED  = 0x02,
-- 
2.17.1

[PATCH V6 4/6] hw/block/nvme: support for multi-controller in subsystem

[Minwoo Im]

We have nvme-subsys and nvme devices mapped together.  To support
multi-controller scheme to this setup, controller identifier(id) has to
be managed.  Earlier, cntlid(controller id) used to be always 0 because
we didn't have any subsystem scheme that controller id matters.

This patch introduced 'cntlid' attribute to the nvme controller
instance(NvmeCtrl) and make it allocated by the nvme-subsys device
mapped to the controller.  If nvme-subsys is not given to the
controller, then it will always be 0 as it was.

Added 'ctrls' array in the nvme-subsys instance to manage attached
controllers to the subsystem with a limit(32).  This patch didn't take
list for the controllers to make it seamless with nvme-ns device.

Signed-off-by: Minwoo Im 
---
 hw/block/nvme-subsys.c | 21 +
 hw/block/nvme-subsys.h |  4 
 hw/block/nvme.c| 29 +
 hw/block/nvme.h|  1 +
 4 files changed, 55 insertions(+)

diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index aa82911b951c..e9d61c993c90 100644
--- a/hw/block/nvme-subsys.c
+++ b/hw/block/nvme-subsys.c
@@ -22,6 +22,27 @@
 #include "nvme.h"
 #include "nvme-subsys.h"
 
+int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
+{
+NvmeSubsystem *subsys = n->subsys;
+int cntlid;
+
+for (cntlid = 0; cntlid < ARRAY_SIZE(subsys->ctrls); cntlid++) {
+if (!subsys->ctrls[cntlid]) {
+break;
+}
+}
+
+if (cntlid == ARRAY_SIZE(subsys->ctrls)) {
+error_setg(errp, "no more free controller id");
+return -1;
+}
+
+subsys->ctrls[cntlid] = n;
+
+return cntlid;
+}
+
 static void nvme_subsys_setup(NvmeSubsystem *subsys)
 {
 snprintf((char *)subsys->subnqn, sizeof(subsys->subnqn),
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index 40f06a4c7db0..4eba50d96a1d 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -20,6 +20,10 @@ typedef struct NvmeNamespace NvmeNamespace;
 typedef struct NvmeSubsystem {
 DeviceState parent_obj;
 uint8_t subnqn[256];
+
+NvmeCtrl*ctrls[NVME_SUBSYS_MAX_CTRLS];
 } NvmeSubsystem;
 
+int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp);
+
 #endif /* NVME_SUBSYS_H */
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index b525fca14103..7138389be4bd 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -4435,6 +4435,9 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 strpadcpy((char *)id->mn, sizeof(id->mn), "QEMU NVMe Ctrl", ' ');
 strpadcpy((char *)id->fr, sizeof(id->fr), "1.0", ' ');
 strpadcpy((char *)id->sn, sizeof(id->sn), n->params.serial, ' ');
+
+id->cntlid = cpu_to_le16(n->cntlid);
+
 id->rab = 6;
 id->ieee[0] = 0x00;
 id->ieee[1] = 0x02;
@@ -4481,6 +4484,10 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 id->psd[0].enlat = cpu_to_le32(0x10);
 id->psd[0].exlat = cpu_to_le32(0x4);
 
+if (n->subsys) {
+id->cmic |= NVME_CMIC_MULTI_CTRL;
+}
+
 NVME_CAP_SET_MQES(n->bar.cap, 0x7ff);
 NVME_CAP_SET_CQR(n->bar.cap, 1);
 NVME_CAP_SET_TO(n->bar.cap, 0xf);
@@ -4495,6 +4502,24 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 n->bar.intmc = n->bar.intms = 0;
 }
 
+static int nvme_init_subsys(NvmeCtrl *n, Error **errp)
+{
+int cntlid;
+
+if (!n->subsys) {
+return 0;
+}
+
+cntlid = nvme_subsys_register_ctrl(n, errp);
+if (cntlid < 0) {
+return -1;
+}
+
+n->cntlid = cntlid;
+
+return 0;
+}
+
 static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 {
 NvmeCtrl *n = NVME(pci_dev);
@@ -4515,6 +4540,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error 
**errp)
 return;
 }
 
+if (nvme_init_subsys(n, errp)) {
+error_propagate(errp, local_err);
+return;
+}
 nvme_init_ctrl(n, pci_dev);
 
 /* setup a namespace if the controller drive property was given */
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 04d4684601fd..b8f5f2d6ffb8 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -134,6 +134,7 @@ typedef struct NvmeCtrl {
 NvmeBus  bus;
 BlockConfconf;
 
+uint16_tcntlid;
 boolqs_created;
 uint32_tpage_size;
 uint16_tpage_bits;
-- 
2.17.1

[PATCH V6 3/6] hw/block/nvme: add CMIC enum value for Identify Controller

[Minwoo Im]

Added Controller Multi-path I/O and Namespace Sharing Capabilities
(CMIC) field to support multi-controller in the following patches.

This field is in Identify Controller data structure in [76].

Signed-off-by: Minwoo Im 
---
 include/block/nvme.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index e4b918064df9..d6415a869c1c 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -1034,6 +1034,10 @@ enum NvmeIdCtrlLpa {
 NVME_LPA_EXTENDED = 1 << 2,
 };
 
+enum NvmeIdCtrlCmic {
+NVME_CMIC_MULTI_CTRL= 1 << 1,
+};
+
 #define NVME_CTRL_SQES_MIN(sqes) ((sqes) & 0xf)
 #define NVME_CTRL_SQES_MAX(sqes) (((sqes) >> 4) & 0xf)
 #define NVME_CTRL_CQES_MIN(cqes) ((cqes) & 0xf)
-- 
2.17.1

[PATCH V6 6/6] hw/block/nvme: support for shared namespace in subsystem

[Minwoo Im]

nvme-ns device is registered to a nvme controller device during the
initialization in nvme_register_namespace() in case that 'bus' property
is given which means it's mapped to a single controller.

This patch introduced a new property 'subsys' just like the controller
device instance did to map a namespace to a NVMe subsystem.

If 'subsys' property is given to the nvme-ns device, it will belong to
the specified subsystem and will be attached to all controllers in that
subsystem by enabling shared namespace capability in NMIC(Namespace
Multi-path I/O and Namespace Capabilities) in Identify Namespace.

Usage:

  -device nvme-subsys,id=subsys0
  -device nvme,serial=foo,id=nvme0,subsys=subsys0
  -device nvme,serial=bar,id=nvme1,subsys=subsys0
  -device nvme,serial=baz,id=nvme2,subsys=subsys0
  -device nvme-ns,id=ns1,drive=,nsid=1,subsys=subsys0  # Shared
  -device nvme-ns,id=ns2,drive=,nsid=2,bus=nvme2   # Non-shared

  In the above example, 'ns1' will be shared to 'nvme0' and 'nvme1' in
  the same subsystem.  On the other hand, 'ns2' will be attached to the
  'nvme2' only as a private namespace in that subsystem.

All the namespace with 'subsys' parameter will attach all controllers in
the subsystem to the namespace by default.

Signed-off-by: Minwoo Im 
---
 hw/block/nvme-ns.c | 23 ++-
 hw/block/nvme-ns.h |  7 +++
 hw/block/nvme-subsys.c | 25 +
 hw/block/nvme-subsys.h |  3 +++
 hw/block/nvme.c| 10 +-
 5 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/hw/block/nvme-ns.c b/hw/block/nvme-ns.c
index 62b25cf69bfa..9b493f2ead03 100644
--- a/hw/block/nvme-ns.c
+++ b/hw/block/nvme-ns.c
@@ -63,6 +63,10 @@ static int nvme_ns_init(NvmeNamespace *ns, Error **errp)
 
 id_ns->npda = id_ns->npdg = npdg - 1;
 
+if (nvme_ns_shared(ns)) {
+id_ns->nmic |= NVME_NMIC_NS_SHARED;
+}
+
 return 0;
 }
 
@@ -365,16 +369,25 @@ static void nvme_ns_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-if (nvme_register_namespace(n, ns, errp)) {
-error_propagate_prepend(errp, local_err,
-"could not register namespace: ");
-return;
+if (ns->subsys) {
+if (nvme_subsys_register_ns(ns, errp)) {
+error_propagate_prepend(errp, local_err,
+"could not setup namespace to subsys: ");
+return;
+}
+} else {
+if (nvme_register_namespace(n, ns, errp)) {
+error_propagate_prepend(errp, local_err,
+"could not register namespace: ");
+return;
+}
 }
-
 }
 
 static Property nvme_ns_props[] = {
 DEFINE_BLOCK_PROPERTIES(NvmeNamespace, blkconf),
+DEFINE_PROP_LINK("subsys", NvmeNamespace, subsys, TYPE_NVME_SUBSYS,
+ NvmeSubsystem *),
 DEFINE_PROP_UINT32("nsid", NvmeNamespace, params.nsid, 0),
 DEFINE_PROP_UUID("uuid", NvmeNamespace, params.uuid),
 DEFINE_PROP_BOOL("zoned", NvmeNamespace, params.zoned, false),
diff --git a/hw/block/nvme-ns.h b/hw/block/nvme-ns.h
index 293ac990e3f6..929e78861903 100644
--- a/hw/block/nvme-ns.h
+++ b/hw/block/nvme-ns.h
@@ -47,6 +47,8 @@ typedef struct NvmeNamespace {
 const uint32_t *iocs;
 uint8_t  csi;
 
+NvmeSubsystem   *subsys;
+
 NvmeIdNsZoned   *id_ns_zoned;
 NvmeZone*zone_array;
 QTAILQ_HEAD(, NvmeZone) exp_open_zones;
@@ -77,6 +79,11 @@ static inline uint32_t nvme_nsid(NvmeNamespace *ns)
 return -1;
 }
 
+static inline bool nvme_ns_shared(NvmeNamespace *ns)
+{
+return !!ns->subsys;
+}
+
 static inline NvmeLBAF *nvme_ns_lbaf(NvmeNamespace *ns)
 {
 NvmeIdNs *id_ns = >id_ns;
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
index e9d61c993c90..641de33e99fc 100644
--- a/hw/block/nvme-subsys.c
+++ b/hw/block/nvme-subsys.c
@@ -43,6 +43,31 @@ int nvme_subsys_register_ctrl(NvmeCtrl *n, Error **errp)
 return cntlid;
 }
 
+int nvme_subsys_register_ns(NvmeNamespace *ns, Error **errp)
+{
+NvmeSubsystem *subsys = ns->subsys;
+NvmeCtrl *n;
+int i;
+
+if (subsys->namespaces[nvme_nsid(ns)]) {
+error_setg(errp, "namespace %d already registerd to subsy %s",
+   nvme_nsid(ns), subsys->parent_obj.id);
+return -1;
+}
+
+subsys->namespaces[nvme_nsid(ns)] = ns;
+
+for (i = 0; i < ARRAY_SIZE(subsys->ctrls); i++) {
+n = subsys->ctrls[i];
+
+if (n && nvme_register_namespace(n, ns, errp)) {
+return -1;
+}
+}
+
+return 0;
+}
+
 static void nvme_subsys_setup(NvmeSubsystem *subsys)
 {
 snprintf((char *)subsys->subnqn, sizeof(subsys->subnqn),
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
index 4eba50d96a1d..ccf6a71398d3 100644
--- a/hw/block/nvme-subsys.h
+++ b/hw/block/nvme-subsys.h
@@ -14,6 +14,7 @@
 OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
 
 #define NVME_SUBSYS_MAX_CTRLS   

[PATCH V6 2/6] hw/block/nvme: support to map controller to a subsystem

[Minwoo Im]

nvme controller(nvme) can be mapped to a NVMe subsystem(nvme-subsys).
This patch maps a controller to a subsystem by adding a parameter
'subsys' to the nvme device.

To map a controller to a subsystem, we need to put nvme-subsys first and
then maps the subsystem to the controller:

  -device nvme-subsys,id=subsys0
  -device nvme,serial=foo,id=nvme0,subsys=subsys0

If 'subsys' property is not given to the nvme controller, then subsystem
NQN will be created with serial (e.g., 'foo' in above example),
Otherwise, it will be based on subsys id (e.g., 'subsys0' in above
example).

Signed-off-by: Minwoo Im 
---
 hw/block/nvme.c | 30 +-
 hw/block/nvme.h |  3 +++
 2 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index aabccdf36f4b..b525fca14103 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -22,7 +22,8 @@
  *  [pmrdev=,] \
  *  max_ioqpairs=, \
  *  aerl=, aer_max_queued=, \
- *  mdts=,zoned.append_size_limit= \
+ *  mdts=,zoned.append_size_limit=, \
+ *  subsys= \
  *  -device nvme-ns,drive=,bus=,nsid=,\
  *  zoned=
  *  -device nvme-subsys,id=
@@ -44,6 +45,13 @@
  *
  * nvme device parameters
  * ~~
+ * - `subsys`
+ *   NVM Subsystem device. If given, a subsystem NQN will be initialized with
+ *given. Otherwise,  will be taken for subsystem NQN.
+ *   Also, it will enable multi controller capability represented in Identify
+ *   Controller data structure in CMIC (Controller Multi-path I/O and Namesapce
+ *   Sharing Capabilities), if given.
+ *
  * - `aerl`
  *   The Asynchronous Event Request Limit (AERL). Indicates the maximum number
  *   of concurrently outstanding Asynchronous Event Request commands support
@@ -4404,11 +4412,23 @@ static int nvme_init_pci(NvmeCtrl *n, PCIDevice 
*pci_dev, Error **errp)
 return 0;
 }
 
+static void nvme_init_subnqn(NvmeCtrl *n)
+{
+NvmeSubsystem *subsys = n->subsys;
+NvmeIdCtrl *id = >id_ctrl;
+
+if (!subsys) {
+snprintf((char *)id->subnqn, sizeof(id->subnqn),
+ "nqn.2019-08.org.qemu:%s", n->params.serial);
+} else {
+pstrcpy((char *)id->subnqn, sizeof(id->subnqn), (char*)subsys->subnqn);
+}
+}
+
 static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
 {
 NvmeIdCtrl *id = >id_ctrl;
 uint8_t *pci_conf = pci_dev->config;
-char *subnqn;
 
 id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
 id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
@@ -4455,9 +4475,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice 
*pci_dev)
 id->sgls = cpu_to_le32(NVME_CTRL_SGLS_SUPPORT_NO_ALIGN |
NVME_CTRL_SGLS_BITBUCKET);
 
-subnqn = g_strdup_printf("nqn.2019-08.org.qemu:%s", n->params.serial);
-strpadcpy((char *)id->subnqn, sizeof(id->subnqn), subnqn, '\0');
-g_free(subnqn);
+nvme_init_subnqn(n);
 
 id->psd[0].mp = cpu_to_le16(0x9c4);
 id->psd[0].enlat = cpu_to_le32(0x10);
@@ -4545,6 +4563,8 @@ static Property nvme_props[] = {
 DEFINE_BLOCK_PROPERTIES(NvmeCtrl, namespace.blkconf),
 DEFINE_PROP_LINK("pmrdev", NvmeCtrl, pmr.dev, TYPE_MEMORY_BACKEND,
  HostMemoryBackend *),
+DEFINE_PROP_LINK("subsys", NvmeCtrl, subsys, TYPE_NVME_SUBSYS,
+ NvmeSubsystem *),
 DEFINE_PROP_STRING("serial", NvmeCtrl, params.serial),
 DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, params.cmb_size_mb, 0),
 DEFINE_PROP_UINT32("num_queues", NvmeCtrl, params.num_queues, 0),
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index dee6092bd45f..04d4684601fd 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -2,6 +2,7 @@
 #define HW_NVME_H
 
 #include "block/nvme.h"
+#include "nvme-subsys.h"
 #include "nvme-ns.h"
 
 #define NVME_MAX_NAMESPACES 256
@@ -170,6 +171,8 @@ typedef struct NvmeCtrl {
 
 uint8_t zasl;
 
+NvmeSubsystem   *subsys;
+
 NvmeNamespace   namespace;
 NvmeNamespace   *namespaces[NVME_MAX_NAMESPACES];
 NvmeSQueue  **sq;
-- 
2.17.1

[PATCH V6 1/6] hw/block/nvme: introduce nvme-subsys device

[Minwoo Im]

To support multi-path in QEMU NVMe device model, We need to have NVMe
subsystem hierarchy to map controllers and namespaces to a NVMe
subsystem.

This patch introduced a simple nvme-subsys device model.  The subsystem
will be prepared with subsystem NQN with  provided in
nvme-subsys device:

  ex) -device nvme-subsys,id=subsys0: nqn.2019-08.org.qemu:subsys0

Signed-off-by: Minwoo Im 
---
 hw/block/meson.build   |  2 +-
 hw/block/nvme-subsys.c | 60 ++
 hw/block/nvme-subsys.h | 25 ++
 hw/block/nvme.c|  3 +++
 4 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 hw/block/nvme-subsys.c
 create mode 100644 hw/block/nvme-subsys.h

diff --git a/hw/block/meson.build b/hw/block/meson.build
index 602ca6c8541d..83ea2d37978d 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -13,7 +13,7 @@ softmmu_ss.add(when: 'CONFIG_SSI_M25P80', if_true: 
files('m25p80.c'))
 softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
 softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
 softmmu_ss.add(when: 'CONFIG_SH4', if_true: files('tc58128.c'))
-softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c'))
+softmmu_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('nvme.c', 'nvme-ns.c', 
'nvme-subsys.c'))
 
 specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
 specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: 
files('vhost-user-blk.c'))
diff --git a/hw/block/nvme-subsys.c b/hw/block/nvme-subsys.c
new file mode 100644
index ..aa82911b951c
--- /dev/null
+++ b/hw/block/nvme-subsys.c
@@ -0,0 +1,60 @@
+/*
+ * QEMU NVM Express Subsystem: nvme-subsys
+ *
+ * Copyright (c) 2021 Minwoo Im 
+ *
+ * This code is licensed under the GNU GPL v2.  Refer COPYING.
+ */
+
+#include "qemu/units.h"
+#include "qemu/osdep.h"
+#include "qemu/uuid.h"
+#include "qemu/iov.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
+#include "hw/qdev-properties.h"
+#include "hw/qdev-core.h"
+#include "hw/block/block.h"
+#include "block/aio.h"
+#include "block/accounting.h"
+#include "sysemu/sysemu.h"
+#include "hw/pci/pci.h"
+#include "nvme.h"
+#include "nvme-subsys.h"
+
+static void nvme_subsys_setup(NvmeSubsystem *subsys)
+{
+snprintf((char *)subsys->subnqn, sizeof(subsys->subnqn),
+ "nqn.2019-08.org.qemu:%s", subsys->parent_obj.id);
+}
+
+static void nvme_subsys_realize(DeviceState *dev, Error **errp)
+{
+NvmeSubsystem *subsys = NVME_SUBSYS(dev);
+
+nvme_subsys_setup(subsys);
+}
+
+static void nvme_subsys_class_init(ObjectClass *oc, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(oc);
+
+set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+
+dc->realize = nvme_subsys_realize;
+dc->desc = "Virtual NVMe subsystem";
+}
+
+static const TypeInfo nvme_subsys_info = {
+.name = TYPE_NVME_SUBSYS,
+.parent = TYPE_DEVICE,
+.class_init = nvme_subsys_class_init,
+.instance_size = sizeof(NvmeSubsystem),
+};
+
+static void nvme_subsys_register_types(void)
+{
+type_register_static(_subsys_info);
+}
+
+type_init(nvme_subsys_register_types)
diff --git a/hw/block/nvme-subsys.h b/hw/block/nvme-subsys.h
new file mode 100644
index ..40f06a4c7db0
--- /dev/null
+++ b/hw/block/nvme-subsys.h
@@ -0,0 +1,25 @@
+/*
+ * QEMU NVM Express Subsystem: nvme-subsys
+ *
+ * Copyright (c) 2021 Minwoo Im 
+ *
+ * This code is licensed under the GNU GPL v2.  Refer COPYING.
+ */
+
+#ifndef NVME_SUBSYS_H
+#define NVME_SUBSYS_H
+
+#define TYPE_NVME_SUBSYS "nvme-subsys"
+#define NVME_SUBSYS(obj) \
+OBJECT_CHECK(NvmeSubsystem, (obj), TYPE_NVME_SUBSYS)
+
+#define NVME_SUBSYS_MAX_CTRLS   32
+
+typedef struct NvmeCtrl NvmeCtrl;
+typedef struct NvmeNamespace NvmeNamespace;
+typedef struct NvmeSubsystem {
+DeviceState parent_obj;
+uint8_t subnqn[256];
+} NvmeSubsystem;
+
+#endif /* NVME_SUBSYS_H */
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 21aec90637fa..aabccdf36f4b 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -25,6 +25,7 @@
  *  mdts=,zoned.append_size_limit= \
  *  -device nvme-ns,drive=,bus=,nsid=,\
  *  zoned=
+ *  -device nvme-subsys,id=
  *
  * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at
  * offset 0 in BAR2 and supports only WDS, RDS and SQS for now. By default, the
@@ -38,6 +39,8 @@
  *
  * The PMR will use BAR 4/5 exclusively.
  *
+ * To place controller(s) and namespace(s) to a subsystem, then provide
+ * nvme-subsys device as above.
  *
  * nvme device parameters
  * ~~
-- 
2.17.1

[PATCH V6 0/6] hw/block/nvme: support multi-path for ctrl/ns

[Minwoo Im]

Hello,

This is sixth patch series for the support of NVMe subsystem scheme with
multi-controller and namespace sharing in a subsystem.

This version has a fix in nvme_init_ctrl() when 'cntlid' is set to the
Identify Controller data structure by making it by cpu_to_le16() as
Keith reviewed.

Here's test result with a simple 'nvme list -v' command from this model:

  -device nvme-subsys,id=subsys0 \
  -device nvme,serial=foo,id=nvme0,subsys=subsys0 \
  -device nvme,serial=bar,id=nvme1,subsys=subsys0 \
  -device nvme,serial=baz,id=nvme2,subsys=subsys0 \
  -device nvme-ns,id=ns1,drive=drv10,nsid=1,subsys=subsys0 \
  -device nvme-ns,id=ns2,drive=drv11,nsid=2,bus=nvme2 \
  \
  -device nvme,serial=qux,id=nvme3 \
  -device nvme-ns,id=ns3,drive=drv12,nsid=3,bus=nvme3 \
  \
  -device nvme-subsys,id=subsys1 \
  -device nvme,serial=quux,id=nvme4,subsys=subsys1 \
  -device nvme-ns,id=ns4,drive=drv13,nsid=1,subsys=subsys1,zoned=true \

  root@vm:~/work# nvme list -v
  NVM Express Subsystems

  SubsystemSubsystem-NQN
Controllers
   

 
  nvme-subsys1 nqn.2019-08.org.qemu:subsys0 
nvme0, nvme1, nvme2
  nvme-subsys3 nqn.2019-08.org.qemu:qux 
nvme3
  nvme-subsys4 nqn.2019-08.org.qemu:subsys1 
nvme4

  NVM Express Controllers

  Device   SN   MN   FR 
  TxPort AddressSubsystemNamespaces
     
 -- --  
  nvme0foo  QEMU NVMe Ctrl   1.0
  pcie   :00:06.0   nvme-subsys1 nvme1c0n1
  nvme1bar  QEMU NVMe Ctrl   1.0
  pcie   :00:07.0   nvme-subsys1 nvme1c1n1
  nvme2baz  QEMU NVMe Ctrl   1.0
  pcie   :00:08.0   nvme-subsys1 nvme1c2n1, nvme1c2n2
  nvme3qux  QEMU NVMe Ctrl   1.0
  pcie   :00:09.0   nvme-subsys3 nvme3n1
  nvme4quux QEMU NVMe Ctrl   1.0
  pcie   :00:0a.0   nvme-subsys4 nvme4c4n1

  NVM Express Namespaces

  Device   NSID Usage  Format   Controllers
    --  

  nvme1n1  1134.22  MB / 134.22  MB512   B +  0 B   nvme0, 
nvme1, nvme2
  nvme1n2  2268.44  MB / 268.44  MB512   B +  0 B   nvme2
  nvme3n1  3268.44  MB / 268.44  MB512   B +  0 B   nvme3
  nvme4n1  1268.44  MB / 268.44  MB512   B +  0 B   nvme4

Thanks,

Since V5:
  - Fix endianness for 'cntlid' in Identify Controller data structure.
(Keith)

Since V4:
  - Code clean-up to snprintf rather than duplicating it and copy.
(Keith)
  - Documentation for 'subsys' clean-up.  (Keith)
  - Remove 'cntlid' param from nvme_init_ctrl().  (Keith)
  - Put error_propagate() in nvme_realize().  (Keith)

Since RFC V3:
  - Exclude 'deatched' scheme from this series.  This will be covered in
the next series by covering all the ns-related admin commands
including ZNS and ns-mgmt. (Niklas)
  - Rebased on nvme-next.
  - Remove RFC tag from this V4.

Since RFC V2:
  - Rebased on nvme-next branch with trivial patches from the previous
version(V2) applied. (Klaus)
  - Fix enumeration type name convention with NvmeIdNs prefix. (Klaus)
  - Put 'cntlid' to NvmeCtrl instance in nvme_init_ctrl() which was
missed in V2.
  - Added 'detached' parameter to nvme-ns device to decide whether to
attach or not to controller(s) in the subsystem. (Klaus)
  - Implemented Identify Active Namespace ID List aprt from Identify
Allocated Namespace ID List by removing fall-thru statement.

Since RFC V1:
  - Updated namespace sharing scheme to be based on nvme-subsys
hierarchy.

Minwoo Im (6):
  hw/block/nvme: introduce nvme-subsys device
  hw/block/nvme: support to map controller to a subsystem
  hw/block/nvme: add CMIC enum value for Identify Controller
  hw/block/nvme: support for multi-controller in subsystem
  hw/block/nvme: add NMIC enum value for Identify Namespace
  hw/block/nvme: support for shared namespace in subsystem

 hw/block/meson.build   |   2 +-
 hw/block/nvme-ns.c |  23 +++--
 hw/block/nvme-ns.h |   7 +++
 hw/block/nvme-subsys.c | 106 +
 hw/block/nvme-subsys.h |  32 +
 hw/block/nvme.c|  72 +---
 hw/block/nvme.h|   

[PATCH v8 4/5] iotests: rewrite check into python

[Vladimir Sementsov-Ogievskiy]

Just use classes introduced in previous three commits. Behavior
difference is described in these three commits.

Drop group file, as it becomes unused.

Drop common.env: now check is in python, and for tests we use same
python interpreter that runs the check itself. Use build environment
PYTHON in check-block instead, to keep "make check" use the same
python.

Checking for virtio-blk moved to iotests.py, as it actually iotests.py
dependency. Actually not all python iotests depend on it, so in future
it may be refactored to checked only when really needed.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 Makefile |1 -
 tests/check-block.sh |2 +-
 tests/qemu-iotests/check | 1095 --
 tests/qemu-iotests/common.env.in |3 -
 tests/qemu-iotests/group |  321 -
 tests/qemu-iotests/iotests.py|8 +
 tests/qemu-iotests/meson.build   |3 -
 7 files changed, 138 insertions(+), 1295 deletions(-)
 delete mode 100644 tests/qemu-iotests/common.env.in
 delete mode 100644 tests/qemu-iotests/group

diff --git a/Makefile b/Makefile
index 2a926aaeb0..d335d257bc 100644
--- a/Makefile
+++ b/Makefile
@@ -236,7 +236,6 @@ distclean: clean
rm -f config-host.mak config-host.h*
rm -f tests/tcg/config-*.mak
rm -f config-all-disas.mak config.status
-   rm -f tests/qemu-iotests/common.env
rm -f roms/seabios/config.mak roms/vgabios/config.mak
rm -f qemu-plugins-ld.symbols qemu-plugins-ld64.symbols
rm -f *-config-target.h *-config-devices.mak *-config-devices.h
diff --git a/tests/check-block.sh b/tests/check-block.sh
index e4f37905be..ac32fd67dd 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -76,7 +76,7 @@ export QEMU_CHECK_BLOCK_AUTO=1
 
 ret=0
 for fmt in $format_list ; do
-./check -makecheck -$fmt $group || ret=1
+${PYTHON} ./check -makecheck -$fmt $group || ret=1
 done
 
 exit $ret
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 952762d5ed..5190dee82e 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -1,7 +1,8 @@
-#!/usr/bin/env bash
+#!/usr/bin/env python3
 #
-# Copyright (C) 2009 Red Hat, Inc.
-# Copyright (c) 2000-2002,2006 Silicon Graphics, Inc.  All Rights Reserved.
+# Configure environment and run group of tests in it.
+#
+# Copyright (c) 2020-2021 Virtuozzo International GmbH
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License as
@@ -14,967 +15,129 @@
 #
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see .
-#
-#
-# Control script for QA
-#
-
-status=0
-needwrap=true
-try=0
-n_bad=0
-bad=""
-notrun=""
-casenotrun=""
-interrupt=true
-makecheck=false
-
-_init_error()
-{
-echo "check: $1" >&2
-exit 1
-}
-
-if [ -L "$0" ]
-then
-# called from the build tree
-source_iotests=$(dirname "$(readlink "$0")")
-if [ -z "$source_iotests" ]
-then
-_init_error "failed to obtain source tree name from check symlink"
-fi
-source_iotests=$(cd "$source_iotests"; pwd) || _init_error "failed to 
enter source tree"
-build_iotests=$(cd "$(dirname "$0")"; pwd)
-else
-# called from the source tree
-source_iotests=$PWD
-# this may be an in-tree build (note that in the following code we may not
-# assume that it truly is and have to test whether the build results
-# actually exist)
-build_iotests=$PWD
-fi
-
-build_root="$build_iotests/../.."
-
-# we need common.env
-if ! . "$build_iotests/common.env"
-then
-_init_error "failed to source common.env (make sure the qemu-iotests are 
run from tests/qemu-iotests in the build tree)"
-fi
-
-# we need common.config
-if ! . "$source_iotests/common.config"
-then
-_init_error "failed to source common.config"
-fi
-
-_full_imgfmt_details()
-{
-if [ -n "$IMGOPTS" ]; then
-echo "$IMGFMT ($IMGOPTS)"
-else
-echo "$IMGFMT"
-fi
-}
-
-_full_platform_details()
-{
-os=$(uname -s)
-host=$(hostname -s)
-kernel=$(uname -r)
-platform=$(uname -m)
-echo "$os/$platform $host $kernel"
-}
-
-_full_env_details()
-{
-cat < /dev/null)
-if [ -n "$p" -a -x "$p" ]; then
-type -p "$p"
-else
-return 1
-fi
-}
-
-if [ -z "$TEST_DIR" ]; then
-TEST_DIR=$PWD/scratch
-fi
-mkdir -p "$TEST_DIR" || _init_error 'Failed to create TEST_DIR'
-
-tmp_sock_dir=false
-if [ -z "$SOCK_DIR" ]; then
-SOCK_DIR=$(mktemp -d)
-tmp_sock_dir=true
-fi
-mkdir -p "$SOCK_DIR" || _init_error 'Failed to create SOCK_DIR'
-
-diff="diff -u"
-verbose=false
-debug=false
-group=false
-xgroup=false
-imgopts=false
-showme=false
-sortme=false
-expunge=true
-have_test_arg=false
-cachemode=false
-aiomode=false
-
-tmp="${TEST_DIR}"/$$
-rm -f $tmp.list $tmp.tmp $tmp.sed
-
-export IMGFMT=raw
-export IMGFMT_GENERIC=true
-export 

[PATCH v8 5/5] iotests: rename and move 169 and 199 tests

[Vladimir Sementsov-Ogievskiy]

Rename bitmaps migration tests and move them to tests subdirectory to
demonstrate new human-friendly test naming.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/{199 => tests/migrate-bitmaps-postcopy-test}   | 0
 .../{199.out => tests/migrate-bitmaps-postcopy-test.out}  | 0
 tests/qemu-iotests/{169 => tests/migrate-bitmaps-test}| 0
 tests/qemu-iotests/{169.out => tests/migrate-bitmaps-test.out}| 0
 4 files changed, 0 insertions(+), 0 deletions(-)
 rename tests/qemu-iotests/{199 => tests/migrate-bitmaps-postcopy-test} (100%)
 rename tests/qemu-iotests/{199.out => tests/migrate-bitmaps-postcopy-test.out} 
(100%)
 rename tests/qemu-iotests/{169 => tests/migrate-bitmaps-test} (100%)
 rename tests/qemu-iotests/{169.out => tests/migrate-bitmaps-test.out} (100%)

diff --git a/tests/qemu-iotests/199 
b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
similarity index 100%
rename from tests/qemu-iotests/199
rename to tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
diff --git a/tests/qemu-iotests/199.out 
b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test.out
similarity index 100%
rename from tests/qemu-iotests/199.out
rename to tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test.out
diff --git a/tests/qemu-iotests/169 
b/tests/qemu-iotests/tests/migrate-bitmaps-test
similarity index 100%
rename from tests/qemu-iotests/169
rename to tests/qemu-iotests/tests/migrate-bitmaps-test
diff --git a/tests/qemu-iotests/169.out 
b/tests/qemu-iotests/tests/migrate-bitmaps-test.out
similarity index 100%
rename from tests/qemu-iotests/169.out
rename to tests/qemu-iotests/tests/migrate-bitmaps-test.out
-- 
2.29.2

[PATCH v8 1/5] iotests: add findtests.py

[Vladimir Sementsov-Ogievskiy]

Add python script with new logic of searching for tests:

Current ./check behavior:
 - tests are named [0-9][0-9][0-9]
 - tests must be registered in group file (even if test doesn't belong
   to any group, like 142)

Behavior of findtests.py:
 - group file is dropped
 - tests are all files in tests/ subdirectory (except for .out files),
   so it's not needed more to "register the test", just create it with
   appropriate name in tests/ subdirectory. Old names like
   [0-9][0-9][0-9] (in root iotests directory) are supported too, but
   not recommended for new tests
 - groups are parsed from '# group: ' line inside test files
 - optional file group.local may be used to define some additional
   groups for downstreams
 - 'disabled' group is used to temporary disable tests. So instead of
   commenting tests in old 'group' file you now can add them to
   disabled group with help of 'group.local' file
 - selecting test ranges like 5-15 are not supported more
   (to support restarting failed ./check command from the middle of the
process, new argument is added: --start-from)

Benefits:
 - no rebase conflicts in group file on patch porting from branch to
   branch
 - no conflicts in upstream, when different series want to occupy same
   test number
 - meaningful names for test files
   For example, with digital number, when some person wants to add some
   test about block-stream, he most probably will just create a new
   test. But if there would be test-block-stream test already, he will
   at first look at it and may be just add a test-case into it.
   And anyway meaningful names are better.

This commit doesn't update check behavior (which will be done in
further commit), still, the documentation changed like new behavior is
already here.  Let's live with this small inconsistency for the
following few commits, until final change.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Eric Blake 
---
 docs/devel/testing.rst  |  50 +-
 tests/qemu-iotests/findtests.py | 159 
 2 files changed, 208 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemu-iotests/findtests.py

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 0aa7a13bba..454b706141 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -111,7 +111,7 @@ check-block
 ---
 
 ``make check-block`` runs a subset of the block layer iotests (the tests that
-are in the "auto" group in ``tests/qemu-iotests/group``).
+are in the "auto" group).
 See the "QEMU iotests" section below for more information.
 
 GCC gcov support
@@ -224,6 +224,54 @@ another application on the host may have locked the file, 
possibly leading to a
 test failure.  If using such devices are explicitly desired, consider adding
 ``locking=off`` option to disable image locking.
 
+Test case groups
+
+
+"Tests may belong to one or more test groups, which are defined in the form
+of a comment in the test source file. By convention, test groups are listed
+in the second line of the test file, after the "#!/..." line, like this:
+
+.. code::
+
+  #!/usr/bin/env python3
+  # group: auto quick
+  #
+  ...
+
+Another way of defining groups is creating the tests/qemu-iotests/group.local
+file. This should be used only for downstream (this file should never appear
+in upstream). This file may be used for defining some downstream test groups
+or for temporarily disabling tests, like this:
+
+.. code::
+
+  # groups for some company downstream process
+  #
+  # ci - tests to run on build
+  # down - our downstream tests, not for upstream
+  #
+  # Format of each line is:
+  # TEST_NAME TEST_GROUP [TEST_GROUP ]...
+
+  013 ci
+  210 disabled
+  215 disabled
+  our-ugly-workaround-test down ci
+
+Note that the following group names have a special meaning:
+
+- quick: Tests in this group should finish within a few seconds.
+
+- auto: Tests in this group are used during "make check" and should be
+  runnable in any case. That means they should run with every QEMU binary
+  (also non-x86), with every QEMU configuration (i.e. must not fail if
+  an optional feature is not compiled in - but reporting a "skip" is ok),
+  work at least with the qcow2 file format, work with all kind of host
+  filesystems and users (e.g. "nobody" or "root") and must not take too
+  much memory and disk space (since CI pipelines tend to fail otherwise).
+
+- disabled: Tests in this group are disabled and ignored by check.
+
 .. _docker-ref:
 
 Docker based tests
diff --git a/tests/qemu-iotests/findtests.py b/tests/qemu-iotests/findtests.py
new file mode 100644
index 00..d0c72efd6a
--- /dev/null
+++ b/tests/qemu-iotests/findtests.py
@@ -0,0 +1,159 @@
+# TestFinder class, define set of tests to run.
+#
+# Copyright (c) 2020-2021 Virtuozzo International GmbH
+#
+# 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 

[PATCH v8 3/5] iotests: add testrunner.py

[Vladimir Sementsov-Ogievskiy]

Add TestRunner class, which will run tests in a new python iotests
running framework.

There are some differences with current ./check behavior, most
significant are:
- Consider all tests self-executable, just run them, don't run python
  by hand.
- Elapsed time is cached in json file
- Elapsed time precision increased a bit
- Instead of using "diff -w" which ignores all whitespace differences,
  manually strip whitespace at line end then use python difflib, which
  no longer ignores spacing mid-line

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/testrunner.py | 366 +++
 1 file changed, 366 insertions(+)
 create mode 100644 tests/qemu-iotests/testrunner.py

diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
new file mode 100644
index 00..e80c25adba
--- /dev/null
+++ b/tests/qemu-iotests/testrunner.py
@@ -0,0 +1,366 @@
+# Class for actually running tests.
+#
+# Copyright (c) 2020-2021 Virtuozzo International GmbH
+#
+# 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.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+from pathlib import Path
+import datetime
+import time
+import difflib
+import subprocess
+import contextlib
+import json
+import termios
+import sys
+from contextlib import contextmanager
+from contextlib import AbstractContextManager
+from typing import List, Optional, Iterator, Any, Sequence
+
+from testenv import TestEnv
+
+
+def silent_unlink(path: Path) -> None:
+try:
+path.unlink()
+except OSError:
+pass
+
+
+def file_diff(file1: str, file2: str) -> List[str]:
+with open(file1) as f1, open(file2) as f2:
+# We want to ignore spaces at line ends. There are a lot of mess about
+# it in iotests.
+# TODO: fix all tests to not produce extra spaces, fix all .out files
+# and use strict diff here!
+seq1 = [line.rstrip() for line in f1]
+seq2 = [line.rstrip() for line in f2]
+res = [line.rstrip()
+   for line in difflib.unified_diff(seq1, seq2, file1, file2)]
+return res
+
+
+# We want to save current tty settings during test run,
+# since an aborting qemu call may leave things screwed up.
+@contextmanager
+def savetty() -> Iterator[None]:
+isterm = sys.stdin.isatty()
+if isterm:
+fd = sys.stdin.fileno()
+attr = termios.tcgetattr(fd)
+
+try:
+yield
+finally:
+if isterm:
+termios.tcsetattr(fd, termios.TCSADRAIN, attr)
+
+
+class LastElapsedTime(AbstractContextManager['LastElapsedTime']):
+""" Cache for elapsed time for tests, to show it during new test run
+
+It is safe to use get() at any time.  To use update(), you must either
+use it inside with-block or use save() after update().
+"""
+def __init__(self, cache_file: str, env: TestEnv) -> None:
+self.env = env
+self.cache_file = cache_file
+
+try:
+with open(cache_file) as f:
+self.cache = json.load(f)
+except (OSError, ValueError):
+self.cache = {}
+
+def get(self, test: str,
+default: Optional[float] = None) -> Optional[float]:
+if test not in self.cache:
+return default
+
+if self.env.imgproto not in self.cache[test]:
+return default
+
+return self.cache[test][self.env.imgproto].get(self.env.imgfmt,
+   default)
+
+def update(self, test: str, elapsed: float) -> None:
+d = self.cache.setdefault(test, {})
+d = d.setdefault(self.env.imgproto, {})
+d[self.env.imgfmt] = elapsed
+
+def save(self) -> None:
+with open(self.cache_file, 'w') as f:
+json.dump(self.cache, f)
+
+def __enter__(self) -> 'LastElapsedTime':
+return self
+
+def __exit__(self, exc_type: Any, exc_value: Any, traceback: Any) -> None:
+self.save()
+
+
+class TestResult:
+def __init__(self, status: str, description: str = '',
+ elapsed: Optional[float] = None, diff: Sequence[str] = (),
+ casenotrun: str = '', interrupted: bool = False) -> None:
+self.status = status
+self.description = description
+self.elapsed = elapsed
+self.diff = diff
+self.casenotrun = casenotrun
+self.interrupted = interrupted
+
+
+class 

[PATCH v8 2/5] iotests: add testenv.py

[Vladimir Sementsov-Ogievskiy]

Add TestEnv class, which will handle test environment in a new python
iotests running framework.

Don't add compat=1.1 for qcow2 IMGOPTS, as v3 is default anyway.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 tests/qemu-iotests/testenv.py | 278 ++
 1 file changed, 278 insertions(+)
 create mode 100644 tests/qemu-iotests/testenv.py

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
new file mode 100644
index 00..348af593e9
--- /dev/null
+++ b/tests/qemu-iotests/testenv.py
@@ -0,0 +1,278 @@
+# TestEnv class to manage test environment variables.
+#
+# Copyright (c) 2020-2021 Virtuozzo International GmbH
+#
+# 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.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+import os
+import sys
+import tempfile
+from pathlib import Path
+import shutil
+import collections
+import random
+import subprocess
+import glob
+from contextlib import AbstractContextManager
+from typing import Dict, Any, Optional
+
+
+def get_default_machine(qemu_prog: str) -> str:
+outp = subprocess.run([qemu_prog, '-machine', 'help'], check=True,
+  universal_newlines=True,
+  stdout=subprocess.PIPE).stdout
+
+machines = outp.split('\n')
+default_machine = next(m for m in machines if m.endswith(' (default)'))
+default_machine = default_machine.split(' ', 1)[0]
+
+alias_suf = ' (alias of {})'.format(default_machine)
+alias = next((m for m in machines if m.endswith(alias_suf)), None)
+if alias is not None:
+default_machine = alias.split(' ', 1)[0]
+
+return default_machine
+
+
+class TestEnv(AbstractContextManager['TestEnv']):
+"""
+Manage system environment for running tests
+
+The following variables are supported/provided. They are represented by
+lower-cased TestEnv attributes.
+"""
+
+# We store environment variables as instance attributes, and there are a
+# lot of them. Silence pylint:
+# pylint: disable=too-many-instance-attributes
+
+env_variables = ['PYTHONPATH', 'TEST_DIR', 'SOCK_DIR', 'SAMPLE_IMG_DIR',
+ 'OUTPUT_DIR', 'PYTHON', 'QEMU_PROG', 'QEMU_IMG_PROG',
+ 'QEMU_IO_PROG', 'QEMU_NBD_PROG', 'QSD_PROG',
+ 'SOCKET_SCM_HELPER', 'QEMU_OPTIONS', 'QEMU_IMG_OPTIONS',
+ 'QEMU_IO_OPTIONS', 'QEMU_IO_OPTIONS_NO_FMT',
+ 'QEMU_NBD_OPTIONS', 'IMGOPTS', 'IMGFMT', 'IMGPROTO',
+ 'AIOMODE', 'CACHEMODE', 'VALGRIND_QEMU',
+ 'CACHEMODE_IS_DEFAULT', 'IMGFMT_GENERIC', 'IMGOPTSSYNTAX',
+ 'IMGKEYSECRET', 'QEMU_DEFAULT_MACHINE', 'MALLOC_PERTURB_']
+
+def get_env(self) -> Dict[str, str]:
+env = {}
+for v in self.env_variables:
+val = getattr(self, v.lower(), None)
+if val is not None:
+env[v] = val
+
+return env
+
+def init_directories(self):
+"""Init directory variables:
+ PYTHONPATH
+ TEST_DIR
+ SOCK_DIR
+ SAMPLE_IMG_DIR
+ OUTPUT_DIR
+"""
+self.pythonpath = os.getenv('PYTHONPATH')
+if self.pythonpath:
+self.pythonpath = self.source_iotests + os.pathsep + \
+self.pythonpath
+else:
+self.pythonpath = self.source_iotests
+
+self.test_dir = os.getenv('TEST_DIR',
+  os.path.join(os.getcwd(), 'scratch'))
+Path(self.test_dir).mkdir(parents=True, exist_ok=True)
+
+self.sock_dir = os.getenv('SOCK_DIR')
+self.tmp_sock_dir = False
+if self.sock_dir:
+Path(self.test_dir).mkdir(parents=True, exist_ok=True)
+else:
+self.sock_dir = tempfile.mkdtemp()
+self.tmp_sock_dir = True
+
+self.sample_img_dir = os.getenv('SAMPLE_IMG_DIR',
+os.path.join(self.source_iotests,
+ 'sample_images'))
+
+self.output_dir = os.getcwd()  # OUTPUT_DIR
+
+def init_binaries(self):
+"""Init binary path variables:
+ PYTHON (for bash tests)
+ QEMU_PROG, QEMU_IMG_PROG, QEMU_IO_PROG, QEMU_NBD_PROG, QSD_PROG
+ SOCKET_SCM_HELPER
+"""
+self.python = sys.executable
+
+def 

[PATCH v8 0/5] Rework iotests/check

[Vladimir Sementsov-Ogievskiy]

v8:

about linters:

I didn't modify 297, as Max already staged 297 modifications to test all files.

Also, now I have two complains:
+* Module testenv
+testenv.py:158:4: R0915: Too many statements (53/50) (too-many-statements)
+* Module testrunner
+testrunner.py:222:4: R0911: Too many return statements (7/6) 
(too-many-return-statements)
 Success: no issues found in 5 source files

And I feel, I'm tired to refactor it now.. Probably we can ignore them in 297. 
Probably I can
do some refactoring as a follow-up.

findtests: grammar, wording, add Eric's r-b
testenv: commit msg adjusted
 wording, grammar
 missed env_variables added
 add logic for searching qemu-system-*, if
 qemu-system-{arch} is not exist
 handle valgrind and MALLOC_PERTURB_
 fix s/vmkd/vmdk/
testrunner: commit msg adjusted
wording
fix extra new lines in diff output
s/0/fd/
use class instead of namedtuple
don't handle MALLOC_PERTURB_
declare self._stack in __init__
change thistime to be float
handle KeyboardInterrupt
casenotrun init to ''
fix "output mismatch .. {f_bad}" to be f-string
smarter terminal color handling
check: wording, grammar, sheepdoG, exClude_groups
   change valgrind to be simple boolean option and pass
  it to TestEnv
   handle group.local
   check virtio-blk in iotests.py
   add --color option

Vladimir Sementsov-Ogievskiy (5):
  iotests: add findtests.py
  iotests: add testenv.py
  iotests: add testrunner.py
  iotests: rewrite check into python
  iotests: rename and move 169 and 199 tests

 docs/devel/testing.rst|   50 +-
 Makefile  |1 -
 tests/check-block.sh  |2 +-
 tests/qemu-iotests/check  | 1095 ++---
 tests/qemu-iotests/common.env.in  |3 -
 tests/qemu-iotests/findtests.py   |  159 +++
 tests/qemu-iotests/group  |  321 -
 tests/qemu-iotests/iotests.py |8 +
 tests/qemu-iotests/meson.build|3 -
 tests/qemu-iotests/testenv.py |  278 +
 tests/qemu-iotests/testrunner.py  |  366 ++
 .../migrate-bitmaps-postcopy-test}|0
 .../migrate-bitmaps-postcopy-test.out}|0
 .../{169 => tests/migrate-bitmaps-test}   |0
 .../migrate-bitmaps-test.out} |0
 15 files changed, 990 insertions(+), 1296 deletions(-)
 delete mode 100644 tests/qemu-iotests/common.env.in
 create mode 100644 tests/qemu-iotests/findtests.py
 delete mode 100644 tests/qemu-iotests/group
 create mode 100644 tests/qemu-iotests/testenv.py
 create mode 100644 tests/qemu-iotests/testrunner.py
 rename tests/qemu-iotests/{199 => tests/migrate-bitmaps-postcopy-test} (100%)
 rename tests/qemu-iotests/{199.out => tests/migrate-bitmaps-postcopy-test.out} 
(100%)
 rename tests/qemu-iotests/{169 => tests/migrate-bitmaps-test} (100%)
 rename tests/qemu-iotests/{169.out => tests/migrate-bitmaps-test.out} (100%)

-- 
2.29.2

Re: [PATCH v8 09/11] block: check availablity for preadv/pwritev on mac

[Joelle van Dyne]

Sorry to rephrase this, we would like a single binary to work on both
macOS 10.15 and macOS 11 as well as on both iOS 13 and iOS 14 while
ALSO supporting preadv/pwritev when running on the newer OS.

-j

On Sat, Jan 23, 2021 at 12:00 PM Joelle van Dyne  wrote:
>
> The problem here is that we would like to compile on macOS 11 and be
> able to run the built binaries on macOS 10.15 or lower. Without the
> runtime checks, you'll have a crash when calling preadv/pwritev.
>
> -j
>
> On Sat, Jan 23, 2021 at 5:53 AM Peter Maydell  
> wrote:
> >
> > On Sat, 23 Jan 2021 at 05:03, Joelle van Dyne  wrote:
> > >
> > > macOS 11/iOS 14 added preadv/pwritev APIs. Due to weak linking, configure
> > > will succeed with CONFIG_PREADV even when targeting a lower OS version.
> > > We therefore need to check at run time if we can actually use these APIs.
> >
> > If you make the preadv check be a meson.build has_function()
> > test, I think (but have not tested) that it ought to correctly
> > fail the link in the targeting-lower-OS-version case, because
> > meson.build's has_function() implementation for clang
> > https://github.com/mesonbuild/meson/blob/bad0e95caec4cf49165572df3cf2edc4832280fa/mesonbuild/compilers/mixins/clang.py#L89
> > adds -Wl,-no-weak-imports when it's building the test-case
> > exactly to force that linker visibility obeys the minimum
> > version targets for OSX.
> >
> > If that works I think it would be neater than delaying the
> > check to runtime (and it would also mean another test moved
> > out of configure and into meson.build, which is the direction
> > we're heading anyway).
> >
> > thanks
> > -- PMM

Re: [PATCH v8 09/11] block: check availablity for preadv/pwritev on mac

[Joelle van Dyne]

The problem here is that we would like to compile on macOS 11 and be
able to run the built binaries on macOS 10.15 or lower. Without the
runtime checks, you'll have a crash when calling preadv/pwritev.

-j

On Sat, Jan 23, 2021 at 5:53 AM Peter Maydell  wrote:
>
> On Sat, 23 Jan 2021 at 05:03, Joelle van Dyne  wrote:
> >
> > macOS 11/iOS 14 added preadv/pwritev APIs. Due to weak linking, configure
> > will succeed with CONFIG_PREADV even when targeting a lower OS version.
> > We therefore need to check at run time if we can actually use these APIs.
>
> If you make the preadv check be a meson.build has_function()
> test, I think (but have not tested) that it ought to correctly
> fail the link in the targeting-lower-OS-version case, because
> meson.build's has_function() implementation for clang
> https://github.com/mesonbuild/meson/blob/bad0e95caec4cf49165572df3cf2edc4832280fa/mesonbuild/compilers/mixins/clang.py#L89
> adds -Wl,-no-weak-imports when it's building the test-case
> exactly to force that linker visibility obeys the minimum
> version targets for OSX.
>
> If that works I think it would be neater than delaying the
> check to runtime (and it would also mean another test moved
> out of configure and into meson.build, which is the direction
> we're heading anyway).
>
> thanks
> -- PMM

Re: [PULL 0/8] Usb 20210122 patches

[Peter Maydell]

On Fri, 22 Jan 2021 at 17:53, Gerd Hoffmann  wrote:
>
> The following changes since commit 0e32462630687a18039464511bd0447ada5709c3:
>
>   Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-6.0-pull-=
> request' into staging (2021-01-22 10:35:55 +)
>
> are available in the Git repository at:
>
>   git://git.kraxel.org/qemu tags/usb-20210122-pull-request
>
> for you to fetch changes up to 2980a316734c420e7398aec026909dcfc8614c1d:
>
>   usb-host: map LIBUSB_SPEED_SUPER_PLUS to USB_SPEED_SUPER (2021-01-22 14:51:=
> 35 +0100)
>
> 
> usb: cleanups and fixes.
> usb: add pcap support.
>

Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM

Re: [PATCH v7 00/11] Rework iotests/check

[Vladimir Sementsov-Ogievskiy]

22.01.2021 19:16, Kevin Wolf wrote:

Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:

Hi all!

These series has 3 goals:

  - get rid of group file (to forget about rebase and in-list conflicts)
  - introduce human-readable names for tests
  - rewrite check into python

v7:
   - fix wording and grammar
   - satisfy python linters
   - move argv interfaces all into one in new check script
   - support '-n' == '--dry-run' option
   - update check-block to run check with correct PYTHON


Okay, I think I'm finished with the review for this version.

I also tried pylint/mypy again and it's mostly clean now (pylint
complains about the TODO comments, I think we should just disable that
warning).

Feel free to include the following as patch 12 in v8.

Kevin


I remember Max already queued good changes for 297 in his block branch




diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 85bc1c0c85..8aaa3e455c 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -31,13 +31,15 @@ if ! type -p "mypy" > /dev/null; then
  _notrun "mypy not found"
  fi

-pylint-3 --score=n iotests.py
+FILES="findtests.py iotests.py testenv.py testrunner.py check"
+
+pylint-3 --score=n $FILES

  MYPYPATH=../../python/ mypy --warn-unused-configs --disallow-subclassing-any \
  --disallow-any-generics --disallow-incomplete-defs \
  --disallow-untyped-decorators --no-implicit-optional \
  --warn-redundant-casts --warn-unused-ignores \
---no-implicit-reexport iotests.py
+--no-implicit-reexport $FILES

  # success, all done
  echo "*** done"
diff --git a/tests/qemu-iotests/297.out b/tests/qemu-iotests/297.out
index 6acc843649..85213ef96e 100644
--- a/tests/qemu-iotests/297.out
+++ b/tests/qemu-iotests/297.out
@@ -1,3 +1,3 @@
  QA output created by 297
-Success: no issues found in 1 source file
+Success: no issues found in 5 source files
  *** done
diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
index cd3702e23c..980bfa7b20 100644
--- a/tests/qemu-iotests/pylintrc
+++ b/tests/qemu-iotests/pylintrc
@@ -9,7 +9,8 @@
  # --enable=similarities". If you want to run only the classes checker, but 
have
  # no Warning level messages displayed, use "--disable=all --enable=classes
  # --disable=W".
-disable=invalid-name,
+disable=fixme,
+invalid-name,


Max just disable it in 297. I think we should not disable it globally, as 
additional hint doesn't hurt (except for test output).


  no-else-return,
  too-few-public-methods,
  too-many-arguments,




--
Best regards,
Vladimir

Re: [PATCH v7 10/11] iotests: rewrite check into python

[Vladimir Sementsov-Ogievskiy]

22.01.2021 19:08, Kevin Wolf wrote:

Am 16.01.2021 um 14:44 hat Vladimir Sementsov-Ogievskiy geschrieben:

Just use classes introduced in previous three commits. Behavior
difference is described in these three commits.

Drop group file, as it becomes unused.

Drop common.env: now check is in python, and for tests we use same
python interpreter that runs the check itself. Use build environment
PYTHON in check-block instead, to keep "make check" use the same
python.

Signed-off-by: Vladimir Sementsov-Ogievskiy 



diff --git a/tests/check-block.sh b/tests/check-block.sh
index fb4c1baae9..26eb1c0a9b 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -69,7 +69,7 @@ export QEMU_CHECK_BLOCK_AUTO=1
  
  ret=0

  for fmt in $format_list ; do
-./check -makecheck -$fmt $group || ret=1
+${PYTHON} ./check -makecheck -$fmt $group || ret=1
  done


When I add an echo to print that command line, it seems that ${PYTHON}
is empty for me. Is this expected?


It seems to be defined defined when called from make check. Did you just call 
check-block directly?

It's not intentional, but I think it's OK: if PYTHON is not defined let's just 
execute check as self-executable. And for make-check PYTHON is defined and 
correct python is used.




  exit $ret
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 952762d5ed..914321806a 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check


[..]


-if [ -x "$binary" ]
-then
-export QEMU_PROG="$build_root/$binary"
-break
-fi
-done
-popd > /dev/null
-[ "$QEMU_PROG" = "" ] && _init_error "qemu not found"
-fi


I think this else branch is kind of important (if there is no system
emulator binary for the host architecture, find _any_ system emulator
binary that was built). I can't find its equivalent in the new code.


Hmm, I decided testing "first found" emulator is strange.. It seems like we 
have several emulators and user don't care which would be tested?

Probably we should instead used qemu-system-* binary only if there is only one 
matching binary. And fail if there are many.



--
Best regards,
Vladimir

Re: [PATCH v4 13/16] block/io: support int64_t bytes in bdrv_aligned_preadv()

[Vladimir Sementsov-Ogievskiy]

22.01.2021 19:54, Eric Blake wrote:

On 12/11/20 12:39 PM, Vladimir Sementsov-Ogievskiy wrote:

We are generally moving to int64_t for both offset and bytes parameters
on all io paths.

Main motivation is realization of 64-bit write_zeroes operation for
fast zeroing large disk chunks, up to the whole disk.

We chose signed type, to be consistent with off_t (which is signed) and
with possibility for signed return type (where negative value means
error).

So, prepare bdrv_aligned_preadv() now.

Make byte variable in bdrv_padding_rmw_read() int64_t, as it defined
only to be passed to bdrv_aligned_preadv().


Reads awkwardly, how about:

Make the byte variable in bdrv_padding_rmw_read() int64_t, as it is only
used for pass-through to bdrv_aligned_preadv().


and also s/byte/bytes/





All bdrv_aligned_preadv() callers are safe as type is widening. Let's
look inside:

  - add a new-style assertion that request is good.
  - callees bdrv_is_allocated(), bdrv_co_do_copy_on_readv() supports
int64_t bytes
  - conversion of bytes_remaining is OK, as we never has requests


have


overflowing BDRV_MAX_LENGTH
  - looping through bytes_remaining is ok, num is updated to int64_t
- for bdrv_driver_preadv we have same limit of max_transfer
- qemu_iovec_memset is OK, as bytes+qiov_offset should not overflow
  qiov->size anyway (thanks to bdrv_check_qiov_request())

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  block/io.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)



Reviewed-by: Eric Blake 




--
Best regards,
Vladimir

Re: [PATCH v8 09/11] block: check availablity for preadv/pwritev on mac

[Peter Maydell]

On Sat, 23 Jan 2021 at 05:03, Joelle van Dyne  wrote:
>
> macOS 11/iOS 14 added preadv/pwritev APIs. Due to weak linking, configure
> will succeed with CONFIG_PREADV even when targeting a lower OS version.
> We therefore need to check at run time if we can actually use these APIs.

If you make the preadv check be a meson.build has_function()
test, I think (but have not tested) that it ought to correctly
fail the link in the targeting-lower-OS-version case, because
meson.build's has_function() implementation for clang
https://github.com/mesonbuild/meson/blob/bad0e95caec4cf49165572df3cf2edc4832280fa/mesonbuild/compilers/mixins/clang.py#L89
adds -Wl,-no-weak-imports when it's building the test-case
exactly to force that linker visibility obeys the minimum
version targets for OSX.

If that works I think it would be neater than delaying the
check to runtime (and it would also mean another test moved
out of configure and into meson.build, which is the direction
we're heading anyway).

thanks
-- PMM

[PATCH v2 16/25] hw/sd: ssi-sd: Support single block write

[Bin Meng]

From: Bin Meng 

Add 2 more states for the block write operation. The SPI host needs
to send a data start token to start the transfer, and the data block
written to the card will be acknowledged by a data response token.

Signed-off-by: Bin Meng 
Acked-by: Alistair Francis 

---

Changes in v2:
- Correct the "token" typo in the commit message
- Add 'write_bytes' in vmstate_ssi_sd

 hw/sd/ssi-sd.c | 38 +-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 75e76cf87a..240cfd919c 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -43,6 +43,8 @@ typedef enum {
 SSI_SD_DATA_START,
 SSI_SD_DATA_READ,
 SSI_SD_DATA_CRC16,
+SSI_SD_DATA_WRITE,
+SSI_SD_SKIP_CRC16,
 } ssi_sd_mode;
 
 struct ssi_sd_state {
@@ -53,6 +55,7 @@ struct ssi_sd_state {
 uint8_t response[5];
 uint16_t crc16;
 int32_t read_bytes;
+int32_t write_bytes;
 int32_t arglen;
 int32_t response_pos;
 int32_t stopping;
@@ -85,6 +88,9 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD)
 /* dummy value - don't care */
 #define SSI_DUMMY   0xff
 
+/* data accepted */
+#define DATA_RESPONSE_ACCEPTED  0x05
+
 static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
 {
 ssi_sd_state *s = SSI_SD(dev);
@@ -113,10 +119,17 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 
 switch (s->mode) {
 case SSI_SD_CMD:
-if (val == SSI_DUMMY) {
+switch (val) {
+case SSI_DUMMY:
 DPRINTF("NULL command\n");
 return SSI_DUMMY;
+break;
+case SSI_TOKEN_SINGLE:
+DPRINTF("Start write block\n");
+s->mode = SSI_SD_DATA_WRITE;
+return SSI_DUMMY;
 }
+
 s->cmd = val & 0x3f;
 s->mode = SSI_SD_CMDARG;
 s->arglen = 0;
@@ -250,6 +263,27 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 s->response_pos = 0;
 }
 return val;
+case SSI_SD_DATA_WRITE:
+sdbus_write_byte(>sdbus, val);
+s->write_bytes++;
+if (!sdbus_receive_ready(>sdbus) || s->write_bytes == 512) {
+DPRINTF("Data write end\n");
+s->mode = SSI_SD_SKIP_CRC16;
+s->response_pos = 0;
+}
+return val;
+case SSI_SD_SKIP_CRC16:
+/* we don't verify the crc16 */
+s->response_pos++;
+if (s->response_pos == 2) {
+DPRINTF("CRC16 receive end\n");
+s->mode = SSI_SD_RESPONSE;
+s->write_bytes = 0;
+s->arglen = 1;
+s->response[0] = DATA_RESPONSE_ACCEPTED;
+s->response_pos = 0;
+}
+return SSI_DUMMY;
 }
 /* Should never happen.  */
 return SSI_DUMMY;
@@ -287,6 +321,7 @@ static const VMStateDescription vmstate_ssi_sd = {
 VMSTATE_UINT8_ARRAY(response, ssi_sd_state, 5),
 VMSTATE_UINT16(crc16, ssi_sd_state),
 VMSTATE_INT32(read_bytes, ssi_sd_state),
+VMSTATE_INT32(write_bytes, ssi_sd_state),
 VMSTATE_INT32(arglen, ssi_sd_state),
 VMSTATE_INT32(response_pos, ssi_sd_state),
 VMSTATE_INT32(stopping, ssi_sd_state),
@@ -340,6 +375,7 @@ static void ssi_sd_reset(DeviceState *dev)
 memset(s->response, 0, sizeof(s->response));
 s->crc16 = 0;
 s->read_bytes = 0;
+s->write_bytes = 0;
 s->arglen = 0;
 s->response_pos = 0;
 s->stopping = 0;
-- 
2.25.1

[PATCH v2 18/25] hw/sd: ssi-sd: Bump up version ids of VMStateDescription

[Bin Meng]

From: Bin Meng 

With all these fixes and improvements, there is no way for the
VMStateDescription to keep backward compatibility. We will have
to bump up version ids.

The s->mode check in the post_load() hook is also updated.

Signed-off-by: Bin Meng 

---

Changes in v2:
- new patch: bump up version ids of VMStateDescription

 hw/sd/ssi-sd.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index ee4fbc3dfe..0c507f3ec5 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -4,6 +4,11 @@
  * Copyright (c) 2007-2009 CodeSourcery.
  * Written by Paul Brook
  *
+ * Copyright (c) 2021 Wind River Systems, Inc.
+ * Improved by Bin Meng 
+ *
+ * Validated with U-Boot v2021.01 and Linux v5.10 mmc_spi driver
+ *
  * This code is licensed under the GNU GPL v2.
  *
  * Contributions after 2012-01-13 are licensed under the terms of the
@@ -319,7 +324,7 @@ static int ssi_sd_post_load(void *opaque, int version_id)
 {
 ssi_sd_state *s = (ssi_sd_state *)opaque;
 
-if (s->mode > SSI_SD_DATA_READ) {
+if (s->mode > SSI_SD_SKIP_CRC16) {
 return -EINVAL;
 }
 if (s->mode == SSI_SD_CMDARG &&
@@ -337,8 +342,8 @@ static int ssi_sd_post_load(void *opaque, int version_id)
 
 static const VMStateDescription vmstate_ssi_sd = {
 .name = "ssi_sd",
-.version_id = 2,
-.minimum_version_id = 2,
+.version_id = 3,
+.minimum_version_id = 3,
 .post_load = ssi_sd_post_load,
 .fields = (VMStateField []) {
 VMSTATE_UINT32(mode, ssi_sd_state),
-- 
2.25.1

[PATCH v2 25/25] docs/system: riscv: Add documentation for sifive_u machine

[Bin Meng]

From: Bin Meng 

This adds detailed documentation for RISC-V `sifive_u` machine,
including the following information:

- Supported devices
- Hardware configuration information
- Boot options
- Machine-specific options
- Running Linux kernel
- Running VxWorks kernel
- Running U-Boot, and with an alternate configuration

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 

---

Changes in v2:
- Correct several typos in sifive_u.rst
- Update doc to mention U-Boot v2021.01

 docs/system/riscv/sifive_u.rst | 336 +
 docs/system/target-riscv.rst   |  10 +
 2 files changed, 346 insertions(+)
 create mode 100644 docs/system/riscv/sifive_u.rst

diff --git a/docs/system/riscv/sifive_u.rst b/docs/system/riscv/sifive_u.rst
new file mode 100644
index 00..3ad2426413
--- /dev/null
+++ b/docs/system/riscv/sifive_u.rst
@@ -0,0 +1,336 @@
+SiFive HiFive Unleashed (``sifive_u``)
+==
+
+SiFive HiFive Unleashed Development Board is the ultimate RISC‑V development
+board featuring the Freedom U540 multi-core RISC‑V processor.
+
+Supported devices
+-
+
+The ``sifive_u`` machine supports the following devices:
+
+ * 1 E51 / E31 core
+ * Up to 4 U54 / U34 cores
+ * Core Level Interruptor (CLINT)
+ * Platform-Level Interrupt Controller (PLIC)
+ * Power, Reset, Clock, Interrupt (PRCI)
+ * L2 Loosely Integrated Memory (L2-LIM)
+ * DDR memory controller
+ * 2 UARTs
+ * 1 GEM Ethernet controller
+ * 1 GPIO controller
+ * 1 One-Time Programmable (OTP) memory with stored serial number
+ * 1 DMA controller
+ * 2 QSPI controllers
+ * 1 ISSI 25WP256 flash
+ * 1 SD card in SPI mode
+
+Please note the real world HiFive Unleashed board has a fixed configuration of
+1 E51 core and 4 U54 core combination and the RISC-V core boots in 64-bit mode.
+With QEMU, one can create a machine with 1 E51 core and up to 4 U54 cores. It
+is also possible to create a 32-bit variant with the same peripherals except
+that the RISC-V cores are replaced by the 32-bit ones (E31 and U34), to help
+testing of 32-bit guest software.
+
+Hardware configuration information
+--
+
+The ``sifive_u`` machine automatically generates a device tree blob ("dtb")
+which it passes to the guest. This provides information about the addresses,
+interrupt lines and other configuration of the various devices in the system.
+Guest software should discover the devices that are present in the generated
+DTB instead of using a DTB for the real hardware, as some of the devices are
+not modeled by QEMU and trying to access these devices may cause unexpected
+behavior.
+
+Boot options
+
+
+The ``sifive_u`` machine can start using the standard -kernel functionality
+for loading a Linux kernel, a VxWorks kernel, a modified U-Boot bootloader
+(S-mode) or ELF executable with the default OpenSBI firmware image as the
+-bios. It also supports booting the unmodified U-Boot bootloader using the
+standard -bios functionality.
+
+Machine-specific options
+
+
+The following machine-specific options are supported:
+
+- serial=nnn
+
+  The board serial number. When not given, the default serial number 1 is used.
+
+  SiFive reserves the first 1 KiB of the 16 KiB OTP memory for internal use.
+  The current usage is only used to store the serial number of the board at
+  offset 0xfc. U-Boot reads the serial number from the OTP memory, and uses
+  it to generate a unique MAC address to be programmed to the on-chip GEM
+  Ethernet controller. When multiple QEMU ``sifive_u`` machines are created
+  and connected to the same subnet, they all have the same MAC address hence
+  it creates an unusable network. In such scenario, user should give different
+  values to serial= when creating different ``sifive_u`` machines.
+
+- start-in-flash
+
+  When given, QEMU's ROM codes jump to QSPI memory-mapped flash directly.
+  Otherwise QEMU will jump to DRAM or L2LIM depending on the msel= value.
+  When not given, it defaults to direct DRAM booting.
+
+- msel=[6|11]
+
+  Mode Select (MSEL[3:0]) pins value, used to control where to boot from.
+
+  The FU540 SoC supports booting from several sources, which are controlled
+  using the Mode Select pins on the chip. Typically, the boot process runs
+  through several stages before it begins execution of user-provided programs.
+  These stages typically include the following:
+
+  1. Zeroth Stage Boot Loader (ZSBL), which is contained in an on-chip mask
+ ROM and provided by QEMU. Note QEMU implemented ROM codes are not the
+ same as what is programmed in the hardware. The QEMU one is a simplified
+ version, but it provides the same functionality as the hardware.
+  2. First Stage Boot Loader (FSBL), which brings up PLLs and DDR memory.
+ This is U-Boot SPL.
+  3. Second Stage Boot Loader (SSBL), which further initializes additional
+ peripherals as needed. This is U-Boot proper combined with an 

[PATCH v2 21/25] hw/riscv: sifive_u: Add QSPI2 controller and connect an SD card

[Bin Meng]

From: Bin Meng 

This adds the QSPI2 controller to the SoC, and connects an SD
card to it. The generation of corresponding device tree source
fragment is also added.

Specify machine property `msel` to 11 to boot the same upstream
U-Boot SPL and payload image for the SiFive HiFive Unleashed board.
Note subsequent payload is stored in the SD card image.

$ qemu-system-riscv64 -nographic -M sifive_u,msel=11 -smp 5 -m 8G \
-bios u-boot-spl.bin -drive file=sdcard.img,if=sd

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 

---

Changes in v2:
- Correct the "connects" typo in the commit message

 include/hw/riscv/sifive_u.h |  3 +++
 hw/riscv/sifive_u.c | 43 +++--
 hw/riscv/Kconfig|  1 +
 3 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index 8824b7c031..de1464a2ce 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -47,6 +47,7 @@ typedef struct SiFiveUSoCState {
 SiFiveUOTPState otp;
 SiFivePDMAState dma;
 SiFiveSPIState spi0;
+SiFiveSPIState spi2;
 CadenceGEMState gem;
 
 uint32_t serial;
@@ -85,6 +86,7 @@ enum {
 SIFIVE_U_DEV_UART1,
 SIFIVE_U_DEV_GPIO,
 SIFIVE_U_DEV_QSPI0,
+SIFIVE_U_DEV_QSPI2,
 SIFIVE_U_DEV_OTP,
 SIFIVE_U_DEV_DMC,
 SIFIVE_U_DEV_FLASH0,
@@ -99,6 +101,7 @@ enum {
 SIFIVE_U_L2CC_IRQ2 = 3,
 SIFIVE_U_UART0_IRQ = 4,
 SIFIVE_U_UART1_IRQ = 5,
+SIFIVE_U_QSPI2_IRQ = 6,
 SIFIVE_U_GPIO_IRQ0 = 7,
 SIFIVE_U_GPIO_IRQ1 = 8,
 SIFIVE_U_GPIO_IRQ2 = 9,
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 43a0e983d2..6c1158a848 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -16,6 +16,7 @@
  * 6) GEM (Gigabit Ethernet Controller) and management block
  * 7) DMA (Direct Memory Access Controller)
  * 8) SPI0 connected to an SPI flash
+ * 9) SPI2 connected to an SD card
  *
  * This board currently generates devicetree dynamically that indicates at 
least
  * two harts and up to five harts.
@@ -77,6 +78,7 @@ static const struct MemmapEntry {
 [SIFIVE_U_DEV_UART0] ={ 0x1001, 0x1000 },
 [SIFIVE_U_DEV_UART1] ={ 0x10011000, 0x1000 },
 [SIFIVE_U_DEV_QSPI0] ={ 0x1004, 0x1000 },
+[SIFIVE_U_DEV_QSPI2] ={ 0x1005, 0x1000 },
 [SIFIVE_U_DEV_GPIO] = { 0x1006, 0x1000 },
 [SIFIVE_U_DEV_OTP] =  { 0x1007, 0x1000 },
 [SIFIVE_U_DEV_GEM] =  { 0x1009, 0x2000 },
@@ -345,6 +347,31 @@ static void create_fdt(SiFiveUState *s, const struct 
MemmapEntry *memmap,
 "sifive,fu540-c000-ccache");
 g_free(nodename);
 
+nodename = g_strdup_printf("/soc/spi@%lx",
+(long)memmap[SIFIVE_U_DEV_QSPI2].base);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_cell(fdt, nodename, "#size-cells", 0);
+qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
+qemu_fdt_setprop_cells(fdt, nodename, "clocks",
+prci_phandle, PRCI_CLK_TLCLK);
+qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_QSPI2_IRQ);
+qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
+qemu_fdt_setprop_cells(fdt, nodename, "reg",
+0x0, memmap[SIFIVE_U_DEV_QSPI2].base,
+0x0, memmap[SIFIVE_U_DEV_QSPI2].size);
+qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,spi0");
+g_free(nodename);
+
+nodename = g_strdup_printf("/soc/spi@%lx/mmc@0",
+(long)memmap[SIFIVE_U_DEV_QSPI2].base);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop(fdt, nodename, "disable-wp", NULL, 0);
+qemu_fdt_setprop_cells(fdt, nodename, "voltage-ranges", 3300, 3300);
+qemu_fdt_setprop_cell(fdt, nodename, "spi-max-frequency", 2000);
+qemu_fdt_setprop_cell(fdt, nodename, "reg", 0);
+qemu_fdt_setprop_string(fdt, nodename, "compatible", "mmc-spi-slot");
+g_free(nodename);
+
 nodename = g_strdup_printf("/soc/spi@%lx",
 (long)memmap[SIFIVE_U_DEV_QSPI0].base);
 qemu_fdt_add_subnode(fdt, nodename);
@@ -469,8 +496,8 @@ static void sifive_u_machine_init(MachineState *machine)
 uint32_t fdt_load_addr;
 uint64_t kernel_entry;
 DriveInfo *dinfo;
-DeviceState *flash_dev;
-qemu_irq flash_cs;
+DeviceState *flash_dev, *sd_dev;
+qemu_irq flash_cs, sd_cs;
 
 /* Initialize SoC */
 object_initialize_child(OBJECT(machine), "soc", >soc, TYPE_RISCV_U_SOC);
@@ -616,6 +643,12 @@ static void sifive_u_machine_init(MachineState *machine)
 
 flash_cs = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
 sysbus_connect_irq(SYS_BUS_DEVICE(>soc.spi0), 1, flash_cs);
+
+/* Connect an SD card to SPI2 */
+sd_dev = ssi_create_peripheral(s->soc.spi2.spi, "ssi-sd");
+
+sd_cs = qdev_get_gpio_in_named(sd_dev, SSI_GPIO_CS, 0);
+sysbus_connect_irq(SYS_BUS_DEVICE(>soc.spi2), 1, sd_cs);
 }
 
 static bool 

[PATCH v2 22/25] hw/riscv: sifive_u: Change SIFIVE_U_GEM_IRQ to decimal value

[Bin Meng]

From: Bin Meng 

All other peripherals' IRQs are in the format of decimal value.
Change SIFIVE_U_GEM_IRQ to be consistent.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
---

(no changes since v1)

 include/hw/riscv/sifive_u.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index de1464a2ce..2656b39808 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -127,7 +127,7 @@ enum {
 SIFIVE_U_PDMA_IRQ6 = 29,
 SIFIVE_U_PDMA_IRQ7 = 30,
 SIFIVE_U_QSPI0_IRQ = 51,
-SIFIVE_U_GEM_IRQ = 0x35
+SIFIVE_U_GEM_IRQ = 53
 };
 
 enum {
-- 
2.25.1

[PATCH v2 20/25] hw/riscv: sifive_u: Add QSPI0 controller and connect a flash

[Bin Meng]

From: Bin Meng 

This adds the QSPI0 controller to the SoC, and connects an ISSI
25WP256 flash to it. The generation of corresponding device tree
source fragment is also added.

Since the direct memory-mapped mode is not supported by the SiFive
SPI model, the  property does not populate the second group
which represents the memory mapped address of the SPI flash.

With this commit, upstream U-Boot for the SiFive HiFive Unleashed
board can boot on QEMU 'sifive_u' out of the box. This allows users
to develop and test the recommended RISC-V boot flow with a real
world use case: ZSBL (in QEMU) loads U-Boot SPL from SPI flash to
L2LIM, then U-Boot SPL loads the payload from SPI flash that is
combined with OpenSBI fw_dynamic firmware and U-Boot proper.

Specify machine property `msel` to 6 to allow booting from the SPI
flash. U-Boot spl is directly loaded via `-bios`, and subsequent
payload is stored in the SPI flash image. Example command line:

$ qemu-system-riscv64 -nographic -M sifive_u,msel=6 -smp 5 -m 8G \
-bios u-boot-spl.bin -drive file=spi-nor.img,if=mtd

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 

---

Changes in v2:
- Correct the "connects" typo in the commit message
- Mention in the commit message that  property does not populate
  the second group which represents the memory mapped address of the
  SPI flash

 include/hw/riscv/sifive_u.h |  4 +++
 hw/riscv/sifive_u.c | 52 +
 hw/riscv/Kconfig|  2 ++
 3 files changed, 58 insertions(+)

diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index a9f7b4a084..8824b7c031 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -26,6 +26,7 @@
 #include "hw/gpio/sifive_gpio.h"
 #include "hw/misc/sifive_u_otp.h"
 #include "hw/misc/sifive_u_prci.h"
+#include "hw/ssi/sifive_spi.h"
 
 #define TYPE_RISCV_U_SOC "riscv.sifive.u.soc"
 #define RISCV_U_SOC(obj) \
@@ -45,6 +46,7 @@ typedef struct SiFiveUSoCState {
 SIFIVEGPIOState gpio;
 SiFiveUOTPState otp;
 SiFivePDMAState dma;
+SiFiveSPIState spi0;
 CadenceGEMState gem;
 
 uint32_t serial;
@@ -82,6 +84,7 @@ enum {
 SIFIVE_U_DEV_UART0,
 SIFIVE_U_DEV_UART1,
 SIFIVE_U_DEV_GPIO,
+SIFIVE_U_DEV_QSPI0,
 SIFIVE_U_DEV_OTP,
 SIFIVE_U_DEV_DMC,
 SIFIVE_U_DEV_FLASH0,
@@ -120,6 +123,7 @@ enum {
 SIFIVE_U_PDMA_IRQ5 = 28,
 SIFIVE_U_PDMA_IRQ6 = 29,
 SIFIVE_U_PDMA_IRQ7 = 30,
+SIFIVE_U_QSPI0_IRQ = 51,
 SIFIVE_U_GEM_IRQ = 0x35
 };
 
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 59b61cea01..43a0e983d2 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -15,6 +15,7 @@
  * 5) OTP (One-Time Programmable) memory with stored serial number
  * 6) GEM (Gigabit Ethernet Controller) and management block
  * 7) DMA (Direct Memory Access Controller)
+ * 8) SPI0 connected to an SPI flash
  *
  * This board currently generates devicetree dynamically that indicates at 
least
  * two harts and up to five harts.
@@ -44,6 +45,7 @@
 #include "hw/char/serial.h"
 #include "hw/cpu/cluster.h"
 #include "hw/misc/unimp.h"
+#include "hw/ssi/ssi.h"
 #include "target/riscv/cpu.h"
 #include "hw/riscv/riscv_hart.h"
 #include "hw/riscv/sifive_u.h"
@@ -74,6 +76,7 @@ static const struct MemmapEntry {
 [SIFIVE_U_DEV_PRCI] = { 0x1000, 0x1000 },
 [SIFIVE_U_DEV_UART0] ={ 0x1001, 0x1000 },
 [SIFIVE_U_DEV_UART1] ={ 0x10011000, 0x1000 },
+[SIFIVE_U_DEV_QSPI0] ={ 0x1004, 0x1000 },
 [SIFIVE_U_DEV_GPIO] = { 0x1006, 0x1000 },
 [SIFIVE_U_DEV_OTP] =  { 0x1007, 0x1000 },
 [SIFIVE_U_DEV_GEM] =  { 0x1009, 0x2000 },
@@ -342,6 +345,32 @@ static void create_fdt(SiFiveUState *s, const struct 
MemmapEntry *memmap,
 "sifive,fu540-c000-ccache");
 g_free(nodename);
 
+nodename = g_strdup_printf("/soc/spi@%lx",
+(long)memmap[SIFIVE_U_DEV_QSPI0].base);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_cell(fdt, nodename, "#size-cells", 0);
+qemu_fdt_setprop_cell(fdt, nodename, "#address-cells", 1);
+qemu_fdt_setprop_cells(fdt, nodename, "clocks",
+prci_phandle, PRCI_CLK_TLCLK);
+qemu_fdt_setprop_cell(fdt, nodename, "interrupts", SIFIVE_U_QSPI0_IRQ);
+qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
+qemu_fdt_setprop_cells(fdt, nodename, "reg",
+0x0, memmap[SIFIVE_U_DEV_QSPI0].base,
+0x0, memmap[SIFIVE_U_DEV_QSPI0].size);
+qemu_fdt_setprop_string(fdt, nodename, "compatible", "sifive,spi0");
+g_free(nodename);
+
+nodename = g_strdup_printf("/soc/spi@%lx/flash@0",
+(long)memmap[SIFIVE_U_DEV_QSPI0].base);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_cell(fdt, nodename, "spi-rx-bus-width", 4);
+qemu_fdt_setprop_cell(fdt, nodename, "spi-tx-bus-width", 4);
+qemu_fdt_setprop(fdt, nodename, "m25p,fast-read", NULL, 0);
+   

[PATCH v2 24/25] docs/system: Add RISC-V documentation

[Bin Meng]

From: Bin Meng 

Add RISC-V system emulator documentation for generic information.
`Board-specific documentation` and `RISC-V CPU features` are only
a placeholder and will be added in the future.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
---

(no changes since v1)

 docs/system/target-riscv.rst | 62 
 docs/system/targets.rst  |  1 +
 2 files changed, 63 insertions(+)
 create mode 100644 docs/system/target-riscv.rst

diff --git a/docs/system/target-riscv.rst b/docs/system/target-riscv.rst
new file mode 100644
index 00..9f4b7586e5
--- /dev/null
+++ b/docs/system/target-riscv.rst
@@ -0,0 +1,62 @@
+.. _RISC-V-System-emulator:
+
+RISC-V System emulator
+==
+
+QEMU can emulate both 32-bit and 64-bit RISC-V CPUs. Use the
+``qemu-system-riscv64`` executable to simulate a 64-bit RISC-V machine,
+``qemu-system-riscv32`` executable to simulate a 32-bit RISC-V machine.
+
+QEMU has generally good support for RISC-V guests. It has support for
+several different machines. The reason we support so many is that
+RISC-V hardware is much more widely varying than x86 hardware. RISC-V
+CPUs are generally built into "system-on-chip" (SoC) designs created by
+many different companies with different devices, and these SoCs are
+then built into machines which can vary still further even if they use
+the same SoC.
+
+For most boards the CPU type is fixed (matching what the hardware has),
+so typically you don't need to specify the CPU type by hand, except for
+special cases like the ``virt`` board.
+
+Choosing a board model
+--
+
+For QEMU's RISC-V system emulation, you must specify which board
+model you want to use with the ``-M`` or ``--machine`` option;
+there is no default.
+
+Because RISC-V systems differ so much and in fundamental ways, typically
+operating system or firmware images intended to run on one machine
+will not run at all on any other. This is often surprising for new
+users who are used to the x86 world where every system looks like a
+standard PC. (Once the kernel has booted, most user space software
+cares much less about the detail of the hardware.)
+
+If you already have a system image or a kernel that works on hardware
+and you want to boot with QEMU, check whether QEMU lists that machine
+in its ``-machine help`` output. If it is listed, then you can probably
+use that board model. If it is not listed, then unfortunately your image
+will almost certainly not boot on QEMU. (You might be able to
+extract the file system and use that with a different kernel which
+boots on a system that QEMU does emulate.)
+
+If you don't care about reproducing the idiosyncrasies of a particular
+bit of hardware, such as small amount of RAM, no PCI or other hard
+disk, etc., and just want to run Linux, the best option is to use the
+``virt`` board. This is a platform which doesn't correspond to any
+real hardware and is designed for use in virtual machines. You'll
+need to compile Linux with a suitable configuration for running on
+the ``virt`` board. ``virt`` supports PCI, virtio, recent CPUs and
+large amounts of RAM. It also supports 64-bit CPUs.
+
+Board-specific documentation
+
+
+Unfortunately many of the RISC-V boards QEMU supports are currently
+undocumented; you can get a complete list by running
+``qemu-system-riscv64 --machine help``, or
+``qemu-system-riscv32 --machine help``.
+
+RISC-V CPU features
+---
diff --git a/docs/system/targets.rst b/docs/system/targets.rst
index 564cea9a9b..75ed1087fd 100644
--- a/docs/system/targets.rst
+++ b/docs/system/targets.rst
@@ -19,6 +19,7 @@ Contents:
target-m68k
target-mips
target-ppc
+   target-riscv
target-rx
target-s390x
target-sparc
-- 
2.25.1

[PATCH v2 23/25] docs/system: Sort targets in alphabetical order

[Bin Meng]

From: Bin Meng 

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
---

(no changes since v1)

 docs/system/targets.rst | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/docs/system/targets.rst b/docs/system/targets.rst
index 560783644d..564cea9a9b 100644
--- a/docs/system/targets.rst
+++ b/docs/system/targets.rst
@@ -7,16 +7,21 @@ various targets are mentioned in the following sections.
 
 Contents:
 
+..
+   This table of contents should be kept sorted alphabetically
+   by the title text of each file, which isn't the same ordering
+   as an alphabetical sort by filename.
+
 .. toctree::
 
-   target-i386
+   target-arm
+   target-avr
+   target-m68k
+   target-mips
target-ppc
+   target-rx
+   target-s390x
target-sparc
target-sparc64
-   target-mips
-   target-arm
-   target-m68k
+   target-i386
target-xtensa
-   target-s390x
-   target-rx
-   target-avr
-- 
2.25.1

[PATCH v2 17/25] hw/sd: ssi-sd: Support multiple block write

[Bin Meng]

From: Bin Meng 

For a multiple block write operation, each block begins with a multi
write start token. Unlike the SD mode that the multiple block write
ends when receiving a STOP_TRAN command (CMD12), a special stop tran
token is used to signal the card.

Emulating this by manually sending a CMD12 to the SD card core, to
bring it out of the receiving data state.

Signed-off-by: Bin Meng 
Acked-by: Alistair Francis 

---

Changes in v2:
- Correct the "token" typo in the commit message
- Introduce multiple write token definitions in this patch

 hw/sd/ssi-sd.c | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 240cfd919c..ee4fbc3dfe 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -82,6 +82,10 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD)
 #define SSI_SDR_ADDRESS_ERROR   0x2000
 #define SSI_SDR_PARAMETER_ERROR 0x4000
 
+/* multiple block write */
+#define SSI_TOKEN_MULTI_WRITE   0xfc
+/* terminate multiple block write */
+#define SSI_TOKEN_STOP_TRAN 0xfd
 /* single block read/write, multiple block read */
 #define SSI_TOKEN_SINGLE0xfe
 
@@ -94,6 +98,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD)
 static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
 {
 ssi_sd_state *s = SSI_SD(dev);
+SDRequest request;
+uint8_t longresp[16];
 
 /*
  * Special case: allow CMD12 (STOP TRANSMISSION) while reading data.
@@ -125,9 +131,31 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 return SSI_DUMMY;
 break;
 case SSI_TOKEN_SINGLE:
+case SSI_TOKEN_MULTI_WRITE:
 DPRINTF("Start write block\n");
 s->mode = SSI_SD_DATA_WRITE;
 return SSI_DUMMY;
+case SSI_TOKEN_STOP_TRAN:
+DPRINTF("Stop multiple write\n");
+
+/* manually issue cmd12 to stop the transfer */
+request.cmd = 12;
+request.arg = 0;
+s->arglen = sdbus_do_command(>sdbus, , longresp);
+if (s->arglen <= 0) {
+s->arglen = 1;
+/* a zero value indicates the card is busy */
+s->response[0] = 0;
+DPRINTF("SD card busy\n");
+} else {
+s->arglen = 1;
+/* a non-zero value indicates the card is ready */
+s->response[0] = SSI_DUMMY;
+}
+
+s->mode = SSI_SD_RESPONSE;
+s->response_pos = 0;
+return SSI_DUMMY;
 }
 
 s->cmd = val & 0x3f;
@@ -136,8 +164,6 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 return SSI_DUMMY;
 case SSI_SD_CMDARG:
 if (s->arglen == 4) {
-SDRequest request;
-uint8_t longresp[16];
 /* FIXME: Check CRC.  */
 request.cmd = s->cmd;
 request.arg = ldl_be_p(s->cmdarg);
-- 
2.25.1

[PATCH v2 19/25] hw/ssi: Add SiFive SPI controller support

[Bin Meng]

From: Bin Meng 

This adds the SiFive SPI controller model for the FU540 SoC.
The direct memory-mapped SPI flash mode is unsupported.

Signed-off-by: Bin Meng 

---

Changes in v2:
- Log guest error when trying to write reserved registers
- Log guest error when trying to access out-of-bounds registers
- log guest error when writing to reserved bits for chip select
  registers and watermark registers
- Log unimplemented warning when trying to write direct-map flash
  interface registers
- Add test tx fifo full logic in sifive_spi_read(), hence remove
  setting the tx fifo full flag in sifive_spi_write().
- Populate register with their default value

 include/hw/ssi/sifive_spi.h |  47 +
 hw/ssi/sifive_spi.c | 367 
 hw/ssi/Kconfig  |   4 +
 hw/ssi/meson.build  |   1 +
 4 files changed, 419 insertions(+)
 create mode 100644 include/hw/ssi/sifive_spi.h
 create mode 100644 hw/ssi/sifive_spi.c

diff --git a/include/hw/ssi/sifive_spi.h b/include/hw/ssi/sifive_spi.h
new file mode 100644
index 00..47d0d6a47c
--- /dev/null
+++ b/include/hw/ssi/sifive_spi.h
@@ -0,0 +1,47 @@
+/*
+ * QEMU model of the SiFive SPI Controller
+ *
+ * Copyright (c) 2021 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#ifndef HW_SIFIVE_SPI_H
+#define HW_SIFIVE_SPI_H
+
+#define SIFIVE_SPI_REG_NUM  (0x78 / 4)
+
+#define TYPE_SIFIVE_SPI "sifive.spi"
+#define SIFIVE_SPI(obj) OBJECT_CHECK(SiFiveSPIState, (obj), TYPE_SIFIVE_SPI)
+
+typedef struct SiFiveSPIState {
+SysBusDevice parent_obj;
+
+MemoryRegion mmio;
+qemu_irq irq;
+
+uint32_t num_cs;
+qemu_irq *cs_lines;
+
+SSIBus *spi;
+
+Fifo8 tx_fifo;
+Fifo8 rx_fifo;
+
+uint32_t regs[SIFIVE_SPI_REG_NUM];
+} SiFiveSPIState;
+
+#endif /* HW_SIFIVE_SPI_H */
diff --git a/hw/ssi/sifive_spi.c b/hw/ssi/sifive_spi.c
new file mode 100644
index 00..61504336ad
--- /dev/null
+++ b/hw/ssi/sifive_spi.c
@@ -0,0 +1,367 @@
+/*
+ * QEMU model of the SiFive SPI Controller
+ *
+ * Copyright (c) 2021 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "hw/ssi/ssi.h"
+#include "sysemu/sysemu.h"
+#include "qemu/fifo8.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "hw/ssi/sifive_spi.h"
+
+#define R_SCKDIV(0x00 / 4)
+#define R_SCKMODE   (0x04 / 4)
+#define R_CSID  (0x10 / 4)
+#define R_CSDEF (0x14 / 4)
+#define R_CSMODE(0x18 / 4)
+#define R_DELAY0(0x28 / 4)
+#define R_DELAY1(0x2C / 4)
+#define R_FMT   (0x40 / 4)
+#define R_TXDATA(0x48 / 4)
+#define R_RXDATA(0x4C / 4)
+#define R_TXMARK(0x50 / 4)
+#define R_RXMARK(0x54 / 4)
+#define R_FCTRL (0x60 / 4)
+#define R_FFMT  (0x64 / 4)
+#define R_IE(0x70 / 4)
+#define R_IP(0x74 / 4)
+
+#define FMT_DIR (1 << 3)
+
+#define TXDATA_FULL (1 << 31)
+#define RXDATA_EMPTY(1 << 31)
+
+#define IE_TXWM (1 << 0)
+#define IE_RXWM (1 << 1)
+
+#define IP_TXWM (1 << 0)
+#define IP_RXWM (1 << 1)
+
+#define FIFO_CAPACITY   8
+
+static void sifive_spi_txfifo_reset(SiFiveSPIState *s)
+{
+fifo8_reset(>tx_fifo);
+
+s->regs[R_TXDATA] &= ~TXDATA_FULL;
+s->regs[R_IP] &= ~IP_TXWM;
+}
+
+static void sifive_spi_rxfifo_reset(SiFiveSPIState *s)
+{
+fifo8_reset(>rx_fifo);
+
+s->regs[R_RXDATA] |= RXDATA_EMPTY;
+s->regs[R_IP] &= ~IP_RXWM;
+}
+
+static void sifive_spi_update_cs(SiFiveSPIState *s)
+{
+int i;
+
+for (i = 0; i < s->num_cs; i++) {
+if (s->regs[R_CSDEF] & (1 << i)) {
+

[PATCH v2 13/25] hw/sd: sd: Allow single/multiple block write for SPI mode

[Bin Meng]

From: Bin Meng 

At present the single/multiple block write in SPI mode is blocked
by sd_normal_command(). Remove the limitation.

Signed-off-by: Bin Meng 
Acked-by: Alistair Francis 
---

(no changes since v1)

 hw/sd/sd.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 09753359bb..946036d87c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1230,9 +1230,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, 
SDRequest req)
 case 25:   /* CMD25:  WRITE_MULTIPLE_BLOCK */
 switch (sd->state) {
 case sd_transfer_state:
-/* Writing in SPI mode not implemented.  */
-if (sd->spi)
-break;
 
 if (addr + sd->blk_len > sd->size) {
 sd->card_status |= ADDRESS_ERROR;
-- 
2.25.1

[PATCH v2 07/25] hw/sd: ssi-sd: Suffix a data block with CRC16

[Bin Meng]

From: Bin Meng 

Per the SD spec, a valid data block is suffixed with a 16-bit CRC
generated by the standard CCITT polynomial x16+x12+x5+1. This part
is currently missing in the ssi-sd state machine. Without it, all
data block transfer fails in guest software because the expected
CRC16 is missing on the data out line.

Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
Signed-off-by: Bin Meng 
Acked-by: Alistair Francis 
---

(no changes since v1)

 hw/sd/ssi-sd.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 043e270066..8bccedfab2 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -17,6 +17,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/sd/sd.h"
 #include "qapi/error.h"
+#include "qemu/crc-ccitt.h"
 #include "qemu/module.h"
 #include "qom/object.h"
 
@@ -40,6 +41,7 @@ typedef enum {
 SSI_SD_RESPONSE,
 SSI_SD_DATA_START,
 SSI_SD_DATA_READ,
+SSI_SD_DATA_CRC16,
 } ssi_sd_mode;
 
 struct ssi_sd_state {
@@ -48,6 +50,7 @@ struct ssi_sd_state {
 int cmd;
 uint8_t cmdarg[4];
 uint8_t response[5];
+uint16_t crc16;
 int32_t arglen;
 int32_t response_pos;
 int32_t stopping;
@@ -194,12 +197,24 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 case SSI_SD_DATA_START:
 DPRINTF("Start read block\n");
 s->mode = SSI_SD_DATA_READ;
+s->response_pos = 0;
 return 0xfe;
 case SSI_SD_DATA_READ:
 val = sdbus_read_byte(>sdbus);
+s->crc16 = crc_ccitt_false(s->crc16, (uint8_t *), 1);
 if (!sdbus_data_ready(>sdbus)) {
 DPRINTF("Data read end\n");
+s->mode = SSI_SD_DATA_CRC16;
+}
+return val;
+case SSI_SD_DATA_CRC16:
+val = (s->crc16 & 0xff00) >> 8;
+s->crc16 <<= 8;
+s->response_pos++;
+if (s->response_pos == 2) {
+DPRINTF("CRC16 read end\n");
 s->mode = SSI_SD_CMD;
+s->response_pos = 0;
 }
 return val;
 }
@@ -237,6 +252,7 @@ static const VMStateDescription vmstate_ssi_sd = {
 VMSTATE_INT32(cmd, ssi_sd_state),
 VMSTATE_UINT8_ARRAY(cmdarg, ssi_sd_state, 4),
 VMSTATE_UINT8_ARRAY(response, ssi_sd_state, 5),
+VMSTATE_UINT16(crc16, ssi_sd_state),
 VMSTATE_INT32(arglen, ssi_sd_state),
 VMSTATE_INT32(response_pos, ssi_sd_state),
 VMSTATE_INT32(stopping, ssi_sd_state),
@@ -288,6 +304,7 @@ static void ssi_sd_reset(DeviceState *dev)
 s->cmd = 0;
 memset(s->cmdarg, 0, sizeof(s->cmdarg));
 memset(s->response, 0, sizeof(s->response));
+s->crc16 = 0;
 s->arglen = 0;
 s->response_pos = 0;
 s->stopping = 0;
-- 
2.25.1

[PATCH v2 05/25] hw/sd: sd: Drop sd_crc16()

[Bin Meng]

From: Bin Meng 

commit f6fb1f9b319f ("sdcard: Correct CRC16 offset in sd_function_switch()")
changed the 16-bit CRC to be stored at offset 64. In fact, this CRC
calculation is completely wrong. From the original codes, it wants
to calculate the CRC16 of the first 64 bytes of sd->data[], however
passing 64 as the `width` to sd_crc16() actually counts 256 bytes
starting from the `message` for the CRC16 calculation, which is not
what we want.

Besides that, it seems existing sd_crc16() algorithm does not match
the SD spec, which says CRC16 is the CCITT one but the calculation
does not produce expected result. It turns out the CRC16 was never
transferred outside the sd core, as in sd_read_byte() we see:

if (sd->data_offset >= 64)
sd->state = sd_transfer_state;

Given above reasons, let's drop it.

Signed-off-by: Bin Meng 
Reviewed-by: Pragnesh Patel 
Tested-by: Pragnesh Patel 

---

Changes in v2:
- Fix several typos in the commit message

 hw/sd/sd.c | 18 --
 1 file changed, 18 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bfea5547d5..b3952514fe 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -271,23 +271,6 @@ static uint8_t sd_crc7(const void *message, size_t width)
 return shift_reg;
 }
 
-static uint16_t sd_crc16(const void *message, size_t width)
-{
-int i, bit;
-uint16_t shift_reg = 0x;
-const uint16_t *msg = (const uint16_t *)message;
-width <<= 1;
-
-for (i = 0; i < width; i ++, msg ++)
-for (bit = 15; bit >= 0; bit --) {
-shift_reg <<= 1;
-if ((shift_reg >> 15) ^ ((*msg >> bit) & 1))
-shift_reg ^= 0x1011;
-}
-
-return shift_reg;
-}
-
 #define OCR_POWER_DELAY_NS  50 /* 0.5ms */
 
 FIELD(OCR, VDD_VOLTAGE_WINDOW,  0, 24)
@@ -843,7 +826,6 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
 sd->data[16 - (i >> 1)] |= new_func << ((i % 2) * 4);
 }
 memset(>data[17], 0, 47);
-stw_be_p(sd->data + 64, sd_crc16(sd->data, 64));
 }
 
 static inline bool sd_wp_addr(SDState *sd, uint64_t addr)
-- 
2.25.1

[PATCH v2 15/25] hw/sd: Introduce receive_ready() callback

[Bin Meng]

From: Bin Meng 

At present there is a data_ready() callback for the SD data read
path. Let's add a receive_ready() for the SD data write path.

Signed-off-by: Bin Meng 
Acked-by: Alistair Francis 
Reviewed-by: Philippe Mathieu-Daudé 
---

(no changes since v1)

 include/hw/sd/sd.h |  2 ++
 hw/sd/core.c   | 13 +
 hw/sd/sd.c |  6 ++
 3 files changed, 21 insertions(+)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 05ef9b73e5..47360ba4ee 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -116,6 +116,7 @@ struct SDCardClass {
  * Return: byte value read
  */
 uint8_t (*read_byte)(SDState *sd);
+bool (*receive_ready)(SDState *sd);
 bool (*data_ready)(SDState *sd);
 void (*set_voltage)(SDState *sd, uint16_t millivolts);
 uint8_t (*get_dat_lines)(SDState *sd);
@@ -187,6 +188,7 @@ void sdbus_write_data(SDBus *sdbus, const void *buf, size_t 
length);
  * Read multiple bytes of data on the data lines of a SD bus.
  */
 void sdbus_read_data(SDBus *sdbus, void *buf, size_t length);
+bool sdbus_receive_ready(SDBus *sd);
 bool sdbus_data_ready(SDBus *sd);
 bool sdbus_get_inserted(SDBus *sd);
 bool sdbus_get_readonly(SDBus *sd);
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 08c93b5903..30ee62c510 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -160,6 +160,19 @@ void sdbus_read_data(SDBus *sdbus, void *buf, size_t 
length)
 }
 }
 
+bool sdbus_receive_ready(SDBus *sdbus)
+{
+SDState *card = get_card(sdbus);
+
+if (card) {
+SDCardClass *sc = SD_CARD_GET_CLASS(card);
+
+return sc->receive_ready(card);
+}
+
+return false;
+}
+
 bool sdbus_data_ready(SDBus *sdbus)
 {
 SDState *card = get_card(sdbus);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 946036d87c..c99c0e93bb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2037,6 +2037,11 @@ uint8_t sd_read_byte(SDState *sd)
 return ret;
 }
 
+static bool sd_receive_ready(SDState *sd)
+{
+return sd->state == sd_receivingdata_state;
+}
+
 static bool sd_data_ready(SDState *sd)
 {
 return sd->state == sd_sendingdata_state;
@@ -2147,6 +2152,7 @@ static void sd_class_init(ObjectClass *klass, void *data)
 sc->do_command = sd_do_command;
 sc->write_byte = sd_write_byte;
 sc->read_byte = sd_read_byte;
+sc->receive_ready = sd_receive_ready;
 sc->data_ready = sd_data_ready;
 sc->enable = sd_enable;
 sc->get_inserted = sd_get_inserted;
-- 
2.25.1

[PATCH v2 08/25] hw/sd: ssi-sd: Add a state representing Nac

[Bin Meng]

From: Bin Meng 

Per the "Physical Layer Specification Version 8.00" chapter 7.5.2,
"Data Read", there is a minimum 8 clock cycles (Nac) after the card
response and before data block shows up on the data out line. This
applies to both single and multiple block read operations.

Current implementation of single block read already satisfies the
timing requirement as in the RESPONSE state after all responses are
transferred the state remains unchanged. In the next 8 clock cycles
it jumps to DATA_START state if data is ready.

However we need an explicit state when expanding our support to
multiple block read in the future. Let's add a new state PREP_DATA
explicitly in the ssi-sd state machine to represent Nac.

Note we don't change the single block read state machine to let it
jump from RESPONSE state to DATA_START state as that effectively
generates a 16 clock cycles Nac, which might not be safe. As the
spec says the maximum Nac shall be calculated from several fields
encoded in the CSD register, we don't want to bother updating CSD
to ensure our Nac is within range to complicate things.

Signed-off-by: Bin Meng 

---

Changes in v2:
- new patch: add a state representing Nac

 hw/sd/ssi-sd.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 8bccedfab2..5763afeba0 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -39,6 +39,7 @@ typedef enum {
 SSI_SD_CMDARG,
 SSI_SD_PREP_RESP,
 SSI_SD_RESPONSE,
+SSI_SD_PREP_DATA,
 SSI_SD_DATA_START,
 SSI_SD_DATA_READ,
 SSI_SD_DATA_CRC16,
@@ -194,6 +195,10 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 s->mode = SSI_SD_CMD;
 }
 return 0xff;
+case SSI_SD_PREP_DATA:
+DPRINTF("Prepare data block (Nac)\n");
+s->mode = SSI_SD_DATA_START;
+return 0xff;
 case SSI_SD_DATA_START:
 DPRINTF("Start read block\n");
 s->mode = SSI_SD_DATA_READ;
-- 
2.25.1

[PATCH v2 14/25] hw/sd: sd.h: Cosmetic change of using spaces

[Bin Meng]

From: Bin Meng 

QEMU coding convention prefers spaces over tabs.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 

---

Changes in v2:
- Correct the "coding" typo in the commit message

 include/hw/sd/sd.h | 42 +-
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 59d108d453..05ef9b73e5 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -33,27 +33,27 @@
 #include "hw/qdev-core.h"
 #include "qom/object.h"
 
-#define OUT_OF_RANGE   (1 << 31)
-#define ADDRESS_ERROR  (1 << 30)
-#define BLOCK_LEN_ERROR(1 << 29)
-#define ERASE_SEQ_ERROR(1 << 28)
-#define ERASE_PARAM(1 << 27)
-#define WP_VIOLATION   (1 << 26)
-#define CARD_IS_LOCKED (1 << 25)
-#define LOCK_UNLOCK_FAILED (1 << 24)
-#define COM_CRC_ERROR  (1 << 23)
-#define ILLEGAL_COMMAND(1 << 22)
-#define CARD_ECC_FAILED(1 << 21)
-#define CC_ERROR   (1 << 20)
-#define SD_ERROR   (1 << 19)
-#define CID_CSD_OVERWRITE  (1 << 16)
-#define WP_ERASE_SKIP  (1 << 15)
-#define CARD_ECC_DISABLED  (1 << 14)
-#define ERASE_RESET(1 << 13)
-#define CURRENT_STATE  (7 << 9)
-#define READY_FOR_DATA (1 << 8)
-#define APP_CMD(1 << 5)
-#define AKE_SEQ_ERROR  (1 << 3)
+#define OUT_OF_RANGE(1 << 31)
+#define ADDRESS_ERROR   (1 << 30)
+#define BLOCK_LEN_ERROR (1 << 29)
+#define ERASE_SEQ_ERROR (1 << 28)
+#define ERASE_PARAM (1 << 27)
+#define WP_VIOLATION(1 << 26)
+#define CARD_IS_LOCKED  (1 << 25)
+#define LOCK_UNLOCK_FAILED  (1 << 24)
+#define COM_CRC_ERROR   (1 << 23)
+#define ILLEGAL_COMMAND (1 << 22)
+#define CARD_ECC_FAILED (1 << 21)
+#define CC_ERROR(1 << 20)
+#define SD_ERROR(1 << 19)
+#define CID_CSD_OVERWRITE   (1 << 16)
+#define WP_ERASE_SKIP   (1 << 15)
+#define CARD_ECC_DISABLED   (1 << 14)
+#define ERASE_RESET (1 << 13)
+#define CURRENT_STATE   (7 << 9)
+#define READY_FOR_DATA  (1 << 8)
+#define APP_CMD (1 << 5)
+#define AKE_SEQ_ERROR   (1 << 3)
 
 enum SDPhySpecificationVersion {
 SD_PHY_SPECv1_10_VERS = 1,
-- 
2.25.1

[PATCH v2 12/25] hw/sd: sd: Remove duplicated codes in single/multiple block read/write

[Bin Meng]

From: Bin Meng 

The single block read (CMD17) codes are the same as the multiple
block read (CMD18). Merge them into one. The same applies to single
block write (CMD24) and multiple block write (CMD25).

Signed-off-by: Bin Meng 
Acked-by: Alistair Francis 
---

(no changes since v1)

 hw/sd/sd.c | 47 ---
 1 file changed, 47 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b3952514fe..09753359bb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1181,24 +1181,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, 
SDRequest req)
 break;
 
 case 17:   /* CMD17:  READ_SINGLE_BLOCK */
-switch (sd->state) {
-case sd_transfer_state:
-
-if (addr + sd->blk_len > sd->size) {
-sd->card_status |= ADDRESS_ERROR;
-return sd_r1;
-}
-
-sd->state = sd_sendingdata_state;
-sd->data_start = addr;
-sd->data_offset = 0;
-return sd_r1;
-
-default:
-break;
-}
-break;
-
 case 18:   /* CMD18:  READ_MULTIPLE_BLOCK */
 switch (sd->state) {
 case sd_transfer_state:
@@ -1245,35 +1227,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, 
SDRequest req)
 
 /* Block write commands (Class 4) */
 case 24:   /* CMD24:  WRITE_SINGLE_BLOCK */
-switch (sd->state) {
-case sd_transfer_state:
-/* Writing in SPI mode not implemented.  */
-if (sd->spi)
-break;
-
-if (addr + sd->blk_len > sd->size) {
-sd->card_status |= ADDRESS_ERROR;
-return sd_r1;
-}
-
-sd->state = sd_receivingdata_state;
-sd->data_start = addr;
-sd->data_offset = 0;
-sd->blk_written = 0;
-
-if (sd_wp_addr(sd, sd->data_start)) {
-sd->card_status |= WP_VIOLATION;
-}
-if (sd->csd[14] & 0x30) {
-sd->card_status |= WP_VIOLATION;
-}
-return sd_r1;
-
-default:
-break;
-}
-break;
-
 case 25:   /* CMD25:  WRITE_MULTIPLE_BLOCK */
 switch (sd->state) {
 case sd_transfer_state:
-- 
2.25.1

[PATCH v2 06/25] util: Add CRC16 (CCITT) calculation routines

[Bin Meng]

From: Bin Meng 

Import CRC16 calculation routines from Linux kernel v5.10:

  include/linux/crc-ccitt.h
  lib/crc-ccitt.c

to QEMU:

  include/qemu/crc-ccitt.h
  util/crc-ccitt.c

Signed-off-by: Bin Meng 
Acked-by: Alistair Francis 
---

(no changes since v1)

 include/qemu/crc-ccitt.h |  33 ++
 util/crc-ccitt.c | 127 +++
 util/meson.build |   1 +
 3 files changed, 161 insertions(+)
 create mode 100644 include/qemu/crc-ccitt.h
 create mode 100644 util/crc-ccitt.c

diff --git a/include/qemu/crc-ccitt.h b/include/qemu/crc-ccitt.h
new file mode 100644
index 00..06ee55b159
--- /dev/null
+++ b/include/qemu/crc-ccitt.h
@@ -0,0 +1,33 @@
+/*
+ * CRC16 (CCITT) Checksum Algorithm
+ *
+ * Copyright (c) 2021 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * From Linux kernel v5.10 include/linux/crc-ccitt.h
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#ifndef _CRC_CCITT_H
+#define _CRC_CCITT_H
+
+extern uint16_t const crc_ccitt_table[256];
+extern uint16_t const crc_ccitt_false_table[256];
+
+extern uint16_t crc_ccitt(uint16_t crc, const uint8_t *buffer, size_t len);
+extern uint16_t crc_ccitt_false(uint16_t crc, const uint8_t *buffer, size_t 
len);
+
+static inline uint16_t crc_ccitt_byte(uint16_t crc, const uint8_t c)
+{
+return (crc >> 8) ^ crc_ccitt_table[(crc ^ c) & 0xff];
+}
+
+static inline uint16_t crc_ccitt_false_byte(uint16_t crc, const uint8_t c)
+{
+return (crc << 8) ^ crc_ccitt_false_table[(crc >> 8) ^ c];
+}
+
+#endif /* _CRC_CCITT_H */
diff --git a/util/crc-ccitt.c b/util/crc-ccitt.c
new file mode 100644
index 00..b981d8ac55
--- /dev/null
+++ b/util/crc-ccitt.c
@@ -0,0 +1,127 @@
+/*
+ * CRC16 (CCITT) Checksum Algorithm
+ *
+ * Copyright (c) 2021 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * From Linux kernel v5.10 lib/crc-ccitt.c
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/crc-ccitt.h"
+
+/*
+ * This mysterious table is just the CRC of each possible byte. It can be
+ * computed using the standard bit-at-a-time methods. The polynomial can
+ * be seen in entry 128, 0x8408. This corresponds to x^0 + x^5 + x^12.
+ * Add the implicit x^16, and you have the standard CRC-CCITT.
+ */
+uint16_t const crc_ccitt_table[256] = {
+0x, 0x1189, 0x2312, 0x329b, 0x4624, 0x57ad, 0x6536, 0x74bf,
+0x8c48, 0x9dc1, 0xaf5a, 0xbed3, 0xca6c, 0xdbe5, 0xe97e, 0xf8f7,
+0x1081, 0x0108, 0x3393, 0x221a, 0x56a5, 0x472c, 0x75b7, 0x643e,
+0x9cc9, 0x8d40, 0xbfdb, 0xae52, 0xdaed, 0xcb64, 0xf9ff, 0xe876,
+0x2102, 0x308b, 0x0210, 0x1399, 0x6726, 0x76af, 0x4434, 0x55bd,
+0xad4a, 0xbcc3, 0x8e58, 0x9fd1, 0xeb6e, 0xfae7, 0xc87c, 0xd9f5,
+0x3183, 0x200a, 0x1291, 0x0318, 0x77a7, 0x662e, 0x54b5, 0x453c,
+0xbdcb, 0xac42, 0x9ed9, 0x8f50, 0xfbef, 0xea66, 0xd8fd, 0xc974,
+0x4204, 0x538d, 0x6116, 0x709f, 0x0420, 0x15a9, 0x2732, 0x36bb,
+0xce4c, 0xdfc5, 0xed5e, 0xfcd7, 0x8868, 0x99e1, 0xab7a, 0xbaf3,
+0x5285, 0x430c, 0x7197, 0x601e, 0x14a1, 0x0528, 0x37b3, 0x263a,
+0xdecd, 0xcf44, 0xfddf, 0xec56, 0x98e9, 0x8960, 0xbbfb, 0xaa72,
+0x6306, 0x728f, 0x4014, 0x519d, 0x2522, 0x34ab, 0x0630, 0x17b9,
+0xef4e, 0xfec7, 0xcc5c, 0xddd5, 0xa96a, 0xb8e3, 0x8a78, 0x9bf1,
+0x7387, 0x620e, 0x5095, 0x411c, 0x35a3, 0x242a, 0x16b1, 0x0738,
+0xffcf, 0xee46, 0xdcdd, 0xcd54, 0xb9eb, 0xa862, 0x9af9, 0x8b70,
+0x8408, 0x9581, 0xa71a, 0xb693, 0xc22c, 0xd3a5, 0xe13e, 0xf0b7,
+0x0840, 0x19c9, 0x2b52, 0x3adb, 0x4e64, 0x5fed, 0x6d76, 0x7cff,
+0x9489, 0x8500, 0xb79b, 0xa612, 0xd2ad, 0xc324, 0xf1bf, 0xe036,
+0x18c1, 0x0948, 0x3bd3, 0x2a5a, 0x5ee5, 0x4f6c, 0x7df7, 0x6c7e,
+0xa50a, 0xb483, 0x8618, 0x9791, 0xe32e, 0xf2a7, 0xc03c, 0xd1b5,
+0x2942, 0x38cb, 0x0a50, 0x1bd9, 0x6f66, 0x7eef, 0x4c74, 0x5dfd,
+0xb58b, 0xa402, 0x9699, 0x8710, 0xf3af, 0xe226, 0xd0bd, 0xc134,
+0x39c3, 0x284a, 0x1ad1, 0x0b58, 0x7fe7, 0x6e6e, 0x5cf5, 0x4d7c,
+0xc60c, 0xd785, 0xe51e, 0xf497, 0x8028, 0x91a1, 0xa33a, 0xb2b3,
+0x4a44, 0x5bcd, 0x6956, 0x78df, 0x0c60, 0x1de9, 0x2f72, 0x3efb,
+0xd68d, 0xc704, 0xf59f, 0xe416, 0x90a9, 0x8120, 0xb3bb, 0xa232,
+0x5ac5, 0x4b4c, 0x79d7, 0x685e, 0x1ce1, 0x0d68, 0x3ff3, 0x2e7a,
+0xe70e, 0xf687, 0xc41c, 0xd595, 0xa12a, 0xb0a3, 0x8238, 0x93b1,
+0x6b46, 0x7acf, 0x4854, 0x59dd, 0x2d62, 0x3ceb, 0x0e70, 0x1ff9,
+0xf78f, 0xe606, 0xd49d, 0xc514, 0xb1ab, 0xa022, 0x92b9, 0x8330,
+0x7bc7, 0x6a4e, 0x58d5, 0x495c, 0x3de3, 0x2c6a, 0x1ef1, 0x0f78
+};
+
+/*
+ * Similar table to calculate CRC16 variant known as CRC-CCITT-FALSE
+ * Reflected bits order, does not augment final value.
+ */
+uint16_t const crc_ccitt_false_table[256] = {
+0x, 0x1021, 0x2042, 0x3063, 0x4084, 0x50A5, 0x60C6, 0x70E7,
+0x8108, 0x9129, 0xA14A, 0xB16B, 0xC18C, 0xD1AD, 0xE1CE, 0xF1EF,
+0x1231, 0x0210, 0x3273, 0x2252, 0x52B5, 0x4294, 0x72F7, 0x62D6,
+0x9339, 0x8318, 0xB37B, 0xA35A, 0xD3BD, 0xC39C, 0xF3FF, 

[PATCH v2 11/25] hw/sd: ssi-sd: Use macros for the dummy value and tokens in the transfer

[Bin Meng]

From: Bin Meng 

At present the codes use hardcoded numbers (0xff/0xfe) for the dummy
value and block start token. Replace them with macros.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 

---

Changes in v2:
- Move multiple write token definitions out of this patch

 hw/sd/ssi-sd.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index c1532b004b..75e76cf87a 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -79,6 +79,12 @@ OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD)
 #define SSI_SDR_ADDRESS_ERROR   0x2000
 #define SSI_SDR_PARAMETER_ERROR 0x4000
 
+/* single block read/write, multiple block read */
+#define SSI_TOKEN_SINGLE0xfe
+
+/* dummy value - don't care */
+#define SSI_DUMMY   0xff
+
 static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
 {
 ssi_sd_state *s = SSI_SD(dev);
@@ -107,14 +113,14 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 
 switch (s->mode) {
 case SSI_SD_CMD:
-if (val == 0xff) {
+if (val == SSI_DUMMY) {
 DPRINTF("NULL command\n");
-return 0xff;
+return SSI_DUMMY;
 }
 s->cmd = val & 0x3f;
 s->mode = SSI_SD_CMDARG;
 s->arglen = 0;
-return 0xff;
+return SSI_DUMMY;
 case SSI_SD_CMDARG:
 if (s->arglen == 4) {
 SDRequest request;
@@ -189,15 +195,15 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 } else {
 s->cmdarg[s->arglen++] = val;
 }
-return 0xff;
+return SSI_DUMMY;
 case SSI_SD_PREP_RESP:
 DPRINTF("Prepare card response (Ncr)\n");
 s->mode = SSI_SD_RESPONSE;
-return 0xff;
+return SSI_DUMMY;
 case SSI_SD_RESPONSE:
 if (s->stopping) {
 s->stopping = 0;
-return 0xff;
+return SSI_DUMMY;
 }
 if (s->response_pos < s->arglen) {
 DPRINTF("Response 0x%02x\n", s->response[s->response_pos]);
@@ -210,16 +216,16 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 DPRINTF("End of command\n");
 s->mode = SSI_SD_CMD;
 }
-return 0xff;
+return SSI_DUMMY;
 case SSI_SD_PREP_DATA:
 DPRINTF("Prepare data block (Nac)\n");
 s->mode = SSI_SD_DATA_START;
-return 0xff;
+return SSI_DUMMY;
 case SSI_SD_DATA_START:
 DPRINTF("Start read block\n");
 s->mode = SSI_SD_DATA_READ;
 s->response_pos = 0;
-return 0xfe;
+return SSI_TOKEN_SINGLE;
 case SSI_SD_DATA_READ:
 val = sdbus_read_byte(>sdbus);
 s->read_bytes++;
@@ -246,7 +252,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 return val;
 }
 /* Should never happen.  */
-return 0xff;
+return SSI_DUMMY;
 }
 
 static int ssi_sd_post_load(void *opaque, int version_id)
-- 
2.25.1

[PATCH v2 10/25] hw/sd: ssi-sd: Support multiple block read

[Bin Meng]

From: Bin Meng 

In the case of a multiple block read operation every transferred
block has its suffix of CRC16. Update the state machine logic to
handle multiple block read.

Signed-off-by: Bin Meng 
Acked-by: Alistair Francis 

---

Changes in v2:
- Fix 2 typos in the commit message
- Add a comment block to explain the CMD12 timing
- Catch CMD12 in all of the data read states per the timing requirement

 hw/sd/ssi-sd.c | 38 +++---
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 9e2f13374a..c1532b004b 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -52,6 +52,7 @@ struct ssi_sd_state {
 uint8_t cmdarg[4];
 uint8_t response[5];
 uint16_t crc16;
+int32_t read_bytes;
 int32_t arglen;
 int32_t response_pos;
 int32_t stopping;
@@ -82,11 +83,26 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 {
 ssi_sd_state *s = SSI_SD(dev);
 
-/* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
-if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
-s->mode = SSI_SD_CMD;
-/* There must be at least one byte delay before the card responds.  */
-s->stopping = 1;
+/*
+ * Special case: allow CMD12 (STOP TRANSMISSION) while reading data.
+ *
+ * See "Physical Layer Specification Version 8.00" chapter 7.5.2.2,
+ * to avoid conflict between CMD12 response and next data block,
+ * timing of CMD12 should be controlled as follows:
+ *
+ * - CMD12 issued at the timing that end bit of CMD12 and end bit of
+ *   data block is overlapped
+ * - CMD12 issued after one clock cycle after host receives a token
+ *   (either Start Block token or Data Error token)
+ *
+ * We need to catch CMD12 in all of the data read states.
+ */
+if (s->mode >= SSI_SD_PREP_DATA && s->mode <= SSI_SD_DATA_CRC16) {
+if (val == 0x4c) {
+s->mode = SSI_SD_CMD;
+/* There must be at least one byte delay before the card responds 
*/
+s->stopping = 1;
+}
 }
 
 switch (s->mode) {
@@ -206,8 +222,9 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 return 0xfe;
 case SSI_SD_DATA_READ:
 val = sdbus_read_byte(>sdbus);
+s->read_bytes++;
 s->crc16 = crc_ccitt_false(s->crc16, (uint8_t *), 1);
-if (!sdbus_data_ready(>sdbus)) {
+if (!sdbus_data_ready(>sdbus) || s->read_bytes == 512) {
 DPRINTF("Data read end\n");
 s->mode = SSI_SD_DATA_CRC16;
 }
@@ -218,7 +235,12 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 s->response_pos++;
 if (s->response_pos == 2) {
 DPRINTF("CRC16 read end\n");
-s->mode = SSI_SD_CMD;
+if (s->read_bytes == 512 && s->cmd != 17) {
+s->mode = SSI_SD_PREP_DATA;
+} else {
+s->mode = SSI_SD_CMD;
+}
+s->read_bytes = 0;
 s->response_pos = 0;
 }
 return val;
@@ -258,6 +280,7 @@ static const VMStateDescription vmstate_ssi_sd = {
 VMSTATE_UINT8_ARRAY(cmdarg, ssi_sd_state, 4),
 VMSTATE_UINT8_ARRAY(response, ssi_sd_state, 5),
 VMSTATE_UINT16(crc16, ssi_sd_state),
+VMSTATE_INT32(read_bytes, ssi_sd_state),
 VMSTATE_INT32(arglen, ssi_sd_state),
 VMSTATE_INT32(response_pos, ssi_sd_state),
 VMSTATE_INT32(stopping, ssi_sd_state),
@@ -310,6 +333,7 @@ static void ssi_sd_reset(DeviceState *dev)
 memset(s->cmdarg, 0, sizeof(s->cmdarg));
 memset(s->response, 0, sizeof(s->response));
 s->crc16 = 0;
+s->read_bytes = 0;
 s->arglen = 0;
 s->response_pos = 0;
 s->stopping = 0;
-- 
2.25.1

[PATCH v2 09/25] hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION

[Bin Meng]

From: Bin Meng 

This fixes the wrong command index for STOP_TRANSMISSION, the
required command to interrupt the multiple block read command,
in the old codes. It should be CMD12 (0x4c), not CMD13 (0x4d).

Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
Signed-off-by: Bin Meng 

---

Changes in v2:
- Make this fix a separate patch from the CMD18 support

 hw/sd/ssi-sd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 5763afeba0..9e2f13374a 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -83,7 +83,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t 
val)
 ssi_sd_state *s = SSI_SD(dev);
 
 /* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.  */
-if (s->mode == SSI_SD_DATA_READ && val == 0x4d) {
+if (s->mode == SSI_SD_DATA_READ && val == 0x4c) {
 s->mode = SSI_SD_CMD;
 /* There must be at least one byte delay before the card responds.  */
 s->stopping = 1;
-- 
2.25.1

[PATCH v2 04/25] hw/sd: sd: Support CMD59 for SPI mode

[Bin Meng]

From: Bin Meng 

After the card is put into SPI mode, CRC check for all commands
including CMD0 will be done according to CMD59 setting. But this
command is currently unimplemented. Simply allow the decoding of
CMD59, but the CRC remains unchecked.

Signed-off-by: Bin Meng 
Reviewed-by: Pragnesh Patel 
Tested-by: Pragnesh Patel 
---

(no changes since v1)

 hw/sd/sd.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 4375ed5b8b..bfea5547d5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1517,18 +1517,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, 
SDRequest req)
 if (!sd->spi) {
 goto bad_cmd;
 }
-goto unimplemented_spi_cmd;
+return sd_r1;
 
 default:
 bad_cmd:
 qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
 return sd_illegal;
-
-unimplemented_spi_cmd:
-/* Commands that are recognised but not yet implemented in SPI mode.  
*/
-qemu_log_mask(LOG_UNIMP, "SD: CMD%i not implemented in SPI mode\n",
-  req.cmd);
-return sd_illegal;
 }
 
 qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state\n", req.cmd);
-- 
2.25.1

[PATCH v2 03/25] hw/sd: ssi-sd: Fix incorrect card response sequence

[Bin Meng]

From: Bin Meng 

Per the "Physical Layer Specification Version 8.00" chapter 7.5.1,
"Command/Response", there is a minimum 8 clock cycles (Ncr) before
the card response shows up on the data out line. However current
implementation jumps directly to the sending response state after
all 6 bytes command is received, which is a spec violation.

Add a new state PREP_RESP in the ssi-sd state machine to handle it.

Fixes: 775616c3ae8c ("Partial SD card SPI mode support")
Signed-off-by: Bin Meng 
Reviewed-by: Pragnesh Patel 
Tested-by: Pragnesh Patel 

---

Changes in v2:
- Add a debug printf in the state handling codes

 hw/sd/ssi-sd.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 9a75e0095c..043e270066 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -36,6 +36,7 @@ do { fprintf(stderr, "ssi_sd: error: " fmt , ## 
__VA_ARGS__);} while (0)
 typedef enum {
 SSI_SD_CMD = 0,
 SSI_SD_CMDARG,
+SSI_SD_PREP_RESP,
 SSI_SD_RESPONSE,
 SSI_SD_DATA_START,
 SSI_SD_DATA_READ,
@@ -163,12 +164,16 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, 
uint32_t val)
 s->response[1] = status;
 DPRINTF("Card status 0x%02x\n", status);
 }
-s->mode = SSI_SD_RESPONSE;
+s->mode = SSI_SD_PREP_RESP;
 s->response_pos = 0;
 } else {
 s->cmdarg[s->arglen++] = val;
 }
 return 0xff;
+case SSI_SD_PREP_RESP:
+DPRINTF("Prepare card response (Ncr)\n");
+s->mode = SSI_SD_RESPONSE;
+return 0xff;
 case SSI_SD_RESPONSE:
 if (s->stopping) {
 s->stopping = 0;
-- 
2.25.1

[PATCH v2 02/25] hw/block: m25p80: Add various ISSI flash information

[Bin Meng]

From: Bin Meng 

This updates the flash information table to include various ISSI
flashes that are supported by upstream U-Boot and Linux kernel.

Signed-off-by: Bin Meng 
Acked-by: Alistair Francis 
---

(no changes since v1)

 hw/block/m25p80.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 217c130f56..4bf8aa8158 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -210,6 +210,19 @@ static const FlashPartInfo known_devices[] = {
 { INFO("640s33b", 0x898913,  0,  64 << 10, 128, 0) },
 { INFO("n25q064", 0x20ba17,  0,  64 << 10, 128, 0) },
 
+/* ISSI */
+{ INFO("is25lq040b",  0x9d4013,  0,  64 << 10,   8, ER_4K) },
+{ INFO("is25lp080d",  0x9d6014,  0,  64 << 10,  16, ER_4K) },
+{ INFO("is25lp016d",  0x9d6015,  0,  64 << 10,  32, ER_4K) },
+{ INFO("is25lp032",   0x9d6016,  0,  64 << 10,  64, ER_4K) },
+{ INFO("is25lp064",   0x9d6017,  0,  64 << 10, 128, ER_4K) },
+{ INFO("is25lp128",   0x9d6018,  0,  64 << 10, 256, ER_4K) },
+{ INFO("is25lp256",   0x9d6019,  0,  64 << 10, 512, ER_4K) },
+{ INFO("is25wp032",   0x9d7016,  0,  64 << 10,  64, ER_4K) },
+{ INFO("is25wp064",   0x9d7017,  0,  64 << 10, 128, ER_4K) },
+{ INFO("is25wp128",   0x9d7018,  0,  64 << 10, 256, ER_4K) },
+{ INFO("is25wp256",   0x9d7019,  0,  64 << 10, 512, ER_4K) },
+
 /* Macronix */
 { INFO("mx25l2005a",  0xc22012,  0,  64 << 10,   4, ER_4K) },
 { INFO("mx25l4005a",  0xc22013,  0,  64 << 10,   8, ER_4K) },
-- 
2.25.1

[PATCH v2 01/25] hw/block: m25p80: Add ISSI SPI flash support

[Bin Meng]

From: Bin Meng 

This adds the ISSI SPI flash support. The number of dummy cycles in
fast read, fast read dual output and fast read quad output commands
is currently using the default 8. Likewise, the same default value
is used for fast read dual/quad I/O command. Per the datasheet [1],
the number of dummy cycles is configurable, but this is not modeled
at present.

For flash whose size is larger than 16 MiB, the sequence of 3-byte
address along with EXTADD bit in the bank address register (BAR) is
not supported. We assume that guest software always uses op codes
with 4-byte address sequence. Fortunately, this is the case for both
U-Boot and Linux spi-nor drivers.

QPI (Quad Peripheral Interface) that supports 2-cycle instruction
has different default values for dummy cycles of fast read family
commands, and is unsupported at the time being.

[1] http://www.issi.com/WW/pdf/25LP-WP256.pdf

Signed-off-by: Bin Meng 

---

Changes in v2:
- Mention QPI (Quad Peripheral Interface) mode is not supported

 hw/block/m25p80.c | 44 +++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index b744a58d1c..217c130f56 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -412,6 +412,7 @@ typedef enum {
 MAN_NUMONYX,
 MAN_WINBOND,
 MAN_SST,
+MAN_ISSI,
 MAN_GENERIC,
 } Manufacturer;
 
@@ -487,6 +488,8 @@ static inline Manufacturer get_man(Flash *s)
 return MAN_MACRONIX;
 case 0xBF:
 return MAN_SST;
+case 0x9D:
+return MAN_ISSI;
 default:
 return MAN_GENERIC;
 }
@@ -706,6 +709,9 @@ static void complete_collecting_data(Flash *s)
 case MAN_SPANSION:
 s->quad_enable = !!(s->data[1] & 0x02);
 break;
+case MAN_ISSI:
+s->quad_enable = extract32(s->data[0], 6, 1);
+break;
 case MAN_MACRONIX:
 s->quad_enable = extract32(s->data[0], 6, 1);
 if (s->len > 1) {
@@ -895,6 +901,19 @@ static void decode_fast_read_cmd(Flash *s)
 SPANSION_DUMMY_CLK_LEN
 );
 break;
+case MAN_ISSI:
+/*
+ * The Fast Read instruction code is followed by address bytes and
+ * dummy cycles, transmitted via the SI line.
+ *
+ * The number of dummy cycles is configurable but this is currently
+ * unmodeled, hence the default value 8 is used.
+ *
+ * QPI (Quad Peripheral Interface) mode has different default value
+ * of dummy cycles, but this is unsupported at the time being.
+ */
+s->needed_bytes += 1;
+break;
 default:
 break;
 }
@@ -934,6 +953,16 @@ static void decode_dio_read_cmd(Flash *s)
 break;
 }
 break;
+case MAN_ISSI:
+/*
+ * The Fast Read Dual I/O instruction code is followed by address bytes
+ * and dummy cycles, transmitted via the IO1 and IO0 line.
+ *
+ * The number of dummy cycles is configurable but this is currently
+ * unmodeled, hence the default value 4 is used.
+ */
+s->needed_bytes += 1;
+break;
 default:
 break;
 }
@@ -974,6 +1003,19 @@ static void decode_qio_read_cmd(Flash *s)
 break;
 }
 break;
+case MAN_ISSI:
+/*
+ * The Fast Read Quad I/O instruction code is followed by address bytes
+ * and dummy cycles, transmitted via the IO3, IO2, IO1 and IO0 line.
+ *
+ * The number of dummy cycles is configurable but this is currently
+ * unmodeled, hence the default value 6 is used.
+ *
+ * QPI (Quad Peripheral Interface) mode has different default value
+ * of dummy cycles, but this is unsupported at the time being.
+ */
+s->needed_bytes += 3;
+break;
 default:
 break;
 }
@@ -1132,7 +1174,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 
 case RDSR:
 s->data[0] = (!!s->write_enable) << 1;
-if (get_man(s) == MAN_MACRONIX) {
+if (get_man(s) == MAN_MACRONIX || get_man(s) == MAN_ISSI) {
 s->data[0] |= (!!s->quad_enable) << 6;
 }
 if (get_man(s) == MAN_SST) {
-- 
2.25.1

[PATCH v2 00/25] hw/riscv: sifive_u: Add missing SPI support

[Bin Meng]

From: Bin Meng 

This adds the missing SPI support to the `sifive_u` machine in the QEMU
mainline. With this series, upstream U-Boot for the SiFive HiFive Unleashed
board can boot on QEMU `sifive_u` out of the box. This allows users to
develop and test the recommended RISC-V boot flow with a real world use
case: ZSBL (in QEMU) loads U-Boot SPL from SD card or SPI flash to L2LIM,
then U-Boot SPL loads the payload from SD card or SPI flash that is a
combination of OpenSBI fw_dynamic firmware and U-Boot proper.

The m25p80 model is updated to support ISSI flash series. A bunch of
ssi-sd issues are fixed, and writing to SD card in SPI mode is supported.

reST documentation for RISC-V is added. Currently only `sifive_u`
machine is documented, but more to come.

Changes in v2:
- Mention QPI (Quad Peripheral Interface) mode is not supported
- Add a debug printf in the state handling codes
- Fix several typos in the commit message
- new patch: add a state representing Nac
- Make this fix a separate patch from the CMD18 support
- Fix 2 typos in the commit message
- Add a comment block to explain the CMD12 timing
- Catch CMD12 in all of the data read states per the timing requirement
- Move multiple write token definitions out of this patch
- Correct the "coding" typo in the commit message
- Correct the "token" typo in the commit message
- Add 'write_bytes' in vmstate_ssi_sd
- Correct the "token" typo in the commit message
- Introduce multiple write token definitions in this patch
- new patch: bump up version ids of VMStateDescription
- Log guest error when trying to write reserved registers
- Log guest error when trying to access out-of-bounds registers
- log guest error when writing to reserved bits for chip select
  registers and watermark registers
- Log unimplemented warning when trying to write direct-map flash
  interface registers
- Add test tx fifo full logic in sifive_spi_read(), hence remove
  setting the tx fifo full flag in sifive_spi_write().
- Populate register with their default value
- Correct the "connects" typo in the commit message
- Mention in the commit message that  property does not populate
  the second group which represents the memory mapped address of the
  SPI flash
- Correct the "connects" typo in the commit message
- Correct several typos in sifive_u.rst
- Update doc to mention U-Boot v2021.01

Bin Meng (25):
  hw/block: m25p80: Add ISSI SPI flash support
  hw/block: m25p80: Add various ISSI flash information
  hw/sd: ssi-sd: Fix incorrect card response sequence
  hw/sd: sd: Support CMD59 for SPI mode
  hw/sd: sd: Drop sd_crc16()
  util: Add CRC16 (CCITT) calculation routines
  hw/sd: ssi-sd: Suffix a data block with CRC16
  hw/sd: ssi-sd: Add a state representing Nac
  hw/sd: ssi-sd: Fix the wrong command index for STOP_TRANSMISSION
  hw/sd: ssi-sd: Support multiple block read
  hw/sd: ssi-sd: Use macros for the dummy value and tokens in the
transfer
  hw/sd: sd: Remove duplicated codes in single/multiple block read/write
  hw/sd: sd: Allow single/multiple block write for SPI mode
  hw/sd: sd.h: Cosmetic change of using spaces
  hw/sd: Introduce receive_ready() callback
  hw/sd: ssi-sd: Support single block write
  hw/sd: ssi-sd: Support multiple block write
  hw/sd: ssi-sd: Bump up version ids of VMStateDescription
  hw/ssi: Add SiFive SPI controller support
  hw/riscv: sifive_u: Add QSPI0 controller and connect a flash
  hw/riscv: sifive_u: Add QSPI2 controller and connect an SD card
  hw/riscv: sifive_u: Change SIFIVE_U_GEM_IRQ to decimal value
  docs/system: Sort targets in alphabetical order
  docs/system: Add RISC-V documentation
  docs/system: riscv: Add documentation for sifive_u machine

 docs/system/riscv/sifive_u.rst | 336 ++
 docs/system/target-riscv.rst   |  72 +++
 docs/system/targets.rst|  20 +-
 include/hw/riscv/sifive_u.h|   9 +-
 include/hw/sd/sd.h |  44 ++--
 include/hw/ssi/sifive_spi.h|  47 +
 include/qemu/crc-ccitt.h   |  33 +++
 hw/block/m25p80.c  |  57 -
 hw/riscv/sifive_u.c|  91 
 hw/sd/core.c   |  13 ++
 hw/sd/sd.c |  82 +---
 hw/sd/ssi-sd.c | 166 +--
 hw/ssi/sifive_spi.c| 367 +
 util/crc-ccitt.c   | 127 
 hw/riscv/Kconfig   |   3 +
 hw/ssi/Kconfig |   4 +
 hw/ssi/meson.build |   1 +
 util/meson.build   |   1 +
 18 files changed, 1347 insertions(+), 126 deletions(-)
 create mode 100644 docs/system/riscv/sifive_u.rst
 create mode 100644 docs/system/target-riscv.rst
 create mode 100644 include/hw/ssi/sifive_spi.h
 create mode 100644 include/qemu/crc-ccitt.h
 create mode 100644 hw/ssi/sifive_spi.c
 create mode 100644 util/crc-ccitt.c

-- 
2.25.1