[Qemu-devel] [PULL v2 01/32] pci: fix pci_requester_id()

[Qemu-devel] [PULL v2 01/32] pci: fix pci_requester_id()

[Michael S. Tsirkin]

From: Peter Xu <peterx@redhat.com>

This fix SID verification failure when IOMMU IR is enabled with PCI
bridges. Existing pci_requester_id() is more like getting BDF info
only. Renaming it to pci_get_bdf(). Meanwhile, we provide the correct
implementation to get requester ID. VT-d spec 5.1.1 is a good reference
to go, though it talks only about interrupt delivery, the rule works
exactly the same for non-interrupt cases.

Currently, there are three use cases for pci_requester_id():

- PCIX status bits: here we need BDF only, not requester ID. Replacing
  with pci_get_bdf().
- PCIe Error injection and MSI delivery: for both these cases, we are
  looking for requester IDs. Here we should use the new impl.

To avoid a PCI walk every time we send MSI message, one requester_id
cache field is added to PCIDevice to cache the result when initialize
PCI device.

Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/pci/pci.h     | 26 +++++++++++++++--
 hw/i386/kvm/pci-assign.c |  2 +-
 hw/pci/pci.c             | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 3 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 4420f47..9ed1624 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -15,6 +15,7 @@
 #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
 #define PCI_SLOT(devfn)         (((devfn) >> 3) & 0x1f)
 #define PCI_FUNC(devfn)         ((devfn) & 0x07)
+#define PCI_BUILD_BDF(bus, devfn)     ((bus << 8) | (devfn))
 #define PCI_SLOT_MAX            32
 #define PCI_FUNC_MAX            8
 
@@ -230,6 +231,20 @@ typedef void (*MSIVectorPollNotifier)(PCIDevice *dev,
                                       unsigned int vector_start,
                                       unsigned int vector_end);
 
+enum PCIReqIDType {
+    PCI_REQ_ID_INVALID = 0,
+    PCI_REQ_ID_BDF,
+    PCI_REQ_ID_SECONDARY_BUS,
+    PCI_REQ_ID_MAX,
+};
+typedef enum PCIReqIDType PCIReqIDType;
+
+struct PCIReqIDCache {
+    PCIDevice *dev;
+    PCIReqIDType type;
+};
+typedef struct PCIReqIDCache PCIReqIDCache;
+
 struct PCIDevice {
     DeviceState qdev;
 
@@ -252,6 +267,11 @@ struct PCIDevice {
     /* the following fields are read only */
     PCIBus *bus;
     int32_t devfn;
+    /* Cached device to fetch requester ID from, to avoid the PCI
+     * tree walking every time we invoke PCI request (e.g.,
+     * MSI). For conventional PCI root complex, this field is
+     * meaningless. */
+    PCIReqIDCache requester_id_cache;
     char name[64];
     PCIIORegion io_regions[PCI_NUM_REGIONS];
     AddressSpace bus_master_as;
@@ -692,11 +712,13 @@ static inline uint32_t pci_config_size(const PCIDevice *d)
     return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE;
 }
 
-static inline uint16_t pci_requester_id(PCIDevice *dev)
+static inline uint16_t pci_get_bdf(PCIDevice *dev)
 {
-    return (pci_bus_num(dev->bus) << 8) | dev->devfn;
+    return PCI_BUILD_BDF(pci_bus_num(dev->bus), dev->devfn);
 }
 
+uint16_t pci_requester_id(PCIDevice *dev);
+
 /* DMA access functions */
 static inline AddressSpace *pci_get_address_space(PCIDevice *dev)
 {
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index db2cbd2..bceed09 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1482,7 +1482,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
          * error bits, leave the rest. */
         status = pci_get_long(pci_dev->config + pos + PCI_X_STATUS);
         status &= ~(PCI_X_STATUS_BUS | PCI_X_STATUS_DEVFN);
-        status |= pci_requester_id(pci_dev);
+        status |= pci_get_bdf(pci_dev);
         status &= ~(PCI_X_STATUS_SPL_DISC | PCI_X_STATUS_UNX_SPL |
                     PCI_X_STATUS_SPL_ERR);
         pci_set_long(pci_dev->config + pos + PCI_X_STATUS, status);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index bb605ef..87bea47 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -836,6 +836,81 @@ static void do_pci_unregister_device(PCIDevice *pci_dev)
     address_space_destroy(&pci_dev->bus_master_as);
 }
 
+/* Extract PCIReqIDCache into BDF format */
+static uint16_t pci_req_id_cache_extract(PCIReqIDCache *cache)
+{
+    uint8_t bus_n;
+    uint16_t result;
+
+    switch (cache->type) {
+    case PCI_REQ_ID_BDF:
+        result = pci_get_bdf(cache->dev);
+        break;
+    case PCI_REQ_ID_SECONDARY_BUS:
+        bus_n = pci_bus_num(cache->dev->bus);
+        result = PCI_BUILD_BDF(bus_n, 0);
+        break;
+    default:
+        error_printf("Invalid PCI requester ID cache type: %d\n",
+                     cache->type);
+        exit(1);
+        break;
+    }
+
+    return result;
+}
+
+/* Parse bridges up to the root complex and return requester ID
+ * cache for specific device.  For full PCIe topology, the cache
+ * result would be exactly the same as getting BDF of the device.
+ * However, several tricks are required when system mixed up with
+ * legacy PCI devices and PCIe-to-PCI bridges.
+ *
+ * Here we cache the proxy device (and type) not requester ID since
+ * bus number might change from time to time.
+ */
+static PCIReqIDCache pci_req_id_cache_get(PCIDevice *dev)
+{
+    PCIDevice *parent;
+    PCIReqIDCache cache = {
+        .dev = dev,
+        .type = PCI_REQ_ID_BDF,
+    };
+
+    while (!pci_bus_is_root(dev->bus)) {
+        /* We are under PCI/PCIe bridges */
+        parent = dev->bus->parent_dev;
+        if (pci_is_express(parent)) {
+            if (pcie_cap_get_type(parent) == PCI_EXP_TYPE_PCI_BRIDGE) {
+                /* When we pass through PCIe-to-PCI/PCIX bridges, we
+                 * override the requester ID using secondary bus
+                 * number of parent bridge with zeroed devfn
+                 * (pcie-to-pci bridge spec chap 2.3). */
+                cache.type = PCI_REQ_ID_SECONDARY_BUS;
+                cache.dev = dev;
+            }
+        } else {
+            /* Legacy PCI, override requester ID with the bridge's
+             * BDF upstream.  When the root complex connects to
+             * legacy PCI devices (including buses), it can only
+             * obtain requester ID info from directly attached
+             * devices.  If devices are attached under bridges, only
+             * the requester ID of the bridge that is directly
+             * attached to the root complex can be recognized. */
+            cache.type = PCI_REQ_ID_BDF;
+            cache.dev = parent;
+        }
+        dev = parent;
+    }
+
+    return cache;
+}
+
+uint16_t pci_requester_id(PCIDevice *dev)
+{
+    return pci_req_id_cache_extract(&dev->requester_id_cache);
+}
+
 /* -1 for devfn means auto assign */
 static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
                                          const char *name, int devfn,
@@ -885,6 +960,7 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
     }
 
     pci_dev->devfn = devfn;
