[Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Haozhong Zhang]

This is the QEMU part patches that works with the associated Xen
patches to enable vNVDIMM support for Xen HVM domains. Xen relies on
QEMU to build guest NFIT and NVDIMM namespace devices, and allocate
guest address space for vNVDIMM devices.

All patches can be found at
  Xen:  https://github.com/hzzhan9/xen.git nvdimm-rfc-v3
  QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3

Patch 1 is to avoid dereferencing the NULL pointer to non-existing
label data, as the Xen side support for labels is not implemented yet.

Patch 2 & 3 add a memory backend dedicated for Xen usage and a hotplug
memory region for Xen guest, in order to make the existing nvdimm
device plugging path work on Xen.

Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when QEMU is
used as the Xen device model.


Haozhong Zhang (10):
  nvdimm: do not intiailize nvdimm->label_data if label size is zero
  hw/xen-hvm: create the hotplug memory region on Xen
  hostmem-xen: add a host memory backend for Xen
  nvdimm acpi: do not use fw_cfg on Xen
  hw/xen-hvm: initialize DM ACPI
  hw/xen-hvm: add function to copy ACPI into guest memory
  nvdimm acpi: copy NFIT to Xen guest
  nvdimm acpi: copy ACPI namespace device of vNVDIMM to Xen guest
  nvdimm acpi: do not build _FIT method on Xen
  hw/xen-hvm: enable building DM ACPI if vNVDIMM is enabled

 backends/Makefile.objs |   1 +
 backends/hostmem-xen.c | 108 ++++++++++++++++++++++++++
 backends/hostmem.c     |   9 +++
 hw/acpi/aml-build.c    |  10 ++-
 hw/acpi/nvdimm.c       |  79 ++++++++++++++-----
 hw/i386/pc.c           | 102 ++++++++++++++-----------
 hw/i386/xen/xen-hvm.c  | 204 ++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/mem/nvdimm.c        |  10 ++-
 hw/mem/pc-dimm.c       |   6 +-
 include/hw/i386/pc.h   |   1 +
 include/hw/xen/xen.h   |  25 ++++++
 stubs/xen-hvm.c        |  10 +++
 12 files changed, 495 insertions(+), 70 deletions(-)
 create mode 100644 backends/hostmem-xen.c

-- 
2.11.0

[Qemu-devel] [RFC QEMU PATCH v3 01/10] nvdimm: do not intiailize nvdimm->label_data if label size is zero

[Haozhong Zhang]

The memory region of vNVDIMM on Xen is a RAM memory region, so
memory_region_get_ram_ptr() cannot be used in nvdimm_realize() to get
a pointer to the label data area in that region. To be worse, it may
abort QEMU. As Xen currently does not support labels (i.e. label size
is 0) and every access in QEMU to labels is led by a label size check,
let's not intiailize nvdimm->label_data if the label size is 0.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
---
 hw/mem/nvdimm.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 952fce5ec8..3e58538b99 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -87,7 +87,15 @@ static void nvdimm_realize(PCDIMMDevice *dimm, Error **errp)
     align = memory_region_get_alignment(mr);
 
     pmem_size = size - nvdimm->label_size;
-    nvdimm->label_data = memory_region_get_ram_ptr(mr) + pmem_size;
+    /*
+     * The memory region of vNVDIMM on Xen is not a RAM memory region,
+     * so memory_region_get_ram_ptr() below will abort QEMU. In
+     * addition that Xen currently does not support vNVDIMM labels
+     * (i.e. label_size is zero here), let's not initialize of the
+     * pointer to label data if the label size is zero.
+     */
+    if (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) {
-- 
2.11.0

[Qemu-devel] [RFC QEMU PATCH v3 02/10] hw/xen-hvm: create the hotplug memory region on Xen

[Haozhong Zhang]

The guest physical address of vNVDIMM is allocated from the hotplug
memory region, which is not created when QEMU is used as Xen device
model. In order to use vNVDIMM for Xen HVM domains, this commit reuses
the code for pc machine type to create the hotplug memory region for
Xen HVM domains.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
CC: Eduardo Habkost <ehabkost@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
---
 hw/i386/pc.c          | 86 ++++++++++++++++++++++++++++-----------------------
 hw/i386/xen/xen-hvm.c |  2 ++
 include/hw/i386/pc.h  |  1 +
 3 files changed, 51 insertions(+), 38 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 21081041d5..5cbdce61a7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1347,6 +1347,53 @@ void xen_load_linux(PCMachineState *pcms)
     pcms->fw_cfg = fw_cfg;
 }
 
+void pc_memory_hotplug_init(PCMachineState *pcms, MemoryRegion *system_memory)
+{
+    MachineState *machine = MACHINE(pcms);
+    PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+    ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
+
+    if (!pcmc->has_reserved_memory || machine->ram_size >= machine->maxram_size)
+        return;
+
+    if (memory_region_size(&pcms->hotplug_memory.mr)) {
+        error_report("hotplug memory region has been initialized");
+        exit(EXIT_FAILURE);
+    }
+
+    if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
+        error_report("unsupported amount of memory slots: %"PRIu64,
+                     machine->ram_slots);
+        exit(EXIT_FAILURE);
+    }
+
+    if (QEMU_ALIGN_UP(machine->maxram_size,
+                      TARGET_PAGE_SIZE) != machine->maxram_size) {
+        error_report("maximum memory size must by aligned to multiple of "
+                     "%d bytes", TARGET_PAGE_SIZE);
+        exit(EXIT_FAILURE);
+    }
+
+    pcms->hotplug_memory.base =
+        ROUND_UP(0x100000000ULL + pcms->above_4g_mem_size, 1ULL << 30);
+
+    if (pcmc->enforce_aligned_dimm) {
+        /* size hotplug region assuming 1G page max alignment per slot */
+        hotplug_mem_size += (1ULL << 30) * machine->ram_slots;
+    }
+
+    if ((pcms->hotplug_memory.base + hotplug_mem_size) < hotplug_mem_size) {
+        error_report("unsupported amount of maximum memory: " RAM_ADDR_FMT,
+                     machine->maxram_size);
+        exit(EXIT_FAILURE);
+    }
+
+    memory_region_init(&pcms->hotplug_memory.mr, OBJECT(pcms),
+                       "hotplug-memory", hotplug_mem_size);
+    memory_region_add_subregion(system_memory, pcms->hotplug_memory.base,
+                                &pcms->hotplug_memory.mr);
+}
+
 void pc_memory_init(PCMachineState *pcms,
                     MemoryRegion *system_memory,
                     MemoryRegion *rom_memory,
@@ -1398,44 +1445,7 @@ void pc_memory_init(PCMachineState *pcms,
     }
 
     /* initialize hotplug memory address space */
-    if (pcmc->has_reserved_memory &&
-        (machine->ram_size < machine->maxram_size)) {
-        ram_addr_t hotplug_mem_size =
-            machine->maxram_size - machine->ram_size;
-
-        if (machine->ram_slots > ACPI_MAX_RAM_SLOTS) {
-            error_report("unsupported amount of memory slots: %"PRIu64,
-                         machine->ram_slots);
-            exit(EXIT_FAILURE);
-        }
-
-        if (QEMU_ALIGN_UP(machine->maxram_size,
-                          TARGET_PAGE_SIZE) != machine->maxram_size) {
-            error_report("maximum memory size must by aligned to multiple of "
-                         "%d bytes", TARGET_PAGE_SIZE);
-            exit(EXIT_FAILURE);
-        }
-
-        pcms->hotplug_memory.base =
-            ROUND_UP(0x100000000ULL + pcms->above_4g_mem_size, 1ULL << 30);
-
-        if (pcmc->enforce_aligned_dimm) {
-            /* size hotplug region assuming 1G page max alignment per slot */
-            hotplug_mem_size += (1ULL << 30) * machine->ram_slots;
-        }
-
-        if ((pcms->hotplug_memory.base + hotplug_mem_size) <
-            hotplug_mem_size) {
-            error_report("unsupported amount of maximum memory: " RAM_ADDR_FMT,
-                         machine->maxram_size);
-            exit(EXIT_FAILURE);
-        }
-
-        memory_region_init(&pcms->hotplug_memory.mr, OBJECT(pcms),
-                           "hotplug-memory", hotplug_mem_size);
-        memory_region_add_subregion(system_memory, pcms->hotplug_memory.base,
-                                    &pcms->hotplug_memory.mr);
-    }
+    pc_memory_hotplug_init(pcms, system_memory);
 
     /* Initialize PC system firmware */
     pc_system_firmware_init(rom_memory, !pcmc->pci_enabled);
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index d9ccd5d0d6..90163e1a1b 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -235,6 +235,8 @@ static void xen_ram_init(PCMachineState *pcms,
                                  pcms->above_4g_mem_size);
         memory_region_add_subregion(sysmem, 0x100000000ULL, &ram_hi);
     }
+
+    pc_memory_hotplug_init(pcms, sysmem);
 }
 
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 8226904524..b65c5dd5ec 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -249,6 +249,7 @@ void pc_memory_init(PCMachineState *pcms,
                     MemoryRegion *system_memory,
                     MemoryRegion *rom_memory,
                     MemoryRegion **ram_memory);
+void pc_memory_hotplug_init(PCMachineState *pcms, MemoryRegion *system_memory);
 qemu_irq pc_allocate_cpu_irq(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
 void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
-- 
2.11.0

[Qemu-devel] [RFC QEMU PATCH v3 03/10] hostmem-xen: add a host memory backend for Xen

[Haozhong Zhang]

vNVDIMM requires a host memory backend to allocate its backend
resources to the guest. When QEMU is used as Xen device model, the
backend resource allocation of vNVDIMM is managed out of QEMU. A new
host memory backend 'memory-backend-xen' is introduced to represent
the backend resource allocated by Xen. It simply creates a memory
region of the specified size as a placeholder in the guest address
space, which will be mapped by Xen to the actual backend resource.

Following example QEMU options create a vNVDIMM device backed by a 4GB
host PMEM region at host physical address 0x100000000:
   -object memory-backend-xen,id=mem1,host-addr=0x100000000,size=4G
   -device nvdimm,id=nvdimm1,memdev=mem1

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
---
 backends/Makefile.objs |   1 +
 backends/hostmem-xen.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++
 backends/hostmem.c     |   9 +++++
 hw/mem/pc-dimm.c       |   6 ++-
 4 files changed, 123 insertions(+), 1 deletion(-)
 create mode 100644 backends/hostmem-xen.c

diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 0400799efd..3096fde21f 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -5,6 +5,7 @@ common-obj-$(CONFIG_TPM) += tpm.o
 
 common-obj-y += hostmem.o hostmem-ram.o
 common-obj-$(CONFIG_LINUX) += hostmem-file.o
+common-obj-${CONFIG_XEN_BACKEND} += hostmem-xen.o
 
 common-obj-y += cryptodev.o
 common-obj-y += cryptodev-builtin.o
diff --git a/backends/hostmem-xen.c b/backends/hostmem-xen.c
new file mode 100644
index 0000000000..99211efd81
--- /dev/null
+++ b/backends/hostmem-xen.c
@@ -0,0 +1,108 @@
+/*
+ * QEMU Host Memory Backend for Xen
+ *
+ * Copyright(C) 2017 Intel Corporation.
+ *
+ * Author:
+ *   Haozhong Zhang <haozhong.zhang@intel.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/hostmem.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
+
+#define TYPE_MEMORY_BACKEND_XEN "memory-backend-xen"
+
+#define MEMORY_BACKEND_XEN(obj) \
+    OBJECT_CHECK(HostMemoryBackendXen, (obj), TYPE_MEMORY_BACKEND_XEN)
+
+typedef struct HostMemoryBackendXen HostMemoryBackendXen;
+
+struct HostMemoryBackendXen {
+    HostMemoryBackend parent_obj;
+
+    uint64_t host_addr;
+};
+
+static void xen_backend_get_host_addr(Object *obj, Visitor *v, const char *name,
+                                      void *opaque, Error **errp)
+{
+    HostMemoryBackendXen *backend = MEMORY_BACKEND_XEN(obj);
+    uint64_t value = backend->host_addr;
+
+    visit_type_size(v, name, &value, errp);
+}
+
+static void xen_backend_set_host_addr(Object *obj, Visitor *v, const char *name,
+                                      void *opaque, Error **errp)
+{
+    HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+    HostMemoryBackendXen *xb = MEMORY_BACKEND_XEN(obj);
+    Error *local_err = NULL;
+    uint64_t value;
+
+    if (memory_region_size(&backend->mr)) {
+        error_setg(&local_err, "cannot change property value");
+        goto out;
+    }
+
+    visit_type_size(v, name, &value, &local_err);
+    if (local_err) {
+        goto out;
+    }
+    xb->host_addr = value;
+
+ out:
+    error_propagate(errp, local_err);
+}
+
+static void xen_backend_alloc(HostMemoryBackend *backend, Error **errp)
+{
+    if (!backend->size) {
+        error_setg(errp, "can't create backend with size 0");
+        return;
+    }
+    memory_region_init(&backend->mr, OBJECT(backend), "hostmem-xen",
+                       backend->size);
+    backend->mr.align = getpagesize();
+}
+
+static void xen_backend_class_init(ObjectClass *oc, void *data)
+{
+    HostMemoryBackendClass *bc = MEMORY_BACKEND_CLASS(oc);
+
+    bc->alloc = xen_backend_alloc;
+
+    object_class_property_add(oc, "host-addr", "int",
+                              xen_backend_get_host_addr,
+                              xen_backend_set_host_addr,
+                              NULL, NULL, &error_abort);
+}
+
+static const TypeInfo xen_backend_info = {
+    .name = TYPE_MEMORY_BACKEND_XEN,
+    .parent = TYPE_MEMORY_BACKEND,
+    .class_init = xen_backend_class_init,
+    .instance_size = sizeof(HostMemoryBackendXen),
+};
+
+static void register_types(void)
+{
+    type_register_static(&xen_backend_info);
+}
+
+type_init(register_types);
diff --git a/backends/hostmem.c b/backends/hostmem.c
index ee2c2d5bfd..ba13a52994 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -12,6 +12,7 @@
 #include "qemu/osdep.h"
 #include "sysemu/hostmem.h"
 #include "hw/boards.h"
+#include "hw/xen/xen.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
 #include "qapi-types.h"
@@ -277,6 +278,14 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
             goto out;
         }
 
+        /*
+         * The backend storage of MEMORY_BACKEND_XEN is managed by Xen,
+         * so no further work in this function is needed.
+         */
+        if (xen_enabled() && !backend->mr.ram_block) {
+            goto out;
+        }
+
         ptr = memory_region_get_ram_ptr(&backend->mr);
         sz = memory_region_size(&backend->mr);
 
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index bdf6649083..7e1fe005ee 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -28,6 +28,7 @@
 #include "sysemu/kvm.h"
 #include "trace.h"
 #include "hw/virtio/vhost.h"
+#include "hw/xen/xen.h"
 
 typedef struct pc_dimms_capacity {
      uint64_t size;
@@ -108,7 +109,10 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms,
     }
 
     memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr);
