On 20 July 2016 at 01:49, Kwangwoo Lee <kwangwoo....@sk.com> wrote:
> Add hotplug memory feature on aarch64 virt platfom. This patch is
> required to emulate a DIMM type memory like NVDIMM.
>
> Signed-off-by: Kwangwoo Lee <kwangwoo....@sk.com>
> ---
> default-configs/aarch64-softmmu.mak | 1 +
> hw/arm/virt.c | 110
> ++++++++++++++++++++++++++++++++++++
> include/hw/arm/virt.h | 3 +
> 3 files changed, 114 insertions(+)
>
> diff --git a/default-configs/aarch64-softmmu.mak
> b/default-configs/aarch64-softmmu.mak
> index 2449483..5790cd2 100644
> --- a/default-configs/aarch64-softmmu.mak
> +++ b/default-configs/aarch64-softmmu.mak
> @@ -7,3 +7,4 @@ CONFIG_AUX=y
> CONFIG_DDC=y
> CONFIG_DPCD=y
> CONFIG_XLNX_ZYNQMP=y
> +CONFIG_MEM_HOTPLUG=y
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index a193b5a..f7ff411 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -58,6 +58,8 @@
> #include "hw/smbios/smbios.h"
> #include "qapi/visitor.h"
> #include "standard-headers/linux/input.h"
> +#include "hw/mem/pc-dimm.h"
> +#include "hw/acpi/acpi.h"
>
> /* Number of external interrupt lines to configure the GIC with */
> #define NUM_IRQS 256
> @@ -91,6 +93,7 @@ typedef struct {
> bool secure;
> bool highmem;
> int32_t gic_version;
> + MemoryHotplugState hotplug_memory;
> } VirtMachineState;
>
> #define TYPE_VIRT_MACHINE MACHINE_TYPE_NAME("virt")
> @@ -1376,6 +1379,40 @@ static void machvirt_init(MachineState *machine)
> guest_info_state->machine_done.notify = virt_guest_info_machine_done;
> qemu_add_machine_init_done_notifier(&guest_info_state->machine_done);
>
> + /* initialize hotplug memory address space */
> + if (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,
"number of"
> + 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
> "
"must be"
> + "%d bytes", TARGET_PAGE_SIZE);
> + exit(EXIT_FAILURE);
> + }
> +
> + vms->hotplug_memory.base =
> + ROUND_UP(vbi->memmap[VIRT_MEM].base + machine->ram_size,
> + ARCH_VIRT_HOTPLUG_MEM_ALIGN);
> +
> + if ((vms->hotplug_memory.base + hotplug_mem_size) <
> + hotplug_mem_size) {
This expression is confusing. Is it trying to test for overflow?
When can that happen?
> + error_report("unsupported amount of maximum memory: "
> RAM_ADDR_FMT,
> + machine->maxram_size);
> + exit(EXIT_FAILURE);
> + }
> +
> + memory_region_init(&vms->hotplug_memory.mr, OBJECT(vms),
> + "hotplug-memory", hotplug_mem_size);
> + memory_region_add_subregion(sysmem, vms->hotplug_memory.base,
> + &vms->hotplug_memory.mr);
> + }
> +
> vbi->bootinfo.ram_size = machine->ram_size;
> vbi->bootinfo.kernel_filename = machine->kernel_filename;
> vbi->bootinfo.kernel_cmdline = machine->kernel_cmdline;
> @@ -1448,9 +1485,75 @@ static void virt_set_gic_version(Object *obj, const
> char *value, Error **errp)
> }
> }
>
> +static void virt_dimm_plug(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> + PCDIMMDevice *dimm = PC_DIMM(dev);
> + PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> + MemoryRegion *mr = ddc->get_memory_region(dimm);
> + Error *local_err = NULL;
> + uint64_t align = memory_region_get_alignment(mr);
> +
> + pc_dimm_memory_plug(dev, &vms->hotplug_memory, mr, align, &local_err);
> + if (local_err) {
> + goto out;
> + }
> +
> +out:
> + error_propagate(errp, local_err);
> +}
> +
> +static void virt_dimm_unplug(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> + PCDIMMDevice *dimm = PC_DIMM(dev);
> + PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
> + MemoryRegion *mr = ddc->get_memory_region(dimm);
> + Error *local_err = NULL;
> +
> + pc_dimm_memory_unplug(dev, &vms->hotplug_memory, mr);
> + object_unparent(OBJECT(dev));
> +
> + error_propagate(errp, local_err);
> +}
> +
> +static void virt_machinedevice_plug_cb(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> + virt_dimm_plug(hotplug_dev, dev, errp);
> + } else {
> + error_setg(errp, "acpi: device plug request for not supported device"
"unsupported"
> + " type: %s", object_get_typename(OBJECT(dev)));
> + }
> +}
> +
> +static void virt_machinedevice_unplug_cb(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> + virt_dimm_unplug(hotplug_dev, dev, errp);
> + } else {
> + error_setg(errp, "acpi: device unplug for not supported device"
"unsupported"
> + " type: %s", object_get_typename(OBJECT(dev)));
> + }
> +}
> +
> +static HotplugHandler *virt_get_hotplug_handler(MachineState *machine,
> + DeviceState *dev)
> +{
> + if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> + return HOTPLUG_HANDLER(machine);
> + }
> + return NULL;
> +}
> +
> static void virt_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> + HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>
> mc->init = machvirt_init;
> /* Start max_cpus at the maximum QEMU supports. We'll further restrict
> @@ -1462,6 +1565,9 @@ static void virt_machine_class_init(ObjectClass *oc,
> void *data)
> mc->block_default_type = IF_VIRTIO;
> mc->no_cdrom = 1;
> mc->pci_allow_0_address = true;
> + mc->get_hotplug_handler = virt_get_hotplug_handler;
> + hc->plug = virt_machinedevice_plug_cb;
> + hc->unplug = virt_machinedevice_unplug_cb;
> }
>
> static const TypeInfo virt_machine_info = {
> @@ -1471,6 +1577,10 @@ static const TypeInfo virt_machine_info = {
> .instance_size = sizeof(VirtMachineState),
> .class_size = sizeof(VirtMachineClass),
> .class_init = virt_machine_class_init,
> + .interfaces = (InterfaceInfo[]) {
> + { TYPE_HOTPLUG_HANDLER },
> + { }
> + },
> };
>
> static void machvirt_machine_init(void)
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 9650193..35aa3cc 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -45,6 +45,9 @@
>
> #define PPI(irq) ((irq) + 16)
>
> +/* 1GB alignment for hotplug memory region */
> +#define ARCH_VIRT_HOTPLUG_MEM_ALIGN (1ULL << 30)
Where does the 1GB alignment come from? Why do we need 1GB
alignment for the base but only 1KB alignment for the size?
> +
> enum {
> VIRT_FLASH,
> VIRT_MEM,
> --
> 2.5.0
thanks
-- PMM