+    pci_dev->requester_id_cache = pci_req_id_cache_get(pci_dev);
     dma_as = pci_device_iommu_address_space(pci_dev);
 
     memory_region_init_alias(&pci_dev->bus_master_enable_region,
-- 
MST

[Qemu-devel] [PULL v2 02/32] vhost-user: add ability to know vhost-user backend disconnection

[Michael S. Tsirkin]

From: Tetsuya Mukawa <mukawa@igel.co.jp>

Current QEMU cannot detect vhost-user backend disconnection. The
patch adds ability to know it.
To know disconnection, add watcher to detect G_IO_HUP event. When
G_IO_HUP event is detected, the disconnected socket will be read
to cause a CHR_EVENT_CLOSED.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Victor Kaplansky <victork@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 net/vhost-user.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 1b9e73a..4a7fd5f 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -22,6 +22,7 @@ typedef struct VhostUserState {
     NetClientState nc;
     CharDriverState *chr;
     VHostNetState *vhost_net;
+    int watch;
 } VhostUserState;
 
 typedef struct VhostUserChardevProps {
@@ -167,6 +168,20 @@ static NetClientInfo net_vhost_user_info = {
         .has_ufo = vhost_user_has_ufo,
 };
 
+static gboolean net_vhost_user_watch(GIOChannel *chan, GIOCondition cond,
+                                           void *opaque)
+{
+    VhostUserState *s = opaque;
+    uint8_t buf[1];
+
+    /* We don't actually want to read anything, but CHR_EVENT_CLOSED will be
+     * raised as a side-effect of the read.
+     */
+    qemu_chr_fe_read_all(s->chr, buf, sizeof(buf));
+
+    return FALSE;
+}
+
 static void net_vhost_user_event(void *opaque, int event)
 {
     const char *name = opaque;
@@ -184,6 +199,8 @@ static void net_vhost_user_event(void *opaque, int event)
     trace_vhost_user_event(s->chr->label, event);
     switch (event) {
     case CHR_EVENT_OPENED:
+        s->watch = qemu_chr_fe_add_watch(s->chr, G_IO_HUP,
+                                         net_vhost_user_watch, s);
         if (vhost_user_start(queues, ncs) < 0) {
             exit(1);
         }
@@ -192,6 +209,8 @@ static void net_vhost_user_event(void *opaque, int event)
     case CHR_EVENT_CLOSED:
         qmp_set_link(name, false, &err);
         vhost_user_stop(queues, ncs);
+        g_source_remove(s->watch);
+        s->watch = 0;
         break;
     }
 
-- 
MST

[Qemu-devel] [PULL v2 03/32] tests/vhost-user-bridge: add client mode

[Michael S. Tsirkin]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

If -c is specified, vubr will try to connect to the socket instead of
listening for connections.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Victor Kaplansky <victork@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/vhost-user-bridge.c | 44 ++++++++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
index 0779ba2..e231ce9 100644
--- a/tests/vhost-user-bridge.c
+++ b/tests/vhost-user-bridge.c
@@ -1204,12 +1204,13 @@ vubr_accept_cb(int sock, void *ctx)
 }
 
 static VubrDev *
-vubr_new(const char *path)
+vubr_new(const char *path, bool client)
 {
     VubrDev *dev = (VubrDev *) calloc(1, sizeof(VubrDev));
     dev->nregions = 0;
     int i;
     struct sockaddr_un un;
+    CallbackFunc cb;
     size_t len;
 
     for (i = 0; i < MAX_NR_VIRTQUEUE; i++) {
@@ -1238,21 +1239,30 @@ vubr_new(const char *path)
     un.sun_family = AF_UNIX;
     strcpy(un.sun_path, path);
     len = sizeof(un.sun_family) + strlen(path);
-    unlink(path);
 
-    if (bind(dev->sock, (struct sockaddr *) &un, len) == -1) {
-        vubr_die("bind");
-    }
+    if (!client) {
+        unlink(path);
 
-    if (listen(dev->sock, 1) == -1) {
-        vubr_die("listen");
+        if (bind(dev->sock, (struct sockaddr *) &un, len) == -1) {
+            vubr_die("bind");
+        }
+
+        if (listen(dev->sock, 1) == -1) {
+            vubr_die("listen");
+        }
+        cb = vubr_accept_cb;
+
+        DPRINT("Waiting for connections on UNIX socket %s ...\n", path);
+    } else {
+        if (connect(dev->sock, (struct sockaddr *)&un, len) == -1) {
+            vubr_die("connect");
+        }
+        cb = vubr_receive_cb;
     }
 
     dispatcher_init(&dev->dispatcher);
-    dispatcher_add(&dev->dispatcher, dev->sock, (void *)dev,
-                   vubr_accept_cb);
+    dispatcher_add(&dev->dispatcher, dev->sock, (void *)dev, cb);
 
-    DPRINT("Waiting for connections on UNIX socket %s ...\n", path);
     return dev;
 }
 
@@ -1369,8 +1379,9 @@ main(int argc, char *argv[])
 {
     VubrDev *dev;
     int opt;
+    bool client = false;
 
-    while ((opt = getopt(argc, argv, "l:r:u:")) != -1) {
+    while ((opt = getopt(argc, argv, "l:r:u:c")) != -1) {
 
         switch (opt) {
         case 'l':
@@ -1386,16 +1397,20 @@ main(int argc, char *argv[])
         case 'u':
             ud_socket_path = strdup(optarg);
             break;
+        case 'c':
+            client = true;
+            break;
         default:
             goto out;
         }
     }
 
-    DPRINT("ud socket: %s\n", ud_socket_path);
+    DPRINT("ud socket: %s (%s)\n", ud_socket_path,
+           client ? "client" : "server");
     DPRINT("local:     %s:%s\n", lhost, lport);
     DPRINT("remote:    %s:%s\n", rhost, rport);
 
-    dev = vubr_new(ud_socket_path);
+    dev = vubr_new(ud_socket_path, client);
     if (!dev) {
         return 1;
     }
@@ -1406,13 +1421,14 @@ main(int argc, char *argv[])
 
 out:
     fprintf(stderr, "Usage: %s ", argv[0]);
-    fprintf(stderr, "[-u ud_socket_path] [-l lhost:lport] [-r rhost:rport]\n");
+    fprintf(stderr, "[-c] [-u ud_socket_path] [-l lhost:lport] [-r rhost:rport]\n");
     fprintf(stderr, "\t-u path to unix doman socket. default: %s\n",
             DEFAULT_UD_SOCKET);
     fprintf(stderr, "\t-l local host and port. default: %s:%s\n",
             DEFAULT_LHOST, DEFAULT_LPORT);
     fprintf(stderr, "\t-r remote host and port. default: %s:%s\n",
             DEFAULT_RHOST, DEFAULT_RPORT);
+    fprintf(stderr, "\t-c client mode\n");
 
     return 1;
 }
-- 
MST

[Qemu-devel] [PULL v2 04/32] tests/vhost-user-bridge: workaround stale vring base

[Michael S. Tsirkin]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

This patch is a similar solution to what Yuanhan Liu/Huawei Xie have
suggested for DPDK. When vubr quits (killed or crashed), a restart of
vubr would get stale vring base from QEMU. That would break the kernel
virtio net completely, making it non-work any more, unless a driver
reset is done.

So, instead of getting the stale vring base from QEMU, Huawei suggested
we could get a proper one from used->idx. This works because the queues
packets are processed in order.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Victor Kaplansky <victork@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/vhost-user-bridge.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/vhost-user-bridge.c b/tests/vhost-user-bridge.c
index e231ce9..36b3cd8 100644
--- a/tests/vhost-user-bridge.c
+++ b/tests/vhost-user-bridge.c
@@ -946,6 +946,13 @@ vubr_set_vring_addr_exec(VubrDev *dev, VhostUserMsg *vmsg)
     DPRINT("    vring_avail at %p\n", vq->avail);
 
     vq->last_used_index = vq->used->idx;
+
+    if (vq->last_avail_index != vq->used->idx) {
+        DPRINT("Last avail index != used index: %d != %d, resuming",
+               vq->last_avail_index, vq->used->idx);
+        vq->last_avail_index = vq->used->idx;
+    }
+
     return 0;
 }
 
-- 
MST

[Qemu-devel] [PULL v2 05/32] qemu-char: add qemu_chr_disconnect to close a fd accepted by listen fd

[Michael S. Tsirkin]

From: Tetsuya Mukawa <mukawa@igel.co.jp>

The patch introduces qemu_chr_disconnect(). The function is used for
closing a fd accepted by listen fd. Though we already have qemu_chr_delete(),
but it closes not only accepted fd but also listen fd. This new function
is used when we still want to keep listen fd.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Victor Kaplansky <victork@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/sysemu/char.h | 7 +++++++
 qemu-char.c           | 8 ++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index 372a6fd..1eb2d0f 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -75,6 +75,7 @@ struct CharDriverState {
     IOReadHandler *chr_read;
     void *handler_opaque;
     void (*chr_close)(struct CharDriverState *chr);
+    void (*chr_disconnect)(struct CharDriverState *chr);
     void (*chr_accept_input)(struct CharDriverState *chr);
     void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
     void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open);
@@ -143,6 +144,12 @@ void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend);
  */
 CharDriverState *qemu_chr_new(const char *label, const char *filename,
                               void (*init)(struct CharDriverState *s));
+/**
+ * @qemu_chr_disconnect:
+ *
+ * Close a fd accpeted by character backend.
+ */
+void qemu_chr_disconnect(CharDriverState *chr);
 
 /**
  * @qemu_chr_new_noreplay:
diff --git a/qemu-char.c b/qemu-char.c
index b13ecbb..e312777 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -4012,6 +4012,13 @@ void qemu_chr_fe_release(CharDriverState *s)
     s->avail_connections++;
 }
 
+void qemu_chr_disconnect(CharDriverState *chr)
+{
+    if (chr->chr_disconnect) {
+        chr->chr_disconnect(chr);
+    }
+}
+
 static void qemu_chr_free_common(CharDriverState *chr)
 {
     g_free(chr->filename);
@@ -4389,6 +4396,7 @@ static CharDriverState *qmp_chardev_open_socket(const char *id,
     chr->chr_write = tcp_chr_write;
     chr->chr_sync_read = tcp_chr_sync_read;
     chr->chr_close = tcp_chr_close;
+    chr->chr_disconnect = tcp_chr_disconnect;
     chr->get_msgfds = tcp_get_msgfds;
     chr->set_msgfds = tcp_set_msgfds;
     chr->chr_add_client = tcp_chr_add_client;
-- 
MST

[Qemu-devel] [PULL v2 06/32] vhost-user: disconnect on start failure

[Michael S. Tsirkin]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

If the backend failed to start (for example feature negociation failed),
do not exit, but disconnect the char device instead. Slightly more
robust for reconnect case.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Victor Kaplansky <victork@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 net/vhost-user.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 4a7fd5f..41ddb4b 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -202,7 +202,8 @@ static void net_vhost_user_event(void *opaque, int event)
         s->watch = qemu_chr_fe_add_watch(s->chr, G_IO_HUP,
                                          net_vhost_user_watch, s);
         if (vhost_user_start(queues, ncs) < 0) {
-            exit(1);
+            qemu_chr_disconnect(s->chr);
+            return;
         }
         qmp_set_link(name, true, &err);
         break;
-- 
MST

[Qemu-devel] [PULL v2 07/32] vhost-net: do not crash if backend is not present

[Michael S. Tsirkin]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Do not crash when backend is not present while enabling the ring. A
following patch will save the enabled state so it can be restored once
the backend is started.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Victor Kaplansky <victork@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/vhost_net.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 6e1032f..805a0df 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -401,8 +401,13 @@ VHostNetState *get_vhost_net(NetClientState *nc)
 int vhost_set_vring_enable(NetClientState *nc, int enable)
 {
     VHostNetState *net = get_vhost_net(nc);
-    const VhostOps *vhost_ops = net->dev.vhost_ops;
+    const VhostOps *vhost_ops;
+
+    if (!net) {
+        return 0;
+    }
 
+    vhost_ops = net->dev.vhost_ops;
     if (vhost_ops->vhost_set_vring_enable) {
         return vhost_ops->vhost_set_vring_enable(&net->dev, enable);
     }
-- 
MST

[Qemu-devel] [PULL v2 08/32] vhost-net: save & restore vhost-user acked features

[Michael S. Tsirkin]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The initial vhost-user connection sets the features to be negotiated
with the driver. Renegotiation isn't possible without device reset.

To handle reconnection of vhost-user backend, ensure the same set of
features are provided, and reuse already acked features.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Victor Kaplansky <victork@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/net/vhost-user.h |  1 +
 include/net/vhost_net.h  |  3 +++
 hw/net/vhost_net.c       | 27 ++++++++++++++++++++++++++-
 net/vhost-user.c         | 10 ++++++++++
 4 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/include/net/vhost-user.h b/include/net/vhost-user.h
index 85109f6..efae35d 100644
--- a/include/net/vhost-user.h
+++ b/include/net/vhost-user.h
@@ -13,5 +13,6 @@
 
 struct vhost_net;
 struct vhost_net *vhost_user_get_vhost_net(NetClientState *nc);
+uint64_t vhost_user_get_acked_features(NetClientState *nc);
 
 #endif /* VHOST_USER_H_ */
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 3389b41..0bd4877 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -31,4 +31,7 @@ int vhost_net_notify_migration_done(VHostNetState *net, char* mac_addr);
 VHostNetState *get_vhost_net(NetClientState *nc);
 
 int vhost_set_vring_enable(NetClientState * nc, int enable);
+
+uint64_t vhost_net_get_acked_features(VHostNetState *net);
+
 #endif
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 805a0df..b28881f 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -120,6 +120,11 @@ uint64_t vhost_net_get_max_queues(VHostNetState *net)
     return net->dev.max_queues;
 }
 
+uint64_t vhost_net_get_acked_features(VHostNetState *net)
+{
+    return net->dev.acked_features;
+}
+
 static int vhost_net_get_fd(NetClientState *backend)
 {
     switch (backend->info->type) {
@@ -136,6 +141,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
     int r;
     bool backend_kernel = options->backend_type == VHOST_BACKEND_TYPE_KERNEL;
     struct vhost_net *net = g_malloc(sizeof *net);
+    uint64_t features = 0;
 
     if (!options->net_backend) {
         fprintf(stderr, "vhost-net requires net backend to be setup\n");
@@ -183,8 +189,21 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options)
             goto fail;
         }
     }
+
     /* Set sane init value. Override when guest acks. */
-    vhost_net_ack_features(net, 0);
+    if (net->nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER) {
+        features = vhost_user_get_acked_features(net->nc);
+        if (~net->dev.features & features) {
+            fprintf(stderr, "vhost lacks feature mask %" PRIu64
+                    " for backend\n",
+                    (uint64_t)(~net->dev.features & features));
+            vhost_dev_cleanup(&net->dev);
+            goto fail;
+        }
+    }
+
+    vhost_net_ack_features(net, features);
+
     return net;
 fail:
     g_free(net);
@@ -447,10 +466,16 @@ uint64_t vhost_net_get_features(struct vhost_net *net, uint64_t features)
 {
     return features;
 }
+
 void vhost_net_ack_features(struct vhost_net *net, uint64_t features)
 {
 }
 
+uint64_t vhost_net_get_acked_features(VHostNetState *net)
+{
+    return 0;
+}
+
 bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
 {
     return false;
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 41ddb4b..d72ce9b 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -23,6 +23,7 @@ typedef struct VhostUserState {
     CharDriverState *chr;
     VHostNetState *vhost_net;
     int watch;
+    uint64_t acked_features;
 } VhostUserState;
 
 typedef struct VhostUserChardevProps {
@@ -37,6 +38,13 @@ VHostNetState *vhost_user_get_vhost_net(NetClientState *nc)
     return s->vhost_net;
 }
 
+uint64_t vhost_user_get_acked_features(NetClientState *nc)
+{
+    VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
+    assert(nc->info->type == NET_CLIENT_OPTIONS_KIND_VHOST_USER);
+    return s->acked_features;
+}
+
 static int vhost_user_running(VhostUserState *s)
 {
     return (s->vhost_net) ? 1 : 0;
@@ -56,6 +64,8 @@ static void vhost_user_stop(int queues, NetClientState *ncs[])
         }
 
         if (s->vhost_net) {
+            /* save acked features */
+            s->acked_features = vhost_net_get_acked_features(s->vhost_net);
             vhost_net_cleanup(s->vhost_net);
             s->vhost_net = NULL;
         }
-- 
MST

[Qemu-devel] [PULL v2 09/32] vhost-net: save & restore vring enable state

[Michael S. Tsirkin]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

A driver may change the vring enable state at run time but vhost-user
backend may not be present (a contrived example is when the backend is
disconnected and the device is reconfigured after driver rebinding)

Restore the vring state when the vhost-user backend is started, so it
can process the ring.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Reviewed-by: Victor Kaplansky <victork@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/net/net.h  |  1 +
 hw/net/vhost_net.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/include/net/net.h b/include/net/net.h
index a69e382..a5c5095 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -99,6 +99,7 @@ struct NetClientState {
     NetClientDestructor *destructor;
     unsigned int queue_index;
     unsigned rxfilter_notify_enabled:1;
+    int vring_enable;
     QTAILQ_HEAD(NetFilterHead, NetFilterState) filters;
 };
 
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index b28881f..50f4dcd 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -329,6 +329,15 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
         if (r < 0) {
             goto err_start;
         }
+
+        if (ncs[i].peer->vring_enable) {
+            /* restore vring enable state */
+            r = vhost_set_vring_enable(ncs[i].peer, ncs[i].peer->vring_enable);
+
+            if (r < 0) {
+                goto err_start;
+            }
+        }
     }
 
     return 0;
@@ -422,6 +431,8 @@ int vhost_set_vring_enable(NetClientState *nc, int enable)
     VHostNetState *net = get_vhost_net(nc);
     const VhostOps *vhost_ops;
 
+    nc->vring_enable = enable;
+
     if (!net) {
         return 0;
     }
-- 
MST

[Qemu-devel] [PULL v2 10/32] tests: append i386 tests

[Michael S. Tsirkin]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Do not overwrite x86-64 tests, re-enable vhost-user-test.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Victor Kaplansky <victork@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index a3e20e3..932ad2a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -228,7 +228,7 @@ endif
 check-qtest-i386-y += tests/test-netfilter$(EXESUF)
 check-qtest-i386-y += tests/test-filter-mirror$(EXESUF)
 check-qtest-i386-y += tests/test-filter-redirector$(EXESUF)
-check-qtest-x86_64-y = $(check-qtest-i386-y)
+check-qtest-x86_64-y += $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
 gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
-- 
MST

[Qemu-devel] [PULL v2 11/32] test: start vhost-user reconnect test

[Michael S. Tsirkin]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

This is a simple reconnect test, that simply checks if vhost-user
reconnection is possible and restore the state. A more complete test
would actually manipulate and check the ring contents (such extended
testing would benefit from the libvhost-user proposed in QEMU list to
avoid duplication of ring manipulations)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Victor Kaplansky <victork@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/vhost-user-test.c | 136 ++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 119 insertions(+), 17 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 6961596..6dfd2ae 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -34,7 +34,7 @@
 #define QEMU_CMD_ACCEL  " -machine accel=tcg"
 #define QEMU_CMD_MEM    " -m %d -object memory-backend-file,id=mem,size=%dM,"\
                         "mem-path=%s,share=on -numa node,memdev=mem"
-#define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s"
+#define QEMU_CMD_CHR    " -chardev socket,id=%s,path=%s%s"
 #define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce"
 #define QEMU_CMD_NET    " -device virtio-net-pci,netdev=net0,romfile=./pc-bios/pxe-virtio.rom"
 
@@ -246,7 +246,12 @@ static void chr_read(void *opaque, const uint8_t *buf, int size)
 
     if (msg.size) {
         p += VHOST_USER_HDR_SIZE;
-        g_assert_cmpint(qemu_chr_fe_read_all(chr, p, msg.size), ==, msg.size);
+        size = qemu_chr_fe_read_all(chr, p, msg.size);
+        if (size != msg.size) {
+            g_test_message("Wrong message size received %d != %d\n",
+                           size, msg.size);
+            return;
+        }
     }
 
     switch (msg.request) {
@@ -363,17 +368,10 @@ static const char *init_hugepagefs(const char *path)
 static TestServer *test_server_new(const gchar *name)
 {
     TestServer *server = g_new0(TestServer, 1);
-    gchar *chr_path;
 
     server->socket_path = g_strdup_printf("%s/%s.sock", tmpfs, name);
     server->mig_path = g_strdup_printf("%s/%s.mig", tmpfs, name);
-
-    chr_path = g_strdup_printf("unix:%s,server,nowait", server->socket_path);
     server->chr_name = g_strdup_printf("chr-%s", name);
-    server->chr = qemu_chr_new(server->chr_name, chr_path, NULL);
-    g_free(chr_path);
-
-    qemu_chr_add_handlers(server->chr, chr_can_read, chr_read, NULL, server);
 
     g_mutex_init(&server->data_mutex);
     g_cond_init(&server->data_cond);
@@ -383,13 +381,34 @@ static TestServer *test_server_new(const gchar *name)
     return server;
 }
 
-#define GET_QEMU_CMD(s)                                                        \
-    g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,                 \
-                    (s)->socket_path, (s)->chr_name)
+static void test_server_create_chr(TestServer *server, const gchar *opt)
+{
+    gchar *chr_path;
 
-#define GET_QEMU_CMDE(s, mem, extra, ...)                                      \
-    g_strdup_printf(QEMU_CMD extra, (mem), (mem), (root), (s)->chr_name,       \
-                    (s)->socket_path, (s)->chr_name, ##__VA_ARGS__)
+    chr_path = g_strdup_printf("unix:%s%s", server->socket_path, opt);
+    server->chr = qemu_chr_new(server->chr_name, chr_path, NULL);
+    g_free(chr_path);
+
+    qemu_chr_add_handlers(server->chr, chr_can_read, chr_read, NULL, server);
+}
+
+static void test_server_listen(TestServer *server)
+{
+    test_server_create_chr(server, ",server,nowait");
+}
+
+static inline void test_server_connect(TestServer *server)
+{
+    test_server_create_chr(server, ",reconnect=1");
+}
+
+#define GET_QEMU_CMD(s)                                         \
+    g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,  \
+                    (s)->socket_path, "", (s)->chr_name)
+
+#define GET_QEMU_CMDE(s, mem, chr_opts, extra, ...)                     \
+    g_strdup_printf(QEMU_CMD extra, (mem), (mem), (root), (s)->chr_name, \
+                    (s)->socket_path, (chr_opts), (s)->chr_name, ##__VA_ARGS__)
 
 static gboolean _test_server_free(TestServer *server)
 {
@@ -537,7 +556,10 @@ static void test_migrate(void)
     guint8 *log;
     guint64 size;
 
-    cmd = GET_QEMU_CMDE(s, 2, "");
+    test_server_listen(s);
+    test_server_listen(dest);
+
+    cmd = GET_QEMU_CMDE(s, 2, "", "");
     from = qtest_start(cmd);
     g_free(cmd);
 
@@ -545,7 +567,7 @@ static void test_migrate(void)
     size = get_log_size(s);
     g_assert_cmpint(size, ==, (2 * 1024 * 1024) / (VHOST_LOG_PAGE * 8));
 
-    cmd = GET_QEMU_CMDE(dest, 2, " -incoming %s", uri);
+    cmd = GET_QEMU_CMDE(dest, 2, "", " -incoming %s", uri);
     to = qtest_init(cmd);
     g_free(cmd);
 
@@ -605,6 +627,80 @@ static void test_migrate(void)
     global_qtest = global;
 }
 
+#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
+static void wait_for_rings_started(TestServer *s, size_t count)
+{
+    gint64 end_time;
+
+    g_mutex_lock(&s->data_mutex);
+    end_time = g_get_monotonic_time() + 5 * G_TIME_SPAN_SECOND;
+    while (ctpop64(s->rings) != count) {
+        if (!g_cond_wait_until(&s->data_cond, &s->data_mutex, end_time)) {
+            /* timeout has passed */
+            g_assert_cmpint(ctpop64(s->rings), ==, count);
+            break;
+        }
+    }
+
+    g_mutex_unlock(&s->data_mutex);
+}
+
+static gboolean
+reconnect_cb(gpointer user_data)
+{
+    TestServer *s = user_data;
+
+    qemu_chr_disconnect(s->chr);
+
+    return FALSE;
+}
+
+static gpointer
+connect_thread(gpointer data)
+{
+    TestServer *s = data;
+
+    /* wait for qemu to start before first try, to avoid extra warnings */
+    g_usleep(G_USEC_PER_SEC);
+    test_server_connect(s);
+
+    return NULL;
+}
+
+static void test_reconnect_subprocess(void)
+{
+    TestServer *s = test_server_new("reconnect");
+    char *cmd;
+
+    g_thread_new("connect", connect_thread, s);
+    cmd = GET_QEMU_CMDE(s, 2, ",server", "");
+    qtest_start(cmd);
+    g_free(cmd);
+
+    wait_for_fds(s);
+    wait_for_rings_started(s, 2);
+
+    /* reconnect */
+    s->fds_num = 0;
+    s->rings = 0;
+    g_idle_add(reconnect_cb, s);
+    wait_for_fds(s);
+    wait_for_rings_started(s, 2);
+
+    qtest_end();
+    test_server_free(s);
+    return;
+}
+
+static void test_reconnect(void)
+{
+    gchar *path = g_strdup_printf("/%s/vhost-user/reconnect/subprocess",
+                                  qtest_get_arch());
+    g_test_trap_subprocess(path, 0, 0);
+    g_test_trap_assert_passed();
+}
+#endif
+
 int main(int argc, char **argv)
 {
     QTestState *s = NULL;
@@ -636,6 +732,7 @@ int main(int argc, char **argv)
     }
 
     server = test_server_new("test");
+    test_server_listen(server);
 
     loop = g_main_loop_new(NULL, FALSE);
     /* run the main loop thread so the chardev may operate */
@@ -648,6 +745,11 @@ int main(int argc, char **argv)
 
     qtest_add_data_func("/vhost-user/read-guest-mem", server, read_guest_mem);
     qtest_add_func("/vhost-user/migrate", test_migrate);
+#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
+    qtest_add_func("/vhost-user/reconnect/subprocess",
+                   test_reconnect_subprocess);
+    qtest_add_func("/vhost-user/reconnect", test_reconnect);
+#endif
 
     ret = g_test_run();
 
-- 
MST

[Qemu-devel] [PULL v2 12/32] smbios: Move table build tools into an include file.

[Michael S. Tsirkin]

From: Corey Minyard <cminyard@mvista.com>

This will let things in other files (like IPMI) build SMBIOS tables.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/smbios/smbios_build.h | 87 ++++++++++++++++++++++++++++++++++++++++++++++++
 hw/smbios/smbios.c       | 70 ++++----------------------------------
 2 files changed, 93 insertions(+), 64 deletions(-)
 create mode 100644 hw/smbios/smbios_build.h

diff --git a/hw/smbios/smbios_build.h b/hw/smbios/smbios_build.h
new file mode 100644
index 0000000..68b8b72
--- /dev/null
+++ b/hw/smbios/smbios_build.h
@@ -0,0 +1,87 @@
+/*
+ * SMBIOS Support
+ *
+ * Copyright (C) 2009 Hewlett-Packard Development Company, L.P.
+ * Copyright (C) 2013 Red Hat, Inc.
+ *
+ * Authors:
+ *  Alex Williamson <alex.williamson@hp.com>
+ *  Markus Armbruster <armbru@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#ifndef QEMU_SMBIOS_BUILD_H
+#define QEMU_SMBIOS_BUILD_H
+
+bool smbios_skip_table(uint8_t type, bool required_table);
+
+extern uint8_t *smbios_tables;
+extern size_t smbios_tables_len;
+extern unsigned smbios_table_max;
+extern unsigned smbios_table_cnt;
+
+#define SMBIOS_BUILD_TABLE_PRE(tbl_type, tbl_handle, tbl_required)        \
+    struct smbios_type_##tbl_type *t;                                     \
+    size_t t_off; /* table offset into smbios_tables */                   \
+    int str_index = 0;                                                    \
+    do {                                                                  \
+        /* should we skip building this table ? */                        \
+        if (smbios_skip_table(tbl_type, tbl_required)) {                  \
+            return;                                                       \
+        }                                                                 \
+                                                                          \
+        /* use offset of table t within smbios_tables */                  \
+        /* (pointer must be updated after each realloc) */                \
+        t_off = smbios_tables_len;                                        \
+        smbios_tables_len += sizeof(*t);                                  \
+        smbios_tables = g_realloc(smbios_tables, smbios_tables_len);      \
+        t = (struct smbios_type_##tbl_type *)(smbios_tables + t_off);     \
+                                                                          \
+        t->header.type = tbl_type;                                        \
+        t->header.length = sizeof(*t);                                    \
+        t->header.handle = cpu_to_le16(tbl_handle);                       \
+    } while (0)
+
+#define SMBIOS_TABLE_SET_STR(tbl_type, field, value)                      \
+    do {                                                                  \
+        int len = (value != NULL) ? strlen(value) + 1 : 0;                \
+        if (len > 1) {                                                    \
+            smbios_tables = g_realloc(smbios_tables,                      \
+                                      smbios_tables_len + len);           \
+            memcpy(smbios_tables + smbios_tables_len, value, len);        \
+            smbios_tables_len += len;                                     \
+            /* update pointer post-realloc */                             \
+            t = (struct smbios_type_##tbl_type *)(smbios_tables + t_off); \
+            t->field = ++str_index;                                       \
+        } else {                                                          \
+            t->field = 0;                                                 \
+        }                                                                 \
+    } while (0)
+
+#define SMBIOS_BUILD_TABLE_POST                                           \
+    do {                                                                  \
+        size_t term_cnt, t_size;                                          \
+                                                                          \
+        /* add '\0' terminator (add two if no strings defined) */         \
+        term_cnt = (str_index == 0) ? 2 : 1;                              \
+        smbios_tables = g_realloc(smbios_tables,                          \
+                                  smbios_tables_len + term_cnt);          \
+        memset(smbios_tables + smbios_tables_len, 0, term_cnt);           \
+        smbios_tables_len += term_cnt;                                    \
+                                                                          \
+        /* update smbios max. element size */                             \
+        t_size = smbios_tables_len - t_off;                               \
+        if (t_size > smbios_table_max) {                                  \
+            smbios_table_max = t_size;                                    \
+        }                                                                 \
+                                                                          \
+        /* update smbios element count */                                 \
+        smbios_table_cnt++;                                               \
+    } while (0)
+
+#endif /* QEMU_SMBIOS_BUILD_H */
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index cb8a111..5dc3e43 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -24,6 +24,7 @@
 #include "hw/smbios/smbios.h"
 #include "hw/loader.h"
 #include "exec/cpu-common.h"
+#include "smbios_build.h"
 
 /* legacy structures and constants for <= 2.0 machines */
 struct smbios_header {
@@ -53,10 +54,10 @@ static bool smbios_uuid_encoded = true;
 /* end: legacy structures & constants for <= 2.0 machines */
 
 
-static uint8_t *smbios_tables;
-static size_t smbios_tables_len;
-static unsigned smbios_table_max;
-static unsigned smbios_table_cnt;
+uint8_t *smbios_tables;
+size_t smbios_tables_len;
+unsigned smbios_table_max;
+unsigned smbios_table_cnt;
 static SmbiosEntryPointType smbios_ep_type = SMBIOS_ENTRY_POINT_21;
 
 static SmbiosEntryPoint ep;
@@ -429,7 +430,7 @@ uint8_t *smbios_get_table_legacy(size_t *length)
 /* end: legacy setup functions for <= 2.0 machines */
 
 
-static bool smbios_skip_table(uint8_t type, bool required_table)
+bool smbios_skip_table(uint8_t type, bool required_table)
 {
     if (test_bit(type, have_binfile_bitmap)) {
         return true; /* user provided their own binary blob(s) */
@@ -443,65 +444,6 @@ static bool smbios_skip_table(uint8_t type, bool required_table)
     return true;
 }
 
-#define SMBIOS_BUILD_TABLE_PRE(tbl_type, tbl_handle, tbl_required)        \
-    struct smbios_type_##tbl_type *t;                                     \
-    size_t t_off; /* table offset into smbios_tables */                   \
-    int str_index = 0;                                                    \
-    do {                                                                  \
-        /* should we skip building this table ? */                        \
-        if (smbios_skip_table(tbl_type, tbl_required)) {                  \
-            return;                                                       \
-        }                                                                 \
-                                                                          \
-        /* use offset of table t within smbios_tables */                  \
-        /* (pointer must be updated after each realloc) */                \
-        t_off = smbios_tables_len;                                        \
-        smbios_tables_len += sizeof(*t);                                  \
-        smbios_tables = g_realloc(smbios_tables, smbios_tables_len);      \
-        t = (struct smbios_type_##tbl_type *)(smbios_tables + t_off);     \
-                                                                          \
-        t->header.type = tbl_type;                                        \
-        t->header.length = sizeof(*t);                                    \
-        t->header.handle = cpu_to_le16(tbl_handle);                       \
-    } while (0)
-
-#define SMBIOS_TABLE_SET_STR(tbl_type, field, value)                      \
-    do {                                                                  \
-        int len = (value != NULL) ? strlen(value) + 1 : 0;                \
-        if (len > 1) {                                                    \
-            smbios_tables = g_realloc(smbios_tables,                      \
-                                      smbios_tables_len + len);           \
-            memcpy(smbios_tables + smbios_tables_len, value, len);        \
-            smbios_tables_len += len;                                     \
-            /* update pointer post-realloc */                             \
-            t = (struct smbios_type_##tbl_type *)(smbios_tables + t_off); \
-            t->field = ++str_index;                                       \
-        } else {                                                          \
-            t->field = 0;                                                 \
-        }                                                                 \
-    } while (0)
-
-#define SMBIOS_BUILD_TABLE_POST                                           \
-    do {                                                                  \
-        size_t term_cnt, t_size;                                          \
-                                                                          \
-        /* add '\0' terminator (add two if no strings defined) */         \
-        term_cnt = (str_index == 0) ? 2 : 1;                              \
-        smbios_tables = g_realloc(smbios_tables,                          \
-                                  smbios_tables_len + term_cnt);          \
-        memset(smbios_tables + smbios_tables_len, 0, term_cnt);           \
-        smbios_tables_len += term_cnt;                                    \
-                                                                          \
-        /* update smbios max. element size */                             \
-        t_size = smbios_tables_len - t_off;                               \
-        if (t_size > smbios_table_max) {                                  \
-            smbios_table_max = t_size;                                    \
-        }                                                                 \
-                                                                          \
-        /* update smbios element count */                                 \
-        smbios_table_cnt++;                                               \
-    } while (0)
-
 static void smbios_build_type_0_table(void)
 {
     SMBIOS_BUILD_TABLE_PRE(0, 0x000, false); /* optional, leave up to BIOS */
-- 
MST

[Qemu-devel] [PULL v2 13/32] ipmi: Add SMBIOS table entry

[Michael S. Tsirkin]

From: Corey Minyard <cminyard@mvista.com>

Add an IPMI table entry to the SMBIOS.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/smbios/ipmi.h      |  15 ++++++
 hw/smbios/no_smbios_type_38.c |  14 +++++
 hw/smbios/smbios.c            |   2 +
 hw/smbios/smbios_type_38.c    | 117 ++++++++++++++++++++++++++++++++++++++++++
 hw/smbios/Makefile.objs       |   2 +
 5 files changed, 150 insertions(+)
 create mode 100644 include/hw/smbios/ipmi.h
 create mode 100644 hw/smbios/no_smbios_type_38.c
 create mode 100644 hw/smbios/smbios_type_38.c

diff --git a/include/hw/smbios/ipmi.h b/include/hw/smbios/ipmi.h
new file mode 100644
index 0000000..1c9aae3
--- /dev/null
+++ b/include/hw/smbios/ipmi.h
@@ -0,0 +1,15 @@
+/*
+ * IPMI SMBIOS firmware handling
+ *
+ * Copyright (c) 2015,2016 Corey Minyard, MontaVista Software, LLC
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_SMBIOS_IPMI_H
+#define QEMU_SMBIOS_IPMI_H
+
+void smbios_build_type_38_table(void);
+
+#endif /* QEMU_SMBIOS_IPMI_H */
diff --git a/hw/smbios/no_smbios_type_38.c b/hw/smbios/no_smbios_type_38.c
new file mode 100644
index 0000000..9528c2c
--- /dev/null
+++ b/hw/smbios/no_smbios_type_38.c
@@ -0,0 +1,14 @@
+/*
+ * IPMI SMBIOS firmware handling
+ *
+ * Copyright (c) 2015,2016 Corey Minyard, MontaVista Software, LLC
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "hw/smbios/ipmi.h"
+
+void smbios_build_type_38_table(void)
+{
+}
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 5dc3e43..74c7102 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -25,6 +25,7 @@
 #include "hw/loader.h"
 #include "exec/cpu-common.h"
 #include "smbios_build.h"
+#include "hw/smbios/ipmi.h"
 
 /* legacy structures and constants for <= 2.0 machines */
 struct smbios_header {
@@ -848,6 +849,7 @@ void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
         }
 
         smbios_build_type_32_table();
+        smbios_build_type_38_table();
         smbios_build_type_127_table();
 
         smbios_validate_table();
diff --git a/hw/smbios/smbios_type_38.c b/hw/smbios/smbios_type_38.c
new file mode 100644
index 0000000..56e8609
--- /dev/null
+++ b/hw/smbios/smbios_type_38.c
@@ -0,0 +1,117 @@
+/*
+ * IPMI SMBIOS firmware handling
+ *
+ * Copyright (c) 2015,2016 Corey Minyard, MontaVista Software, LLC
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/ipmi/ipmi.h"
+#include "hw/smbios/ipmi.h"
+#include "hw/smbios/smbios.h"
+#include "qemu/error-report.h"
+#include "smbios_build.h"
+
+/* SMBIOS type 38 - IPMI */
+struct smbios_type_38 {
+    struct smbios_structure_header header;
+    uint8_t interface_type;
+    uint8_t ipmi_spec_revision;
+    uint8_t i2c_slave_address;
+    uint8_t nv_storage_device_address;
+    uint64_t base_address;
+    uint8_t base_address_modifier;
+    uint8_t interrupt_number;
+} QEMU_PACKED;
+
+static void smbios_build_one_type_38(IPMIFwInfo *info)
+{
+    uint64_t baseaddr = info->base_address;
+    SMBIOS_BUILD_TABLE_PRE(38, 0x3000, true);
+
+    t->interface_type = info->interface_type;
+    t->ipmi_spec_revision = ((info->ipmi_spec_major_revision << 4)
+                             | info->ipmi_spec_minor_revision);
+    t->i2c_slave_address = info->i2c_slave_address;
+    t->nv_storage_device_address = 0;
+
+    assert(info->ipmi_spec_minor_revision <= 15);
+    assert(info->ipmi_spec_major_revision <= 15);
+
+    /* or 1 to set it to I/O space */
+    switch (info->memspace) {
+    case IPMI_MEMSPACE_IO:
+        baseaddr |= 1;
+        break;
+    case IPMI_MEMSPACE_MEM32:
+    case IPMI_MEMSPACE_MEM64:
+        break;
+    case IPMI_MEMSPACE_SMBUS:
+        baseaddr <<= 1;
+        break;
+    }
+
+    t->base_address = cpu_to_le64(baseaddr);
+
+    t->base_address_modifier = 0;
+    if (info->irq_type == IPMI_LEVEL_IRQ) {
+        t->base_address_modifier |= 1;
+    }
+    switch (info->register_spacing) {
+    case 1:
+        break;
+    case 4:
+        t->base_address_modifier |= 1 << 6;
+        break;
+    case 16:
+        t->base_address_modifier |= 2 << 6;
+        break;
+    default:
+        error_report("IPMI register spacing %d is not compatible with"
+                     " SMBIOS, ignoring this entry.", info->register_spacing);
+        return;
+    }
+    t->interrupt_number = info->interrupt_number;
+
+    SMBIOS_BUILD_TABLE_POST;
+}
+
+static void smbios_add_ipmi_devices(BusState *bus)
+{
+    BusChild *kid;
+
+    QTAILQ_FOREACH(kid, &bus->children,  sibling) {
+        DeviceState *dev = kid->child;
+        Object *obj = object_dynamic_cast(OBJECT(dev), TYPE_IPMI_INTERFACE);
+        BusState *childbus;
+
+        if (obj) {
+            IPMIInterface *ii;
+            IPMIInterfaceClass *iic;
+            IPMIFwInfo info;
+
+            ii = IPMI_INTERFACE(obj);
+            iic = IPMI_INTERFACE_GET_CLASS(obj);
+            memset(&info, 0, sizeof(info));
+            iic->get_fwinfo(ii, &info);
+            smbios_build_one_type_38(&info);
+            continue;
+        }
+
+        QLIST_FOREACH(childbus, &dev->child_bus, sibling) {
+            smbios_add_ipmi_devices(childbus);
+        }
+    }
+}
+
+void smbios_build_type_38_table(void)
+{
+    BusState *bus;
+
+    bus = sysbus_get_default();
+    if (bus) {
+        smbios_add_ipmi_devices(bus);
+    }
+}
diff --git a/hw/smbios/Makefile.objs b/hw/smbios/Makefile.objs
index f69a92f..7dcb8f6 100644
--- a/hw/smbios/Makefile.objs
+++ b/hw/smbios/Makefile.objs
@@ -1 +1,3 @@
 common-obj-$(CONFIG_SMBIOS) += smbios.o
+common-obj-$(call land,$(CONFIG_SMBIOS),$(CONFIG_IPMI)) += smbios_type_38.o
+common-obj-$(call land,$(CONFIG_SMBIOS),$(call lnot,$(CONFIG_IPMI))) += no_smbios_type_38.o
-- 
MST

[Qemu-devel] [PULL v2 14/32] acpi: Add IPMI table entries

[Michael S. Tsirkin]

From: Corey Minyard <cminyard@mvista.com>

Use the ACPI table construction tools to create an ACPI entry
for IPMI.  This adds a function called build_acpi_ipmi_devices
to add an DSDT entry for IPMI if IPMI is compiled in and an
IPMI device exists.  It also adds a dummy function if IPMI
is not compiled in.

This conforms to section "C3-2 Locating IPMI System Interfaces in
ACPI Name Space" in the IPMI 2.0 specification.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/ipmi.h |  22 +++++++++++
 hw/acpi/ipmi.c         | 105 +++++++++++++++++++++++++++++++++++++++++++++++++
 hw/acpi/noipmi.c       |  14 +++++++
 hw/i386/acpi-build.c   |  12 ++++++
 hw/acpi/Makefile.objs  |   2 +
 5 files changed, 155 insertions(+)
 create mode 100644 include/hw/acpi/ipmi.h
 create mode 100644 hw/acpi/ipmi.c
 create mode 100644 hw/acpi/noipmi.c

diff --git a/include/hw/acpi/ipmi.h b/include/hw/acpi/ipmi.h
new file mode 100644
index 0000000..ab2bb29
--- /dev/null
+++ b/include/hw/acpi/ipmi.h
@@ -0,0 +1,22 @@
+/*
+ * QEMU IPMI ACPI handling
+ *
+ * Copyright (c) 2015,2016 Corey Minyard <cminyard@mvista.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef HW_ACPI_IPMI_H
+#define HW_ACPI_IPMI_H
+
+#include "qemu/osdep.h"
+#include "hw/acpi/aml-build.h"
+
+/*
+ * Add ACPI IPMI entries for all registered IPMI devices whose parent
+ * bus matches the given bus.  The resource is the ACPI resource that
+ * contains the IPMI device, this is required for the I2C CRS.
+ */
+void build_acpi_ipmi_devices(Aml *table, BusState *bus);
+
+#endif /* HW_ACPI_IPMI_H */
diff --git a/hw/acpi/ipmi.c b/hw/acpi/ipmi.c
new file mode 100644
index 0000000..7e74ce4
--- /dev/null
+++ b/hw/acpi/ipmi.c
@@ -0,0 +1,105 @@
+/*
+ * IPMI ACPI firmware handling
+ *
+ * Copyright (c) 2015,2016 Corey Minyard, MontaVista Software, LLC
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/ipmi/ipmi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/ipmi.h"
+
+static Aml *aml_ipmi_crs(IPMIFwInfo *info)
+{
+    Aml *crs = aml_resource_template();
+
+    /*
+     * The base address is fixed and cannot change.  That may be different
+     * if someone does PCI, but we aren't there yet.
+     */
+    switch (info->memspace) {
+    case IPMI_MEMSPACE_IO:
+        aml_append(crs, aml_io(AML_DECODE16, info->base_address,
+                               info->base_address + info->register_length - 1,
+                               info->register_spacing, info->register_length));
+        break;
+    case IPMI_MEMSPACE_MEM32:
+        aml_append(crs,
+                   aml_dword_memory(AML_POS_DECODE,
+                            AML_MIN_FIXED, AML_MAX_FIXED,
+                            AML_NON_CACHEABLE, AML_READ_WRITE,
+                            0xffffffff,
+                            info->base_address,
+                            info->base_address + info->register_length - 1,
+                            info->register_spacing, info->register_length));
+        break;
+    case IPMI_MEMSPACE_MEM64:
+        aml_append(crs,
+                   aml_qword_memory(AML_POS_DECODE,
+                            AML_MIN_FIXED, AML_MAX_FIXED,
+                            AML_NON_CACHEABLE, AML_READ_WRITE,
+                            0xffffffffffffffffULL,
+                            info->base_address,
+                            info->base_address + info->register_length - 1,
+                            info->register_spacing, info->register_length));
+        break;
+    case IPMI_MEMSPACE_SMBUS:
+        aml_append(crs, aml_return(aml_int(info->base_address)));
+        break;
+    default:
+        abort();
+    }
+
+    if (info->interrupt_number) {
+        aml_append(crs, aml_irq_no_flags(info->interrupt_number));
+    }
+
+    return crs;
+}
+
+static Aml *aml_ipmi_device(IPMIFwInfo *info)
+{
+    Aml *dev;
+    uint16_t version = ((info->ipmi_spec_major_revision << 8)
+                        | (info->ipmi_spec_minor_revision << 4));
+
+    assert(info->ipmi_spec_minor_revision <= 15);
+
+    dev = aml_device("MI%d", info->uuid);
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("IPI0001")));
+    aml_append(dev, aml_name_decl("_STR", aml_string("ipmi_%s",
+                                                     info->interface_name)));
+    aml_append(dev, aml_name_decl("_UID", aml_int(info->uuid)));
+    aml_append(dev, aml_name_decl("_CRS", aml_ipmi_crs(info)));
+    aml_append(dev, aml_name_decl("_IFT", aml_int(info->interface_type)));
+    aml_append(dev, aml_name_decl("_SRV", aml_int(version)));
+
+    return dev;
+}
+
+void build_acpi_ipmi_devices(Aml *scope, BusState *bus)
+{
+
+    BusChild *kid;
+
+    QTAILQ_FOREACH(kid, &bus->children,  sibling) {
+        IPMIInterface *ii;
+        IPMIInterfaceClass *iic;
+        IPMIFwInfo info;
+        Object *obj = object_dynamic_cast(OBJECT(kid->child),
+                                          TYPE_IPMI_INTERFACE);
+
+        if (!obj) {
+            continue;
+        }
+
+        ii = IPMI_INTERFACE(obj);
+        iic = IPMI_INTERFACE_GET_CLASS(obj);
+        iic->get_fwinfo(ii, &info);
+        aml_append(scope, aml_ipmi_device(&info));
+    }
+}
diff --git a/hw/acpi/noipmi.c b/hw/acpi/noipmi.c
new file mode 100644
index 0000000..98b6dce
--- /dev/null
+++ b/hw/acpi/noipmi.c
@@ -0,0 +1,14 @@
+/*
+ * IPMI ACPI firmware handling
+ *
+ * Copyright (c) 2015,2016 Corey Minyard, MontaVista Software, LLC
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "hw/acpi/ipmi.h"
+
+void build_acpi_ipmi_devices(Aml *table, BusState *bus)
+{
+}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 06d6204..676e40a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -59,6 +59,8 @@
 #include "qapi/qmp/qint.h"
 #include "qom/qom-qobject.h"
 
+#include "hw/acpi/ipmi.h"
+
 /* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
  * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
  * a little bit, there should be plenty of free space since the DSDT
@@ -1335,8 +1337,10 @@ static Aml *build_com_device_aml(uint8_t uid)
 static void build_isa_devices_aml(Aml *table)
 {
     ISADevice *fdc = pc_find_fdc0();
+    bool ambiguous;
 
     Aml *scope = aml_scope("_SB.PCI0.ISA");
+    Object *obj = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
 
     aml_append(scope, build_rtc_device_aml());
     aml_append(scope, build_kbd_device_aml());
@@ -1348,6 +1352,14 @@ static void build_isa_devices_aml(Aml *table)
     aml_append(scope, build_com_device_aml(1));
     aml_append(scope, build_com_device_aml(2));
 
+    if (ambiguous) {
+        error_report("Multiple ISA busses, unable to define IPMI ACPI data");
+    } else if (!obj) {
+        error_report("No ISA bus, unable to define IPMI ACPI data");
+    } else {
+        build_acpi_ipmi_devices(scope, BUS(obj));
+    }
+
     aml_append(table, scope);
 }
 
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 66bd727..3390593 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -6,3 +6,5 @@ obj-$(CONFIG_ACPI_NVDIMM) += nvdimm.o
 common-obj-$(CONFIG_ACPI) += acpi_interface.o
 common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
 common-obj-$(CONFIG_ACPI) += aml-build.o
+common-obj-$(call land,$(CONFIG_ACPI),$(CONFIG_IPMI)) += ipmi.o
+common-obj-$(call land,$(CONFIG_SMBIOS),$(call lnot,$(CONFIG_IPMI))) += noipmi.o
-- 
MST

[Qemu-devel] [PULL v2 15/32] bios: Add tests for the IPMI ACPI and SMBIOS entries

[Michael S. Tsirkin]

From: Corey Minyard <cminyard@mvista.com>

Signed-off-by: Corey Minyard <cminyard@mvista.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 tests/bios-tables-test.c             |  60 ++++++++++++++++++++++++++++++++---
 tests/acpi-test-data/pc/DSDT.ipmikcs | Bin 0 -> 5575 bytes
 tests/acpi-test-data/q35/DSDT.ipmibt | Bin 0 -> 8340 bytes
 3 files changed, 56 insertions(+), 4 deletions(-)
 create mode 100644 tests/acpi-test-data/pc/DSDT.ipmikcs
 create mode 100644 tests/acpi-test-data/q35/DSDT.ipmibt

diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index f0493f8..9fba813 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -50,6 +50,8 @@ typedef struct {
     GArray *tables;
     uint32_t smbios_ep_addr;
     struct smbios_21_entry_point smbios_ep_table;
+    uint8_t *required_struct_types;
+    int required_struct_types_len;
 } test_data;
 
 #define ACPI_READ_FIELD(field, addr)           \
@@ -335,7 +337,7 @@ static void test_acpi_tables(test_data *data)
     for (i = 0; i < tables_nr; i++) {
         AcpiSdtTable ssdt_table;
 
-        memset(&ssdt_table, 0 , sizeof(ssdt_table));
+        memset(&ssdt_table, 0, sizeof(ssdt_table));
         uint32_t addr = data->rsdt_tables_addr[i + 1]; /* fadt is first */
         test_dst_table(&ssdt_table, addr);
         g_array_append_val(data->tables, ssdt_table);
@@ -662,7 +664,6 @@ static void test_smbios_structs(test_data *data)
     uint32_t addr = ep_table->structure_table_address;
     int i, len, max_len = 0;
     uint8_t type, prv, crt;
-    uint8_t required_struct_types[] = {0, 1, 3, 4, 16, 17, 19, 32, 127};
 
     /* walk the smbios tables */
     for (i = 0; i < ep_table->number_of_structures; i++) {
@@ -702,8 +703,8 @@ static void test_smbios_structs(test_data *data)
     g_assert_cmpuint(ep_table->max_structure_size, ==, max_len);
 
     /* required struct types must all be present */
-    for (i = 0; i < ARRAY_SIZE(required_struct_types); i++) {
-        g_assert(test_bit(required_struct_types[i], struct_bitmap));
+    for (i = 0; i < data->required_struct_types_len; i++) {
+        g_assert(test_bit(data->required_struct_types[i], struct_bitmap));
     }
 }
 
@@ -743,6 +744,10 @@ static void test_acpi_one(const char *params, test_data *data)
     g_free(args);
 }
 
+static uint8_t base_required_struct_types[] = {
+    0, 1, 3, 4, 16, 17, 19, 32, 127
+};
+
 static void test_acpi_piix4_tcg(void)
 {
     test_data data;
@@ -752,6 +757,8 @@ static void test_acpi_piix4_tcg(void)
      */
     memset(&data, 0, sizeof(data));
     data.machine = MACHINE_PC;
+    data.required_struct_types = base_required_struct_types;
+    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
     test_acpi_one("-machine accel=tcg", &data);
     free_test_data(&data);
 }
@@ -763,6 +770,8 @@ static void test_acpi_piix4_tcg_bridge(void)
     memset(&data, 0, sizeof(data));
     data.machine = MACHINE_PC;
     data.variant = ".bridge";
+    data.required_struct_types = base_required_struct_types;
+    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
     test_acpi_one("-machine accel=tcg -device pci-bridge,chassis_nr=1", &data);
     free_test_data(&data);
 }
@@ -773,6 +782,8 @@ static void test_acpi_q35_tcg(void)
 
     memset(&data, 0, sizeof(data));
     data.machine = MACHINE_Q35;
+    data.required_struct_types = base_required_struct_types;
+    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
     test_acpi_one("-machine q35,accel=tcg", &data);
     free_test_data(&data);
 }
@@ -784,11 +795,50 @@ static void test_acpi_q35_tcg_bridge(void)
     memset(&data, 0, sizeof(data));
     data.machine = MACHINE_Q35;
     data.variant = ".bridge";
+    data.required_struct_types = base_required_struct_types;
+    data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
     test_acpi_one("-machine q35,accel=tcg -device pci-bridge,chassis_nr=1",
                   &data);
     free_test_data(&data);
 }
 
+static uint8_t ipmi_required_struct_types[] = {
+    0, 1, 3, 4, 16, 17, 19, 32, 38, 127
+};
+
+static void test_acpi_q35_tcg_ipmi(void)
+{
+    test_data data;
+
+    memset(&data, 0, sizeof(data));
+    data.machine = MACHINE_Q35;
+    data.variant = ".ipmibt";
+    data.required_struct_types = ipmi_required_struct_types;
+    data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
+    test_acpi_one("-machine q35,accel=tcg -device ipmi-bmc-sim,id=bmc0"
+                  " -device isa-ipmi-bt,bmc=bmc0",
+                  &data);
+    free_test_data(&data);
+}
+
+static void test_acpi_piix4_tcg_ipmi(void)
+{
+    test_data data;
+
+    /* Supplying -machine accel argument overrides the default (qtest).
+     * This is to make guest actually run.
+     */
+    memset(&data, 0, sizeof(data));
+    data.machine = MACHINE_PC;
+    data.variant = ".ipmikcs";
+    data.required_struct_types = ipmi_required_struct_types;
+    data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
+    test_acpi_one("-machine accel=tcg -device ipmi-bmc-sim,id=bmc0"
+                  " -device isa-ipmi-kcs,irq=0,bmc=bmc0",
+                  &data);
+    free_test_data(&data);
+}
+
 int main(int argc, char *argv[])
 {
     const char *arch = qtest_get_arch();
@@ -805,6 +855,8 @@ int main(int argc, char *argv[])
         qtest_add_func("acpi/piix4/tcg/bridge", test_acpi_piix4_tcg_bridge);
         qtest_add_func("acpi/q35/tcg", test_acpi_q35_tcg);
         qtest_add_func("acpi/q35/tcg/bridge", test_acpi_q35_tcg_bridge);
+        qtest_add_func("acpi/piix4/tcg/ipmi", test_acpi_piix4_tcg_ipmi);
+        qtest_add_func("acpi/q35/tcg/ipmi", test_acpi_q35_tcg_ipmi);
     }
     ret = g_test_run();
     boot_sector_cleanup(disk);
diff --git a/tests/acpi-test-data/pc/DSDT.ipmikcs b/tests/acpi-test-data/pc/DSDT.ipmikcs
new file mode 100644
index 0000000000000000000000000000000000000000..f10cd9e296c942b66d6f2404f1a95fe5cc4a6796
GIT binary patch
literal 5575
zcmb7I-ESMm5ud#yrQ?#6j?!6{?Ib4Tug0xye%Nw?7KO+=N~FxA&OFL>aYpYb$tcw(
z^+Ab&$U*>_0Td^1DdVDjqg_7Re?s%tKITtI|AiDy{1kQOj!Ushu1E>+I3K^Ancbb4
z-J@X}t-qfDU|*Ei^s1dL-?elDWefm_reC!dy9ds`QC@LbQi?I>aWbIsQC75{8RhjA
z_UDfGC(nETV~@3c*m$l!-DvwCJ@GbxKo2*3&WTVLT)XT#-7}V6%PDTPW~54Aa#?Xp
zMiUfwy=E}gR6<Nesalnm05Jri8pEhsEfZ1zUw)T_nDbaAe)dkKVPvhQ;byf(*KU=-
zCyD2isQt*}s@G3_*7Km}B@Yh=(4YFO*M`sADfs8V;{zD*X~or>b|%B84iY$HG!8L<
zT~IGvrD!Nzr+dn}n=b~jret6kij^<w67WNHDLw|%V>P?vhB#bU!d=hl68AWUd+|3{
z$qJ1Gs9uA~x>ac~cnf5`M1-uD@khON?wTwdlQpM%rP<QaXt-oaTr7%ro}XA@KKP(Y
z3&2*`4(xz5u?tT3=B=_Zg3Gd&2v%h@(O{ql9xxoW1zY&LAmMzS?j>_=eKc@-_V_@B
z#}_~)U`bgv^wB0x4kJ3l=U3R*^4BuNR@k@lx42LXOK>?B*v>E4t&-U6WurEeRe{I(
z8qYEQl1uyoerNa@e#qb9-|%U!DDJXR$(-aGrQ!HTLdNO-vTn6TvBp4~54WC{)ay^+
zb`RiLY&2(;r&rhq@&~e_EbIsIRATy?nHP&1XJG~kGqfR5W?sp!WKb5=z|1n4DI7Dy
zrI8)n(MUVeD+%o=Lpx?IKb+6S1YkIJtC`;R-`#e9zxm#<KEjPpbH2i!r~aB^(k?1$
z%Pm4T?<7G|tIAO_?pwOuVo7+x6)Y8Ebj;=TEIy2^vu{^cv80t3l$l9l^m4AUii&c>
zY_g;pDu~g`jde5YuyS3$1=<BmudGVWIpPiv{SGPpUA7H0+_^;J0jLcu#j>m3;#5AG
z!CkA_0)Ix@_k3tSFDIV1`M&?C%_Z3I_jcMFz-8F%tG!JGC9wSJjavq6_0RV<SwGWj
z^Vmpi!`8F(h$|r|)7#|INKzh3<dH;&E&s#a4Xy$<0C=(m;P183-;*r{{?-$(4G-G`
z&v|_QnoQAB%~91dt9d)8;3H3u0zHa~He0)mtJ+WO@%;`qkHml{wZ+<Py_rBjGnbj-
z$xkLA5hU#{d`dDN$7ijdeA>Y|cZeR4*y+|Mahg~ZVPU7R5mB2{aINgj9H0GU3KDW0
zvz_BNiQiq&ro&pMASgU7;~F&^J|vp&2a;*(4ltmQ0{$fChRr_hgi&BuY}QCV^j<#Z
z9aHH?d(V$~&j;S~7~i<}y!ON1^G?^Kx$DQgbLszZUl?;=2;3Krxi3VxFAUuq$KB&W
z^P^{99CKd`+!v3zFGjd8I^DGx&fY%mJ{GzcDMY6cZn<dRNo^w7Y%LkCF@=gE+FiAR
z?XT==H+0J^NqG3`)>@+jo}Wo_1r)S{pSHjKgbL2@;ExQxhd(hGw6)W^g(-bBsvq~T
zaXym=pmE)zVqf6@&{ikVpAMKnwtFAXc^|yz%kih)$K-1`*n@VT?|FO@9&NCVXDJll
z#}ilvs`3qn6~6GtUVGEu3<C6^&#}H6t>s^P?3oh$a`~a>bZ^D3QYBi=%4m)FL{J@R
z-CEC9AH46s^YFLXo%`=Sc(}89pZX&n{5(0<AmJsvWLZ3{uquK!_>yOx$Ekp!DJ-eZ
zN7r>LmkSq1OS@*N1p`_@04)`k87&N;s<N76sH&_&N6S#05=P#E0~)`r&;rcJjA?WC
zuDUB$#jWHDfFxZWaLLrhm7}KIEnP9IR+xBct6}9zxy7)%r_KBCKG@mKcD^?`Z8nIe
zpXixJqlpI__8f<qK{qZ<n6}=4BOPvwbB=U}htGfU)T0r;XWEr|f3|1V8a6Y7<@Omd
z&*u7C&up4^LMfL~jNFdLJm(S_88GdNN$WRe*8fWf0)xq(S+QN_^J#(hw9>qZ4Im*x
z5Lm@*E?&<RdG6Dn0^kkm*yp>$@FT=S_7EE=4dhc8Y&|4rkgSH}#d<|ZFC#gW&Y7jV
zL+oiJtG?MRS7{ItbuxrRy-EX!sFRT$5)(mVi3!v+y01MY8R>V3XMr#i2-*812~l;@
z)%(U|kHmmeE*3Ip`D-7a9G@~xm8MBk45n8saE|6(oD-5$ICe<RVL+}=o=NSKaZ>se
zC#i-zM8ihp;&tu3Y27v1@eS}%>K<*=Ps}yD75g4&+gz*M@qdR(Jv6B;n3(%`+J_k>
zwiD9?FAO0>txZtU2fZv^(ydx9hbgVSja85b=iI%c%WQaK7!z1}qvc#06A(f>g(Hh_
z?^;Y;f~jCA%hgI|SXI~?!cs3!@;^LDgM{w{w5IgHOYN)Fs}xgE&camt7S*p(BR(DM
zz`w@#h2wtE2}>Ud0*$zKDu;)E`!3Xc@eN&m^%zb!y!;-kV3PmOru5azuU@hk8Ci{4
zRjU~;+JCnJY<xei!M%VNOvBfdH}6sVE4niiF@`}Sechc=VtAj%n=*1YNJA4)e}1%l
zPWS5auk__GfZG*w9e1rj=woeEjjM5%LNBYk_ktUpsf_Jc(8_|QG~RhRURdU-^Hwdo
zNdo8d&f~Mh$W($^DoA|PiK-)onTf6Wbpn)Ni3d#uQyyDY(3$9->O4LXj>krpr6k3N
zt!K3f>SmH^KA_`5DWewGHfiT_c$KF2N~e3-&JP((0tqynV&1?WN<={rMIxN;n|A&!
z9B8jMhVP{~L<Vgp=>K%>B?qJ?Xo1WU6i;n*g;0D8iGT{LaRH4-L5spF9(SSDiAboh
zIw_!&QPAtcD!x^PR+UJou$mCiL=^Ofu*wCLM?!^FJfgz@l2On_VKpV7sYs}>nikM>
z6!fOBIwhb}kx*e3@8e;t(^1g3gjH2Q)kvtYni0@U6!dLjbw)sEBB8?Stboo&K}*6a
zW_~y=EfOlMo)FLzQP8rmIwzoWkx*gvq=24`g6hKRynxO}LWR{+0(vS6Y6z>R1@v?z
zR9HPDpl70>6=C%?0evkJDy+UPpsz<kmxa|g1oVwasIYogK+paVRI8pb?lf9bKegAv
zmx2T8b=v`>B7@y5WU|Bv%@Q)$kwVQrF{DZFb&~ks)`5*}WyndyjdU^OusIBM<m<*r
zN6xUshdT1MIMR_jEYzWn{M{to^j-(MD;{kJHWr-V@o+=_NRLO{fsLgj^!V0;{E=QI
z9*aQevjKmkUm_mUS?Koz{zxCp7!y<I@h~~C^)Wro8dGefr+FJ=dYU;V%SfLkeoViF
gJZ}3)zfU~swX<|j!aw@xpBCCeJnPokkTAIXABOi<OaK4?

literal 0
HcmV?d00001

diff --git a/tests/acpi-test-data/q35/DSDT.ipmibt b/tests/acpi-test-data/q35/DSDT.ipmibt
new file mode 100644
index 0000000000000000000000000000000000000000..3db2b0b5f96e567d42d3abc672c95e9724243aaf
GIT binary patch
literal 8340
zcmb7JO>Y~=8J;C6YBeOKrL>miuZU2Kq)BT>wj8(UM__XKX>+ADNjnK>fRr4!a?>;m
z#RifX0c-`xhYtl3#X)<c4V?q@7c|Enb8CQ}3KTu`S`_gq>hr$Ck!MJ&h<VtZclLSS
zcb?hZd6v87cDeI;K?vv1rL~~mE|gx4f-?FXAq2H)?=@0aMfXm*RO>mJM9S&j&2Sr=
zlI#DZT)JFy{<zuyqTj#yQQujQ#l}PL{>FOz!N>g#L7-b3v6R&Cp6|6wz3!bTXm^~9
z_)=!MU2-znFDWhhOXs7YQF6VtEJL?zS;Wh3uihwUi(gCE?FFipbbG~eOSrwujk4pm
zFGrDZgHEd{{PI+9tsSK9y&;0~E2jt5$(_yoob<Zk-&ej}JbwO*S8f)+`qkfleeVsK
z0^khxF7_)i1(b&xE0pRsZR82s4!qBHZaDAb$Z1BO9?XBXqj9R+qk}PlssBM%^-_t`
zyKi`*tMpXddQ|CXJq}c0{FvQ()Comah_AoPh*bBkNAcX1%4)d~waUGMf3DZ=6h%xu
z{g}Ge-|I`SKgfQ%(-%Aa%--Iv7)*b<vo1bg&x(KicVbuU${Ea7tDVow=?|xHB9UCA
zgm_nY=O<6maA<0~+d;a~F7|jjF@Qx3CTg@UC1jsYPZQTeLf(;y?%gBQ7QtFrObQ3P
z5_Zj~Q%*3dl91bkIs{Zj1jWj#6O^!NM%Z{4kG+`G6{%Wx`(&#Vpr|^p8JtHN>4H2|
zb7Bz-FR>(?nzJRgL?ZpJ=x+b?VyV0jPbU2-o{xQrOzexkaKL(9ypH`+0!q8v&xLE3
z_lYxeqeNFuED4W*FGI^dFeUs2HRCk?{veH6-W3m0`vpCjt2yr^-$}ae(j6{TNnuV-
zYef@|x-xYdnvjFiJW4)F3U|rlG*wD-W+aVF?B~Ip9+)TbD1kgK=V_$E#_6G+XQu>)
z)9$o#H{v&M^fqql+lD8drTkO;OE}4;0=RCkQSC@vneEn<j7YdqBd7{L+r!?8H!1S|
z@R~zs(+mF1cW2b<M6(y)zB8Li;X)M6)r%FO+78-PF`K1+nLtp}K@ve-%;r+ry*;cl
z3O9_11>m$g#ymiENKE8sY=W7<4kKb}XhJX(P-Y?|CPu^9Bs290=u+ma3{5H~pgJU`
zu7X=gVL4BVnHri>Og(snrp^gV=L9n~bV4!p;1QZS)0WOOGc`1A>qKbkoV0XKS~@3f
zod`{xuBFqpbh@@qgr-g`HpWcj;#0H7lR0JUL}==imQHEul(tTUrp}C|Gh^w@*g6rK
zI<e>(=QC^R%-T8;nmTiq&YYz)XX`|0>YTQ8PFp&sZJh{Boimos8B6DktrMZC)3bDX
zmQK&siO|%Uw{+$$oq1a)LR06grE}KOIcw`gXzHA^bk12i=WLw_O`X1_)3<c`woZhm
z&O?^YLzd1%woZhm&Us7cyrpyA)``&6dDzl<*wT5})``&6xnSvBuyiiiIuV*Wk1(@1
zTnUdbvou`njwojEI3#9yh&^gCk6O&5HWQ)AJZ3SES<GWL6QRjGZZVHr%;Po_p~*a9
zF;7^`6E+i}$$W;H+GU<$re5QpQB1vXBP6DFl0~cLqE&Ozu8C09bYY-H!$528#=e1~
zjlx3%MS!v@NFvZhVW0}fmgL~kiGjS}Iw-23IwYo+1_M=CZJ-8}3{*jxNd}4#qtOPc
zu)~OIYOf^&RZwOkBqoN=SQx0n4kKdf$t43-P-c>WBGfuzpbE=O7^ndy165FFl7S-B
zI$@v+%S;%k0VM-fP-c>WBGfuzpbE=O7^ndy165FFl7S-BI$@v+%S;%k0VM-fP-c>W
zBGfuzpbE=O7^ndy165FFl7S-BI$@v+%S;%k0VM-fP-c>WBGfuzpbE=O7^ndy165FF
zl7S-BI$@v+%S;%k0VM-fP-c>WBGfuzpbE=O7^ndy165FFl7S-BI$@v+%S;%k0VM-f
zP-c>WBGfuzpbE=O7^ndy165FFl7S-BI$@v+%S;%k0VM-fP-c>WBGfuzpbE=O7^ndy
z165FFl7S-BI$@v+%S;%k0VM-fP-c>WBGfuzpbE=O7^ndy165FFl7S-BI$@v+%S;%k
z0VM-fP-c>WBGfuzpbE=O7^ndy165FFl7S-BI$@v+%S;%k0VM-fP-c>WBGfuzpbE=O
z7^ndy165FFl7S-BI$@v+%S;%k0VM-fP-c>WBGfuzpok0uMPwQ%LeoGI8U|`GVW0++
z4Afwfff`I0sKJDR8cZ@!gGmNzFkzqu69#H9$v_Pz8K}X8ff`I0sKF!yHJD_e1``H~
zNS|F8C?cFOo=h@Ogg6a4B&MEQ7$_n=w=hsddTz-;5$d@m14URC{?Q&RR1C5^{AYES
z{*XRM)2`UP`|}r*^e>fOJJ5>_Cu&_P!f`K^XcN$;><YYQxm2Ncl{Pin)LnnBaS3m#
z22;1?olU$?nc9^Z|7_z@(3%3EwVa=pnNQMUir$jpkMlk1lM~oB;@g{YMo<fbXyGMG
za}v)8Mg6xvi#Id$-p9qT+xdAp_sO)FO2XX))^CB87}@+8^@@XrfYB8Fe6QSSi}$H5
z?{Loy{eKs`G$4)-<1c&LxwknS1C^)#VfE5Tb-WVz-qlM+^%7SvVSHxwlK<rDrS5h}
z+JlkmGVv7U1EYMv<%5y(!2#uiVfpfCdA#5F-s;On`7)O;kCZPTP`*4YUl}c*dW!ND
zqkM(SS4PTL4k%w4mamSMpLmM$Rik{B%U4IrR}Uy(9hR?+mQO!L`I=F_#^q}x<!c9&
zuMNxBN6Sw>Mfti>zRu<ABjxJ{l&_<Fik7m`@}4TM7Y{sE@LM-}0c!X8X};yLqx*W8
zZuB;mc{?4RHOA7Zz9*&|yDw+$ba((6OQ-stm~QMoo3qp5sbnmj>U(0kvHQli)8SEN
zES>6mV!E;W;E<gT&o5)?RNoWRja}FCb~-%NjHOe3PfRy<ogTK+;YnvKo$9mGc@@07
zF7^DEf+#E|@Es<I)>bz~Kh9^QEAX#`e(Uj#kJn{F#GBY9Mcl_OC3e^Sqd^4Ed(0Y4
z^zqjFL2_64GKeY{65S>FSN}p9t7jnaDWcuGj!!<Z=s&DZ-0y!xwT9%rSRcsSeK{%K
z+i*4>WYK*cUoG(FbtAsL;fQO4rO$TOuf^ARfPFELJJ+1mPW83E^T6f3Oy25uw=bqn
zk%338N?YGb5-SJ{G#5md3-z~d#=pDuo5I%hx8J(8b?y4wuvL64ks0+(Atz<c`6l~K
z)+yjMKa7*UJ$;jvh3?%f2PEOV*ZtsZ5G^mO$?<c&u-Fm1G=YNNebdJmMD32ip~{72
z2SMe6*!1%>&ICrjDLy3r4VNZhHt9J2{MA#vT2$#(md^+@>3a*N%v(Kae0wRL4C|4)
zXL8YMv|L<1r{2xy!s~Cowe@acb4;8+$D<ic?S$pk7QRa0p``1C&ETT&i|Mc(tcu6S
za9X-QpksS`4}W>TPmH)9wkyrS+)mh7Z95@PckEE`V1D55gst$3>Xk`!Biiaa{qA#=
zWY_UehBSXh+W1eKf)i$T!b-d6#BxTX`+jjD#Hu!>hoG@;*gAJMzap3K{aA=Bt_S<~
z-JbgC-eI_hCoCe$X$-chnzLxuRrA?qMfbjd=CF4@EWSF#9!0YrhpkebNYrRjA~c$H
zB2c4ANw%n)8pNf54-<Rf-={u0zD2Raoq6tbZtnL`t4YV2gYx7Kbqh#dIg?+M$3May
z`&cqbC7C3V<*l5N66t%;gyuB3!{$5&)QhPy9gZnE9gJz5bZB^nh_>H6ch>)57`+-g
zqbDFnZ}0JG`cb&n?xe;*H^Q~bmH4+f7^sx~Qiy9mF$Q$dJsg0?*F4{*xndACmX~ow
z`$upStca^Grt~*Jy6Wjt^%~jXt-)V;$pTB0^7kL-L;`OWXkuuCZ+efikFt)7en(B-
zw>bPLyC3Ix8vkQXuEeC$lQjJjf{*ZXI>N8^_Wtr69}r)?OyB;36mC@Ym3ibgr2hZA
zvJ}#Vw1W?b-t$*!-C60)rc(|Ip?$VD>!xG^i!4Pi(Ge=@{o=9Y?)ItbD}l15+@(r*
z8S{09&?D=YH{ne<SyTz8ukx3-(7pQjiz=@nyc!rw9{x&5u|P{+Jqla++%Dxp_wFJE
zNxJq6bSGfbq*fCxToPClPY|Fe&P%?VajjvE;3Wa47~kj*DLz(^0$m;0w1a}5re!4I
wRa5GVa;Ck!j8BjBeZIT>LVNjVD!?go1Pr`=x{PIu8fSE4g&XN8SnY`a0dp?SY5)KL

literal 0
HcmV?d00001

-- 
MST

[Qemu-devel] [PULL v2 16/32] pci core: assert ENOSPC when add capability

[Michael S. Tsirkin]

From: Cao jin <caoj.fnst@cn.fujitsu.com>

ENOSPC is programming error, assert it for debugging.

cc: Michael S. Tsirkin <mst@redhat.com>
cc: Marcel Apfelbaum <marcel@redhat.com>
cc: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/pci.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 87bea47..4b585f4 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2228,10 +2228,8 @@ int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id,
 
     if (!offset) {
         offset = pci_find_space(pdev, size);
-        if (!offset) {
-            error_setg(errp, "out of PCI config space");
-            return -ENOSPC;
-        }
+        /* out of PCI config space is programming error */
+        assert(offset);
     } else {
         /* Verify that capabilities don't overlap.  Note: device assignment
          * depends on this check to verify that the device is not broken.
-- 
MST

[Qemu-devel] [PULL v2 17/32] fix some coding style problems

[Michael S. Tsirkin]

From: Cao jin <caoj.fnst@cn.fujitsu.com>

It has:
1. More newlines make the code block well separated.
2. Add more comments for msi_init.
3. Fix a indentation in vmxnet3.c.
4. ioh3420 & xio3130_downstream: put PCI Express capability init function
   together, make it more readable.

cc: Michael S. Tsirkin <mst@redhat.com>
cc: Markus Armbruster <armbru@redhat.com>
cc: Marcel Apfelbaum <marcel@redhat.com>
cc: Dmitry Fleytman <dmitry@daynix.com>
cc: Jason Wang <jasowang@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/net/vmxnet3.c                   |  2 +-
 hw/pci-bridge/ioh3420.c            |  7 ++++++-
 hw/pci-bridge/pci_bridge_dev.c     |  4 ++++
 hw/pci-bridge/xio3130_downstream.c |  6 +++++-
 hw/pci-bridge/xio3130_upstream.c   |  3 +++
 hw/pci/msi.c                       | 16 ++++++++++++++++
 6 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 16645e6..d978976 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -348,7 +348,7 @@ typedef struct {
 /* Interrupt management */
 
 /*
- *This function returns sign whether interrupt line is in asserted state
+ * This function returns sign whether interrupt line is in asserted state
  * This depends on the type of interrupt used. For INTX interrupt line will
  * be asserted until explicit deassertion, for MSI(X) interrupt line will
  * be deasserted automatically due to notification semantics of the MSI(X)
diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index 0937fa3..b4a7806 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -106,12 +106,14 @@ static int ioh3420_initfn(PCIDevice *d)
     if (rc < 0) {
         goto err_bridge;
     }
+
     rc = msi_init(d, IOH_EP_MSI_OFFSET, IOH_EP_MSI_NR_VECTOR,
                   IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
                   IOH_EP_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT);
     if (rc < 0) {
         goto err_bridge;
     }
+
     rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT, p->port);
     if (rc < 0) {
         goto err_msi;
@@ -120,18 +122,21 @@ static int ioh3420_initfn(PCIDevice *d)
     pcie_cap_arifwd_init(d);
     pcie_cap_deverr_init(d);
     pcie_cap_slot_init(d, s->slot);
+    pcie_cap_root_init(d);
+
     pcie_chassis_create(s->chassis);
     rc = pcie_chassis_add_slot(s);
     if (rc < 0) {
         goto err_pcie_cap;
     }
-    pcie_cap_root_init(d);
+
     rc = pcie_aer_init(d, IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
     if (rc < 0) {
         goto err;
     }
     pcie_aer_root_init(d);
     ioh3420_aer_vector_update(d);
+
     return 0;
 
 err:
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index 7b582e9..41ca47b 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -68,10 +68,12 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
         /* MSI is not applicable without SHPC */
         bridge_dev->flags &= ~(1 << PCI_BRIDGE_DEV_F_MSI_REQ);
     }
+
     err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0);
     if (err) {
         goto slotid_error;
     }
+
     if ((bridge_dev->flags & (1 << PCI_BRIDGE_DEV_F_MSI_REQ)) &&
         msi_nonbroken) {
         err = msi_init(dev, 0, 1, true, true);
@@ -79,6 +81,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
             goto msi_error;
         }
     }
+
     if (shpc_present(dev)) {
         /* TODO: spec recommends using 64 bit prefetcheable BAR.
          * Check whether that works well. */
@@ -86,6 +89,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
                          PCI_BASE_ADDRESS_MEM_TYPE_64, &bridge_dev->bar);
     }
     return 0;
+
 msi_error:
     slotid_cap_cleanup(dev);
 slotid_error:
diff --git a/hw/pci-bridge/xio3130_downstream.c b/hw/pci-bridge/xio3130_downstream.c
index cf1ee63..e6d653d 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -70,11 +70,13 @@ static int xio3130_downstream_initfn(PCIDevice *d)
     if (rc < 0) {
         goto err_bridge;
     }
+
     rc = pci_bridge_ssvid_init(d, XIO3130_SSVID_OFFSET,
                                XIO3130_SSVID_SVID, XIO3130_SSVID_SSID);
     if (rc < 0) {
         goto err_bridge;
     }
+
     rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_DOWNSTREAM,
                        p->port);
     if (rc < 0) {
@@ -83,12 +85,14 @@ static int xio3130_downstream_initfn(PCIDevice *d)
     pcie_cap_flr_init(d);
     pcie_cap_deverr_init(d);
     pcie_cap_slot_init(d, s->slot);
+    pcie_cap_arifwd_init(d);
+
     pcie_chassis_create(s->chassis);
     rc = pcie_chassis_add_slot(s);
     if (rc < 0) {
         goto err_pcie_cap;
     }
-    pcie_cap_arifwd_init(d);
+
     rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
     if (rc < 0) {
         goto err;
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index 164ef58..d976844 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -66,11 +66,13 @@ static int xio3130_upstream_initfn(PCIDevice *d)
     if (rc < 0) {
         goto err_bridge;
     }
+
     rc = pci_bridge_ssvid_init(d, XIO3130_SSVID_OFFSET,
                                XIO3130_SSVID_SVID, XIO3130_SSVID_SSID);
     if (rc < 0) {
         goto err_bridge;
     }
+
     rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_UPSTREAM,
                        p->port);
     if (rc < 0) {
@@ -78,6 +80,7 @@ static int xio3130_upstream_initfn(PCIDevice *d)
     }
     pcie_cap_flr_init(d);
     pcie_cap_deverr_init(d);
+
     rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
     if (rc < 0) {
         goto err;
diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a87ef4d..359058e 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -165,6 +165,22 @@ bool msi_enabled(const PCIDevice *dev)
          PCI_MSI_FLAGS_ENABLE);
 }
 
+/*
+ * Make PCI device @dev MSI-capable.
+ * Non-zero @offset puts capability MSI at that offset in PCI config
+ * space.
+ * @nr_vectors is the number of MSI vectors (1, 2, 4, 8, 16 or 32).
+ * If @msi64bit, make the device capable of sending a 64-bit message
+ * address.
+ * If @msi_per_vector_mask, make the device support per-vector masking.
+ * Return the offset of capability MSI in config space on success,
+ * return -errno on error.
+ *
+ * -ENOTSUP means lacking msi support for a msi-capable platform.
+ * -EINVAL means capability overlap, happens when @offset is non-zero,
+ *  also means a programming error, except device assignment, which can check
+ *  if a real HW is broken.
+ */
 int msi_init(struct PCIDevice *dev, uint8_t offset,
              unsigned int nr_vectors, bool msi64bit, bool msi_per_vector_mask)
 {
-- 
MST

[Qemu-devel] [PULL v2 18/32] msi_init: change return value to 0 on success

[Michael S. Tsirkin]

From: Cao jin <caoj.fnst@cn.fujitsu.com>

No caller use its return value as msi capability offset, also in order
to make its return behaviour consistent with msix_init().

cc: Michael S. Tsirkin <mst@redhat.com>
cc: Paolo Bonzini <pbonzini@redhat.com>
cc: Hannes Reinecke <hare@suse.de>
cc: Markus Armbruster <armbru@redhat.com>
cc: Marcel Apfelbaum <marcel@redhat.com>

Acked-by: Hannes Reinecke <hare@suse.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/pci/msi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index 359058e..ed79225 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -173,8 +173,7 @@ bool msi_enabled(const PCIDevice *dev)
  * If @msi64bit, make the device capable of sending a 64-bit message
  * address.
  * If @msi_per_vector_mask, make the device support per-vector masking.
- * Return the offset of capability MSI in config space on success,
- * return -errno on error.
+ * Return 0 on success, return -errno on error.
  *
  * -ENOTSUP means lacking msi support for a msi-capable platform.
  * -EINVAL means capability overlap, happens when @offset is non-zero,
@@ -236,7 +235,8 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
         pci_set_long(dev->wmask + msi_mask_off(dev, msi64bit),
                      0xffffffff >> (PCI_MSI_VECTORS_MAX - nr_vectors));
     }
-    return config_offset;
+
+    return 0;
 }
 
 void msi_uninit(struct PCIDevice *dev)
-- 
MST

[Qemu-devel] [PULL v2 19/32] pc-dimm: introduce get_vmstate_memory_region callback

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

This callback returns the MemoryRegion that is the memory of dimm should
be kept during live migration

nvdimm device is different with pc-dimm as its memory includes not only
the MemoryRegion directly mapping to guest's address space but also the
memory used as label data

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/mem/pc-dimm.h |  5 ++++-
 hw/mem/pc-dimm.c         | 14 ++++++++++++--
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 67e92d8..1e483f2 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -61,7 +61,9 @@ typedef struct PCDIMMDevice {
  * @realize: called after common dimm is realized so that the dimm based
  * devices get the chance to do specified operations.
  * @get_memory_region: returns #MemoryRegion associated with @dimm which
- * is directly mapped into the physical address space of guest
+ * is directly mapped into the physical address space of guest.
+ * @get_vmstate_memory_region: returns #MemoryRegion which indicates the
+ * memory of @dimm should be kept during live migration.
  */
 typedef struct PCDIMMDeviceClass {
     /* private */
@@ -70,6 +72,7 @@ typedef struct PCDIMMDeviceClass {
     /* public */
     void (*realize)(PCDIMMDevice *dimm, Error **errp);
     MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm);
+    MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm);
 } PCDIMMDeviceClass;
 
 /**
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 6de2275..249193a 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -40,6 +40,8 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
     int slot;
     MachineState *machine = MACHINE(qdev_get_machine());
     PCDIMMDevice *dimm = PC_DIMM(dev);
+    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
+    MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm);
     Error *local_err = NULL;
     uint64_t existing_dimms_capacity = 0;
     uint64_t addr;
@@ -105,7 +107,7 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
     }
 
     memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr);
-    vmstate_register_ram(mr, dev);
+    vmstate_register_ram(vmstate_mr, dev);
     numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node);
 
 out:
@@ -116,10 +118,12 @@ void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
                            MemoryRegion *mr)
 {
     PCDIMMDevice *dimm = PC_DIMM(dev);
+    PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
+    MemoryRegion *vmstate_mr = ddc->get_vmstate_memory_region(dimm);
 
     numa_unset_mem_node_id(dimm->addr, memory_region_size(mr), dimm->node);
     memory_region_del_subregion(&hpms->mr, mr);
-    vmstate_unregister_ram(mr, dev);
+    vmstate_unregister_ram(vmstate_mr, dev);
 }
 
 static int pc_existing_dimms_capacity_internal(Object *obj, void *opaque)
@@ -424,6 +428,11 @@ static MemoryRegion *pc_dimm_get_memory_region(PCDIMMDevice *dimm)
     return host_memory_backend_get_memory(dimm->hostmem, &error_abort);
 }
 
+static MemoryRegion *pc_dimm_get_vmstate_memory_region(PCDIMMDevice *dimm)
+{
+    return host_memory_backend_get_memory(dimm->hostmem, &error_abort);
+}
+
 static void pc_dimm_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -434,6 +443,7 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data)
     dc->desc = "DIMM memory module";
 
     ddc->get_memory_region = pc_dimm_get_memory_region;
+    ddc->get_vmstate_memory_region = pc_dimm_get_vmstate_memory_region;
 }
 
 static TypeInfo pc_dimm_info = {
-- 
MST

[Qemu-devel] [PULL v2 20/32] nvdimm: support nvdimm label

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Introduce a parameter, 'label-size', which is the size of nvdimm label
data area which is reserved at the end of backend memory. It is required
at least 128k

Two callbacks, read_label_data() and write_label_data(), are used to
operate the label area

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/mem/nvdimm.h |  55 +++++++++++++++++++-
 hw/mem/nvdimm.c         | 132 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 186 insertions(+), 1 deletion(-)

diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index 60ee92b..1cfe9e0 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -34,7 +34,60 @@
         }                                                     \
     } while (0)
 
-#define TYPE_NVDIMM             "nvdimm"
+/*
+ * The minimum label data size is required by NVDIMM Namespace
+ * specification, see the chapter 2 Namespaces:
+ *   "NVDIMMs following the NVDIMM Block Mode Specification use an area
+ *    at least 128KB in size, which holds around 1000 labels."
+ */
+#define MIN_NAMESPACE_LABEL_SIZE      (128UL << 10)
+
+#define TYPE_NVDIMM      "nvdimm"
+#define NVDIMM(obj)      OBJECT_CHECK(NVDIMMDevice, (obj), TYPE_NVDIMM)
+#define NVDIMM_CLASS(oc) OBJECT_CLASS_CHECK(NVDIMMClass, (oc), TYPE_NVDIMM)
+#define NVDIMM_GET_CLASS(obj) OBJECT_GET_CLASS(NVDIMMClass, (obj), \
+                                               TYPE_NVDIMM)
+struct NVDIMMDevice {
+    /* private */
+    PCDIMMDevice parent_obj;
+
+    /* public */
+
+    /*
+     * the size of label data in NVDIMM device which is presented to
+     * guest via __DSM "Get Namespace Label Size" function.
+     */
+    uint64_t label_size;
+
+    /*
+     * the address of label data which is read by __DSM "Get Namespace
+     * Label Data" function and written by __DSM "Set Namespace Label
+     * Data" function.
+     */
+    void *label_data;
+
+    /*
+     * it's the PMEM region in NVDIMM device, which is presented to
+     * guest via ACPI NFIT and _FIT method if NVDIMM hotplug is supported.
+     */
+    MemoryRegion nvdimm_mr;
+};
+typedef struct NVDIMMDevice NVDIMMDevice;
+
+struct NVDIMMClass {
+    /* private */
+    PCDIMMDeviceClass parent_class;
+
+    /* public */
+
+    /* read @size bytes from NVDIMM label data at @offset into @buf. */
+    void (*read_label_data)(NVDIMMDevice *nvdimm, void *buf,
+                            uint64_t size, uint64_t offset);
+    /* write @size bytes from @buf to NVDIMM label data at @offset. */
+    void (*write_label_data)(NVDIMMDevice *nvdimm, const void *buf,
+                             uint64_t size, uint64_t offset);
+};
+typedef struct NVDIMMClass NVDIMMClass;
 
 #define NVDIMM_DSM_MEM_FILE     "etc/acpi/nvdimm-mem"
 
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 0a602f2..81896c0 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -23,20 +23,152 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
 #include "hw/mem/nvdimm.h"
 
+static void nvdimm_get_label_size(Object *obj, Visitor *v, const char *name,
+                                  void *opaque, Error **errp)
+{
+    NVDIMMDevice *nvdimm = NVDIMM(obj);
+    uint64_t value = nvdimm->label_size;
+
+    visit_type_size(v, name, &value, errp);
+}
+
+static void nvdimm_set_label_size(Object *obj, Visitor *v, const char *name,
+                                  void *opaque, Error **errp)
+{
+    NVDIMMDevice *nvdimm = NVDIMM(obj);
+    Error *local_err = NULL;
+    uint64_t value;
+
+    if (memory_region_size(&nvdimm->nvdimm_mr)) {
+        error_setg(&local_err, "cannot change property value");
+        goto out;
+    }
+
+    visit_type_size(v, name, &value, &local_err);
+    if (local_err) {
+        goto out;
+    }
+    if (value < MIN_NAMESPACE_LABEL_SIZE) {
+        error_setg(&local_err, "Property '%s.%s' (0x%" PRIx64 ") is required"
+                   " at least 0x%lx", object_get_typename(obj),
+                   name, value, MIN_NAMESPACE_LABEL_SIZE);
+        goto out;
+    }
+
+    nvdimm->label_size = value;
+out:
+    error_propagate(errp, local_err);
+}
+
+static void nvdimm_init(Object *obj)
+{
+    object_property_add(obj, "label-size", "int",
+                        nvdimm_get_label_size, nvdimm_set_label_size, NULL,
+                        NULL, NULL);
+}
+
+static MemoryRegion *nvdimm_get_memory_region(PCDIMMDevice *dimm)
+{
+    NVDIMMDevice *nvdimm = NVDIMM(dimm);
+
+    return &nvdimm->nvdimm_mr;
+}
+
+static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp)
+{
+    MemoryRegion *mr = host_memory_backend_get_memory(dimm->hostmem, errp);
+    NVDIMMDevice *nvdimm = NVDIMM(dimm);
+    uint64_t align, pmem_size, size = memory_region_size(mr);
+
+    align = memory_region_get_alignment(mr);
+
+    pmem_size = size - nvdimm->label_size;
+    nvdimm->label_data = memory_region_get_ram_ptr(mr) + pmem_size;
+    pmem_size = QEMU_ALIGN_DOWN(pmem_size, align);
+
+    if (size <= nvdimm->label_size || !pmem_size) {
+        HostMemoryBackend *hostmem = dimm->hostmem;
+        char *path = object_get_canonical_path_component(OBJECT(hostmem));
+
+        error_setg(errp, "the size of memdev %s (0x%" PRIx64 ") is too "
+                   "small to contain nvdimm label (0x%" PRIx64 ") and "
+                   "aligned PMEM (0x%" PRIx64 ")",
+                   path, memory_region_size(mr), nvdimm->label_size, align);
+        return;
+    }
+
+    memory_region_init_alias(&nvdimm->nvdimm_mr, OBJECT(dimm),
+                             "nvdimm-memory", mr, 0, pmem_size);
+    nvdimm->nvdimm_mr.align = align;
+}
+
+/*
+ * the caller should check the input parameters before calling
+ * label read/write functions.
+ */
+static void nvdimm_validate_rw_label_data(NVDIMMDevice *nvdimm, uint64_t size,
+                                        uint64_t offset)
+{
+    assert((nvdimm->label_size >= size + offset) && (offset + size > offset));
+}
+
+static void nvdimm_read_label_data(NVDIMMDevice *nvdimm, void *buf,
+                                   uint64_t size, uint64_t offset)
+{
+    nvdimm_validate_rw_label_data(nvdimm, size, offset);
+
+    memcpy(buf, nvdimm->label_data + offset, size);
+}
+
+static void nvdimm_write_label_data(NVDIMMDevice *nvdimm, const void *buf,
+                                    uint64_t size, uint64_t offset)
+{
+    MemoryRegion *mr;
+    PCDIMMDevice *dimm = PC_DIMM(nvdimm);
+    uint64_t backend_offset;
+
+    nvdimm_validate_rw_label_data(nvdimm, size, offset);
+
+    memcpy(nvdimm->label_data + offset, buf, size);
+
+    mr = host_memory_backend_get_memory(dimm->hostmem, &error_abort);
+    backend_offset = memory_region_size(mr) - nvdimm->label_size + offset;
+    memory_region_set_dirty(mr, backend_offset, size);
+}
+
+static MemoryRegion *nvdimm_get_vmstate_memory_region(PCDIMMDevice *dimm)
+{
+    return host_memory_backend_get_memory(dimm->hostmem, &error_abort);
+}
+
 static void nvdimm_class_init(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
+    PCDIMMDeviceClass *ddc = PC_DIMM_CLASS(oc);
+    NVDIMMClass *nvc = NVDIMM_CLASS(oc);
 
     /* nvdimm hotplug has not been supported yet. */
     dc->hotpluggable = false;
+
+    ddc->realize = nvdimm_realize;
+    ddc->get_memory_region = nvdimm_get_memory_region;
+    ddc->get_vmstate_memory_region = nvdimm_get_vmstate_memory_region;
+
+    nvc->read_label_data = nvdimm_read_label_data;
+    nvc->write_label_data = nvdimm_write_label_data;
 }
 
 static TypeInfo nvdimm_info = {
     .name          = TYPE_NVDIMM,
     .parent        = TYPE_PC_DIMM,
+    .class_size    = sizeof(NVDIMMClass),
     .class_init    = nvdimm_class_init,
+    .instance_size = sizeof(NVDIMMDevice),
+    .instance_init = nvdimm_init,
 };
 
 static void nvdimm_register_types(void)
-- 
MST

[Qemu-devel] [PULL v2 21/32] acpi: add aml_object_type

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Implement ObjectType which is used by NVDIMM _DSM method in
later patch

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/aml-build.h | 1 +
 hw/acpi/aml-build.c         | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 3952f85..90d733f 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -364,6 +364,7 @@ Aml *aml_refof(Aml *arg);
 Aml *aml_derefof(Aml *arg);
 Aml *aml_sizeof(Aml *arg);
 Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target);
+Aml *aml_object_type(Aml *object);
 
 void
 build_header(BIOSLinker *linker, GArray *table_data,
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 123160a..c807afa 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1488,6 +1488,14 @@ Aml *aml_concatenate(Aml *source1, Aml *source2, Aml *target)
                                  target);
 }
 
+/* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefObjectType */
+Aml *aml_object_type(Aml *object)
+{
+    Aml *var = aml_opcode(0x8E /* ObjectTypeOp */);
+    aml_append(var, object);
+    return var;
+}
+
 void
 build_header(BIOSLinker *linker, GArray *table_data,
              AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
-- 
MST

[Qemu-devel] [PULL v2 22/32] acpi: add aml_call5

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It will be used by NVDIMM ACPI

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/acpi/aml-build.h |  2 ++
 hw/acpi/aml-build.c         | 14 ++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 90d733f..eebfa5d 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -278,6 +278,8 @@ Aml *aml_call1(const char *method, Aml *arg1);
 Aml *aml_call2(const char *method, Aml *arg1, Aml *arg2);
 Aml *aml_call3(const char *method, Aml *arg1, Aml *arg2, Aml *arg3);
 Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4);
+Aml *aml_call5(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
+               Aml *arg5);
 Aml *aml_gpio_int(AmlConsumerAndProducer con_and_pro,
                   AmlLevelAndEdge edge_level,
                   AmlActiveHighAndLow active_level, AmlShared shared,
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index c807afa..b6a9019 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -665,6 +665,20 @@ Aml *aml_call4(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4)
     return var;
 }
 
+/* helper to call method with 5 arguments */
+Aml *aml_call5(const char *method, Aml *arg1, Aml *arg2, Aml *arg3, Aml *arg4,
+               Aml *arg5)
+{
+    Aml *var = aml_alloc();
+    build_append_namestring(var->buf, "%s", method);
+    aml_append(var, arg1);
+    aml_append(var, arg2);
+    aml_append(var, arg3);
+    aml_append(var, arg4);
+    aml_append(var, arg5);
+    return var;
+}
+
 /*
  * ACPI 5.0: 6.4.3.8.1 GPIO Connection Descriptor
  * Type 1, Large Item Name 0xC
-- 
MST

[Qemu-devel] [PULL v2 23/32] nvdimm acpi: set HDLE properly

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Now we pass HDLE to Qemu properly, use 0 for root device and use the
handle for nvdimm devices

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/nvdimm.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index b4c2262..14355f8 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -490,7 +490,7 @@ static void nvdimm_build_common_dsm(Aml *dev)
     Aml *method, *ifctx, *function, *dsm_mem, *unpatched, *result_size;
     uint8_t byte_list[1];
 
-    method = aml_method(NVDIMM_COMMON_DSM, 4, AML_SERIALIZED);
+    method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
     function = aml_arg(2);
     dsm_mem = aml_name(NVDIMM_ACPI_MEM_ADDR);
 
@@ -516,11 +516,10 @@ static void nvdimm_build_common_dsm(Aml *dev)
 
     /*
      * The HDLE indicates the DSM function is issued from which device,
-     * it is not used at this time as no function is supported yet.
-     * Currently we make it always be 0 for all the devices and will set
-     * the appropriate value once real function is implemented.
+     * it reserves 0 for root device and is the handle for NVDIMM devices.
+     * See the comments in nvdimm_slot_to_handle().
      */
-    aml_append(method, aml_store(aml_int(0x0), aml_name("HDLE")));
+    aml_append(method, aml_store(aml_arg(4), aml_name("HDLE")));
     aml_append(method, aml_store(aml_arg(1), aml_name("REVS")));
     aml_append(method, aml_store(aml_arg(2), aml_name("FUNC")));
 
@@ -542,13 +541,14 @@ static void nvdimm_build_common_dsm(Aml *dev)
     aml_append(dev, method);
 }
 
-static void nvdimm_build_device_dsm(Aml *dev)
+static void nvdimm_build_device_dsm(Aml *dev, uint32_t handle)
 {
     Aml *method;
 
     method = aml_method("_DSM", 4, AML_NOTSERIALIZED);
-    aml_append(method, aml_return(aml_call4(NVDIMM_COMMON_DSM, aml_arg(0),
-                                  aml_arg(1), aml_arg(2), aml_arg(3))));
+    aml_append(method, aml_return(aml_call5(NVDIMM_COMMON_DSM, aml_arg(0),
+                                  aml_arg(1), aml_arg(2), aml_arg(3),
+                                  aml_int(handle))));
     aml_append(dev, method);
 }
 
@@ -573,7 +573,7 @@ static void nvdimm_build_nvdimm_devices(GSList *device_list, Aml *root_dev)
          */
         aml_append(nvdimm_dev, aml_name_decl("_ADR", aml_int(handle)));
 
-        nvdimm_build_device_dsm(nvdimm_dev);
+        nvdimm_build_device_dsm(nvdimm_dev, handle);
         aml_append(root_dev, nvdimm_dev);
     }
 }
@@ -665,7 +665,9 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray *table_offsets,
     aml_append(dev, field);
 
     nvdimm_build_common_dsm(dev);
-    nvdimm_build_device_dsm(dev);
+
+    /* 0 is reserved for root device. */
+    nvdimm_build_device_dsm(dev, 0);
 
     nvdimm_build_nvdimm_devices(device_list, dev);
 
-- 
MST

[Qemu-devel] [PULL v2 24/32] nvdimm acpi: save arg3 of _DSM method

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Check if the input Arg3 is valid then store it into ARG3 if it is
needed

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/nvdimm.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 14355f8..95504e9 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -488,6 +488,7 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
 static void nvdimm_build_common_dsm(Aml *dev)
 {
     Aml *method, *ifctx, *function, *dsm_mem, *unpatched, *result_size;
+    Aml *pckg, *pckg_index, *pckg_buf;
     uint8_t byte_list[1];
 
     method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
@@ -524,6 +525,25 @@ static void nvdimm_build_common_dsm(Aml *dev)
     aml_append(method, aml_store(aml_arg(2), aml_name("FUNC")));
 
     /*
+     * The fourth parameter (Arg3) of _DSM is a package which contains
+     * a buffer, the layout of the buffer is specified by UUID (Arg0),
+     * Revision ID (Arg1) and Function Index (Arg2) which are documented
+     * in the DSM Spec.
+     */
+    pckg = aml_arg(3);
+    ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
+                   aml_int(4 /* Package */)) /* It is a Package? */,
+                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */,
+                   NULL));
+
+    pckg_index = aml_local(2);
+    pckg_buf = aml_local(3);
+    aml_append(ifctx, aml_store(aml_index(pckg, aml_int(0)), pckg_index));
+    aml_append(ifctx, aml_store(aml_derefof(pckg_index), pckg_buf));
+    aml_append(ifctx, aml_store(pckg_buf, aml_name("ARG3")));
+    aml_append(method, ifctx);
+
+    /*
      * tell QEMU about the real address of DSM memory, then QEMU
      * gets the control and fills the result in DSM memory.
      */
-- 
MST

[Qemu-devel] [PULL v2 25/32] nvdimm acpi: check UUID

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Check arg0 which indicates UUID to see if it is valid

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/nvdimm.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 95504e9..b01f2c6 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -487,19 +487,39 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
 
 static void nvdimm_build_common_dsm(Aml *dev)
 {
-    Aml *method, *ifctx, *function, *dsm_mem, *unpatched, *result_size;
+    Aml *method, *ifctx, *function, *handle, *uuid, *dsm_mem, *result_size;
+    Aml *elsectx, *unsupport, *unpatched, *expected_uuid, *uuid_invalid;
     Aml *pckg, *pckg_index, *pckg_buf;
     uint8_t byte_list[1];
 
     method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
+    uuid = aml_arg(0);
     function = aml_arg(2);
+    handle = aml_arg(4);
     dsm_mem = aml_name(NVDIMM_ACPI_MEM_ADDR);
 
     /*
      * do not support any method if DSM memory address has not been
      * patched.
      */
-    unpatched = aml_if(aml_equal(dsm_mem, aml_int(0x0)));
+    unpatched = aml_equal(dsm_mem, aml_int(0x0));
+
+    expected_uuid = aml_local(0);
+
+    ifctx = aml_if(aml_equal(handle, aml_int(0x0)));
+    aml_append(ifctx, aml_store(
+               aml_touuid("2F10E7A4-9E91-11E4-89D3-123B93F75CBA")
+               /* UUID for NVDIMM Root Device */, expected_uuid));
+    aml_append(method, ifctx);
+    elsectx = aml_else();
+    aml_append(elsectx, aml_store(
+               aml_touuid("4309AC30-0D11-11E4-9191-0800200C9A66")
+               /* UUID for NVDIMM Devices */, expected_uuid));
+    aml_append(method, elsectx);
+
+    uuid_invalid = aml_lnot(aml_equal(uuid, expected_uuid));
+
+    unsupport = aml_if(aml_or(unpatched, uuid_invalid, NULL));
 
     /*
      * function 0 is called to inquire what functions are supported by
@@ -508,19 +528,19 @@ static void nvdimm_build_common_dsm(Aml *dev)
     ifctx = aml_if(aml_equal(function, aml_int(0)));
     byte_list[0] = 0 /* No function Supported */;
     aml_append(ifctx, aml_return(aml_buffer(1, byte_list)));
-    aml_append(unpatched, ifctx);
+    aml_append(unsupport, ifctx);
 
     /* No function is supported yet. */
     byte_list[0] = 1 /* Not Supported */;
-    aml_append(unpatched, aml_return(aml_buffer(1, byte_list)));
-    aml_append(method, unpatched);
+    aml_append(unsupport, aml_return(aml_buffer(1, byte_list)));
+    aml_append(method, unsupport);
 
     /*
      * The HDLE indicates the DSM function is issued from which device,
      * it reserves 0 for root device and is the handle for NVDIMM devices.
      * See the comments in nvdimm_slot_to_handle().
      */
-    aml_append(method, aml_store(aml_arg(4), aml_name("HDLE")));
+    aml_append(method, aml_store(handle, aml_name("HDLE")));
     aml_append(method, aml_store(aml_arg(1), aml_name("REVS")));
     aml_append(method, aml_store(aml_arg(2), aml_name("FUNC")));
 
-- 
MST

[Qemu-devel] [PULL v2 26/32] nvdimm acpi: abstract the operations for root & nvdimm devices

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It separates the operations between root device and nvdimm devices
in order to introducing label functions support for nvdimm device

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/nvdimm.c | 74 ++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 56 insertions(+), 18 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index b01f2c6..07c95c1 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -406,6 +406,55 @@ struct NvdimmDsmFuncNoPayloadOut {
 } QEMU_PACKED;
 typedef struct NvdimmDsmFuncNoPayloadOut NvdimmDsmFuncNoPayloadOut;
 
+static void
+nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
+{
+    NvdimmDsmFunc0Out func0 = {
+        .len = cpu_to_le32(sizeof(func0)),
+        .supported_func = cpu_to_le32(supported_func),
+    };
+    cpu_physical_memory_write(dsm_mem_addr, &func0, sizeof(func0));
+}
+
+static void
+nvdimm_dsm_no_payload(uint32_t func_ret_status, hwaddr dsm_mem_addr)
+{
+    NvdimmDsmFuncNoPayloadOut out = {
+        .len = cpu_to_le32(sizeof(out)),
+        .func_ret_status = cpu_to_le32(func_ret_status),
+    };
+    cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
+}
+
+static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
+{
+    /*
+     * function 0 is called to inquire which functions are supported by
+     * OSPM
+     */
+    if (!in->function) {
+        nvdimm_dsm_function0(0 /* No function supported other than
+                                  function 0 */, dsm_mem_addr);
+        return;
+    }
+
+    /* No function except function 0 is supported yet. */
+    nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
+}
+
+static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
+{
+    /* See the comments in nvdimm_dsm_root(). */
+    if (!in->function) {
+        nvdimm_dsm_function0(0 /* No function supported other than
+                                  function 0 */, dsm_mem_addr);
+        return;
+    }
+
+    /* No function except function 0 is supported yet. */
+    nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
+}
+
 static uint64_t
 nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
 {
@@ -436,26 +485,15 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
     nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
                  in->handle, in->function);
 
-    /*
-     * function 0 is called to inquire which functions are supported by
-     * OSPM
-     */
-    if (in->function == 0) {
-        NvdimmDsmFunc0Out func0 = {
-            .len = cpu_to_le32(sizeof(func0)),
-             /* No function supported other than function 0 */
-            .supported_func = cpu_to_le32(0),
-        };
-        cpu_physical_memory_write(dsm_mem_addr, &func0, sizeof func0);
-    } else {
-        /* No function except function 0 is supported yet. */
-        NvdimmDsmFuncNoPayloadOut out = {
-            .len = cpu_to_le32(sizeof(out)),
-            .func_ret_status = cpu_to_le32(1)  /* Not Supported */,
-        };
-        cpu_physical_memory_write(dsm_mem_addr, &out, sizeof(out));
+     /* Handle 0 is reserved for NVDIMM Root Device. */
+    if (!in->handle) {
+        nvdimm_dsm_root(in, dsm_mem_addr);
+        goto exit;
     }
 
+    nvdimm_dsm_device(in, dsm_mem_addr);
+
+exit:
     g_free(in);
 }
 
-- 
MST

[Qemu-devel] [PULL v2 27/32] nvdimm acpi: check revision

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Currently only revision 1 is supported

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/nvdimm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 07c95c1..8b89285 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -485,6 +485,13 @@ nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
     nvdimm_debug("Revision %#x Handler %#x Function %#x.\n", in->revision,
                  in->handle, in->function);
 
+    if (in->revision != 0x1 /* Currently we only support DSM Spec Rev1. */) {
+        nvdimm_debug("Revision %#x is not supported, expect %#x.\n",
+                     in->revision, 0x1);
+        nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
+        goto exit;
+    }
+
      /* Handle 0 is reserved for NVDIMM Root Device. */
     if (!in->handle) {
         nvdimm_dsm_root(in, dsm_mem_addr);
-- 
MST

[Qemu-devel] [PULL v2 28/32] nvdimm acpi: support Get Namespace Label Size function

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Function 4 is used to get Namespace label size

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/nvdimm.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 127 insertions(+), 3 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 8b89285..4a25d8f 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -216,6 +216,26 @@ static uint32_t nvdimm_slot_to_dcr_index(int slot)
     return nvdimm_slot_to_spa_index(slot) + 1;
 }
 
+static NVDIMMDevice *nvdimm_get_device_by_handle(uint32_t handle)
+{
+    NVDIMMDevice *nvdimm = NULL;
+    GSList *list, *device_list = nvdimm_get_plugged_device_list();
+
+    for (list = device_list; list; list = list->next) {
+        NVDIMMDevice *nvd = list->data;
+        int slot = object_property_get_int(OBJECT(nvd), PC_DIMM_SLOT_PROP,
+                                           NULL);
+
+        if (nvdimm_slot_to_handle(slot) == handle) {
+            nvdimm = nvd;
+            break;
+        }
+    }
+
+    g_slist_free(device_list);
+    return nvdimm;
+}
+
 /* ACPI 6.0: 5.2.25.1 System Physical Address Range Structure */
 static void
 nvdimm_build_structure_spa(GArray *structures, DeviceState *dev)
@@ -406,6 +426,35 @@ struct NvdimmDsmFuncNoPayloadOut {
 } QEMU_PACKED;
 typedef struct NvdimmDsmFuncNoPayloadOut NvdimmDsmFuncNoPayloadOut;
 
+struct NvdimmFuncGetLabelSizeOut {
+    /* the size of buffer filled by QEMU. */
+    uint32_t len;
+    uint32_t func_ret_status; /* return status code. */
+    uint32_t label_size; /* the size of label data area. */
+    /*
+     * Maximum size of the namespace label data length supported by
+     * the platform in Get/Set Namespace Label Data functions.
+     */
+    uint32_t max_xfer;
+} QEMU_PACKED;
+typedef struct NvdimmFuncGetLabelSizeOut NvdimmFuncGetLabelSizeOut;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncGetLabelSizeOut) > 4096);
+
+struct NvdimmFuncGetLabelDataOut {
+    /* the size of buffer filled by QEMU. */
+    uint32_t len;
+    uint32_t func_ret_status; /* return status code. */
+    uint8_t out_buf[0]; /* the data got via Get Namesapce Label function. */
+} QEMU_PACKED;
+typedef struct NvdimmFuncGetLabelDataOut NvdimmFuncGetLabelDataOut;
+
+struct NvdimmFuncSetLabelDataIn {
+    uint32_t offset; /* the offset in the namespace label data area. */
+    uint32_t length; /* the size of data is to be written via the function. */
+    uint8_t in_buf[0]; /* the data written to label data area. */
+} QEMU_PACKED;
+typedef struct NvdimmFuncSetLabelDataIn NvdimmFuncSetLabelDataIn;
+
 static void
 nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
 {
@@ -442,16 +491,91 @@ static void nvdimm_dsm_root(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
     nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
 }
 
+/*
+ * the max transfer size is the max size transferred by both a
+ * 'Get Namespace Label Data' function and a 'Set Namespace Label Data'
+ * function.
+ */
+static uint32_t nvdimm_get_max_xfer_label_size(void)
+{
+    uint32_t max_get_size, max_set_size, dsm_memory_size = 4096;
+
+    /*
+     * the max data ACPI can read one time which is transferred by
+     * the response of 'Get Namespace Label Data' function.
+     */
+    max_get_size = dsm_memory_size - sizeof(NvdimmFuncGetLabelDataOut);
+
+    /*
+     * the max data ACPI can write one time which is transferred by
+     * 'Set Namespace Label Data' function.
+     */
+    max_set_size = dsm_memory_size - offsetof(NvdimmDsmIn, arg3) -
+                   sizeof(NvdimmFuncSetLabelDataIn);
+
+    return MIN(max_get_size, max_set_size);
+}
+
+/*
+ * DSM Spec Rev1 4.4 Get Namespace Label Size (Function Index 4).
+ *
+ * It gets the size of Namespace Label data area and the max data size
+ * that Get/Set Namespace Label Data functions can transfer.
+ */
+static void nvdimm_dsm_label_size(NVDIMMDevice *nvdimm, hwaddr dsm_mem_addr)
+{
+    NvdimmFuncGetLabelSizeOut label_size_out = {
+        .len = cpu_to_le32(sizeof(label_size_out)),
+    };
+    uint32_t label_size, mxfer;
+
+    label_size = nvdimm->label_size;
+    mxfer = nvdimm_get_max_xfer_label_size();
+
+    nvdimm_debug("label_size %#x, max_xfer %#x.\n", label_size, mxfer);
+
+    label_size_out.func_ret_status = cpu_to_le32(0 /* Success */);
+    label_size_out.label_size = cpu_to_le32(label_size);
+    label_size_out.max_xfer = cpu_to_le32(mxfer);
+
+    cpu_physical_memory_write(dsm_mem_addr, &label_size_out,
+                              sizeof(label_size_out));
+}
+
 static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
 {
+    NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(in->handle);
+
     /* See the comments in nvdimm_dsm_root(). */
     if (!in->function) {
-        nvdimm_dsm_function0(0 /* No function supported other than
-                                  function 0 */, dsm_mem_addr);
+        uint32_t supported_func = 0;
+
+        if (nvdimm && nvdimm->label_size) {
+            supported_func |= 0x1 /* Bit 0 indicates whether there is
+                                     support for any functions other
+                                     than function 0. */ |
+                              1 << 4 /* Get Namespace Label Size */;
+        }
+        nvdimm_dsm_function0(supported_func, dsm_mem_addr);
         return;
     }
 
-    /* No function except function 0 is supported yet. */
+    if (!nvdimm) {
+        nvdimm_dsm_no_payload(2 /* Non-Existing Memory Device */,
+                              dsm_mem_addr);
+        return;
+    }
+
+    /* Encode DSM function according to DSM Spec Rev1. */
+    switch (in->function) {
+    case 4 /* Get Namespace Label Size */:
+        if (nvdimm->label_size) {
+            nvdimm_dsm_label_size(nvdimm, dsm_mem_addr);
+            return;
+        }
+        break;
+    }
+
     nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
 }
 
-- 
MST

[Qemu-devel] [PULL v2 29/32] nvdimm acpi: support Get Namespace Label Data function

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Function 5 is used to get Namespace Label Data

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/nvdimm.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 82 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 4a25d8f..8ce0ea3 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -440,6 +440,14 @@ struct NvdimmFuncGetLabelSizeOut {
 typedef struct NvdimmFuncGetLabelSizeOut NvdimmFuncGetLabelSizeOut;
 QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncGetLabelSizeOut) > 4096);
 
+struct NvdimmFuncGetLabelDataIn {
+    uint32_t offset; /* the offset in the namespace label data area. */
+    uint32_t length; /* the size of data is to be read via the function. */
+} QEMU_PACKED;
+typedef struct NvdimmFuncGetLabelDataIn NvdimmFuncGetLabelDataIn;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncGetLabelDataIn) +
+                  offsetof(NvdimmDsmIn, arg3) > 4096);
+
 struct NvdimmFuncGetLabelDataOut {
     /* the size of buffer filled by QEMU. */
     uint32_t len;
@@ -447,6 +455,7 @@ struct NvdimmFuncGetLabelDataOut {
     uint8_t out_buf[0]; /* the data got via Get Namesapce Label function. */
 } QEMU_PACKED;
 typedef struct NvdimmFuncGetLabelDataOut NvdimmFuncGetLabelDataOut;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncGetLabelDataOut) > 4096);
 
 struct NvdimmFuncSetLabelDataIn {
     uint32_t offset; /* the offset in the namespace label data area. */
@@ -542,6 +551,71 @@ static void nvdimm_dsm_label_size(NVDIMMDevice *nvdimm, hwaddr dsm_mem_addr)
                               sizeof(label_size_out));
 }
 