-    vmstate_register_ram(vmstate_mr, dev);
+    /* memory-backend-xen is not backed by RAM. */
+    if (!xen_enabled()) {
+        vmstate_register_ram(vmstate_mr, dev);
+    }
     numa_set_mem_node_id(addr, memory_region_size(mr), dimm->node);
 
 out:
-- 
2.11.0

[Qemu-devel] [RFC QEMU PATCH v3 04/10] nvdimm acpi: do not use fw_cfg on Xen

[Haozhong Zhang]

Xen relies on QEMU to build guest ACPI for NVDIMM. However, no fw_cfg
is created when QEMU is used as Xen device model, so QEMU should avoid
using fw_cfg on Xen.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
---
 hw/acpi/nvdimm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 6ceea196e7..9121a766c6 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -32,6 +32,7 @@
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/nvram/fw_cfg.h"
 #include "hw/mem/nvdimm.h"
+#include "hw/xen/xen.h"
 
 static int nvdimm_device_list(Object *obj, void *opaque)
 {
@@ -890,8 +891,12 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
 
     state->dsm_mem = g_array_new(false, true /* clear */, 1);
     acpi_data_push(state->dsm_mem, sizeof(NvdimmDsmIn));
-    fw_cfg_add_file(fw_cfg, NVDIMM_DSM_MEM_FILE, state->dsm_mem->data,
-                    state->dsm_mem->len);
+
+    /* No fw_cfg is created when QEMU is used as Xen device model. */
+    if (!xen_enabled()) {
+        fw_cfg_add_file(fw_cfg, NVDIMM_DSM_MEM_FILE, state->dsm_mem->data,
+                        state->dsm_mem->len);
+    }
 
     nvdimm_init_fit_buffer(&state->fit_buf);
 }
-- 
2.11.0

[Qemu-devel] [RFC QEMU PATCH v3 05/10] hw/xen-hvm: initialize DM ACPI

[Haozhong Zhang]

Probe the base address and the length of guest ACPI buffer reserved
for copying ACPI from QEMU.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
cc: Anthony Perard <anthony.perard@citrix.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/xen/xen-hvm.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 90163e1a1b..ae895aaf03 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -18,6 +18,7 @@
 #include "hw/xen/xen_backend.h"
 #include "qmp-commands.h"
 
+#include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include "qemu/range.h"
 #include "sysemu/xen-mapcache.h"
@@ -86,6 +87,18 @@ typedef struct XenPhysmap {
     QLIST_ENTRY(XenPhysmap) list;
 } XenPhysmap;
 
+#define HVM_XS_DM_ACPI_ROOT    "/hvmloader/dm-acpi"
+#define HVM_XS_DM_ACPI_ADDRESS HVM_XS_DM_ACPI_ROOT"/address"
+#define HVM_XS_DM_ACPI_LENGTH  HVM_XS_DM_ACPI_ROOT"/length"
+
+typedef struct XenAcpiBuf {
+    ram_addr_t base;
+    ram_addr_t length;
+    ram_addr_t used;
+} XenAcpiBuf;
+
+static XenAcpiBuf *dm_acpi_buf;
+
 typedef struct XenIOState {
     ioservid_t ioservid;
     shared_iopage_t *shared_page;
@@ -110,6 +123,8 @@ typedef struct XenIOState {
     hwaddr free_phys_offset;
     const XenPhysmap *log_for_dirtybit;
 
+    XenAcpiBuf dm_acpi_buf;
+
     Notifier exit;
     Notifier suspend;
     Notifier wakeup;
@@ -1234,6 +1249,52 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
     xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
 }
 
+static int xen_dm_acpi_needed(PCMachineState *pcms)
+{
+    return 0;
+}
+
+static int dm_acpi_buf_init(XenIOState *state)
+{
+    char path[80], *value;
+    unsigned int len;
+
+    dm_acpi_buf = &state->dm_acpi_buf;
+
+    snprintf(path, sizeof(path),
+             "/local/domain/%d"HVM_XS_DM_ACPI_ADDRESS, xen_domid);
+    value = xs_read(state->xenstore, 0, path, &len);
+    if (!value) {
+        return -EINVAL;
+    }
+    if (qemu_strtoul(value, NULL, 16, &dm_acpi_buf->base)) {
+        return -EINVAL;
+    }
+
+    snprintf(path, sizeof(path),
+             "/local/domain/%d"HVM_XS_DM_ACPI_LENGTH, xen_domid);
+    value = xs_read(state->xenstore, 0, path, &len);
+    if (!value) {
+        return -EINVAL;
+    }
+    if (qemu_strtoul(value, NULL, 16, &dm_acpi_buf->length)) {
+        return -EINVAL;
+    }
+
+    dm_acpi_buf->used = 0;
+
+    return 0;
+}
+
+static int xen_dm_acpi_init(PCMachineState *pcms, XenIOState *state)
+{
+    if (!xen_dm_acpi_needed(pcms)) {
+        return 0;
+    }
+
+    return dm_acpi_buf_init(state);
+}
+
 void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
 {
     int i, rc;
@@ -1385,6 +1446,11 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
     /* Disable ACPI build because Xen handles it */
     pcms->acpi_build_enabled = false;
 
+    if (xen_dm_acpi_init(pcms, state)) {
+        error_report("failed to initialize xen ACPI");
+        goto err;
+    }
+
     return;
 
 err:
-- 
2.11.0

[Qemu-devel] [RFC QEMU PATCH v3 06/10] hw/xen-hvm: add function to copy ACPI into guest memory

[Haozhong Zhang]

Xen relies on QEMU to build guest NFIT and NVDIMM namespace devices,
and implements an interface to allow QEMU to copy its ACPI into guest
memory. This commit implements the QEMU side support.

The location of guest memory that can receive QEMU ACPI can be found
from XenStore entries /local/domain/$dom_id/hvmloader/dm-acpi/{address,length},
which have been handled by previous commit.

QEMU ACPI copied to guest is organized in blobs. For each blob, QEMU
creates following XenStore entries under
/local/domain/$dom_id/hvmloader/dm-acpi/$name to indicate its type,
location in above guest memory region and size.
 - type   the type of the passed ACPI, which can be the following
          values.
    * XEN_DM_ACPI_BLOB_TYPE_TABLE (0) indicates it's a complete ACPI
      table, and its signature is indicated by $name in the XenStore
      path.
    * XEN_DM_ACPI_BLOB_TYPE_NSDEV (1) indicates it's the body of a
      namespace device, and its device name is indicated by $name in
      the XenStore path.
 - offset  offset in byte from the beginning of above guest memory region
 - length  size in byte of the copied ACPI

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/i386/xen/xen-hvm.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/hw/xen/xen.h  |  18 ++++++++
 stubs/xen-hvm.c       |   6 +++
 3 files changed, 137 insertions(+)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index ae895aaf03..b74c4ffb9c 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -1286,6 +1286,20 @@ static int dm_acpi_buf_init(XenIOState *state)
     return 0;
 }
 
+static ram_addr_t dm_acpi_buf_alloc(size_t length)
+{
+    ram_addr_t addr;
+
+    if (dm_acpi_buf->length - dm_acpi_buf->used < length) {
+        return 0;
+    }
+
+    addr = dm_acpi_buf->base + dm_acpi_buf->used;
+    dm_acpi_buf->used += length;
+
+    return addr;
+}
+
 static int xen_dm_acpi_init(PCMachineState *pcms, XenIOState *state)
 {
     if (!xen_dm_acpi_needed(pcms)) {
@@ -1295,6 +1309,105 @@ static int xen_dm_acpi_init(PCMachineState *pcms, XenIOState *state)
     return dm_acpi_buf_init(state);
 }
 
+static int xs_write_dm_acpi_blob_entry(const char *name,
+                                       const char *entry, const char *value)
+{
+    XenIOState *state = container_of(dm_acpi_buf, XenIOState, dm_acpi_buf);
+    char path[80];
+
+    snprintf(path, sizeof(path),
+             "/local/domain/%d"HVM_XS_DM_ACPI_ROOT"/%s/%s",
+             xen_domid, name, entry);
+    if (!xs_write(state->xenstore, 0, path, value, strlen(value))) {
+        return -EIO;
+    }
+
+    return 0;
+}
+
+static size_t xen_memcpy_to_guest(ram_addr_t gpa,
+                                  const void *buf, size_t length)
+{
+    size_t copied = 0, size;
+    ram_addr_t s, e, offset, cur = gpa;
+    xen_pfn_t cur_pfn;
+    void *page;
+
+    if (!buf || !length) {
+        return 0;
+    }
+
+    s = gpa & TARGET_PAGE_MASK;
+    e = gpa + length;
+    if (e < s) {
+        return 0;
+    }
+
+    while (cur < e) {
+        cur_pfn = cur >> TARGET_PAGE_BITS;
+        offset = cur - (cur_pfn << TARGET_PAGE_BITS);
+        size = (length >= TARGET_PAGE_SIZE - offset) ?
+               TARGET_PAGE_SIZE - offset : length;
+
+        page = xenforeignmemory_map(xen_fmem, xen_domid, PROT_READ | PROT_WRITE,
+                                    1, &cur_pfn, NULL);
+        if (!page) {
+            break;
+        }
+
+        memcpy(page + offset, buf, size);
+        xenforeignmemory_unmap(xen_fmem, page, 1);
+
+        copied += size;
+        buf += size;
+        cur += size;
+        length -= size;
+    }
+
+    return copied;
+}
+
+int xen_acpi_copy_to_guest(const char *name, const void *blob, size_t length,
+                           int type)
+{
+    char value[21];
+    ram_addr_t buf_addr;
+    int rc;
+
+    if (type != XEN_DM_ACPI_BLOB_TYPE_TABLE &&
+        type != XEN_DM_ACPI_BLOB_TYPE_NSDEV) {
+        return -EINVAL;
+    }
+
+    buf_addr = dm_acpi_buf_alloc(length);
+    if (!buf_addr) {
+        return -ENOMEM;
+    }
+    if (xen_memcpy_to_guest(buf_addr, blob, length) != length) {
+        return -EIO;
+    }
+
+    snprintf(value, sizeof(value), "%d", type);
+    rc = xs_write_dm_acpi_blob_entry(name, "type", value);
+    if (rc) {
+        return rc;
+    }
+
+    snprintf(value, sizeof(value), "%"PRIu64, buf_addr - dm_acpi_buf->base);
+    rc = xs_write_dm_acpi_blob_entry(name, "offset", value);
+    if (rc) {
+        return rc;
+    }
+
+    snprintf(value, sizeof(value), "%"PRIu64, length);
+    rc = xs_write_dm_acpi_blob_entry(name, "length", value);
+    if (rc) {
+        return rc;
+    }
+
+    return 0;
+}
+
 void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
 {
     int i, rc;
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 7efcdaa8fe..38dcd1a7d4 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -48,4 +48,22 @@ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
 
 void xen_register_framebuffer(struct MemoryRegion *mr);
 
+/*
+ * Copy an ACPI blob from QEMU to HVM guest.
+ *
+ * Parameters:
+ *  name:   a unique name of the data blob; for XEN_DM_ACPI_BLOB_TYPE_NSDEV,
+ *          name should be less then 4 characters
+ *  blob:   the ACPI blob to be copied
+ *  length: the length in bytes of the ACPI blob
+ *  type:   the type of content in the ACPI blob, one of XEN_DM_ACPI_BLOB_TYPE_*
+ *
+ * Return:
+ *   0 on success; a non-zero error code on failures.
+ */
+#define XEN_DM_ACPI_BLOB_TYPE_TABLE 0 /* ACPI table */
+#define XEN_DM_ACPI_BLOB_TYPE_NSDEV 1 /* AML of ACPI namespace device */
+int xen_acpi_copy_to_guest(const char *name, const void *blob, size_t length,
+                           int type);
+
 #endif /* QEMU_HW_XEN_H */
diff --git a/stubs/xen-hvm.c b/stubs/xen-hvm.c
index 3ca6c51b21..58889ae0fb 100644
--- a/stubs/xen-hvm.c
+++ b/stubs/xen-hvm.c
@@ -61,3 +61,9 @@ void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
 void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
 {
 }
+
+int xen_acpi_copy_to_guest(const char *name, const void *blob, size_t length,
+                           int type)
+{
+    return -1;
+}
-- 
2.11.0

[Qemu-devel] [RFC QEMU PATCH v3 07/10] nvdimm acpi: copy NFIT to Xen guest

[Haozhong Zhang]

Xen relies on QEMU to build the guest NFIT.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
---
 hw/acpi/nvdimm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 9121a766c6..d9cdc5a531 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -404,6 +404,12 @@ static void nvdimm_build_nfit(AcpiNVDIMMState *state, GArray *table_offsets,
     build_header(linker, table_data,
                  (void *)(table_data->data + header), "NFIT",
                  sizeof(NvdimmNfitHeader) + fit_buf->fit->len, 1, NULL, NULL);
+
+    if (xen_enabled()) {
+        xen_acpi_copy_to_guest("NFIT", table_data->data + header,
+                               sizeof(NvdimmNfitHeader) + fit_buf->fit->len,
+                               XEN_DM_ACPI_BLOB_TYPE_TABLE);
+    }
 }
 
 #define NVDIMM_DSM_MEMORY_SIZE      4096
-- 
2.11.0

[Qemu-devel] [RFC QEMU PATCH v3 08/10] nvdimm acpi: copy ACPI namespace device of vNVDIMM to Xen guest

[Haozhong Zhang]

Xen relies on QEMU to build the ACPI namespace device of vNVDIMM for
Xen guest.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
---
 hw/acpi/nvdimm.c | 55 ++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 17 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index d9cdc5a531..bf887512ad 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -1226,22 +1226,8 @@ static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots)
     }
 }
 
