On Wed, Jul 11, 2012 at 10:31 AM, Vasilis Liaskovitis <vasilis.liaskovitis@xxxxxxxxxxxxxxxx> wrote: > Each hotplug-able memory slot is a SysBusDevice. A hot-add operation for a > particular dimm creates a new MemoryRegion of the given physical address > offset, size and node proximity, and attaches it to main system memory as a > sub_region. A hot-remove operation detaches and frees the MemoryRegion from > system memory. > > This prototype still lacks proper qdev integration: a separate > hotplug side-channel is used and main system bus hotplug capability is > ignored. > > Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@xxxxxxxxxxxxxxxx> > --- > hw/Makefile.objs | 2 +- > hw/dimm.c | 234 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/dimm.h | 58 +++++++++++++ > 3 files changed, 293 insertions(+), 1 deletions(-) > create mode 100644 hw/dimm.c > create mode 100644 hw/dimm.h > > diff --git a/hw/Makefile.objs b/hw/Makefile.objs > index 3d77259..e2184bf 100644 > --- a/hw/Makefile.objs > +++ b/hw/Makefile.objs > @@ -26,7 +26,7 @@ hw-obj-$(CONFIG_I8254) += i8254_common.o i8254.o > hw-obj-$(CONFIG_PCSPK) += pcspk.o > hw-obj-$(CONFIG_PCKBD) += pckbd.o > hw-obj-$(CONFIG_FDC) += fdc.o > -hw-obj-$(CONFIG_ACPI) += acpi.o acpi_piix4.o > +hw-obj-$(CONFIG_ACPI) += acpi.o acpi_piix4.o dimm.o > hw-obj-$(CONFIG_APM) += pm_smbus.o apm.o > hw-obj-$(CONFIG_DMA) += dma.o > hw-obj-$(CONFIG_I82374) += i82374.o > diff --git a/hw/dimm.c b/hw/dimm.c > new file mode 100644 > index 0000000..00c4623 > --- /dev/null > +++ b/hw/dimm.c > @@ -0,0 +1,234 @@ > +/* > + * Dimm device for Memory Hotplug > + * > + * Copyright ProfitBricks GmbH 2012 > + * 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 "trace.h" > +#include "qdev.h" > +#include "dimm.h" > +#include <time.h> > +#include "../exec-memory.h" > +#include "qmp-commands.h" > + > +static DeviceState *dimm_hotplug_qdev; > +static dimm_hotplug_fn dimm_hotplug; > +static QTAILQ_HEAD(Dimmlist, DimmState) dimmlist; Using global state does not look right. It should always be possible to pass around structures to avoid it. > + > +static Property dimm_properties[] = { > + DEFINE_PROP_END_OF_LIST() > +}; > + > +void dimm_populate(DimmState *s) All functions are global and exported but there does not seem to be users. Please make all static which you can. > +{ > + DeviceState *dev= (DeviceState*)s; > + MemoryRegion *new = NULL; > + > + new = g_malloc(sizeof(MemoryRegion)); > + memory_region_init_ram(new, dev->id, s->size); > + vmstate_register_ram_global(new); > + memory_region_add_subregion(get_system_memory(), s->start, new); > + s->mr = new; > + s->populated = true; > +} > + > + > +void dimm_depopulate(DimmState *s) > +{ > + assert(s); > + if (s->populated) { > + vmstate_unregister_ram(s->mr, NULL); > + memory_region_del_subregion(get_system_memory(), s->mr); > + memory_region_destroy(s->mr); > + s->populated = false; > + s->mr = NULL; > + } > +} > + > +DimmState *dimm_create(char *id, uint64_t size, uint64_t node, uint32_t > + dimm_idx, bool populated) > +{ > + DeviceState *dev; > + DimmState *mdev; > + > + dev = sysbus_create_simple("dimm", -1, NULL); > + dev->id = id; > + > + mdev = DIMM(dev); > + mdev->idx = dimm_idx; > + mdev->start = 0; > + mdev->size = size; > + mdev->node = node; > + mdev->populated = populated; > + QTAILQ_INSERT_TAIL(&dimmlist, mdev, nextdimm); > + return mdev; > +} > + > +void dimm_register_hotplug(dimm_hotplug_fn hotplug, DeviceState *qdev) > +{ > + dimm_hotplug_qdev = qdev; > + dimm_hotplug = hotplug; > + dimm_scan_populated(); > +} > + > +void dimm_activate(DimmState *slot) > +{ > + dimm_populate(slot); > + if (dimm_hotplug) > + dimm_hotplug(dimm_hotplug_qdev, (SysBusDevice*)slot, 1); Why the cast? Also braces, please check your patches with checkpatch.pl. > +} > + > +void dimm_deactivate(DimmState *slot) > +{ > + if (dimm_hotplug) > + dimm_hotplug(dimm_hotplug_qdev, (SysBusDevice*)slot, 0); > +} > + > +DimmState *dimm_find_from_name(char *id) const char *id? > +{ > + Error *err = NULL; > + DeviceState *qdev; > + const char *type; > + qdev = qdev_find_recursive(sysbus_get_default(), id); > + if (qdev) { > + type = object_property_get_str(OBJECT(qdev), "type", &err); > + if (!type) { > + return NULL; > + } > + if (!strcmp(type, "dimm")) { > + return DIMM(qdev); > + } > + } > + return NULL; > +} > + > +int dimm_do(Monitor *mon, const QDict *qdict, bool add) > +{ > + DimmState *slot = NULL; > + > + char *id = (char*) qdict_get_try_str(qdict, "id"); Why this cast? > + if (!id) { > + fprintf(stderr, "ERROR %s invalid id\n",__FUNCTION__); > + return 1; > + } > + > + slot = dimm_find_from_name(id); > + > + if (!slot) { > + fprintf(stderr, "%s no slot %s found\n", __FUNCTION__, id); > + return 1; > + } > + > + if (add) { > + if (slot->populated) { > + fprintf(stderr, "ERROR %s slot %s already populated\n", > + __FUNCTION__, id); > + return 1; > + } > + dimm_activate(slot); > + } > + else { > + if (!slot->populated) { > + fprintf(stderr, "ERROR %s slot %s is not populated\n", > + __FUNCTION__, id); > + return 1; > + } > + dimm_deactivate(slot); > + } > + > + return 0; > +} > + > +DimmState *dimm_find_from_idx(uint32_t idx) > +{ > + DimmState *slot; > + > + QTAILQ_FOREACH(slot, &dimmlist, nextdimm) { > + if (slot->idx == idx) { > + return slot; > + } > + } > + return NULL; > +} > + > +/* used to calculate physical address offsets for all dimms */ > +void dimm_calc_offsets(dimm_calcoffset_fn calcfn) > +{ > + DimmState *slot; > + QTAILQ_FOREACH(slot, &dimmlist, nextdimm) { > + if (!slot->start) > + slot->start = calcfn(slot->size); > + } > +} > + > +/* used to populate and activate dimms at boot time */ > +void dimm_scan_populated(void) > +{ > + DimmState *slot; > + QTAILQ_FOREACH(slot, &dimmlist, nextdimm) { > + if (slot->populated && !slot->mr) { > + dimm_activate(slot); > + } > + } > +} > + > +void dimm_notify(uint32_t idx, uint32_t event) > +{ > + DimmState *s; > + s = dimm_find_from_idx(idx); > + assert(s != NULL); > + > + switch(event) { > + case DIMM_REMOVE_SUCCESS: > + dimm_depopulate(s); > + break; > + default: > + break; > + } > +} > + > +static int dimm_init(SysBusDevice *s) > +{ > + DimmState *slot; > + slot = DIMM(s); > + slot->mr = NULL; > + slot->populated = false; > + return 0; > +} > + > +static void dimm_class_init(ObjectClass *klass, void *data) > +{ > + SysBusDeviceClass *sc = SYS_BUS_DEVICE_CLASS(klass); > + DeviceClass *dc = DEVICE_CLASS(klass); > + > + dc->props = dimm_properties; > + sc->init = dimm_init; > + dimm_hotplug = NULL; > + QTAILQ_INIT(&dimmlist); > +} > + > +static TypeInfo dimm_info = { > + .name = "dimm", > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(DimmState), > + .class_init = dimm_class_init, > +}; > + > +static void dimm_register_types(void) > +{ > + type_register_static(&dimm_info); > +} > + > +type_init(dimm_register_types) > diff --git a/hw/dimm.h b/hw/dimm.h > new file mode 100644 > index 0000000..643f319 > --- /dev/null > +++ b/hw/dimm.h > @@ -0,0 +1,58 @@ > +#ifndef QEMU_DIMM_H > +#define QEMU_DIMM_H Should be HW_DIMM_H. > + > +#include "qemu-common.h" > +#include "memory.h" > +#include "sysbus.h" > +#include "qapi-types.h" > +#include "qemu-queue.h" > +#include "cpus.h" > +#define MAX_DIMMS 255 > +#define DIMM_BITMAP_BYTES (MAX_DIMMS + 7) / 8 > +#define DEFAULT_DIMMSIZE 1024*1024*1024 > + > +typedef enum { > + DIMM_REMOVE_SUCCESS = 0, > + DIMM_REMOVE_FAIL = 1, > + DIMM_ADD_SUCCESS = 2, > + DIMM_ADD_FAIL = 3 > +} dimm_hp_result_code; > + > +#define TYPE_DIMM "dimm" > +#define DIMM(obj) \ > + OBJECT_CHECK(DimmState, (obj), TYPE_DIMM) > +#define DIMM_CLASS(klass) \ > + OBJECT_CLASS_CHECK(DimmClass, (obj), TYPE_DIMM) > +#define DIMM_GET_CLASS(obj) \ > + OBJECT_GET_CLASS(DimmClass, (obj), TYPE_DIMM) > + > +typedef struct DimmState { > + SysBusDevice busdev; > + uint32_t idx; /* index in memory hotplug register/bitmap */ > + ram_addr_t start; /* starting physical address */ > + ram_addr_t size; > + uint32_t node; /* numa node proximity */ > + MemoryRegion *mr; /* MemoryRegion for this slot. !NULL only if populated */ > + bool populated; /* 1 means device has been hotplugged. Default is 0. */ > + QTAILQ_ENTRY (DimmState) nextdimm; > +} DimmState; > + > +typedef int (*dimm_hotplug_fn)(DeviceState *qdev, SysBusDevice *dev, int add); > +typedef target_phys_addr_t (*dimm_calcoffset_fn)(uint64_t size); > + > +DimmState *dimm_create(char *id, uint64_t size, uint64_t node, uint32_t > + dimm_idx, bool populated); > +void dimm_populate(DimmState *s); > +void dimm_depopulate(DimmState *s); > +int dimm_do(Monitor *mon, const QDict *qdict, bool add); > +DimmState *dimm_find_from_idx(uint32_t idx); > +DimmState *dimm_find_from_name(char *id); > +void dimm_register_hotplug(dimm_hotplug_fn hotplug, DeviceState *qdev); > +void dimm_calc_offsets(dimm_calcoffset_fn calcfn); > +void dimm_activate(DimmState *slot); > +void dimm_deactivate(DimmState *slot); > +void dimm_scan_populated(void); > +void dimm_notify(uint32_t idx, uint32_t event); > + > + > +#endif > -- > 1.7.9 > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html