+static uint32_t nvdimm_rw_label_data_check(NVDIMMDevice *nvdimm,
+                                           uint32_t offset, uint32_t length)
+{
+    uint32_t ret = 3 /* Invalid Input Parameters */;
+
+    if (offset + length < offset) {
+        nvdimm_debug("offset %#x + length %#x is overflow.\n", offset,
+                     length);
+        return ret;
+    }
+
+    if (nvdimm->label_size < offset + length) {
+        nvdimm_debug("position %#x is beyond label data (len = %#lx).\n",
+                     offset + length, nvdimm->label_size);
+        return ret;
+    }
+
+    if (length > nvdimm_get_max_xfer_label_size()) {
+        nvdimm_debug("length (%#x) is larger than max_xfer (%#x).\n",
+                     length, nvdimm_get_max_xfer_label_size());
+        return ret;
+    }
+
+    return 0 /* Success */;
+}
+
+/*
+ * DSM Spec Rev1 4.5 Get Namespace Label Data (Function Index 5).
+ */
+static void nvdimm_dsm_get_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
+                                      hwaddr dsm_mem_addr)
+{
+    NVDIMMClass *nvc = NVDIMM_GET_CLASS(nvdimm);
+    NvdimmFuncGetLabelDataIn *get_label_data;
+    NvdimmFuncGetLabelDataOut *get_label_data_out;
+    uint32_t status;
+    int size;
+
+    get_label_data = (NvdimmFuncGetLabelDataIn *)in->arg3;
+    le32_to_cpus(&get_label_data->offset);
+    le32_to_cpus(&get_label_data->length);
+
+    nvdimm_debug("Read Label Data: offset %#x length %#x.\n",
+                 get_label_data->offset, get_label_data->length);
+
+    status = nvdimm_rw_label_data_check(nvdimm, get_label_data->offset,
+                                        get_label_data->length);
+    if (status != 0 /* Success */) {
+        nvdimm_dsm_no_payload(status, dsm_mem_addr);
+        return;
+    }
+
+    size = sizeof(*get_label_data_out) + get_label_data->length;
+    assert(size <= 4096);
+    get_label_data_out = g_malloc(size);
+
+    get_label_data_out->len = cpu_to_le32(size);
+    get_label_data_out->func_ret_status = cpu_to_le32(0 /* Success */);
+    nvc->read_label_data(nvdimm, get_label_data_out->out_buf,
+                         get_label_data->length, get_label_data->offset);
+
+    cpu_physical_memory_write(dsm_mem_addr, get_label_data_out, size);
+    g_free(get_label_data_out);
+}
+
 static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
 {
     NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(in->handle);
@@ -554,7 +628,8 @@ static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
             supported_func |= 0x1 /* Bit 0 indicates whether there is
                                      support for any functions other
                                      than function 0. */ |
-                              1 << 4 /* Get Namespace Label Size */;
+                              1 << 4 /* Get Namespace Label Size */ |
+                              1 << 5 /* Get Namespace Label Data */;
         }
         nvdimm_dsm_function0(supported_func, dsm_mem_addr);
         return;