-static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
-                              BIOSLinker *linker, GArray *dsm_dma_arrea,
-                              uint32_t ram_slots)
+static void nvdimm_build_ssdt_device(Aml *dev, uint32_t ram_slots)
 {
-    Aml *ssdt, *sb_scope, *dev;
-    int mem_addr_offset, nvdimm_ssdt;
-
-    acpi_add_table(table_offsets, table_data);
-
-    ssdt = init_aml_allocator();
-    acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
-
-    sb_scope = aml_scope("\\_SB");
-
-    dev = aml_device("NVDR");
-
     /*
      * ACPI 6.0: 9.20 NVDIMM Devices:
      *
@@ -1262,6 +1248,25 @@ static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
     nvdimm_build_fit(dev);
 
     nvdimm_build_nvdimm_devices(dev, ram_slots);
+}
+
+static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
+                              BIOSLinker *linker, GArray *dsm_dma_arrea,
+                              uint32_t ram_slots)
+{
+    Aml *ssdt, *sb_scope, *dev;
+    int mem_addr_offset, nvdimm_ssdt;
+
+    acpi_add_table(table_offsets, table_data);
+
+    ssdt = init_aml_allocator();
+    acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
+
+    sb_scope = aml_scope("\\_SB");
+
+    dev = aml_device("NVDR");
+
+    nvdimm_build_ssdt_device(dev, ram_slots);
 
     aml_append(sb_scope, dev);
     aml_append(ssdt, sb_scope);
@@ -1285,6 +1290,18 @@ static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
     free_aml_allocator();
 }
 
+static void nvdimm_build_xen_ssdt(uint32_t ram_slots)
+{
+    Aml *dev = init_aml_allocator();
+
+    nvdimm_build_ssdt_device(dev, ram_slots);
+    build_append_named_dword(dev->buf, NVDIMM_ACPI_MEM_ADDR);
+    xen_acpi_copy_to_guest("NVDR", dev->buf->data, dev->buf->len,
+                           XEN_DM_ACPI_BLOB_TYPE_NSDEV);
+
+    free_aml_allocator();
+}
+
 void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
                        BIOSLinker *linker, AcpiNVDIMMState *state,
                        uint32_t ram_slots)
@@ -1296,8 +1313,12 @@ void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
         return;
     }
 
-    nvdimm_build_ssdt(table_offsets, table_data, linker, state->dsm_mem,
-                      ram_slots);
+    if (!xen_enabled()) {
+        nvdimm_build_ssdt(table_offsets, table_data, linker, state->dsm_mem,
+                          ram_slots);
+    } else {
+        nvdimm_build_xen_ssdt(ram_slots);
+    }
 
     device_list = nvdimm_get_device_list();
     /* no NVDIMM device is plugged. */
-- 
2.11.0

[Qemu-devel] [RFC QEMU PATCH v3 09/10] nvdimm acpi: do not build _FIT method on Xen

[Haozhong Zhang]

Xen currently does not support vNVDIMM hotplug and always sets QEMU
option "maxmem" to be just enough for RAM and vNVDIMM, so it's not
necessary to build _FIT method when QEMU is used as Xen device model.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
---
 hw/acpi/nvdimm.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index bf887512ad..61789c3966 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -1245,7 +1245,14 @@ static void nvdimm_build_ssdt_device(Aml *dev, uint32_t ram_slots)
 
     /* 0 is reserved for root device. */
     nvdimm_build_device_dsm(dev, 0);
-    nvdimm_build_fit(dev);
+    /*
+     * Xen does not support vNVDIMM hotplug, and always sets the QEMU
+     * option "maxmem" to be just enough for RAM and static plugged
+     * vNVDIMM, so it's unnecessary to build _FIT method on Xen.
+     */
+    if (!xen_enabled()) {
+        nvdimm_build_fit(dev);
+    }
 
     nvdimm_build_nvdimm_devices(dev, ram_slots);
 }
-- 
2.11.0

[Qemu-devel] [RFC QEMU PATCH v3 10/10] hw/xen-hvm: enable building DM ACPI if vNVDIMM is enabled

[Haozhong Zhang]

If the machine option 'nvdimm' is enabled and QEMU is used as Xen
device model, construct the guest NFIT and ACPI namespace devices of
vNVDIMM and copy them into guest memory.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Anthony Perard <anthony.perard@citrix.com>
---
 hw/acpi/aml-build.c   | 10 +++++++---
 hw/i386/pc.c          | 16 ++++++++++------
 hw/i386/xen/xen-hvm.c | 25 +++++++++++++++++++++++--
 include/hw/xen/xen.h  |  7 +++++++
 stubs/xen-hvm.c       |  4 ++++
 5 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 36a6cc450e..5f57c1bef3 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -22,6 +22,7 @@
 #include "qemu/osdep.h"
 #include <glib/gprintf.h>
 #include "hw/acpi/aml-build.h"
+#include "hw/xen/xen.h"
 #include "qemu/bswap.h"
 #include "qemu/bitops.h"
 #include "sysemu/numa.h"
@@ -1531,9 +1532,12 @@ build_header(BIOSLinker *linker, GArray *table_data,
     h->oem_revision = cpu_to_le32(1);
     memcpy(h->asl_compiler_id, ACPI_BUILD_APPNAME4, 4);
     h->asl_compiler_revision = cpu_to_le32(1);
-    /* Checksum to be filled in by Guest linker */
-    bios_linker_loader_add_checksum(linker, ACPI_BUILD_TABLE_FILE,
-        tbl_offset, len, checksum_offset);
+    /* No linker is used when QEMU is used as Xen device model. */
+    if (!xen_enabled()) {
+        /* Checksum to be filled in by Guest linker */
+        bios_linker_loader_add_checksum(linker, ACPI_BUILD_TABLE_FILE,
+                                        tbl_offset, len, checksum_offset);
+    }
 }
 
 void *acpi_data_push(GArray *table_data, unsigned size)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5cbdce61a7..7101d380a0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1252,12 +1252,16 @@ void pc_machine_done(Notifier *notifier, void *data)
         }
     }
 
-    acpi_setup();
-    if (pcms->fw_cfg) {
-        pc_build_smbios(pcms);
-        pc_build_feature_control_file(pcms);
-        /* update FW_CFG_NB_CPUS to account for -device added CPUs */
-        fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
+    if (!xen_enabled()) {
+        acpi_setup();
+        if (pcms->fw_cfg) {
+            pc_build_smbios(pcms);
+            pc_build_feature_control_file(pcms);
+            /* update FW_CFG_NB_CPUS to account for -device added CPUs */
+            fw_cfg_modify_i16(pcms->fw_cfg, FW_CFG_NB_CPUS, pcms->boot_cpus);
+        }
+    } else {
+        xen_dm_acpi_setup(pcms);
     }
 
     if (pcms->apic_id_limit > 255) {
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index b74c4ffb9c..d81cc7dbbc 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -265,7 +265,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
         /* RAM already populated in Xen */
         fprintf(stderr, "%s: do not alloc "RAM_ADDR_FMT
                 " bytes of ram at "RAM_ADDR_FMT" when runstate is INMIGRATE\n",
-                __func__, size, ram_addr); 
+                __func__, size, ram_addr);
         return;
     }
 
@@ -1251,7 +1251,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
 
 static int xen_dm_acpi_needed(PCMachineState *pcms)
 {
-    return 0;
+    return pcms->acpi_nvdimm_state.is_enabled;
 }
 
 static int dm_acpi_buf_init(XenIOState *state)
@@ -1309,6 +1309,20 @@ static int xen_dm_acpi_init(PCMachineState *pcms, XenIOState *state)
     return dm_acpi_buf_init(state);
 }
 
