pc-dimm: factor out MemoryDevice
v2 cover pc-dimm: factor out MemoryDevice
Right now we can only map PCDIMM/NVDIMM into guest address space. In the future, we might want to do the same for virtio devices - e.g. virtio-pmem or virtio-mem. Especially, they should be able to live side by side to each other.
E.g. the virto based memory devices regions will not be exposed via ACPI and friends. They will be detected just like other virtio devices and indicate the applicable memory region. This makes it possible to also use them on architectures without memory device detection support (e.g. s390x).
Let's factor out the memory device code into a MemoryDevice interface.
See the patch descriptions for details.
v1 -> v2:
- Fix compile issues on ppc (still untested O:-) )
David Hildenbrand (3):
pc-dimm: factor out MemoryDevice interface
machine: make MemoryHotplugState accessible via the machine
pc-dimm: factor out address space logic into MemoryDevice code
hw/i386/acpi-build.c | 3 +-
hw/i386/pc.c | 24 ++-
hw/mem/Makefile.objs | 1 +
hw/mem/memory-device.c | 281 +++++++++++++++++++++++++
hw/mem/pc-dimm.c | 304 +++++++--------------------
hw/ppc/spapr.c | 24 ++-
hw/ppc/spapr_hcall.c | 1 +
include/hw/boards.h | 16 ++
include/hw/mem/memory-device.h | 48 +++++
include/hw/mem/pc-dimm.h | 26 +--
numa.c | 3 +-
qmp.c | 4 +-
stubs/Makefile.objs | 2 +-
stubs/{qmp_pc_dimm.c => qmp_memory_device.c} | 4 +-
14 files changed, 464 insertions(+), 277 deletions(-)
create mode 100644 hw/mem/memory-device.c
create mode 100644 include/hw/mem/memory-device.h
rename stubs/{qmp_pc_dimm.c => qmp_memory_device.c} (61%)Signature · 1 line
2.14.3
PATCH v2 1/3 pc-dimm: factor out MemoryDevice interface
On the qmp level, we already have the concept of memory devices:
"query-memory-devices"
Right now, we only support NVDIMM and PCDIMM.
We want to map other devices later into the address space of the guest. Such device could e.g. be virtio devices. These devices will have a guest memory range assigned but won't be exposed via e.g. ACPI. We want to make them look like memory device, but not glued to pc-dimm.
Especially, it will not always be possible to have TYPE_PC_DIMM as a parent class (e.g. virtio devices). Let's use an interface instead. As a first part, convert handling of
- qmp_pc_dimm_device_list
- get_plugged_memory_size
to our new model. plug/unplug stuff etc. will follow later.
A memory device will have to provide the following functions:
- get_addr(): Necessary, as the property "addr" can e.g. not be used for virtio devices (already defined).
- get_plugged_size(): The amount this device offers to the guest as of now.
- get_region_size(): Because this can later on be bigger than the plugged size.
- fill_device_info(): Fill MemoryDeviceInfo, e.g. for qmp.
---
hw/i386/acpi-build.c | 3 +-
hw/mem/Makefile.objs | 1 +
hw/mem/memory-device.c | 119 +++++++++++++++++++++++++++
hw/mem/pc-dimm.c | 119 ++++++++++++++-------------
hw/ppc/spapr.c | 3 +-
hw/ppc/spapr_hcall.c | 1 +
include/hw/mem/memory-device.h | 44 ++++++++++
include/hw/mem/pc-dimm.h | 2 -
numa.c | 3 +-
qmp.c | 4 +-
stubs/Makefile.objs | 2 +-
stubs/{qmp_pc_dimm.c => qmp_memory_device.c} | 4 +-
12 files changed, 239 insertions(+), 66 deletions(-)
create mode 100644 hw/mem/memory-device.c
create mode 100644 include/hw/mem/memory-device.h
rename stubs/{qmp_pc_dimm.c => qmp_memory_device.c} (61%)hw/i386/acpi-build.c +2 -1
@@ -46,6 +46,7 @@#include "hw/acpi/vmgenid.h" #include "sysemu/tpm_backend.h" #include "hw/timer/mc146818rtc_regs.h" +#include "hw/mem/memory-device.h" #include "sysemu/numa.h" /* Supported chipsets: */
@@ -2253,7 +2254,7 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)static void build_srat_hotpluggable_memory (GArray *table_data,uint64_t base,uint64_t len,int default_node) { -MemoryDeviceInfoList *info_list =qmp_pc_dimm_device_list (); +MemoryDeviceInfoList *info_list =qmp_memory_device_list ();MemoryDeviceInfoList *info;MemoryDeviceInfo *mi;PCDIMMDeviceInfo *di;
hw/mem/Makefile.objs +1 -0
@@ -1,2 +1,3 @@ common-obj-$(CONFIG_MEM_HOTPLUG) += pc-dimm.o +common-obj-$(CONFIG_MEM_HOTPLUG) += memory-device.o common-obj-$(CONFIG_NVDIMM) += nvdimm.o
hw/mem/memory-device.c new file +119 -0
119 lines not on this page · raw message
hw/mem/pc-dimm.c +63 -56
159 lines not on this page · raw message
hw/ppc/spapr.c +2 -1
@@ -74,6 +74,7 @@#include "hw/compat.h" #include "qemu/cutils.h" #include "hw/ppc/spapr_cpu_core.h" +#include "hw/mem/memory-device.h" #include <libfdt.h>
@@ -722,7 +723,7 @@ static int spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt) }if (hotplug_lmb_start) { - dimms =qmp_pc_dimm_device_list (); + dimms =qmp_memory_device_list (); }/* ibm,dynamic-memory */
hw/ppc/spapr_hcall.c +1 -0
@@ -14,6 +14,7 @@#include "kvm_ppc.h" #include "hw/ppc/spapr_ovec.h" #include "mmu-book3s-v3.h" +#include "hw/mem/memory-device.h" struct SPRSyncState {int spr;
include/hw/mem/memory-device.h new file +44 -0
44 lines not on this page · raw message
include/hw/mem/pc-dimm.h +0 -2
@@ -93,9 +93,7 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,int pc_dimm_get_free_slot (const int *hint,int max_slots,Error **errp); -MemoryDeviceInfoList *qmp_pc_dimm_device_list (void );uint64_t pc_existing_dimms_capacity (Error **errp); -uint64_t get_plugged_memory_size (void );void pc_dimm_memory_plug (DeviceState *dev,MemoryHotplugState *hpms,MemoryRegion *mr,uint64_t align,Error **errp); void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
numa.c +2 -1
@@ -36,6 +36,7 @@#include "hw/boards.h" #include "sysemu/hostmem.h" #include "hw/mem/pc-dimm.h" +#include "hw/mem/memory-device.h" #include "qemu/option.h" #include "qemu/config-file.h" #include "qemu/cutils.h"
@@ -520,7 +521,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,static void numa_stat_memory_devices (NumaNodeMem node_mem[]) { -MemoryDeviceInfoList *info_list =qmp_pc_dimm_device_list (); +MemoryDeviceInfoList *info_list =qmp_memory_device_list ();MemoryDeviceInfoList *info;PCDIMMDeviceInfo *pcdimm_info;
qmp.c +2 -2
@@ -39,7 +39,7 @@#include "qapi/qobject-input-visitor.h" #include "hw/boards.h" #include "qom/object_interfaces.h" -#include "hw/mem/pc-dimm.h" +#include "hw/mem/memory-device.h" #include "hw/acpi/acpi_dev_interface.h" NameInfo *qmp_query_name (Error **errp)
@@ -731,7 +731,7 @@ void qmp_object_del(const char *id, Error **errp)MemoryDeviceInfoList *qmp_query_memory_devices (Error **errp) { -return qmp_pc_dimm_device_list (); +return qmp_memory_device_list (); }ACPIOSTInfoList *qmp_query_acpi_ospm_status (Error **errp)
stubs/Makefile.objs +1 -1
@@ -34,7 +34,7 @@ stub-obj-y += uuid.o stub-obj-y += vm-stop.o stub-obj-y += vmstate.o stub-obj-$(CONFIG_WIN32) += fd-register.o -stub-obj-y += qmp_pc_dimm.o +stub-obj-y += qmp_memory_device.o stub-obj-y += target-monitor-defs.o stub-obj-y += target-get-monitor-def.o stub-obj-y += pc_madt_cpu_entry.o
stubs/qmp_memory_device.c renamed from stubs/qmp_pc_dimm.c +2 -2
@@ -1,8 +1,8 @@#include "qemu/osdep.h" #include "qom/object.h" -#include "hw/mem/pc-dimm.h" +#include "hw/mem/memory-device.h" -MemoryDeviceInfoList *qmp_pc_dimm_device_list (void ) +MemoryDeviceInfoList *qmp_memory_device_list (void ) {return NULL ; }
Signature · 1 line
2.14.3
Re: [Qemu-devel] [PATCH v2 1/3] pc-dimm: factor out MemoryDevice interface
Quoted 16 lines · David Hildenbrand
On the qmp level, we already have the concept of memory devices:
"query-memory-devices"
Right now, we only support NVDIMM and PCDIMM.We want to map other devices later into the address space of the guest. Such device could e.g. be virtio devices. These devices will have a guest memory range assigned but won't be exposed via e.g. ACPI. We want to make them look like memory device, but not glued to pc-dimm.
Especially, it will not always be possible to have
TYPE_PC_DIMMas a parent class (e.g. virtio devices). Let's use an interface instead. As a first part, convert handling of
-qmp_pc_dimm_device_list
-get_plugged_memory_size
to our new model. plug/unplug stuff etc. will follow later.
Quoted 3 lines
A memory device will have to provide the following functions:
-get_addr(): Necessary, as the property "addr" can e.g. not be used for virtio devices (already defined).
Is the assumption here that a memory device has a fixed RAM address (GPA) from when it is realized? It seems to me plausible that you could have a memory device that has a specific size, but has a BAR of some sort that's programmed by the guest as part of the "plug" process.
Quoted 73 lines
-
get_plugged_size(): The amount this device offers to the guest as of now.
-get_region_size(): Because this can later on be bigger than the plugged size.
-fill_device_info(): Fill MemoryDeviceInfo, e.g. for qmp.Signed-off-by David Hildenbrand <[email protected]>--- hw/i386/acpi-build.c | 3 +- hw/mem/Makefile.objs | 1 + hw/mem/memory-device.c | 119 +++++++++++++++++++++++++++ hw/mem/pc-dimm.c | 119 ++++++++++++++------------- hw/ppc/spapr.c | 3 +- hw/ppc/spapr_hcall.c | 1 + include/hw/mem/memory-device.h | 44 ++++++++++ include/hw/mem/pc-dimm.h | 2 - numa.c | 3 +- qmp.c | 4 +- stubs/Makefile.objs | 2 +- stubs/{qmp_pc_dimm.c => qmp_memory_device.c} | 4 +- 12 files changed, 239 insertions(+), 66 deletions(-) create mode 100644 hw/mem/memory-device.c create mode 100644 include/hw/mem/memory-device.h rename stubs/{qmp_pc_dimm.c => qmp_memory_device.c} (61%)hw/i386/acpi-build.c +2
-1@@ -46,6 +46,7 @@#include "hw/acpi/vmgenid.h" #include "sysemu/tpm_backend.h" #include "hw/timer/mc146818rtc_regs.h" +#include "hw/mem/memory-device.h" #include "sysemu/numa.h" /* Supported chipsets: */ @@ -2253,7 +2254,7 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)static void build_srat_hotpluggable_memory (GArray *table_data,uint64_t base,uint64_t len,int default_node) { -MemoryDeviceInfoList *info_list =qmp_pc_dimm_device_list (); +MemoryDeviceInfoList *info_list =qmp_memory_device_list ();MemoryDeviceInfoList *info;MemoryDeviceInfo *mi;PCDIMMDeviceInfo *di;hw/mem/Makefile.objs +1
-0@@ -1,2 +1,3 @@ common-obj-$(CONFIG_MEM_HOTPLUG) += pc-dimm.o +common-obj-$(CONFIG_MEM_HOTPLUG) += memory-device.o common-obj-$(CONFIG_NVDIMM) += nvdimm.ohw/mem/memory-device.c new file +13
-0@@ -0,0 +1,119 @@ +/* +* Memory Device Interface +* +* Copyright ProfitBricks GmbH 2012 +* Copyright (C) 2014 Red Hat Inc +* Copyright (c) 2018 Red Hat Inc +* +* This work is licensed under the terms of the GNU GPL, version 2 or later. +* See the COPYING file in the top-level directory. +*/ + +#include "qemu/osdep.h" +#include "hw/mem/memory-device.h"
hw/mem/memory-device.c line 23
Quoted 10 lines
hw/mem/memory-device.c new file +10
-0+#include "hw/qdev.h" +#include "qapi/error.h" +#include "hw/boards.h" +#include "qemu/range.h" + +static gint memory_device_addr_sort (gconstpointer a,gconstpointer b) +{ +MemoryDeviceState *md_a =MEMORY_DEVICE (a); +MemoryDeviceState *md_b =MEMORY_DEVICE (b); +MemoryDeviceClass *mdc =MEMORY_DEVICE_GET_CLASS (a);
AFAICT from the code below, the two devices don't necessarily have to have the same class, so I think you need to get the class pointers for each of them separately. (Or if they do have to be the same by design, I think there should at least be an assert).
Quoted 436 lines
hw/mem/memory-device.c new file +96
-0+const uint64_t addr_a = mdc->get_addr (md_a); +const uint64_t addr_b = mdc->get_addr (md_b); + +if (addr_a > addr_b) { +return 1 ; + }else if (addr_a < addr_b) { +return -1 ; + } +return 0 ; +} + +static int memory_device_built_list (Object *obj,void *opaque) +{ +GSList **list = opaque; + +if (object_dynamic_cast (obj,TYPE_MEMORY_DEVICE )) { +DeviceState *dev =DEVICE (obj); +if (dev->realized) {/* only realized memory devices matter */ + *list =g_slist_insert_sorted (*list, dev, memory_device_addr_sort); + } + } + +object_child_foreach (obj, memory_device_built_list, opaque); +return 0 ; +} + +MemoryDeviceInfoList *qmp_memory_device_list (void ) +{ +GSList *devices =NULL , *item; +MemoryDeviceInfoList *list =NULL , *prev =NULL ; + +object_child_foreach (qdev_get_machine (), memory_device_built_list, + &devices); + +for (item = devices; item; item =g_slist_next (item)) { +MemoryDeviceState *md =MEMORY_DEVICE (item->data); +MemoryDeviceClass *mdc =MEMORY_DEVICE_GET_CLASS (item->data); +MemoryDeviceInfoList *elem =g_new0 (MemoryDeviceInfoList,1 ); +MemoryDeviceInfo *info =g_new0 (MemoryDeviceInfo,1 ); + + mdc->fill_device_info (md, info); + + elem->value = info; + elem->next =NULL ; +if (prev) { + prev->next = elem; + }else { + list = elem; + } + prev = elem; + } + +g_slist_free (devices); + +return list; +} + +static int memory_device_plugged_size (Object *obj,void *opaque) +{ +uint64_t *size = opaque; + +if (object_dynamic_cast (obj,TYPE_MEMORY_DEVICE )) { +DeviceState *dev =DEVICE (obj); +MemoryDeviceState *md =MEMORY_DEVICE (obj); +MemoryDeviceClass *mdc =MEMORY_DEVICE_GET_CLASS (obj); + +if (dev->realized) { + *size += mdc->get_plugged_size (md, &error_abort); + } + } + +object_child_foreach (obj, memory_device_plugged_size, opaque); +return 0 ; +} + +uint64_t get_plugged_memory_size (void ) +{ +uint64_t size =0 ; + +memory_device_plugged_size (qdev_get_machine (), &size); + +return size; +} + +static const TypeInfo memory_device_info = { + .name =TYPE_MEMORY_DEVICE , + .parent =TYPE_INTERFACE , + .class_size =sizeof (MemoryDeviceClass), +}; + +static void memory_device_register_types (void ) +{ +type_register_static (&memory_device_info); +} + +type_init (memory_device_register_types)hw/mem/pc-dimm.c +63
-56@@ -21,6 +21,7 @@#include "qemu/osdep.h" #include "hw/mem/pc-dimm.h" #include "hw/mem/nvdimm.h" +#include "hw/mem/memory-device.h" #include "qapi/error.h" #include "qemu/config-file.h" #include "qapi/visitor.h" @@ -158,11 +159,6 @@ uint64_t pc_existing_dimms_capacity(Error **errp)return cap.size; } -uint64_t get_plugged_memory_size (void ) -{ -return pc_existing_dimms_capacity (&error_abort); -} -static int pc_dimm_slot2bitmap (Object *obj,void *opaque) {unsigned long *bitmap = opaque;@@ -238,57 +234,6 @@ static int pc_dimm_built_list(Object *obj, void *opaque)return 0 ; } -MemoryDeviceInfoList *qmp_pc_dimm_device_list (void ) -{ -GSList *dimms =NULL , *item; -MemoryDeviceInfoList *list =NULL , *prev =NULL ; - -object_child_foreach (qdev_get_machine (), pc_dimm_built_list, &dimms); - -for (item = dimms; item; item =g_slist_next (item)) { -PCDIMMDevice *dimm =PC_DIMM (item->data); -Object *obj =OBJECT (dimm); -MemoryDeviceInfoList *elem =g_new0 (MemoryDeviceInfoList,1 ); -MemoryDeviceInfo *info =g_new0 (MemoryDeviceInfo,1 ); -PCDIMMDeviceInfo *di =g_new0 (PCDIMMDeviceInfo,1 ); -bool is_nvdimm =object_dynamic_cast (obj,TYPE_NVDIMM ); -DeviceClass *dc =DEVICE_GET_CLASS (obj); -DeviceState *dev =DEVICE (obj); - -if (dev->id) { - di->has_id = true; - di->id =g_strdup (dev->id); - } - di->hotplugged = dev->hotplugged; - di->hotpluggable = dc->hotpluggable; - di->addr = dimm->addr; - di->slot = dimm->slot; - di->node = dimm->node; - di->size =object_property_get_uint (obj,PC_DIMM_SIZE_PROP ,NULL ); - di->memdev =object_get_canonical_path (OBJECT (dimm->hostmem)); - -if (!is_nvdimm) { - info->u.dimm.data = di; - info->type =MEMORY_DEVICE_INFO_KIND_DIMM ; - }else { - info->u.nvdimm.data = di; - info->type =MEMORY_DEVICE_INFO_KIND_NVDIMM ; - } - elem->value = info; - elem->next =NULL ; -if (prev) { - prev->next = elem; - }else { - list = elem; - } - prev = elem; - } - -g_slist_free (dimms); - -return list; -} - uint64_t pc_dimm_get_free_addr(uint64_t address_space_start, uint64_t address_space_size, uint64_t *hint, uint64_t align, uint64_t size,@@ -445,10 +390,62 @@ static MemoryRegion *pc_dimm_get_vmstate_memory_region(PCDIMMDevice *dimm)return host_memory_backend_get_memory (dimm->hostmem, &error_abort); } +static uint64_t pc_dimm_md_get_addr (MemoryDeviceState *md) +{ +PCDIMMDevice *dimm =PC_DIMM (md); + +return dimm->addr; +} + +static uint64_t pc_dimm_md_get_region_size (MemoryDeviceState *md,Error **errp) +{ +PCDIMMDevice *dimm =PC_DIMM (md); +PCDIMMDeviceClass *ddc =PC_DIMM_GET_CLASS (md); +MemoryRegion *mr; + + mr = ddc->get_memory_region (dimm, errp); +if (!mr) { +return 0 ; + } + +return memory_region_size (mr); +} + +static void pc_dimm_md_fill_device_info (MemoryDeviceState *md, +MemoryDeviceInfo *info) +{ +PCDIMMDeviceInfo *di =g_new0 (PCDIMMDeviceInfo,1 ); +DeviceClass *dc =DEVICE_GET_CLASS (md); +PCDIMMDevice *dimm =PC_DIMM (md); +DeviceState *dev =DEVICE (md); + +if (dev->id) { + di->has_id = true; + di->id =g_strdup (dev->id); + } + di->hotplugged = dev->hotplugged; + di->hotpluggable = dc->hotpluggable; + di->addr = dimm->addr; + di->slot = dimm->slot; + di->node = dimm->node; + di->size =object_property_get_uint (OBJECT (dimm),PC_DIMM_SIZE_PROP , +NULL ); + di->memdev =object_get_canonical_path (OBJECT (dimm->hostmem)); + +if (object_dynamic_cast (OBJECT (dev),TYPE_NVDIMM )) { + info->u.nvdimm.data = di; + info->type =MEMORY_DEVICE_INFO_KIND_NVDIMM ; + }else { + info->u.dimm.data = di; + info->type =MEMORY_DEVICE_INFO_KIND_DIMM ; + } +} +static void pc_dimm_class_init (ObjectClass *oc,void *data) {DeviceClass *dc =DEVICE_CLASS (oc);PCDIMMDeviceClass *ddc =PC_DIMM_CLASS (oc); +MemoryDeviceClass *mdc =MEMORY_DEVICE_CLASS (oc); dc->realize = pc_dimm_realize; dc->unrealize = pc_dimm_unrealize;@@ -457,6 +454,12 @@ static void pc_dimm_class_init(ObjectClass *oc, void *data) ddc->get_memory_region = pc_dimm_get_memory_region; ddc->get_vmstate_memory_region = pc_dimm_get_vmstate_memory_region; + + mdc->get_addr = pc_dimm_md_get_addr; +/* for a dimm plugged_size == region_size */ + mdc->get_plugged_size = pc_dimm_md_get_region_size; + mdc->get_region_size = pc_dimm_md_get_region_size; + mdc->fill_device_info = pc_dimm_md_fill_device_info; }static TypeInfo pc_dimm_info = {@@ -466,6 +469,10 @@ static TypeInfo pc_dimm_info = { .instance_init = pc_dimm_init, .class_init = pc_dimm_class_init, .class_size = sizeof(PCDIMMDeviceClass), + .interfaces = (InterfaceInfo []) { + {TYPE_MEMORY_DEVICE }, + { } + }, };static void pc_dimm_register_types (void )hw/ppc/spapr.c +2
-1@@ -74,6 +74,7 @@#include "hw/compat.h" #include "qemu/cutils.h" #include "hw/ppc/spapr_cpu_core.h" +#include "hw/mem/memory-device.h" #include <libfdt.h> @@ -722,7 +723,7 @@ static int spapr_populate_drconf_memory(sPAPRMachineState *spapr, void *fdt) }if (hotplug_lmb_start) { - dimms =qmp_pc_dimm_device_list (); + dimms =qmp_memory_device_list (); }/* ibm,dynamic-memory */ hw/ppc/spapr_hcall.c +1
-0@@ -14,6 +14,7 @@#include "kvm_ppc.h" #include "hw/ppc/spapr_ovec.h" #include "mmu-book3s-v3.h" +#include "hw/mem/memory-device.h" struct SPRSyncState {int spr;include/hw/mem/memory-device.h new file +44
-0@@ -0,0 +1,44 @@ +/* +* Memory Device Interface +* +* Copyright (c) 2018 Red Hat, Inc. +* +* Authors: +* David Hildenbrand <[email protected]> +* +* This work is licensed under the terms of the GNU GPL, version 2 or later. +* See the COPYING file in the top-level directory. +*/ + +#ifndef MEMORY_DEVICE_H +#define MEMORY_DEVICE_H + +#include "qom/object.h" +#include "hw/qdev.h" + +#define TYPE_MEMORY_DEVICE "memory-device" + +#define MEMORY_DEVICE_CLASS (klass) \ + OBJECT_CLASS_CHECK(MemoryDeviceClass, (klass), TYPE_MEMORY_DEVICE) +#define MEMORY_DEVICE_GET_CLASS (obj) \ + OBJECT_GET_CLASS(MemoryDeviceClass, (obj), TYPE_MEMORY_DEVICE) +#define MEMORY_DEVICE (obj) \ + INTERFACE_CHECK(MemoryDeviceState, (obj), TYPE_MEMORY_DEVICE) + +typedef struct MemoryDeviceState { +Object parent_obj; +}MemoryDeviceState ; + +typedef struct MemoryDeviceClass { +InterfaceClass parent_class; + +uint64_t (*get_addr)(MemoryDeviceState *md); +uint64_t (*get_plugged_size)(MemoryDeviceState *md,Error **errp); +uint64_t (*get_region_size)(MemoryDeviceState *md,Error **errp); +void (*fill_device_info)(MemoryDeviceState *md,MemoryDeviceInfo *info); +}MemoryDeviceClass ; + +MemoryDeviceInfoList *qmp_memory_device_list (void ); +uint64_t get_plugged_memory_size (void ); + +#endif include/hw/mem/pc-dimm.h +0
-2@@ -93,9 +93,7 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,int pc_dimm_get_free_slot (const int *hint,int max_slots,Error **errp); -MemoryDeviceInfoList *qmp_pc_dimm_device_list (void );uint64_t pc_existing_dimms_capacity (Error **errp); -uint64_t get_plugged_memory_size (void );void pc_dimm_memory_plug (DeviceState *dev,MemoryHotplugState *hpms,MemoryRegion *mr,uint64_t align,Error **errp); void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,numa.c +2
-1@@ -36,6 +36,7 @@#include "hw/boards.h" #include "sysemu/hostmem.h" #include "hw/mem/pc-dimm.h" +#include "hw/mem/memory-device.h" #include "qemu/option.h" #include "qemu/config-file.h" #include "qemu/cutils.h" @@ -520,7 +521,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,static void numa_stat_memory_devices (NumaNodeMem node_mem[]) { -MemoryDeviceInfoList *info_list =qmp_pc_dimm_device_list (); +MemoryDeviceInfoList *info_list =qmp_memory_device_list ();MemoryDeviceInfoList *info;PCDIMMDeviceInfo *pcdimm_info;qmp.c +2
-2@@ -39,7 +39,7 @@#include "qapi/qobject-input-visitor.h" #include "hw/boards.h" #include "qom/object_interfaces.h" -#include "hw/mem/pc-dimm.h" +#include "hw/mem/memory-device.h" #include "hw/acpi/acpi_dev_interface.h" NameInfo *qmp_query_name (Error **errp)@@ -731,7 +731,7 @@ void qmp_object_del(const char *id, Error **errp)MemoryDeviceInfoList *qmp_query_memory_devices (Error **errp) { -return qmp_pc_dimm_device_list (); +return qmp_memory_device_list (); }ACPIOSTInfoList *qmp_query_acpi_ospm_status (Error **errp)stubs/Makefile.objs +1
-1@@ -34,7 +34,7 @@ stub-obj-y += uuid.o stub-obj-y += vm-stop.o stub-obj-y += vmstate.o stub-obj-$(CONFIG_WIN32) += fd-register.o -stub-obj-y += qmp_pc_dimm.o +stub-obj-y += qmp_memory_device.o stub-obj-y += target-monitor-defs.o stub-obj-y += target-get-monitor-def.o stub-obj-y += pc_madt_cpu_entry.ostubs/qmp_memory_device.c renamed from stubs/qmp_pc_dimm.c +2
-2@@ -1,8 +1,8 @@#include "qemu/osdep.h" #include "qom/object.h" -#include "hw/mem/pc-dimm.h" +#include "hw/mem/memory-device.h" -MemoryDeviceInfoList *qmp_pc_dimm_device_list (void ) +MemoryDeviceInfoList *qmp_memory_device_list (void ) {return NULL ; }
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] [PATCH v2 1/3] pc-dimm: factor out MemoryDevice interface
Quoted 21 lines · David Gibson
Quoted 16 lines · David Hildenbrand
On the qmp level, we already have the concept of memory devices:
"query-memory-devices"
Right now, we only support NVDIMM and PCDIMM.We want to map other devices later into the address space of the guest. Such device could e.g. be virtio devices. These devices will have a guest memory range assigned but won't be exposed via e.g. ACPI. We want to make them look like memory device, but not glued to pc-dimm.
Especially, it will not always be possible to have
TYPE_PC_DIMMas a parent class (e.g. virtio devices). Let's use an interface instead. As a first part, convert handling of
-qmp_pc_dimm_device_list
-get_plugged_memory_size
to our new model. plug/unplug stuff etc. will follow later.Quoted 3 lines
A memory device will have to provide the following functions:
-get_addr(): Necessary, as the property "addr" can e.g. not be used for virtio devices (already defined).
Quoted 5 lines
Is the assumption here that a memory device has a fixed RAM address (GPA) from when it is realized? It seems to me plausible that you could have a memory device that has a specific size, but has a BAR of some sort that's programmed by the guest as part of the "plug" process.
Yes, the assumption is that the
a) address in physical memory
b) region size
Are defined at realize time. Which holds true for DIMM/NVDIMM and will hold true for the two virtio device types we have in mind (virtio-mem/virtio-pmem).
Both basically allow to find free address ranges.
Quoted 81 lines
Quoted 81 lines
-
get_plugged_size(): The amount this device offers to the guest as of now.
-get_region_size(): Because this can later on be bigger than the plugged size.
-fill_device_info(): Fill MemoryDeviceInfo, e.g. for qmp.Signed-off-by David Hildenbrand <[email protected]>--- hw/i386/acpi-build.c | 3 +- hw/mem/Makefile.objs | 1 + hw/mem/memory-device.c | 119 +++++++++++++++++++++++++++ hw/mem/pc-dimm.c | 119 ++++++++++++++------------- hw/ppc/spapr.c | 3 +- hw/ppc/spapr_hcall.c | 1 + include/hw/mem/memory-device.h | 44 ++++++++++ include/hw/mem/pc-dimm.h | 2 - numa.c | 3 +- qmp.c | 4 +- stubs/Makefile.objs | 2 +- stubs/{qmp_pc_dimm.c => qmp_memory_device.c} | 4 +- 12 files changed, 239 insertions(+), 66 deletions(-) create mode 100644 hw/mem/memory-device.c create mode 100644 include/hw/mem/memory-device.h rename stubs/{qmp_pc_dimm.c => qmp_memory_device.c} (61%)hw/i386/acpi-build.c +2
-1@@ -46,6 +46,7 @@#include "hw/acpi/vmgenid.h" #include "sysemu/tpm_backend.h" #include "hw/timer/mc146818rtc_regs.h" +#include "hw/mem/memory-device.h" #include "sysemu/numa.h" /* Supported chipsets: */ @@ -2253,7 +2254,7 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog)static void build_srat_hotpluggable_memory (GArray *table_data,uint64_t base,uint64_t len,int default_node) { -MemoryDeviceInfoList *info_list =qmp_pc_dimm_device_list (); +MemoryDeviceInfoList *info_list =qmp_memory_device_list ();MemoryDeviceInfoList *info;MemoryDeviceInfo *mi;PCDIMMDeviceInfo *di;hw/mem/Makefile.objs +1
-0@@ -1,2 +1,3 @@ common-obj-$(CONFIG_MEM_HOTPLUG) += pc-dimm.o +common-obj-$(CONFIG_MEM_HOTPLUG) += memory-device.o common-obj-$(CONFIG_NVDIMM) += nvdimm.ohw/mem/memory-device.c new file +20
-0@@ -0,0 +1,119 @@ +/* +* Memory Device Interface +* +* Copyright ProfitBricks GmbH 2012 +* Copyright (C) 2014 Red Hat Inc +* Copyright (c) 2018 Red Hat Inc +* +* This work is licensed under the terms of the GNU GPL, version 2 or later. +* See the COPYING file in the top-level directory. +*/ + +#include "qemu/osdep.h" +#include "hw/mem/memory-device.h" +#include "hw/qdev.h" +#include "qapi/error.h" +#include "hw/boards.h" +#include "qemu/range.h" + +static gint memory_device_addr_sort (gconstpointer a,gconstpointer b) +{
Quoted 8 lines
hw/mem/memory-device.c line 23
Quoted 3 lines
hw/mem/memory-device.c new file +3
-0+MemoryDeviceState *md_a =MEMORY_DEVICE (a); +MemoryDeviceState *md_b =MEMORY_DEVICE (b); +MemoryDeviceClass *mdc =MEMORY_DEVICE_GET_CLASS (a);AFAICT from the code below, the two devices don't necessarily have to have the same class, so I think you need to get the class pointers for each of them separately. (Or if they do have to be the same by design, I think there should at least be an assert).
Haven't considered that, my assumption was that any memory device class would be sufficient to find/use the pointer (as that layout should not change). But I might be wrong, so I'll lookup both pointers, thanks!
Quoted 11 lines
Quoted 11 lines
hw/mem/memory-device.c new file +10
-0+const uint64_t addr_a = mdc->get_addr (md_a); +const uint64_t addr_b = mdc->get_addr (md_b); + +if (addr_a > addr_b) { +return 1 ; + }else if (addr_a < addr_b) { +return -1 ; + } +return 0 ; +}
Signature · 3 lines
Thanks, David / dhildenb
PATCH v2 2/3 machine: make MemoryHotplugState accessible via the machine
In reply to David HildenbrandLet's allow to query the MemoryHotplugState from the machine.
This allows us to generically detect if a certain machine has support for memory devices, and to generically manage it (find free address range, plug/unplug a memory region).
--- hw/i386/pc.c | 12 ++++++++++++ hw/ppc/spapr.c | 12 ++++++++++++ include/hw/boards.h | 16 ++++++++++++++++ include/hw/mem/pc-dimm.h | 12 +----------- 4 files changed, 41 insertions(+), 11 deletions(-)
hw/i386/pc.c +12 -0
@@ -2337,6 +2337,17 @@ static void x86_nmi(NMIState *n, int cpu_index, Error **errp) } } +static MemoryHotplugState *pc_machine_get_memory_hotplug_state (MachineState *ms) +{ +PCMachineState *pcms =PC_MACHINE (ms); + +if (!pcms->hotplug_memory.base) { +/* hotplug not supported */ +return NULL ; + } +return &pcms->hotplug_memory; +} +static void pc_machine_class_init (ObjectClass *oc,void *data) {MachineClass *mc =MACHINE_CLASS (oc);
@@ -2376,6 +2387,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) hc->unplug = pc_machine_device_unplug_cb; nc->nmi_monitor_handler = x86_nmi; mc->default_cpu_type =TARGET_DEFAULT_CPU_TYPE ; + mc->get_memory_hotplug_state = pc_machine_get_memory_hotplug_state; object_class_property_add(oc, PC_MACHINE_MEMHP_REGION_SIZE, "int", pc_machine_get_hotplug_memory_region_size, NULL,
hw/ppc/spapr.c +12 -0
@@ -3868,6 +3868,17 @@ PowerPCCPU *spapr_find_cpu(int vcpu_id)return NULL ; } +static MemoryHotplugState *spapr_get_memory_hotplug_state (MachineState *ms) +{ +sPAPRMachineState *spapr =SPAPR_MACHINE (ms); + +if (!spapr->hotplug_memory.base) { +/* hotplug not supported */ +return NULL ; + } +return &spapr->hotplug_memory; +} +static void spapr_machine_class_init (ObjectClass *oc,void *data) {MachineClass *mc =MACHINE_CLASS (oc);
@@ -3927,6 +3938,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)* in which LMBs are represented and hot-added */ mc->numa_mem_align_shift =28 ; + mc->get_memory_hotplug_state = spapr_get_memory_hotplug_state; smc->default_caps.caps[SPAPR_CAP_HTM ] =SPAPR_CAP_OFF ; smc->default_caps.caps[SPAPR_CAP_VSX ] =SPAPR_CAP_ON ;
include/hw/boards.h +16 -0
@@ -105,6 +105,17 @@ typedef struct {CPUArchId cpus[0 ]; } CPUArchIdList; +/** +* MemoryHotplugState: +* @base: address in guest physical address space where hotplug memory +* address space begins. +* @mr: hotplug memory address space container +*/ +typedef struct MemoryHotplugState { +hwaddr base; +MemoryRegion mr; +}MemoryHotplugState ; +/** * MachineClass: * @max_cpus: maximum number of CPUs supported. Default: 1
@@ -156,6 +167,10 @@ typedef struct {* should instead use "unimplemented-device" for all memory ranges where * the guest will attempt to probe for a device that QEMU doesn't * implement and a stub device is required. +* @get_memory_hotplug_state: +* If a machine support memory hotplug, the returned data ontains +* information about the portion of guest physical address space +* where memory devices can be mapped to (e.g. to hotplug a pc-dimm). */ struct MachineClass {/*< private >*/
@@ -212,6 +227,7 @@ struct MachineClass { unsigned cpu_index);const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);int64_t (*get_default_cpu_node_id)(const MachineState *ms,int idx); + MemoryHotplugState *(*get_memory_hotplug_state)(MachineState *ms); };/**
include/hw/mem/pc-dimm.h +1 -11
@@ -19,6 +19,7 @@#include "exec/memory.h" #include "sysemu/hostmem.h" #include "hw/qdev.h" +#include "hw/boards.h" #define TYPE_PC_DIMM "pc-dimm"#define PC_DIMM (obj) \
@@ -75,17 +76,6 @@ typedef struct PCDIMMDeviceClass { MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm); } PCDIMMDeviceClass; -/** -* MemoryHotplugState: -* @base: address in guest physical address space where hotplug memory -* address space begins. -* @mr: hotplug memory address space container -*/ -typedef struct MemoryHotplugState { -hwaddr base; -MemoryRegion mr; -}MemoryHotplugState ; - uint64_t pc_dimm_get_free_addr(uint64_t address_space_start, uint64_t address_space_size, uint64_t *hint, uint64_t align, uint64_t size,
Signature · 1 line
2.14.3
PATCH v2 3/3 pc-dimm: factor out address space logic into MemoryDevice code
In reply to David HildenbrandTo be able to reuse MemoryDevice logic from other devices besides pc-dimm, factor the relevant stuff out into the MemoryDevice code.
As we don't care about slots for memory devices that are not pc-dimm, don't factor that part out.
Most of this patch just moves checks and logic around. While at it, make the code properly detect certain error conditions better (e.g. fragmented memory).
--- hw/i386/pc.c | 12 +-- hw/mem/memory-device.c | 162 ++++++++++++++++++++++++++++++++++++ hw/mem/pc-dimm.c | 185 +++-------------------------------------- hw/ppc/spapr.c | 9 +- include/hw/mem/memory-device.h | 4 + include/hw/mem/pc-dimm.h | 14 +--- 6 files changed, 185 insertions(+), 201 deletions(-)
hw/i386/pc.c +2 -10
@@ -1711,7 +1711,7 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, goto out; } -pc_dimm_memory_plug (dev, &pcms->hotplug_memory, mr, align, &local_err); +pc_dimm_memory_plug (dev, align, &local_err);if (local_err) { goto out; }
@@ -1761,17 +1761,9 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) {PCMachineState *pcms =PC_MACHINE (hotplug_dev); -PCDIMMDevice *dimm =PC_DIMM (dev); -PCDIMMDeviceClass *ddc =PC_DIMM_GET_CLASS (dimm); -MemoryRegion *mr;HotplugHandlerClass *hhc;Error *local_err =NULL ; - mr = ddc->get_memory_region (dimm, &local_err); -if (local_err) { - goto out; - } - hhc =HOTPLUG_HANDLER_GET_CLASS (pcms->acpi_dev); hhc->unplug (HOTPLUG_HANDLER (pcms->acpi_dev), dev, &local_err);
@@ -1779,7 +1771,7 @@ static void pc_dimm_unplug(HotplugHandler *hotplug_dev, goto out; } -pc_dimm_memory_unplug (dev, &pcms->hotplug_memory, mr); +pc_dimm_memory_unplug (dev);object_unparent (OBJECT (dev)); out:
hw/mem/memory-device.c +162 -0
174 lines not on this page · raw message
hw/mem/pc-dimm.c +12 -173
242 lines not on this page · raw message
hw/ppc/spapr.c +3 -6
@@ -3041,7 +3041,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev, align =memory_region_get_alignment (mr); size =memory_region_size (mr); -pc_dimm_memory_plug (dev, &ms->hotplug_memory, mr, align, &local_err); +pc_dimm_memory_plug (dev, align, &local_err);if (local_err) { goto out; }
@@ -3062,7 +3062,7 @@ static void spapr_memory_plug(HotplugHandler *hotplug_dev, DeviceState *dev,return ; out_unplug: -pc_dimm_memory_unplug (dev, &ms->hotplug_memory, mr); +pc_dimm_memory_unplug (dev); out:error_propagate (errp, local_err); }
@@ -3180,9 +3180,6 @@ static sPAPRDIMMState *spapr_recover_pending_dimm_state(sPAPRMachineState *ms,void spapr_lmb_release (DeviceState *dev) {sPAPRMachineState *spapr =SPAPR_MACHINE (qdev_get_hotplug_handler (dev)); -PCDIMMDevice *dimm =PC_DIMM (dev); -PCDIMMDeviceClass *ddc =PC_DIMM_GET_CLASS (dimm); -MemoryRegion *mr = ddc->get_memory_region (dimm, &error_abort);sPAPRDIMMState *ds =spapr_pending_dimm_unplugs_find (spapr,PC_DIMM (dev));/* This information will get lost if a migration occurs
@@ -3202,7 +3199,7 @@ void spapr_lmb_release(DeviceState *dev)* Now that all the LMBs have been removed by the guest, call the * pc-dimm unplug handler to cleanup up the pc-dimm device. */ -pc_dimm_memory_unplug (dev, &spapr->hotplug_memory, mr); +pc_dimm_memory_unplug (dev);object_unparent (OBJECT (dev));spapr_pending_dimm_unplugs_remove (spapr, ds); }
include/hw/mem/memory-device.h +4 -0
@@ -40,5 +40,9 @@ typedef struct MemoryDeviceClass {MemoryDeviceInfoList *qmp_memory_device_list (void );uint64_t get_plugged_memory_size (void ); +uint64_t memory_device_get_free_addr (uint64_t *hint,uint64_t align, +uint64_t size,Error **errp); +void memory_device_plug_region (MemoryRegion *mr,uint64_t addr,Error **errp); +void memory_device_unplug_region (MemoryRegion *mr);#endif
include/hw/mem/pc-dimm.h +2 -12
@@ -19,7 +19,6 @@#include "exec/memory.h" #include "sysemu/hostmem.h" #include "hw/qdev.h" -#include "hw/boards.h" #define TYPE_PC_DIMM "pc-dimm"#define PC_DIMM (obj) \
@@ -76,16 +75,7 @@ typedef struct PCDIMMDeviceClass { MemoryRegion *(*get_vmstate_memory_region)(PCDIMMDevice *dimm); } PCDIMMDeviceClass; -uint64_t pc_dimm_get_free_addr (uint64_t address_space_start, -uint64_t address_space_size, -uint64_t *hint,uint64_t align,uint64_t size, -Error **errp); -int pc_dimm_get_free_slot (const int *hint,int max_slots,Error **errp); - -uint64_t pc_existing_dimms_capacity (Error **errp); -void pc_dimm_memory_plug (DeviceState *dev,MemoryHotplugState *hpms, -MemoryRegion *mr,uint64_t align,Error **errp); -void pc_dimm_memory_unplug (DeviceState *dev,MemoryHotplugState *hpms, -MemoryRegion *mr); +void pc_dimm_memory_plug (DeviceState *dev,uint64_t align,Error **errp); +void pc_dimm_memory_unplug (DeviceState *dev);#endif
Signature · 1 line
2.14.3