@@ -574,6 +649,12 @@ static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
             return;
         }
         break;
+    case 5 /* Get Namespace Label Data */:
+        if (nvdimm->label_size) {
+            nvdimm_dsm_get_label_data(nvdimm, in, dsm_mem_addr);
+            return;
+        }
+        break;
     }
 
     nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
-- 
MST

[Qemu-devel] [PULL v2 30/32] nvdimm acpi: support Set Namespace Label Data function

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

Function 6 is used to set Namespace Label Data

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/nvdimm.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 8ce0ea3..f11342b 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -463,6 +463,8 @@ struct NvdimmFuncSetLabelDataIn {
     uint8_t in_buf[0]; /* the data written to label data area. */
 } QEMU_PACKED;
 typedef struct NvdimmFuncSetLabelDataIn NvdimmFuncSetLabelDataIn;
+QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncSetLabelDataIn) +
+                  offsetof(NvdimmDsmIn, arg3) > 4096);
 
 static void
 nvdimm_dsm_function0(uint32_t supported_func, hwaddr dsm_mem_addr)
@@ -616,6 +618,39 @@ static void nvdimm_dsm_get_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
     g_free(get_label_data_out);
 }
 
+/*
+ * DSM Spec Rev1 4.6 Set Namespace Label Data (Function Index 6).
+ */
+static void nvdimm_dsm_set_label_data(NVDIMMDevice *nvdimm, NvdimmDsmIn *in,
+                                      hwaddr dsm_mem_addr)
+{
+    NVDIMMClass *nvc = NVDIMM_GET_CLASS(nvdimm);
+    NvdimmFuncSetLabelDataIn *set_label_data;
+    uint32_t status;
+
+    set_label_data = (NvdimmFuncSetLabelDataIn *)in->arg3;
+
+    le32_to_cpus(&set_label_data->offset);
+    le32_to_cpus(&set_label_data->length);
+
+    nvdimm_debug("Write Label Data: offset %#x length %#x.\n",
+                 set_label_data->offset, set_label_data->length);
+
+    status = nvdimm_rw_label_data_check(nvdimm, set_label_data->offset,
+                                        set_label_data->length);
+    if (status != 0 /* Success */) {
+        nvdimm_dsm_no_payload(status, dsm_mem_addr);
+        return;
+    }
+
+    assert(sizeof(*in) + sizeof(*set_label_data) + set_label_data->length <=
+           4096);
+
+    nvc->write_label_data(nvdimm, set_label_data->in_buf,
+                          set_label_data->length, set_label_data->offset);
+    nvdimm_dsm_no_payload(0 /* Success */, dsm_mem_addr);
+}
+
 static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
 {
     NVDIMMDevice *nvdimm = nvdimm_get_device_by_handle(in->handle);
@@ -629,7 +664,8 @@ static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
                                      support for any functions other
                                      than function 0. */ |
                               1 << 4 /* Get Namespace Label Size */ |
-                              1 << 5 /* Get Namespace Label Data */;
+                              1 << 5 /* Get Namespace Label Data */ |
+                              1 << 6 /* Set Namespace Label Data */;
         }
         nvdimm_dsm_function0(supported_func, dsm_mem_addr);
         return;