+static void xen_dm_acpi_nvdimm_setup(PCMachineState *pcms)
+{
+    GArray *table_offsets = g_array_new(false, true /* clear */,
+                                        sizeof(uint32_t));
+    GArray *table_data = g_array_new(false, true /* clear */, 1);
+
+    nvdimm_build_acpi(table_offsets, table_data,
+                      NULL, &pcms->acpi_nvdimm_state,
+                      MACHINE(pcms)->ram_slots);
+
+    g_array_free(table_offsets, true);
+    g_array_free(table_data, true);
+}
+
 static int xs_write_dm_acpi_blob_entry(const char *name,
                                        const char *entry, const char *value)
 {
@@ -1408,6 +1422,13 @@ int xen_acpi_copy_to_guest(const char *name, const void *blob, size_t length,
     return 0;
 }
 
+void xen_dm_acpi_setup(PCMachineState *pcms)
+{
+    if (pcms->acpi_nvdimm_state.is_enabled) {
+        xen_dm_acpi_nvdimm_setup(pcms);
+    }
+}
+
 void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
 {
     int i, rc;
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 38dcd1a7d4..8c48195e12 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -66,4 +66,11 @@ void xen_register_framebuffer(struct MemoryRegion *mr);
 int xen_acpi_copy_to_guest(const char *name, const void *blob, size_t length,
                            int type);
 
+/*
+ * Build guest ACPI (i.e. DM ACPI, or ACPI built by device model) and
+ * copy them into guest memory. Xen hvmloader will load and merge DM
+ * ACPI with the guest ACPI built by itself.
+ */
+void xen_dm_acpi_setup(PCMachineState *pcms);
+
 #endif /* QEMU_HW_XEN_H */
diff --git a/stubs/xen-hvm.c b/stubs/xen-hvm.c
index 58889ae0fb..c1a6d21efa 100644
--- a/stubs/xen-hvm.c
+++ b/stubs/xen-hvm.c
@@ -67,3 +67,7 @@ int xen_acpi_copy_to_guest(const char *name, const void *blob, size_t length,
 {
     return -1;
 }
+
+void xen_dm_acpi_setup(PCMachineState *pcms)
+{
+}
-- 
2.11.0

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest
Message-id: 20170911044157.15403-1-haozhong.zhang@intel.com
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20170911044157.15403-1-haozhong.zhang@intel.com -> patchew/20170911044157.15403-1-haozhong.zhang@intel.com
Switched to a new branch 'test'
d5f5b8faf2 hw/xen-hvm: enable building DM ACPI if vNVDIMM is enabled
73b52971f5 nvdimm acpi: do not build _FIT method on Xen
1c6eeac40e nvdimm acpi: copy ACPI namespace device of vNVDIMM to Xen guest
f2d6097366 nvdimm acpi: copy NFIT to Xen guest
69ddac3d65 hw/xen-hvm: add function to copy ACPI into guest memory
cae88474b2 hw/xen-hvm: initialize DM ACPI
23a0e4204a nvdimm acpi: do not use fw_cfg on Xen
e298be5d96 hostmem-xen: add a host memory backend for Xen
f069bbb659 hw/xen-hvm: create the hotplug memory region on Xen
69b6b6e9fa nvdimm: do not intiailize nvdimm->label_data if label size is zero

=== OUTPUT BEGIN ===
Checking PATCH 1/10: nvdimm: do not intiailize nvdimm->label_data if label size is zero...
ERROR: braces {} are necessary for all arms of this statement
#33: FILE: hw/mem/nvdimm.c:97:
+    if (nvdimm->label_size)
[...]

total: 1 errors, 0 warnings, 16 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/10: hw/xen-hvm: create the hotplug memory region on Xen...
ERROR: braces {} are necessary for all arms of this statement
#29: FILE: hw/i386/pc.c:1356:
+    if (!pcmc->has_reserved_memory || machine->ram_size >= machine->maxram_size)
[...]

total: 1 errors, 0 warnings, 113 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/10: hostmem-xen: add a host memory backend for Xen...
Checking PATCH 4/10: nvdimm acpi: do not use fw_cfg on Xen...
Checking PATCH 5/10: hw/xen-hvm: initialize DM ACPI...
Checking PATCH 6/10: hw/xen-hvm: add function to copy ACPI into guest memory...
Checking PATCH 7/10: nvdimm acpi: copy NFIT to Xen guest...
Checking PATCH 8/10: nvdimm acpi: copy ACPI namespace device of vNVDIMM to Xen guest...
Checking PATCH 9/10: nvdimm acpi: do not build _FIT method on Xen...
Checking PATCH 10/10: hw/xen-hvm: enable building DM ACPI if vNVDIMM is enabled...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Igor Mammedov]

On Mon, 11 Sep 2017 12:41:47 +0800
Haozhong Zhang <haozhong.zhang@intel.com> wrote:

> This is the QEMU part patches that works with the associated Xen
> patches to enable vNVDIMM support for Xen HVM domains. Xen relies on
> QEMU to build guest NFIT and NVDIMM namespace devices, and allocate
> guest address space for vNVDIMM devices.
> 
> All patches can be found at
>   Xen:  https://github.com/hzzhan9/xen.git nvdimm-rfc-v3
>   QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3
> 
> Patch 1 is to avoid dereferencing the NULL pointer to non-existing
> label data, as the Xen side support for labels is not implemented yet.
> 
> Patch 2 & 3 add a memory backend dedicated for Xen usage and a hotplug
> memory region for Xen guest, in order to make the existing nvdimm
> device plugging path work on Xen.
> 
> Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when QEMU is
> used as the Xen device model.

I've skimmed over patch-set and can't say that I'm happy with
number of xen_enabled() invariants it introduced as well as
with partial blobs it creates.

I'd like to reduce above and a way to do this might be making xen 
 1. use fw_cfg
 2. fetch QEMU build acpi tables from fw_cfg
 3. extract nvdim tables (which is trivial) and use them

looking at xen_load_linux(), it seems possible to use fw_cfg.

So what's stopping xen from using it elsewhere?,
instead of adding more xen specific code to do 'the same'
job and not reusing/sharing common code with tcg/kvm.


> Haozhong Zhang (10):
>   nvdimm: do not intiailize nvdimm->label_data if label size is zero
>   hw/xen-hvm: create the hotplug memory region on Xen
>   hostmem-xen: add a host memory backend for Xen
>   nvdimm acpi: do not use fw_cfg on Xen
>   hw/xen-hvm: initialize DM ACPI
>   hw/xen-hvm: add function to copy ACPI into guest memory
>   nvdimm acpi: copy NFIT to Xen guest
>   nvdimm acpi: copy ACPI namespace device of vNVDIMM to Xen guest
>   nvdimm acpi: do not build _FIT method on Xen
>   hw/xen-hvm: enable building DM ACPI if vNVDIMM is enabled
> 
>  backends/Makefile.objs |   1 +
>  backends/hostmem-xen.c | 108 ++++++++++++++++++++++++++
>  backends/hostmem.c     |   9 +++
>  hw/acpi/aml-build.c    |  10 ++-
>  hw/acpi/nvdimm.c       |  79 ++++++++++++++-----
>  hw/i386/pc.c           | 102 ++++++++++++++-----------
>  hw/i386/xen/xen-hvm.c  | 204 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  hw/mem/nvdimm.c        |  10 ++-
>  hw/mem/pc-dimm.c       |   6 +-
>  include/hw/i386/pc.h   |   1 +
>  include/hw/xen/xen.h   |  25 ++++++
>  stubs/xen-hvm.c        |  10 +++
>  12 files changed, 495 insertions(+), 70 deletions(-)
>  create mode 100644 backends/hostmem-xen.c
> 

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Stefano Stabellini]

CC'ing xen-devel, and the Xen tools and x86 maintainers.

On Mon, 11 Sep 2017, Igor Mammedov wrote:
> On Mon, 11 Sep 2017 12:41:47 +0800
> Haozhong Zhang <haozhong.zhang@intel.com> wrote:
> 
> > This is the QEMU part patches that works with the associated Xen
> > patches to enable vNVDIMM support for Xen HVM domains. Xen relies on
> > QEMU to build guest NFIT and NVDIMM namespace devices, and allocate
> > guest address space for vNVDIMM devices.
> > 
> > All patches can be found at
> >   Xen:  https://github.com/hzzhan9/xen.git nvdimm-rfc-v3
> >   QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3
> > 
> > Patch 1 is to avoid dereferencing the NULL pointer to non-existing
> > label data, as the Xen side support for labels is not implemented yet.
> > 
> > Patch 2 & 3 add a memory backend dedicated for Xen usage and a hotplug
> > memory region for Xen guest, in order to make the existing nvdimm
> > device plugging path work on Xen.
> > 
> > Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when QEMU is
> > used as the Xen device model.
> 
> I've skimmed over patch-set and can't say that I'm happy with
> number of xen_enabled() invariants it introduced as well as
> with partial blobs it creates.

I have not read the series (Haozhong, please CC me, Anthony and
xen-devel to the whole series next time), but yes, indeed. Let's not add
more xen_enabled() if possible.

Haozhong, was there a design document thread on xen-devel about this? If
so, did it reach a conclusion? Was the design accepted? If so, please
add a link to the design doc in the introductory email, so that
everybody can read it and be on the same page.


> I'd like to reduce above and a way to do this might be making xen 
>  1. use fw_cfg
>  2. fetch QEMU build acpi tables from fw_cfg
>  3. extract nvdim tables (which is trivial) and use them
> 
> looking at xen_load_linux(), it seems possible to use fw_cfg.
> 
> So what's stopping xen from using it elsewhere?,
> instead of adding more xen specific code to do 'the same'
> job and not reusing/sharing common code with tcg/kvm.

So far, ACPI tables have not been generated by QEMU. Xen HVM machines
rely on a firmware-like application called "hvmloader" that runs in
guest context and generates the ACPI tables. I have no opinions on
hvmloader and I'll let the Xen maintainers talk about it. However, keep
in mind that with an HVM guest some devices are emulated by Xen and/or
by other device emulators that can run alongside QEMU. QEMU doesn't have
a full few of the system.

Here the question is: does it have to be QEMU the one to generate the
ACPI blobs for the nvdimm? It would be nicer if it was up to hvmloader
like the rest, instead of introducing this split-brain design about
ACPI. We need to see a design doc to fully understand this.

If the design doc thread led into thinking that it has to be QEMU to
generate them, then would it make the code nicer if we used fw_cfg to
get the (full or partial) tables from QEMU, as Igor suggested?


> > Haozhong Zhang (10):
> >   nvdimm: do not intiailize nvdimm->label_data if label size is zero
> >   hw/xen-hvm: create the hotplug memory region on Xen
> >   hostmem-xen: add a host memory backend for Xen
> >   nvdimm acpi: do not use fw_cfg on Xen
> >   hw/xen-hvm: initialize DM ACPI
> >   hw/xen-hvm: add function to copy ACPI into guest memory
> >   nvdimm acpi: copy NFIT to Xen guest
> >   nvdimm acpi: copy ACPI namespace device of vNVDIMM to Xen guest
> >   nvdimm acpi: do not build _FIT method on Xen
> >   hw/xen-hvm: enable building DM ACPI if vNVDIMM is enabled
> > 
> >  backends/Makefile.objs |   1 +
> >  backends/hostmem-xen.c | 108 ++++++++++++++++++++++++++
> >  backends/hostmem.c     |   9 +++
> >  hw/acpi/aml-build.c    |  10 ++-
> >  hw/acpi/nvdimm.c       |  79 ++++++++++++++-----
> >  hw/i386/pc.c           | 102 ++++++++++++++-----------
> >  hw/i386/xen/xen-hvm.c  | 204 ++++++++++++++++++++++++++++++++++++++++++++++++-
> >  hw/mem/nvdimm.c        |  10 ++-
> >  hw/mem/pc-dimm.c       |   6 +-
> >  include/hw/i386/pc.h   |   1 +
> >  include/hw/xen/xen.h   |  25 ++++++
> >  stubs/xen-hvm.c        |  10 +++
> >  12 files changed, 495 insertions(+), 70 deletions(-)
> >  create mode 100644 backends/hostmem-xen.c
> > 
> 

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Haozhong Zhang]

On 09/11/17 11:52 -0700, Stefano Stabellini wrote:
> CC'ing xen-devel, and the Xen tools and x86 maintainers.
> 
> On Mon, 11 Sep 2017, Igor Mammedov wrote:
> > On Mon, 11 Sep 2017 12:41:47 +0800
> > Haozhong Zhang <haozhong.zhang@intel.com> wrote:
> > 
> > > This is the QEMU part patches that works with the associated Xen
> > > patches to enable vNVDIMM support for Xen HVM domains. Xen relies on
> > > QEMU to build guest NFIT and NVDIMM namespace devices, and allocate
> > > guest address space for vNVDIMM devices.
> > > 
> > > All patches can be found at
> > >   Xen:  https://github.com/hzzhan9/xen.git nvdimm-rfc-v3
> > >   QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3
> > > 
> > > Patch 1 is to avoid dereferencing the NULL pointer to non-existing
> > > label data, as the Xen side support for labels is not implemented yet.
> > > 
> > > Patch 2 & 3 add a memory backend dedicated for Xen usage and a hotplug
> > > memory region for Xen guest, in order to make the existing nvdimm
> > > device plugging path work on Xen.
> > > 
> > > Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when QEMU is
> > > used as the Xen device model.
> > 
> > I've skimmed over patch-set and can't say that I'm happy with
> > number of xen_enabled() invariants it introduced as well as
> > with partial blobs it creates.
> 
> I have not read the series (Haozhong, please CC me, Anthony and
> xen-devel to the whole series next time), but yes, indeed. Let's not add
> more xen_enabled() if possible.
> 
> Haozhong, was there a design document thread on xen-devel about this? If
> so, did it reach a conclusion? Was the design accepted? If so, please
> add a link to the design doc in the introductory email, so that
> everybody can read it and be on the same page.

Yes, there is a design [1] discussed and reviewed. Section 4.3 discussed
the guest ACPI.

[1] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01921.html

> 
> 
> > I'd like to reduce above and a way to do this might be making xen 
> >  1. use fw_cfg
> >  2. fetch QEMU build acpi tables from fw_cfg
> >  3. extract nvdim tables (which is trivial) and use them
> > 
> > looking at xen_load_linux(), it seems possible to use fw_cfg.
> > 
> > So what's stopping xen from using it elsewhere?,
> > instead of adding more xen specific code to do 'the same'
> > job and not reusing/sharing common code with tcg/kvm.
> 
> So far, ACPI tables have not been generated by QEMU. Xen HVM machines
> rely on a firmware-like application called "hvmloader" that runs in
> guest context and generates the ACPI tables. I have no opinions on
> hvmloader and I'll let the Xen maintainers talk about it. However, keep
> in mind that with an HVM guest some devices are emulated by Xen and/or
> by other device emulators that can run alongside QEMU. QEMU doesn't have
> a full few of the system.
> 
> Here the question is: does it have to be QEMU the one to generate the
> ACPI blobs for the nvdimm? It would be nicer if it was up to hvmloader
> like the rest, instead of introducing this split-brain design about
> ACPI. We need to see a design doc to fully understand this.
>

hvmloader runs in the guest and is responsible to build/load guest
ACPI. However, it's not capable to build AML at runtime (for the lack
of AML builder). If any guest ACPI object is needed (e.g. by guest
DSDT), it has to be generated from ASL by iasl at Xen compile time and
then be loaded by hvmloader at runtime.

Xen includes an OperationRegion "BIOS" in the static generated guest
DSDT, whose address is hardcoded and which contains a list of values
filled by hvmloader at runtime. Other ACPI objects can refer to those
values (e.g., the number of vCPUs). But it's not enough for generating
guest NVDIMM ACPI objects at compile time and then being customized
and loaded by hvmload, because its structure (i.e., the number of
namespace devices) cannot be decided util the guest config is known.

Alternatively, we may introduce an AML builder in hvmloader and build
all guest ACPI completely in hvmloader. Looking at the similar
implementation in QEMU, it would not be small, compared to the current
size of hvmloader. Besides, I'm still going to let QEMU handle guest
NVDIMM _DSM and _FIT calls, which is another reason I use QEMU to
build NVDIMM ACPI.

> If the design doc thread led into thinking that it has to be QEMU to
> generate them, then would it make the code nicer if we used fw_cfg to
> get the (full or partial) tables from QEMU, as Igor suggested?

I'll have a look at the code (which I didn't notice) pointed by Igor.

One possible issue to use fw_cfg is how to avoid the conflict between
ACPI built by QEMU and ACPI built by hvmloader (e.g., both may use the
same table signature / device names / ...). In my current design, QEMU
will pass the table signatures and device names used in its ACPI to
Xen, and Xen can check the conflict with its own ACPI. Perhaps we can
add necessary functions in fw_cfg as well. Anyway, let me first look
at the code.

Thanks,
Haozhong

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Konrad Rzeszutek Wilk]

On Tue, Sep 12, 2017 at 11:15:09AM +0800, Haozhong Zhang wrote:
> On 09/11/17 11:52 -0700, Stefano Stabellini wrote:
> > CC'ing xen-devel, and the Xen tools and x86 maintainers.
> > 
> > On Mon, 11 Sep 2017, Igor Mammedov wrote:
> > > On Mon, 11 Sep 2017 12:41:47 +0800
> > > Haozhong Zhang <haozhong.zhang@intel.com> wrote:
> > > 
> > > > This is the QEMU part patches that works with the associated Xen
> > > > patches to enable vNVDIMM support for Xen HVM domains. Xen relies on
> > > > QEMU to build guest NFIT and NVDIMM namespace devices, and allocate
> > > > guest address space for vNVDIMM devices.
> > > > 
> > > > All patches can be found at
> > > >   Xen:  https://github.com/hzzhan9/xen.git nvdimm-rfc-v3
> > > >   QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3
> > > > 
> > > > Patch 1 is to avoid dereferencing the NULL pointer to non-existing
> > > > label data, as the Xen side support for labels is not implemented yet.
> > > > 
> > > > Patch 2 & 3 add a memory backend dedicated for Xen usage and a hotplug
> > > > memory region for Xen guest, in order to make the existing nvdimm
> > > > device plugging path work on Xen.
> > > > 
> > > > Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when QEMU is
> > > > used as the Xen device model.
> > > 
> > > I've skimmed over patch-set and can't say that I'm happy with
> > > number of xen_enabled() invariants it introduced as well as
> > > with partial blobs it creates.
> > 
> > I have not read the series (Haozhong, please CC me, Anthony and
> > xen-devel to the whole series next time), but yes, indeed. Let's not add
> > more xen_enabled() if possible.
> > 
> > Haozhong, was there a design document thread on xen-devel about this? If
> > so, did it reach a conclusion? Was the design accepted? If so, please
> > add a link to the design doc in the introductory email, so that
> > everybody can read it and be on the same page.
> 
> Yes, there is a design [1] discussed and reviewed. Section 4.3 discussed
> the guest ACPI.
> 
> [1] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01921.html

Igor, did you have a chance to read it?

.. see below
> 
> > 
> > 
> > > I'd like to reduce above and a way to do this might be making xen 
> > >  1. use fw_cfg
> > >  2. fetch QEMU build acpi tables from fw_cfg
> > >  3. extract nvdim tables (which is trivial) and use them
> > > 
> > > looking at xen_load_linux(), it seems possible to use fw_cfg.
> > > 
> > > So what's stopping xen from using it elsewhere?,
> > > instead of adding more xen specific code to do 'the same'
> > > job and not reusing/sharing common code with tcg/kvm.
> > 
> > So far, ACPI tables have not been generated by QEMU. Xen HVM machines
> > rely on a firmware-like application called "hvmloader" that runs in
> > guest context and generates the ACPI tables. I have no opinions on
> > hvmloader and I'll let the Xen maintainers talk about it. However, keep
> > in mind that with an HVM guest some devices are emulated by Xen and/or
> > by other device emulators that can run alongside QEMU. QEMU doesn't have
> > a full few of the system.
> > 
> > Here the question is: does it have to be QEMU the one to generate the
> > ACPI blobs for the nvdimm? It would be nicer if it was up to hvmloader
> > like the rest, instead of introducing this split-brain design about
> > ACPI. We need to see a design doc to fully understand this.
> >
> 
> hvmloader runs in the guest and is responsible to build/load guest
> ACPI. However, it's not capable to build AML at runtime (for the lack
> of AML builder). If any guest ACPI object is needed (e.g. by guest
> DSDT), it has to be generated from ASL by iasl at Xen compile time and
> then be loaded by hvmloader at runtime.
> 
> Xen includes an OperationRegion "BIOS" in the static generated guest
> DSDT, whose address is hardcoded and which contains a list of values
> filled by hvmloader at runtime. Other ACPI objects can refer to those
> values (e.g., the number of vCPUs). But it's not enough for generating
> guest NVDIMM ACPI objects at compile time and then being customized
> and loaded by hvmload, because its structure (i.e., the number of
> namespace devices) cannot be decided util the guest config is known.
> 
> Alternatively, we may introduce an AML builder in hvmloader and build
> all guest ACPI completely in hvmloader. Looking at the similar
> implementation in QEMU, it would not be small, compared to the current
> size of hvmloader. Besides, I'm still going to let QEMU handle guest
> NVDIMM _DSM and _FIT calls, which is another reason I use QEMU to
> build NVDIMM ACPI.
> 
> > If the design doc thread led into thinking that it has to be QEMU to
> > generate them, then would it make the code nicer if we used fw_cfg to
> > get the (full or partial) tables from QEMU, as Igor suggested?
> 
> I'll have a look at the code (which I didn't notice) pointed by Igor.

And there is a spec too!

https://github.com/qemu/qemu/blob/master/docs/specs/fw_cfg.txt

Igor, did you have in mind to use FW_CFG_FILE_DIR to retrieve the
ACPI AML code?

> 
> One possible issue to use fw_cfg is how to avoid the conflict between
> ACPI built by QEMU and ACPI built by hvmloader (e.g., both may use the
> same table signature / device names / ...). In my current design, QEMU
> will pass the table signatures and device names used in its ACPI to
> Xen, and Xen can check the conflict with its own ACPI. Perhaps we can
> add necessary functions in fw_cfg as well. Anyway, let me first look
> at the code.
> 
> Thanks,
> Haozhong

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Haozhong Zhang]

On 10/10/17 12:05 -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Sep 12, 2017 at 11:15:09AM +0800, Haozhong Zhang wrote:
> > On 09/11/17 11:52 -0700, Stefano Stabellini wrote:
> > > CC'ing xen-devel, and the Xen tools and x86 maintainers.
> > > 
> > > On Mon, 11 Sep 2017, Igor Mammedov wrote:
> > > > On Mon, 11 Sep 2017 12:41:47 +0800
> > > > Haozhong Zhang <haozhong.zhang@intel.com> wrote:
> > > > 
> > > > > This is the QEMU part patches that works with the associated Xen
> > > > > patches to enable vNVDIMM support for Xen HVM domains. Xen relies on
> > > > > QEMU to build guest NFIT and NVDIMM namespace devices, and allocate
> > > > > guest address space for vNVDIMM devices.
> > > > > 
> > > > > All patches can be found at
> > > > >   Xen:  https://github.com/hzzhan9/xen.git nvdimm-rfc-v3
> > > > >   QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3
> > > > > 
> > > > > Patch 1 is to avoid dereferencing the NULL pointer to non-existing
> > > > > label data, as the Xen side support for labels is not implemented yet.
> > > > > 
> > > > > Patch 2 & 3 add a memory backend dedicated for Xen usage and a hotplug
> > > > > memory region for Xen guest, in order to make the existing nvdimm
> > > > > device plugging path work on Xen.
> > > > > 
> > > > > Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when QEMU is
> > > > > used as the Xen device model.
> > > > 
> > > > I've skimmed over patch-set and can't say that I'm happy with
> > > > number of xen_enabled() invariants it introduced as well as
> > > > with partial blobs it creates.
> > > 
> > > I have not read the series (Haozhong, please CC me, Anthony and
> > > xen-devel to the whole series next time), but yes, indeed. Let's not add
> > > more xen_enabled() if possible.
> > > 
> > > Haozhong, was there a design document thread on xen-devel about this? If
> > > so, did it reach a conclusion? Was the design accepted? If so, please
> > > add a link to the design doc in the introductory email, so that
> > > everybody can read it and be on the same page.
> > 
> > Yes, there is a design [1] discussed and reviewed. Section 4.3 discussed
> > the guest ACPI.
> > 
> > [1] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01921.html
> 
> Igor, did you have a chance to read it?
> 
> .. see below
> > 
> > > 
> > > 
> > > > I'd like to reduce above and a way to do this might be making xen 
> > > >  1. use fw_cfg
> > > >  2. fetch QEMU build acpi tables from fw_cfg
> > > >  3. extract nvdim tables (which is trivial) and use them
> > > > 
> > > > looking at xen_load_linux(), it seems possible to use fw_cfg.
> > > > 
> > > > So what's stopping xen from using it elsewhere?,
> > > > instead of adding more xen specific code to do 'the same'
> > > > job and not reusing/sharing common code with tcg/kvm.
> > > 
> > > So far, ACPI tables have not been generated by QEMU. Xen HVM machines
> > > rely on a firmware-like application called "hvmloader" that runs in
> > > guest context and generates the ACPI tables. I have no opinions on
> > > hvmloader and I'll let the Xen maintainers talk about it. However, keep
> > > in mind that with an HVM guest some devices are emulated by Xen and/or
> > > by other device emulators that can run alongside QEMU. QEMU doesn't have
> > > a full few of the system.
> > > 
> > > Here the question is: does it have to be QEMU the one to generate the
> > > ACPI blobs for the nvdimm? It would be nicer if it was up to hvmloader
> > > like the rest, instead of introducing this split-brain design about
> > > ACPI. We need to see a design doc to fully understand this.
> > >
> > 
> > hvmloader runs in the guest and is responsible to build/load guest
> > ACPI. However, it's not capable to build AML at runtime (for the lack
> > of AML builder). If any guest ACPI object is needed (e.g. by guest
> > DSDT), it has to be generated from ASL by iasl at Xen compile time and
> > then be loaded by hvmloader at runtime.
> > 
> > Xen includes an OperationRegion "BIOS" in the static generated guest
> > DSDT, whose address is hardcoded and which contains a list of values
> > filled by hvmloader at runtime. Other ACPI objects can refer to those
> > values (e.g., the number of vCPUs). But it's not enough for generating
> > guest NVDIMM ACPI objects at compile time and then being customized
> > and loaded by hvmload, because its structure (i.e., the number of
> > namespace devices) cannot be decided util the guest config is known.
> > 
> > Alternatively, we may introduce an AML builder in hvmloader and build
> > all guest ACPI completely in hvmloader. Looking at the similar
> > implementation in QEMU, it would not be small, compared to the current
> > size of hvmloader. Besides, I'm still going to let QEMU handle guest
> > NVDIMM _DSM and _FIT calls, which is another reason I use QEMU to
> > build NVDIMM ACPI.
> > 
> > > If the design doc thread led into thinking that it has to be QEMU to
> > > generate them, then would it make the code nicer if we used fw_cfg to
> > > get the (full or partial) tables from QEMU, as Igor suggested?
> > 
> > I'll have a look at the code (which I didn't notice) pointed by Igor.
> 
> And there is a spec too!
> 
> https://github.com/qemu/qemu/blob/master/docs/specs/fw_cfg.txt
> 
> Igor, did you have in mind to use FW_CFG_FILE_DIR to retrieve the
> ACPI AML code?
> 

Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and
/rom@etc/table-loader. The former is unstructured to guest, and
contains all data of guest ACPI. The latter is a BIOSLinkerLoader
organized as a set of commands, which direct the guest (e.g., SeaBIOS
on KVM/QEMU) to relocate data in the former file, recalculate checksum
of specified area, and fill guest address in specified ACPI field.

One part of my patches is to implement a mechanism to tell Xen which
part of ACPI data is a table (NFIT), and which part defines a
namespace device and what the device name is. I can add two new loader
commands for them respectively.

Because they just provide information and SeaBIOS in non-xen
environment ignores unrecognized commands, they will not break SeaBIOS
in non-xen environment.

On QEMU side, most Xen-specific hacks in ACPI builder could be
dropped, and replaced by adding the new loader commands (though they
may be used only by Xen).

On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor
are needed in, perhaps, hvmloader.


Haozhong

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Paolo Bonzini]

On 12/10/2017 14:45, Haozhong Zhang wrote:
> Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and
> /rom@etc/table-loader. The former is unstructured to guest, and
> contains all data of guest ACPI. The latter is a BIOSLinkerLoader
> organized as a set of commands, which direct the guest (e.g., SeaBIOS
> on KVM/QEMU) to relocate data in the former file, recalculate checksum
> of specified area, and fill guest address in specified ACPI field.
> 
> One part of my patches is to implement a mechanism to tell Xen which
> part of ACPI data is a table (NFIT), and which part defines a
> namespace device and what the device name is. I can add two new loader
> commands for them respectively.
> 
> Because they just provide information and SeaBIOS in non-xen
> environment ignores unrecognized commands, they will not break SeaBIOS
> in non-xen environment.
> 
> On QEMU side, most Xen-specific hacks in ACPI builder could be
> dropped, and replaced by adding the new loader commands (though they
> may be used only by Xen).
> 
> On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor
> are needed in, perhaps, hvmloader.

If Xen has to parse BIOSLinkerLoader, it can use the existing commands
to process a reduced set of ACPI tables.  In other words,
etc/acpi/tables would only include the NFIT, the SSDT with namespace
devices, and the XSDT.  etc/acpi/rsdp would include the RSDP table as usual.

hvmloader can then:

1) allocate some memory for where the XSDT will go

2) process the BIOSLinkerLoader like SeaBIOS would do

3) find the RSDP in low memory, since the loader script must have placed
it there.  If it cannot find it, allocate some low memory, fill it with
the RSDP header and revision, and and jump to step 6

4) If it found QEMU's RSDP, use it to find QEMU's XSDT

5) Copy ACPI table pointers from QEMU to hvmloader's RSDT and/or XSDT.

6) build hvmloader tables and link them into the RSDT and/or XSDT as usual.

7) overwrite the RSDP in low memory with a pointer to hvmloader's own
RSDT and/or XSDT, and updated the checksums

QEMU's XSDT remains there somewhere in memory, unused but harmless.

Paolo

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Konrad Rzeszutek Wilk]