@@ -655,6 +691,12 @@ static void nvdimm_dsm_device(NvdimmDsmIn *in, hwaddr dsm_mem_addr)
             return;
         }
         break;
+    case 0x6 /* Set Namespace Label Data */:
+        if (nvdimm->label_size) {
+            nvdimm_dsm_set_label_data(nvdimm, in, dsm_mem_addr);
+            return;
+        }
+        break;
     }
 
     nvdimm_dsm_no_payload(1 /* Not Supported */, dsm_mem_addr);
-- 
MST

[Qemu-devel] [PULL v2 31/32] docs: add NVDIMM ACPI documentation

[Michael S. Tsirkin]

From: Xiao Guangrong <guangrong.xiao@linux.intel.com>

It describes the basic concepts of NVDIMM ACPI and the interfaces
between QEMU and the ACPI BIOS

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 docs/specs/acpi_nvdimm.txt | 132 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 docs/specs/acpi_nvdimm.txt

diff --git a/docs/specs/acpi_nvdimm.txt b/docs/specs/acpi_nvdimm.txt
new file mode 100644
index 0000000..0fdd251
--- /dev/null
+++ b/docs/specs/acpi_nvdimm.txt
@@ -0,0 +1,132 @@
+QEMU<->ACPI BIOS NVDIMM interface
+---------------------------------
+
+QEMU supports NVDIMM via ACPI. This document describes the basic concepts of
+NVDIMM ACPI and the interface between QEMU and the ACPI BIOS.
+
+NVDIMM ACPI Background
+----------------------
+NVDIMM is introduced in ACPI 6.0 which defines an NVDIMM root device under
+_SB scope with a _HID of “ACPI0012”. For each NVDIMM present or intended
+to be supported by platform, platform firmware also exposes an ACPI
+Namespace Device under the root device.
+
+The NVDIMM child devices under the NVDIMM root device are defined with _ADR
+corresponding to the NFIT device handle. The NVDIMM root device and the
+NVDIMM devices can have device specific methods (_DSM) to provide additional
+functions specific to a particular NVDIMM implementation.
+
+This is an example from ACPI 6.0, a platform contains one NVDIMM:
+
+Scope (\_SB){
+   Device (NVDR) // Root device
+   {
+      Name (_HID, “ACPI0012”)
+      Method (_STA) {...}
+      Method (_FIT) {...}
+      Method (_DSM, ...) {...}
+      Device (NVD)
+      {
+         Name(_ADR, h) //where h is NFIT Device Handle for this NVDIMM
+         Method (_DSM, ...) {...}
+      }
+   }
+}
+
+Method supported on both NVDIMM root device and NVDIMM device
+_DSM (Device Specific Method)
+   It is a control method that enables devices to provide device specific
+   control functions that are consumed by the device driver.
+   The NVDIMM DSM specification can be found at:
+        http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf
+
+   Arguments:
+   Arg0 – A Buffer containing a UUID (16 Bytes)
+   Arg1 – An Integer containing the Revision ID (4 Bytes)
+   Arg2 – An Integer containing the Function Index (4 Bytes)
+   Arg3 – A package containing parameters for the function specified by the
+          UUID, Revision ID, and Function Index
+
+   Return Value:
+   If Function Index = 0, a Buffer containing a function index bitfield.
+   Otherwise, the return value and type depends on the UUID, revision ID
+   and function index which are described in the DSM specification.
+
+Methods on NVDIMM ROOT Device
+_FIT(Firmware Interface Table)
+   It evaluates to a buffer returning data in the format of a series of NFIT
+   Type Structure.
+
+   Arguments: None
+
+   Return Value:
+   A Buffer containing a list of NFIT Type structure entries.
+
+   The detailed definition of the structure can be found at ACPI 6.0: 5.2.25
+   NVDIMM Firmware Interface Table (NFIT).
+
+QEMU NVDIMM Implemention
+========================
+QEMU uses 4 bytes IO Port starting from 0x0a18 and a RAM-based memory page
+for NVDIMM ACPI.
+
+Memory:
+   QEMU uses BIOS Linker/loader feature to ask BIOS to allocate a memory
+   page and dynamically patch its into a int32 object named "MEMA" in ACPI.
+
+   This page is RAM-based and it is used to transfer data between _DSM
+   method and QEMU. If ACPI has control, this pages is owned by ACPI which
+   writes _DSM input data to it, otherwise, it is owned by QEMU which
+   emulates _DSM access and writes the output data to it.
+
+   ACPI writes _DSM Input Data (based on the offset in the page):
+   [0x0 - 0x3]: 4 bytes, NVDIMM Device Handle, 0 is reserved for NVDIMM
+                Root device.
+   [0x4 - 0x7]: 4 bytes, Revision ID, that is the Arg1 of _DSM method.
+   [0x8 - 0xB]: 4 bytes. Function Index, that is the Arg2 of _DSM method.
+   [0xC - 0xFFF]: 4084 bytes, the Arg3 of _DSM method.
+
+   QEMU Writes Output Data (based on the offset in the page):
+   [0x0 - 0x3]: 4 bytes, the length of result
+   [0x4 - 0xFFF]: 4092 bytes, the DSM result filled by QEMU
+
+IO Port 0x0a18 - 0xa1b:
+   ACPI writes the address of the memory page allocated by BIOS to this
+   port then QEMU gets the control and fills the result in the memory page.
+
+   write Access:
+   [0x0a18 - 0xa1b]: 4 bytes, the address of the memory page allocated
+                     by BIOS.
+
+_DSM process diagram:
+---------------------
+"MEMA" indicates the address of memory page allocated by BIOS.
+
+ +----------------------+      +-----------------------+
+ |    1. OSPM           |      |    2. OSPM            |
+ | save _DSM input data |      |  write "MEMA" to      | Exit to QEMU
+ | to the page          +----->|  IO port 0x0a18       +------------+
+ | indicated by "MEMA"  |      |                       |            |
+ +----------------------+      +-----------------------+            |
+                                                                    |
+                                                                    v
+ +-------------   ----+       +-----------+      +------------------+--------+
+ |      5 QEMU        |       | 4 QEMU    |      |        3. QEMU            |
+ | write _DSM result  |       |  emulate  |      | get _DSM input data from  |
+ | to the page        +<------+ _DSM      +<-----+ the page indicated by the |
+ |                    |       |           |      | value from the IO port    |
+ +--------+-----------+       +-----------+      +---------------------------+
+          |
+          | Enter Guest
+          |
+          v
+ +--------------------------+      +--------------+
+ |     6 OSPM               |      |   7 OSPM     |
+ | result size is returned  |      |  _DSM return |
+ | by reading  DSM          +----->+              |
+ | result from the page     |      |              |
+ +--------------------------+      +--------------+
+
+ _FIT implementation
+ -------------------
+ TODO (will fill it when nvdimm hotplug is introduced)
-- 
MST

[Qemu-devel] [PULL v2 32/32] MAINTAINERS: add Marcel to PCI

[Michael S. Tsirkin]

Marcel is reviewing PCI patches anyway, things will
be easier if people remember to Cc him.

Cc: Marcel Apfelbaum <marcel@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index df990a8..fe2279e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -780,6 +780,7 @@ F: hw/ipack/
 
 PCI
 M: Michael S. Tsirkin <mst@redhat.com>
+M: Marcel Apfelbaum <marcel@redhat.com>
 S: Supported
 F: include/hw/pci/*
 F: hw/misc/pci-testdev.c
-- 
MST