On Thu, Oct 12, 2017 at 08:45:44PM +0800, Haozhong Zhang wrote:
> On 10/10/17 12:05 -0400, Konrad Rzeszutek Wilk wrote:
> > On Tue, Sep 12, 2017 at 11:15:09AM +0800, Haozhong Zhang wrote:
> > > On 09/11/17 11:52 -0700, Stefano Stabellini wrote:
> > > > CC'ing xen-devel, and the Xen tools and x86 maintainers.
> > > > 
> > > > On Mon, 11 Sep 2017, Igor Mammedov wrote:
> > > > > On Mon, 11 Sep 2017 12:41:47 +0800
> > > > > Haozhong Zhang <haozhong.zhang@intel.com> wrote:
> > > > > 
> > > > > > This is the QEMU part patches that works with the associated Xen
> > > > > > patches to enable vNVDIMM support for Xen HVM domains. Xen relies on
> > > > > > QEMU to build guest NFIT and NVDIMM namespace devices, and allocate
> > > > > > guest address space for vNVDIMM devices.
> > > > > > 
> > > > > > All patches can be found at
> > > > > >   Xen:  https://github.com/hzzhan9/xen.git nvdimm-rfc-v3
> > > > > >   QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3
> > > > > > 
> > > > > > Patch 1 is to avoid dereferencing the NULL pointer to non-existing
> > > > > > label data, as the Xen side support for labels is not implemented yet.
> > > > > > 
> > > > > > Patch 2 & 3 add a memory backend dedicated for Xen usage and a hotplug
> > > > > > memory region for Xen guest, in order to make the existing nvdimm
> > > > > > device plugging path work on Xen.
> > > > > > 
> > > > > > Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when QEMU is
> > > > > > used as the Xen device model.
> > > > > 
> > > > > I've skimmed over patch-set and can't say that I'm happy with
> > > > > number of xen_enabled() invariants it introduced as well as
> > > > > with partial blobs it creates.
> > > > 
> > > > I have not read the series (Haozhong, please CC me, Anthony and
> > > > xen-devel to the whole series next time), but yes, indeed. Let's not add
> > > > more xen_enabled() if possible.
> > > > 
> > > > Haozhong, was there a design document thread on xen-devel about this? If
> > > > so, did it reach a conclusion? Was the design accepted? If so, please
> > > > add a link to the design doc in the introductory email, so that
> > > > everybody can read it and be on the same page.
> > > 
> > > Yes, there is a design [1] discussed and reviewed. Section 4.3 discussed
> > > the guest ACPI.
> > > 
> > > [1] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01921.html
> > 
> > Igor, did you have a chance to read it?
> > 
> > .. see below
> > > 
> > > > 
> > > > 
> > > > > I'd like to reduce above and a way to do this might be making xen 
> > > > >  1. use fw_cfg
> > > > >  2. fetch QEMU build acpi tables from fw_cfg
> > > > >  3. extract nvdim tables (which is trivial) and use them
> > > > > 
> > > > > looking at xen_load_linux(), it seems possible to use fw_cfg.
> > > > > 
> > > > > So what's stopping xen from using it elsewhere?,
> > > > > instead of adding more xen specific code to do 'the same'
> > > > > job and not reusing/sharing common code with tcg/kvm.
> > > > 
> > > > So far, ACPI tables have not been generated by QEMU. Xen HVM machines
> > > > rely on a firmware-like application called "hvmloader" that runs in
> > > > guest context and generates the ACPI tables. I have no opinions on
> > > > hvmloader and I'll let the Xen maintainers talk about it. However, keep
> > > > in mind that with an HVM guest some devices are emulated by Xen and/or
> > > > by other device emulators that can run alongside QEMU. QEMU doesn't have
> > > > a full few of the system.
> > > > 
> > > > Here the question is: does it have to be QEMU the one to generate the
> > > > ACPI blobs for the nvdimm? It would be nicer if it was up to hvmloader
> > > > like the rest, instead of introducing this split-brain design about
> > > > ACPI. We need to see a design doc to fully understand this.
> > > >
> > > 
> > > hvmloader runs in the guest and is responsible to build/load guest
> > > ACPI. However, it's not capable to build AML at runtime (for the lack
> > > of AML builder). If any guest ACPI object is needed (e.g. by guest
> > > DSDT), it has to be generated from ASL by iasl at Xen compile time and
> > > then be loaded by hvmloader at runtime.
> > > 
> > > Xen includes an OperationRegion "BIOS" in the static generated guest
> > > DSDT, whose address is hardcoded and which contains a list of values
> > > filled by hvmloader at runtime. Other ACPI objects can refer to those
> > > values (e.g., the number of vCPUs). But it's not enough for generating
> > > guest NVDIMM ACPI objects at compile time and then being customized
> > > and loaded by hvmload, because its structure (i.e., the number of
> > > namespace devices) cannot be decided util the guest config is known.
> > > 
> > > Alternatively, we may introduce an AML builder in hvmloader and build
> > > all guest ACPI completely in hvmloader. Looking at the similar
> > > implementation in QEMU, it would not be small, compared to the current
> > > size of hvmloader. Besides, I'm still going to let QEMU handle guest
> > > NVDIMM _DSM and _FIT calls, which is another reason I use QEMU to
> > > build NVDIMM ACPI.
> > > 
> > > > If the design doc thread led into thinking that it has to be QEMU to
> > > > generate them, then would it make the code nicer if we used fw_cfg to
> > > > get the (full or partial) tables from QEMU, as Igor suggested?
> > > 
> > > I'll have a look at the code (which I didn't notice) pointed by Igor.
> > 
> > And there is a spec too!
> > 
> > https://github.com/qemu/qemu/blob/master/docs/specs/fw_cfg.txt
> > 
> > Igor, did you have in mind to use FW_CFG_FILE_DIR to retrieve the
> > ACPI AML code?
> > 
> 
> Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and
> /rom@etc/table-loader. The former is unstructured to guest, and
> contains all data of guest ACPI. The latter is a BIOSLinkerLoader
> organized as a set of commands, which direct the guest (e.g., SeaBIOS
> on KVM/QEMU) to relocate data in the former file, recalculate checksum
> of specified area, and fill guest address in specified ACPI field.
> 
> One part of my patches is to implement a mechanism to tell Xen which
> part of ACPI data is a table (NFIT), and which part defines a
> namespace device and what the device name is. I can add two new loader
> commands for them respectively.

<nods>
> 
> Because they just provide information and SeaBIOS in non-xen
> environment ignores unrecognized commands, they will not break SeaBIOS
> in non-xen environment.
> 
> On QEMU side, most Xen-specific hacks in ACPI builder could be

Wooot!
> dropped, and replaced by adding the new loader commands (though they
> may be used only by Xen).

And eventually all of the hvmloader ACPI built in code could be dropped
and use all of the loader commands?

> 
> On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor
> are needed in, perhaps, hvmloader.

<nods>
> 
> 
> Haozhong
> 

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Haozhong Zhang]

On 10/12/17 17:45 +0200, Paolo Bonzini wrote:
> On 12/10/2017 14:45, Haozhong Zhang wrote:
> > Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and
> > /rom@etc/table-loader. The former is unstructured to guest, and
> > contains all data of guest ACPI. The latter is a BIOSLinkerLoader
> > organized as a set of commands, which direct the guest (e.g., SeaBIOS
> > on KVM/QEMU) to relocate data in the former file, recalculate checksum
> > of specified area, and fill guest address in specified ACPI field.
> > 
> > One part of my patches is to implement a mechanism to tell Xen which
> > part of ACPI data is a table (NFIT), and which part defines a
> > namespace device and what the device name is. I can add two new loader
> > commands for them respectively.
> > 
> > Because they just provide information and SeaBIOS in non-xen
> > environment ignores unrecognized commands, they will not break SeaBIOS
> > in non-xen environment.
> > 
> > On QEMU side, most Xen-specific hacks in ACPI builder could be
> > dropped, and replaced by adding the new loader commands (though they
> > may be used only by Xen).
> > 
> > On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor
> > are needed in, perhaps, hvmloader.
> 
> If Xen has to parse BIOSLinkerLoader, it can use the existing commands
> to process a reduced set of ACPI tables.  In other words,
> etc/acpi/tables would only include the NFIT, the SSDT with namespace
> devices, and the XSDT.  etc/acpi/rsdp would include the RSDP table as usual.
>
> hvmloader can then:
> 
> 1) allocate some memory for where the XSDT will go
> 
> 2) process the BIOSLinkerLoader like SeaBIOS would do
> 
> 3) find the RSDP in low memory, since the loader script must have placed
> it there.  If it cannot find it, allocate some low memory, fill it with
> the RSDP header and revision, and and jump to step 6
> 
> 4) If it found QEMU's RSDP, use it to find QEMU's XSDT
> 
> 5) Copy ACPI table pointers from QEMU to hvmloader's RSDT and/or XSDT.
> 
> 6) build hvmloader tables and link them into the RSDT and/or XSDT as usual.
> 
> 7) overwrite the RSDP in low memory with a pointer to hvmloader's own
> RSDT and/or XSDT, and updated the checksums
> 
> QEMU's XSDT remains there somewhere in memory, unused but harmless.
> 

It can work for plan tables which do not contain AML.

However, for a namespace device, Xen needs to know its name in order
to detect the potential name conflict with those used in Xen built
ACPI. Xen does not (and is not going to) introduce an AML parser, so
it cannot get those device names from QEMU built ACPI by its own.

The idea of either this patch series or the new BIOSLinkerLoader
command is to let QEMU tell Xen where the definition body of a
namespace device (i.e. that part within the outmost "Device(NAME)") is
and what the device name is. Xen, after the name conflict check, can
re-package the definition body in a namespace device (w/ minimal AML
builder code added in Xen) and then in SSDT.


Haozhong

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Haozhong Zhang]

On 10/12/17 13:39 -0400, Konrad Rzeszutek Wilk wrote:
> On Thu, Oct 12, 2017 at 08:45:44PM +0800, Haozhong Zhang wrote:
> > On 10/10/17 12:05 -0400, Konrad Rzeszutek Wilk wrote:
> > > On Tue, Sep 12, 2017 at 11:15:09AM +0800, Haozhong Zhang wrote:
> > > > On 09/11/17 11:52 -0700, Stefano Stabellini wrote:
> > > > > CC'ing xen-devel, and the Xen tools and x86 maintainers.
> > > > > 
> > > > > On Mon, 11 Sep 2017, Igor Mammedov wrote:
> > > > > > On Mon, 11 Sep 2017 12:41:47 +0800
> > > > > > Haozhong Zhang <haozhong.zhang@intel.com> wrote:
> > > > > > 
> > > > > > > This is the QEMU part patches that works with the associated Xen
> > > > > > > patches to enable vNVDIMM support for Xen HVM domains. Xen relies on
> > > > > > > QEMU to build guest NFIT and NVDIMM namespace devices, and allocate
> > > > > > > guest address space for vNVDIMM devices.
> > > > > > > 
> > > > > > > All patches can be found at
> > > > > > >   Xen:  https://github.com/hzzhan9/xen.git nvdimm-rfc-v3
> > > > > > >   QEMU: https://github.com/hzzhan9/qemu.git xen-nvdimm-rfc-v3
> > > > > > > 
> > > > > > > Patch 1 is to avoid dereferencing the NULL pointer to non-existing
> > > > > > > label data, as the Xen side support for labels is not implemented yet.
> > > > > > > 
> > > > > > > Patch 2 & 3 add a memory backend dedicated for Xen usage and a hotplug
> > > > > > > memory region for Xen guest, in order to make the existing nvdimm
> > > > > > > device plugging path work on Xen.
> > > > > > > 
> > > > > > > Patch 4 - 10 build and cooy NFIT from QEMU to Xen guest, when QEMU is
> > > > > > > used as the Xen device model.
> > > > > > 
> > > > > > I've skimmed over patch-set and can't say that I'm happy with
> > > > > > number of xen_enabled() invariants it introduced as well as
> > > > > > with partial blobs it creates.
> > > > > 
> > > > > I have not read the series (Haozhong, please CC me, Anthony and
> > > > > xen-devel to the whole series next time), but yes, indeed. Let's not add
> > > > > more xen_enabled() if possible.
> > > > > 
> > > > > Haozhong, was there a design document thread on xen-devel about this? If
> > > > > so, did it reach a conclusion? Was the design accepted? If so, please
> > > > > add a link to the design doc in the introductory email, so that
> > > > > everybody can read it and be on the same page.
> > > > 
> > > > Yes, there is a design [1] discussed and reviewed. Section 4.3 discussed
> > > > the guest ACPI.
> > > > 
> > > > [1] https://lists.xenproject.org/archives/html/xen-devel/2016-07/msg01921.html
> > > 
> > > Igor, did you have a chance to read it?
> > > 
> > > .. see below
> > > > 
> > > > > 
> > > > > 
> > > > > > I'd like to reduce above and a way to do this might be making xen 
> > > > > >  1. use fw_cfg
> > > > > >  2. fetch QEMU build acpi tables from fw_cfg
> > > > > >  3. extract nvdim tables (which is trivial) and use them
> > > > > > 
> > > > > > looking at xen_load_linux(), it seems possible to use fw_cfg.
> > > > > > 
> > > > > > So what's stopping xen from using it elsewhere?,
> > > > > > instead of adding more xen specific code to do 'the same'
> > > > > > job and not reusing/sharing common code with tcg/kvm.
> > > > > 
> > > > > So far, ACPI tables have not been generated by QEMU. Xen HVM machines
> > > > > rely on a firmware-like application called "hvmloader" that runs in
> > > > > guest context and generates the ACPI tables. I have no opinions on
> > > > > hvmloader and I'll let the Xen maintainers talk about it. However, keep
> > > > > in mind that with an HVM guest some devices are emulated by Xen and/or
> > > > > by other device emulators that can run alongside QEMU. QEMU doesn't have
> > > > > a full few of the system.
> > > > > 
> > > > > Here the question is: does it have to be QEMU the one to generate the
> > > > > ACPI blobs for the nvdimm? It would be nicer if it was up to hvmloader
> > > > > like the rest, instead of introducing this split-brain design about
> > > > > ACPI. We need to see a design doc to fully understand this.
> > > > >
> > > > 
> > > > hvmloader runs in the guest and is responsible to build/load guest
> > > > ACPI. However, it's not capable to build AML at runtime (for the lack
> > > > of AML builder). If any guest ACPI object is needed (e.g. by guest
> > > > DSDT), it has to be generated from ASL by iasl at Xen compile time and
> > > > then be loaded by hvmloader at runtime.
> > > > 
> > > > Xen includes an OperationRegion "BIOS" in the static generated guest
> > > > DSDT, whose address is hardcoded and which contains a list of values
> > > > filled by hvmloader at runtime. Other ACPI objects can refer to those
> > > > values (e.g., the number of vCPUs). But it's not enough for generating
> > > > guest NVDIMM ACPI objects at compile time and then being customized
> > > > and loaded by hvmload, because its structure (i.e., the number of
> > > > namespace devices) cannot be decided util the guest config is known.
> > > > 
> > > > Alternatively, we may introduce an AML builder in hvmloader and build
> > > > all guest ACPI completely in hvmloader. Looking at the similar
> > > > implementation in QEMU, it would not be small, compared to the current
> > > > size of hvmloader. Besides, I'm still going to let QEMU handle guest
> > > > NVDIMM _DSM and _FIT calls, which is another reason I use QEMU to
> > > > build NVDIMM ACPI.
> > > > 
> > > > > If the design doc thread led into thinking that it has to be QEMU to
> > > > > generate them, then would it make the code nicer if we used fw_cfg to
> > > > > get the (full or partial) tables from QEMU, as Igor suggested?
> > > > 
> > > > I'll have a look at the code (which I didn't notice) pointed by Igor.
> > > 
> > > And there is a spec too!
> > > 
> > > https://github.com/qemu/qemu/blob/master/docs/specs/fw_cfg.txt
> > > 
> > > Igor, did you have in mind to use FW_CFG_FILE_DIR to retrieve the
> > > ACPI AML code?
> > > 
> > 
> > Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and
> > /rom@etc/table-loader. The former is unstructured to guest, and
> > contains all data of guest ACPI. The latter is a BIOSLinkerLoader
> > organized as a set of commands, which direct the guest (e.g., SeaBIOS
> > on KVM/QEMU) to relocate data in the former file, recalculate checksum
> > of specified area, and fill guest address in specified ACPI field.
> > 
> > One part of my patches is to implement a mechanism to tell Xen which
> > part of ACPI data is a table (NFIT), and which part defines a
> > namespace device and what the device name is. I can add two new loader
> > commands for them respectively.
> 
> <nods>
> > 
> > Because they just provide information and SeaBIOS in non-xen
> > environment ignores unrecognized commands, they will not break SeaBIOS
> > in non-xen environment.
> > 
> > On QEMU side, most Xen-specific hacks in ACPI builder could be
> 
> Wooot!
> > dropped, and replaced by adding the new loader commands (though they
> > may be used only by Xen).
> 
> And eventually all of the hvmloader ACPI built in code could be dropped
> and use all of the loader commands?

If Xen is going to rely on QEMU to build the entire ACPI for a HVM
guest, then there would be no need to check the signature/name
conflict, so the new BIOSLinkerLoader commands here would not be
necessary in that case (or only for backwards compatible). I don't
know how much work would be needed, and that could be another project.

Haozhong

> 
> > 
> > On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor
> > are needed in, perhaps, hvmloader.
> 
> <nods>
> > 
> > 
> > Haozhong
> > 
> 

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Igor Mammedov]

On Fri, 13 Oct 2017 15:53:26 +0800
Haozhong Zhang <haozhong.zhang@intel.com> wrote:

> On 10/12/17 17:45 +0200, Paolo Bonzini wrote:
> > On 12/10/2017 14:45, Haozhong Zhang wrote:  
> > > Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and
> > > /rom@etc/table-loader. The former is unstructured to guest, and
> > > contains all data of guest ACPI. The latter is a BIOSLinkerLoader
> > > organized as a set of commands, which direct the guest (e.g., SeaBIOS
> > > on KVM/QEMU) to relocate data in the former file, recalculate checksum
> > > of specified area, and fill guest address in specified ACPI field.
> > > 
> > > One part of my patches is to implement a mechanism to tell Xen which
> > > part of ACPI data is a table (NFIT), and which part defines a
> > > namespace device and what the device name is. I can add two new loader
> > > commands for them respectively.
> > > 
> > > Because they just provide information and SeaBIOS in non-xen
> > > environment ignores unrecognized commands, they will not break SeaBIOS
> > > in non-xen environment.
> > > 
> > > On QEMU side, most Xen-specific hacks in ACPI builder could be
> > > dropped, and replaced by adding the new loader commands (though they
> > > may be used only by Xen).
> > > 
> > > On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor
> > > are needed in, perhaps, hvmloader.  
> > 
> > If Xen has to parse BIOSLinkerLoader, it can use the existing commands
> > to process a reduced set of ACPI tables.  In other words,
> > etc/acpi/tables would only include the NFIT, the SSDT with namespace
> > devices, and the XSDT.  etc/acpi/rsdp would include the RSDP table as usual.
> >
> > hvmloader can then:
> > 
> > 1) allocate some memory for where the XSDT will go
> > 
> > 2) process the BIOSLinkerLoader like SeaBIOS would do
> > 
> > 3) find the RSDP in low memory, since the loader script must have placed
> > it there.  If it cannot find it, allocate some low memory, fill it with
> > the RSDP header and revision, and and jump to step 6
> > 
> > 4) If it found QEMU's RSDP, use it to find QEMU's XSDT
> > 
> > 5) Copy ACPI table pointers from QEMU to hvmloader's RSDT and/or XSDT.
> > 
> > 6) build hvmloader tables and link them into the RSDT and/or XSDT as usual.
> > 
> > 7) overwrite the RSDP in low memory with a pointer to hvmloader's own
> > RSDT and/or XSDT, and updated the checksums
> > 
> > QEMU's XSDT remains there somewhere in memory, unused but harmless.
> >   
+1 to Paolo's suggestion, i.e.
 1. add BIOSLinkerLoader into hvmloader
 2. load/process QEMU's tables with #1
 3. get pointers to QEMU generated NFIT and NVDIMM SSDT from QEMU's RSDT/XSDT
    and put them in hvmloader's RSDT

> It can work for plan tables which do not contain AML.
> 
> However, for a namespace device, Xen needs to know its name in order
> to detect the potential name conflict with those used in Xen built
> ACPI. Xen does not (and is not going to) introduce an AML parser, so
> it cannot get those device names from QEMU built ACPI by its own.
> 
> The idea of either this patch series or the new BIOSLinkerLoader
> command is to let QEMU tell Xen where the definition body of a
> namespace device (i.e. that part within the outmost "Device(NAME)") is
> and what the device name is. Xen, after the name conflict check, can
> re-package the definition body in a namespace device (w/ minimal AML
> builder code added in Xen) and then in SSDT.

I'd skip conflict check at runtime as hvmloader doesn't currently
have "\\_SB\NVDR" device so instead of doing runtime check it might
do primitive check at build time that ASL sources in hvmloader do
not contain reserved for QEMU "NVDR" keyword to avoid its addition
by accident in future. (it also might be reused in future if some
other tables from QEMU will be reused).
It's a bit hackinsh but at least it does the job and keeps
BIOSLinkerLoader interface the same for all supported firmwares
(I'd consider it as a temporary hack on the way to fully build
by QEMU ACPI tables for Xen).

Ideally it would be better for QEMU to build all ACPI tables for
hvmloader to avoid split brain issues and need to invent extra
interfaces every time a feature is added to pass configuration
data from QEMU to firmware.
But that's probably out of scope of this project, it could be
done on top of this if Xen folks would like to do it. Adding
BIOSLinkerLoader to hvmloader would be a good starting point
for that future effort.

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Haozhong Zhang]

On 10/13/17 10:44 +0200, Igor Mammedov wrote:
> On Fri, 13 Oct 2017 15:53:26 +0800
> Haozhong Zhang <haozhong.zhang@intel.com> wrote:
> 
> > On 10/12/17 17:45 +0200, Paolo Bonzini wrote:
> > > On 12/10/2017 14:45, Haozhong Zhang wrote:  
> > > > Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and
> > > > /rom@etc/table-loader. The former is unstructured to guest, and
> > > > contains all data of guest ACPI. The latter is a BIOSLinkerLoader
> > > > organized as a set of commands, which direct the guest (e.g., SeaBIOS
> > > > on KVM/QEMU) to relocate data in the former file, recalculate checksum
> > > > of specified area, and fill guest address in specified ACPI field.
> > > > 
> > > > One part of my patches is to implement a mechanism to tell Xen which
> > > > part of ACPI data is a table (NFIT), and which part defines a
> > > > namespace device and what the device name is. I can add two new loader
> > > > commands for them respectively.
> > > > 
> > > > Because they just provide information and SeaBIOS in non-xen
> > > > environment ignores unrecognized commands, they will not break SeaBIOS
> > > > in non-xen environment.
> > > > 
> > > > On QEMU side, most Xen-specific hacks in ACPI builder could be
> > > > dropped, and replaced by adding the new loader commands (though they
> > > > may be used only by Xen).
> > > > 
> > > > On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor
> > > > are needed in, perhaps, hvmloader.  
> > > 
> > > If Xen has to parse BIOSLinkerLoader, it can use the existing commands
> > > to process a reduced set of ACPI tables.  In other words,
> > > etc/acpi/tables would only include the NFIT, the SSDT with namespace
> > > devices, and the XSDT.  etc/acpi/rsdp would include the RSDP table as usual.
> > >
> > > hvmloader can then:
> > > 
> > > 1) allocate some memory for where the XSDT will go
> > > 
> > > 2) process the BIOSLinkerLoader like SeaBIOS would do
> > > 
> > > 3) find the RSDP in low memory, since the loader script must have placed
> > > it there.  If it cannot find it, allocate some low memory, fill it with
> > > the RSDP header and revision, and and jump to step 6
> > > 
> > > 4) If it found QEMU's RSDP, use it to find QEMU's XSDT
> > > 
> > > 5) Copy ACPI table pointers from QEMU to hvmloader's RSDT and/or XSDT.
> > > 
> > > 6) build hvmloader tables and link them into the RSDT and/or XSDT as usual.
> > > 
> > > 7) overwrite the RSDP in low memory with a pointer to hvmloader's own
> > > RSDT and/or XSDT, and updated the checksums
> > > 
> > > QEMU's XSDT remains there somewhere in memory, unused but harmless.
> > >   
> +1 to Paolo's suggestion, i.e.
>  1. add BIOSLinkerLoader into hvmloader
>  2. load/process QEMU's tables with #1
>  3. get pointers to QEMU generated NFIT and NVDIMM SSDT from QEMU's RSDT/XSDT
>     and put them in hvmloader's RSDT
> 
> > It can work for plan tables which do not contain AML.
> > 
> > However, for a namespace device, Xen needs to know its name in order
> > to detect the potential name conflict with those used in Xen built
> > ACPI. Xen does not (and is not going to) introduce an AML parser, so
> > it cannot get those device names from QEMU built ACPI by its own.
> > 
> > The idea of either this patch series or the new BIOSLinkerLoader
> > command is to let QEMU tell Xen where the definition body of a
> > namespace device (i.e. that part within the outmost "Device(NAME)") is
> > and what the device name is. Xen, after the name conflict check, can
> > re-package the definition body in a namespace device (w/ minimal AML
> > builder code added in Xen) and then in SSDT.
> 
> I'd skip conflict check at runtime as hvmloader doesn't currently
> have "\\_SB\NVDR" device so instead of doing runtime check it might
> do primitive check at build time that ASL sources in hvmloader do
> not contain reserved for QEMU "NVDR" keyword to avoid its addition
> by accident in future. (it also might be reused in future if some
> other tables from QEMU will be reused).
> It's a bit hackinsh but at least it does the job and keeps
> BIOSLinkerLoader interface the same for all supported firmwares
> (I'd consider it as a temporary hack on the way to fully build
> by QEMU ACPI tables for Xen).
> 
> Ideally it would be better for QEMU to build all ACPI tables for
> hvmloader to avoid split brain issues and need to invent extra
> interfaces every time a feature is added to pass configuration
> data from QEMU to firmware.
> But that's probably out of scope of this project, it could be
> done on top of this if Xen folks would like to do it. Adding
> BIOSLinkerLoader to hvmloader would be a good starting point
> for that future effort.

If we can let QEMU build the entire guest ACPI, we may even not need
to introduce fw_cfg and BIOSLinkerLoader code to hvmloader.  SeaBIOS
is currently loaded after hvmloader and can be used to load QEMU built
ACPI.

To Jan, Andrew, Stefano and Anthony,

what do you think about allowing QEMU to build the entire guest ACPI
and letting SeaBIOS to load it? ACPI builder code in hvmloader is
still there and just bypassed in this case.


Haozhong

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Jan Beulich]

>>> On 13.10.17 at 13:13, <haozhong.zhang@intel.com> wrote:
> To Jan, Andrew, Stefano and Anthony,
> 
> what do you think about allowing QEMU to build the entire guest ACPI
> and letting SeaBIOS to load it? ACPI builder code in hvmloader is
> still there and just bypassed in this case.

Well, if that can be made work in a non-quirky way and without
loss of functionality, I'd probably be fine. I do think, however,
that there's a reason this is being handled in hvmloader right now.

Jan

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Stefano Stabellini]

On Fri, 13 Oct 2017, Jan Beulich wrote:
> >>> On 13.10.17 at 13:13, <haozhong.zhang@intel.com> wrote:
> > To Jan, Andrew, Stefano and Anthony,
> > 
> > what do you think about allowing QEMU to build the entire guest ACPI
> > and letting SeaBIOS to load it? ACPI builder code in hvmloader is
> > still there and just bypassed in this case.
> 
> Well, if that can be made work in a non-quirky way and without
> loss of functionality, I'd probably be fine. I do think, however,
> that there's a reason this is being handled in hvmloader right now.

And not to discourage you, just as a clarification, you'll also need to
consider backward compatibility: unless the tables are identical, I
imagine we'll have to keep using the old tables for already installed
virtual machines.

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Michael S. Tsirkin]

On Fri, Oct 13, 2017 at 03:46:39PM -0700, Stefano Stabellini wrote:
> On Fri, 13 Oct 2017, Jan Beulich wrote:
> > >>> On 13.10.17 at 13:13, <haozhong.zhang@intel.com> wrote:
> > > To Jan, Andrew, Stefano and Anthony,
> > > 
> > > what do you think about allowing QEMU to build the entire guest ACPI
> > > and letting SeaBIOS to load it? ACPI builder code in hvmloader is
> > > still there and just bypassed in this case.
> > 
> > Well, if that can be made work in a non-quirky way and without
> > loss of functionality, I'd probably be fine. I do think, however,
> > that there's a reason this is being handled in hvmloader right now.
> 
> And not to discourage you, just as a clarification, you'll also need to
> consider backward compatibility: unless the tables are identical, I
> imagine we'll have to keep using the old tables for already installed
> virtual machines.

Maybe you can handle this using machine type versioning.
Installed guests would use the old type.

-- 
MST

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Michael S. Tsirkin]

On Fri, Oct 13, 2017 at 03:53:26PM +0800, Haozhong Zhang wrote:
> On 10/12/17 17:45 +0200, Paolo Bonzini wrote:
> > On 12/10/2017 14:45, Haozhong Zhang wrote:
> > > Basically, QEMU builds two ROMs for guest, /rom@etc/acpi/tables and
> > > /rom@etc/table-loader. The former is unstructured to guest, and
> > > contains all data of guest ACPI. The latter is a BIOSLinkerLoader
> > > organized as a set of commands, which direct the guest (e.g., SeaBIOS
> > > on KVM/QEMU) to relocate data in the former file, recalculate checksum
> > > of specified area, and fill guest address in specified ACPI field.
> > > 
> > > One part of my patches is to implement a mechanism to tell Xen which
> > > part of ACPI data is a table (NFIT), and which part defines a
> > > namespace device and what the device name is. I can add two new loader
> > > commands for them respectively.
> > > 
> > > Because they just provide information and SeaBIOS in non-xen
> > > environment ignores unrecognized commands, they will not break SeaBIOS
> > > in non-xen environment.
> > > 
> > > On QEMU side, most Xen-specific hacks in ACPI builder could be
> > > dropped, and replaced by adding the new loader commands (though they
> > > may be used only by Xen).
> > > 
> > > On Xen side, a fw_cfg driver and a BIOSLinkerLoader command executor
> > > are needed in, perhaps, hvmloader.
> > 
> > If Xen has to parse BIOSLinkerLoader, it can use the existing commands
> > to process a reduced set of ACPI tables.  In other words,
> > etc/acpi/tables would only include the NFIT, the SSDT with namespace
> > devices, and the XSDT.  etc/acpi/rsdp would include the RSDP table as usual.
> >
> > hvmloader can then:
> > 
> > 1) allocate some memory for where the XSDT will go
> > 
> > 2) process the BIOSLinkerLoader like SeaBIOS would do
> > 
> > 3) find the RSDP in low memory, since the loader script must have placed
> > it there.  If it cannot find it, allocate some low memory, fill it with
> > the RSDP header and revision, and and jump to step 6
> > 
> > 4) If it found QEMU's RSDP, use it to find QEMU's XSDT
> > 
> > 5) Copy ACPI table pointers from QEMU to hvmloader's RSDT and/or XSDT.
> > 
> > 6) build hvmloader tables and link them into the RSDT and/or XSDT as usual.
> > 
> > 7) overwrite the RSDP in low memory with a pointer to hvmloader's own
> > RSDT and/or XSDT, and updated the checksums
> > 
> > QEMU's XSDT remains there somewhere in memory, unused but harmless.
> > 
> 
> It can work for plan tables which do not contain AML.
> 
> However, for a namespace device, Xen needs to know its name in order
> to detect the potential name conflict with those used in Xen built
> ACPI. Xen does not (and is not going to) introduce an AML parser, so
> it cannot get those device names from QEMU built ACPI by its own.
> 
> The idea of either this patch series or the new BIOSLinkerLoader
> command is to let QEMU tell Xen where the definition body of a
> namespace device (i.e. that part within the outmost "Device(NAME)") is
> and what the device name is. Xen, after the name conflict check, can
> re-package the definition body in a namespace device (w/ minimal AML
> builder code added in Xen) and then in SSDT.
> 
> 
> Haozhong

You most likely can do this without a new command.
You can use something similiar to build_append_named_dword
in combination with BIOS_LINKER_LOADER_COMMAND_ADD_POINTER
like vm gen id does.

-- 
MST

Re: [Qemu-devel] [Xen-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Konrad Rzeszutek Wilk]

On Sun, Oct 15, 2017 at 03:31:15AM +0300, Michael S. Tsirkin wrote:
> On Fri, Oct 13, 2017 at 03:46:39PM -0700, Stefano Stabellini wrote:
> > On Fri, 13 Oct 2017, Jan Beulich wrote:
> > > >>> On 13.10.17 at 13:13, <haozhong.zhang@intel.com> wrote:
> > > > To Jan, Andrew, Stefano and Anthony,
> > > > 
> > > > what do you think about allowing QEMU to build the entire guest ACPI
> > > > and letting SeaBIOS to load it? ACPI builder code in hvmloader is
> > > > still there and just bypassed in this case.
> > > 
> > > Well, if that can be made work in a non-quirky way and without
> > > loss of functionality, I'd probably be fine. I do think, however,
> > > that there's a reason this is being handled in hvmloader right now.
> > 
> > And not to discourage you, just as a clarification, you'll also need to
> > consider backward compatibility: unless the tables are identical, I
> > imagine we'll have to keep using the old tables for already installed
> > virtual machines.
> 
> Maybe you can handle this using machine type versioning.

<nods> And the type could be v2 if nvdimm was provided (which is
something that the toolstack would figure out).

The toolstack could also have a seperate 'v2' config flag if somebody
wanted to play with this _outside_ of having NVDIMM in the guest?


> Installed guests would use the old type.

<nods>

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Paolo Bonzini]

On 14/10/2017 00:46, Stefano Stabellini wrote:
> On Fri, 13 Oct 2017, Jan Beulich wrote:
>>>>> On 13.10.17 at 13:13, <haozhong.zhang@intel.com> wrote:
>>> To Jan, Andrew, Stefano and Anthony,
>>>
>>> what do you think about allowing QEMU to build the entire guest ACPI
>>> and letting SeaBIOS to load it? ACPI builder code in hvmloader is
>>> still there and just bypassed in this case.
>> Well, if that can be made work in a non-quirky way and without
>> loss of functionality, I'd probably be fine. I do think, however,
>> that there's a reason this is being handled in hvmloader right now.
> And not to discourage you, just as a clarification, you'll also need to
> consider backward compatibility: unless the tables are identical, I
> imagine we'll have to keep using the old tables for already installed
> virtual machines.

I agree.  Some of them are already identical, some are not but the QEMU
version should be okay, and for yet more it's probably better to keep
the Xen-specific parts in hvmloader.

The good thing is that it's possible to proceed incrementally once you
have the hvmloader support for merging the QEMU and hvmloader RSDT or
XSDT (whatever you are using), starting with just NVDIMM and proceeding
later with whatever you see fit.

Paolo

Re: [Qemu-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Haozhong Zhang]

On 10/17/17 13:45 +0200, Paolo Bonzini wrote:
> On 14/10/2017 00:46, Stefano Stabellini wrote:
> > On Fri, 13 Oct 2017, Jan Beulich wrote:
> >>>>> On 13.10.17 at 13:13, <haozhong.zhang@intel.com> wrote:
> >>> To Jan, Andrew, Stefano and Anthony,
> >>>
> >>> what do you think about allowing QEMU to build the entire guest ACPI
> >>> and letting SeaBIOS to load it? ACPI builder code in hvmloader is
> >>> still there and just bypassed in this case.
> >> Well, if that can be made work in a non-quirky way and without
> >> loss of functionality, I'd probably be fine. I do think, however,
> >> that there's a reason this is being handled in hvmloader right now.
> > And not to discourage you, just as a clarification, you'll also need to
> > consider backward compatibility: unless the tables are identical, I
> > imagine we'll have to keep using the old tables for already installed
> > virtual machines.
> 
> I agree.  Some of them are already identical, some are not but the QEMU
> version should be okay, and for yet more it's probably better to keep
> the Xen-specific parts in hvmloader.
> 
> The good thing is that it's possible to proceed incrementally once you
> have the hvmloader support for merging the QEMU and hvmloader RSDT or
> XSDT (whatever you are using), starting with just NVDIMM and proceeding
> later with whatever you see fit.
> 

I'll have a try to check how much the differences would affect. If it
would not take too much work, I'd like to adapt Xen NVDIMM enabling
patches to the all QEMU built ACPI. Otherwise, I'll fall back to Paolo
and MST's suggestions.

Thanks,
Haozhong

Re: [Qemu-devel] [Xen-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Roger Pau Monné]

On Tue, Oct 17, 2017 at 08:16:47PM +0800, Haozhong Zhang wrote:
> On 10/17/17 13:45 +0200, Paolo Bonzini wrote:
> > On 14/10/2017 00:46, Stefano Stabellini wrote:
> > > On Fri, 13 Oct 2017, Jan Beulich wrote:
> > >>>>> On 13.10.17 at 13:13, <haozhong.zhang@intel.com> wrote:
> > >>> To Jan, Andrew, Stefano and Anthony,
> > >>>
> > >>> what do you think about allowing QEMU to build the entire guest ACPI
> > >>> and letting SeaBIOS to load it? ACPI builder code in hvmloader is
> > >>> still there and just bypassed in this case.
> > >> Well, if that can be made work in a non-quirky way and without
> > >> loss of functionality, I'd probably be fine. I do think, however,
> > >> that there's a reason this is being handled in hvmloader right now.
> > > And not to discourage you, just as a clarification, you'll also need to
> > > consider backward compatibility: unless the tables are identical, I
> > > imagine we'll have to keep using the old tables for already installed
> > > virtual machines.
> > 
> > I agree.  Some of them are already identical, some are not but the QEMU
> > version should be okay, and for yet more it's probably better to keep
> > the Xen-specific parts in hvmloader.
> > 
> > The good thing is that it's possible to proceed incrementally once you
> > have the hvmloader support for merging the QEMU and hvmloader RSDT or
> > XSDT (whatever you are using), starting with just NVDIMM and proceeding
> > later with whatever you see fit.
> > 
> 
> I'll have a try to check how much the differences would affect. If it
> would not take too much work, I'd like to adapt Xen NVDIMM enabling
> patches to the all QEMU built ACPI. Otherwise, I'll fall back to Paolo
> and MST's suggestions.

I don't agree with the end goal of fully switching to the QEMU build
ACPI tables. First of all, the only entity that has all the
information about the guest it's the toolstack, and so it should be
the one in control of the ACPI tables.

Also, Xen guests can use several device models concurrently (via the
ioreq server interface), and each should be able to contribute to the
information presented in the ACPI tables. Intel is also working on
adding IOMMU emulation to the Xen hypervisor, in which case the vIOMMU
ACPI tables should be created by the toolstack and not QEMU. And
finally keep in mind that there are Xen guests (PVH) that use ACPI
tables but not QEMU.

Thanks, Roger.

Re: [Qemu-devel] [Xen-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Paolo Bonzini]

On 18/10/2017 10:32, Roger Pau Monné wrote:
>> I'll have a try to check how much the differences would affect. If it
>> would not take too much work, I'd like to adapt Xen NVDIMM enabling
>> patches to the all QEMU built ACPI. Otherwise, I'll fall back to Paolo
>> and MST's suggestions.
> I don't agree with the end goal of fully switching to the QEMU build
> ACPI tables. First of all, the only entity that has all the
> information about the guest it's the toolstack, and so it should be
> the one in control of the ACPI tables.
> 
> Also, Xen guests can use several device models concurrently (via the
> ioreq server interface), and each should be able to contribute to the
> information presented in the ACPI tables. Intel is also working on
> adding IOMMU emulation to the Xen hypervisor, in which case the vIOMMU
> ACPI tables should be created by the toolstack and not QEMU. And
> finally keep in mind that there are Xen guests (PVH) that use ACPI
> tables but not QEMU.

I agree with this in fact; QEMU has a view of _most_ of the emulated
hardware, but not all.

However, I disagree that the toolstack should be alone in controlling
the ACPI tables; rather, each involved part of the stack should be
providing its own part of the tables.  For example, QEMU (in addition to
NVDIMM information) should be the one providing an SSDT for southbridge
devices (floppy, COMx, LPTx, etc.).

The Xen stack (or more likely, hvmloader itself) would provide all the
bits that are provided by the hypervisor (MADT for the IOAPIC, another
SSDT for the HPET and RTC, DMAR tables for IOMMU, and so on).  This
should also work just fine for PVH.  Of course backwards compatibility
is the enemy of simplification, but in the end things _should_ actually
be simpler and I think it's a good idea if a prerequisite for Xen
vNVDIMM is to move AML code for QEMU devices out of hvmloader.

Paolo

Re: [Qemu-devel] [Xen-devel] [RFC QEMU PATCH v3 00/10] Implement vNVDIMM for Xen HVM guest

[Roger Pau Monné]

On Wed, Oct 18, 2017 at 10:46:57AM +0200, Paolo Bonzini wrote:
> On 18/10/2017 10:32, Roger Pau Monné wrote:
> >> I'll have a try to check how much the differences would affect. If it
> >> would not take too much work, I'd like to adapt Xen NVDIMM enabling
> >> patches to the all QEMU built ACPI. Otherwise, I'll fall back to Paolo
> >> and MST's suggestions.
> > I don't agree with the end goal of fully switching to the QEMU build
> > ACPI tables. First of all, the only entity that has all the
> > information about the guest it's the toolstack, and so it should be
> > the one in control of the ACPI tables.
> > 
> > Also, Xen guests can use several device models concurrently (via the
> > ioreq server interface), and each should be able to contribute to the
> > information presented in the ACPI tables. Intel is also working on
> > adding IOMMU emulation to the Xen hypervisor, in which case the vIOMMU
> > ACPI tables should be created by the toolstack and not QEMU. And
> > finally keep in mind that there are Xen guests (PVH) that use ACPI
> > tables but not QEMU.
> 
> I agree with this in fact; QEMU has a view of _most_ of the emulated
> hardware, but not all.
> 
> However, I disagree that the toolstack should be alone in controlling
> the ACPI tables; rather, each involved part of the stack should be
> providing its own part of the tables.  For example, QEMU (in addition to
> NVDIMM information) should be the one providing an SSDT for southbridge
> devices (floppy, COMx, LPTx, etc.).

Yes, that's what I wanted to say, rather than the toolstack providing
all the ACPI tables by itself. Every component should provide the
tables of the devices under it's control, and that should be glued
together by the toolstack (ie: hvmloader).

Roger.