On Fri, 19 Jun 2015 10:40:21 +0530 Bharata B Rao <[email protected]> wrote:
> pc_dimm_plug() has code that will be needed for memory plug handlers > in other archs too. Extract code from pc_dimm_plug() into a generic > routine pc_dimm_memory_plug() that resides in pc-dimm.c. Also > correspondingly refactor re-usable unplug code into > pc_dimm_memory_unplug(). > > Signed-off-by: Bharata B Rao <[email protected]> > Reviewed-by: David Gibson <[email protected]> > --- > hw/i386/acpi-build.c | 2 +- > hw/i386/pc.c | 90 > +++++++++--------------------------------------- > hw/mem/pc-dimm.c | 80 > ++++++++++++++++++++++++++++++++++++++++++ include/hw/i386/pc.h > | 4 +-- include/hw/mem/pc-dimm.h | 9 +++++ 5 files changed, 109 > insertions(+), 76 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index b71e942..5f6fa95 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -1512,7 +1512,7 @@ build_srat(GArray *table_data, GArray *linker, > PcGuestInfo *guest_info) */ > if (hotplugabble_address_space_size) { > numamem = acpi_data_push(table_data, sizeof *numamem); > - acpi_build_srat_memory(numamem, pcms->hotplug_memory_base, > + acpi_build_srat_memory(numamem, pcms->hotplug_memory.base, > hotplugabble_address_space_size, 0, > MEM_AFFINITY_HOTPLUGGABLE | > MEM_AFFINITY_ENABLED); > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 3f0d435..c869588 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -64,7 +64,6 @@ > #include "hw/pci/pci_host.h" > #include "acpi-build.h" > #include "hw/mem/pc-dimm.h" > -#include "trace.h" > #include "qapi/visitor.h" > #include "qapi-visit.h" > > @@ -1297,7 +1296,7 @@ FWCfgState *pc_memory_init(MachineState > *machine, exit(EXIT_FAILURE); > } > > - pcms->hotplug_memory_base = > + pcms->hotplug_memory.base = could you split out conversion to structure into separate patch, pls? > ROUND_UP(0x100000000ULL + above_4g_mem_size, 1ULL << 30); > > if (pcms->enforce_aligned_dimm) { > @@ -1305,17 +1304,17 @@ FWCfgState *pc_memory_init(MachineState > *machine, hotplug_mem_size += (1ULL << 30) * machine->ram_slots; > } > > - if ((pcms->hotplug_memory_base + hotplug_mem_size) < > + 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, OBJECT(pcms), > + 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); > + memory_region_add_subregion(system_memory, > pcms->hotplug_memory.base, > + &pcms->hotplug_memory.mr); > } > > /* Initialize PC system firmware */ > @@ -1333,9 +1332,9 @@ FWCfgState *pc_memory_init(MachineState > *machine, fw_cfg = bochs_bios_init(); > rom_set_fw(fw_cfg); > > - if (guest_info->has_reserved_memory && > pcms->hotplug_memory_base) { > + if (guest_info->has_reserved_memory && > pcms->hotplug_memory.base) { uint64_t *val = g_malloc(sizeof(*val)); > - *val = cpu_to_le64(ROUND_UP(pcms->hotplug_memory_base, > 0x1ULL << 30)); > + *val = cpu_to_le64(ROUND_UP(pcms->hotplug_memory.base, > 0x1ULL << 30)); fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", > val, sizeof(*val)); } > > @@ -1554,20 +1553,17 @@ void ioapic_init_gsi(GSIState *gsi_state, > const char *parent_name) static void pc_dimm_plug(HotplugHandler > *hotplug_dev, DeviceState *dev, Error **errp) > { > - int slot; > HotplugHandlerClass *hhc; > Error *local_err = NULL; > PCMachineState *pcms = PC_MACHINE(hotplug_dev); > - MachineState *machine = MACHINE(hotplug_dev); > PCDIMMDevice *dimm = PC_DIMM(dev); > PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm); > MemoryRegion *mr = ddc->get_memory_region(dimm); > - uint64_t existing_dimms_capacity = 0; > uint64_t align = TARGET_PAGE_SIZE; > - uint64_t addr; > > - addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, > &local_err); > - if (local_err) { > + if (!pcms->acpi_dev) { > + error_setg(&local_err, > + "memory hotplug is not enabled: missing acpi > device"); goto out; > } > > @@ -1575,67 +1571,18 @@ static void pc_dimm_plug(HotplugHandler > *hotplug_dev, align = memory_region_get_alignment(mr); > } > > - addr = pc_dimm_get_free_addr(pcms->hotplug_memory_base, > - > memory_region_size(&pcms->hotplug_memory), > - !addr ? NULL : &addr, align, > - memory_region_size(mr), &local_err); > - if (local_err) { > - goto out; > - } > - > - existing_dimms_capacity = pc_existing_dimms_capacity(&local_err); > - if (local_err) { > - goto out; > - } > - > - if (existing_dimms_capacity + memory_region_size(mr) > > - machine->maxram_size - machine->ram_size) { > - error_setg(&local_err, "not enough space, currently 0x%" > PRIx64 > - " in use of total hot pluggable 0x" RAM_ADDR_FMT, > - existing_dimms_capacity, > - machine->maxram_size - machine->ram_size); > - goto out; > - } > - > - object_property_set_int(OBJECT(dev), addr, PC_DIMM_ADDR_PROP, > &local_err); > + pc_dimm_memory_plug(dev, &pcms->hotplug_memory, mr, align, > &local_err); if (local_err) { > + pc_dimm_memory_unplug(dev, &pcms->hotplug_memory, mr); > goto out; > } > - trace_mhp_pc_dimm_assigned_address(addr); > > - slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP, > &local_err); > - if (local_err) { > - goto out; > - } > - > - slot = pc_dimm_get_free_slot(slot == PC_DIMM_UNASSIGNED_SLOT ? > NULL : &slot, > - machine->ram_slots, &local_err); > - if (local_err) { > - goto out; > - } > - object_property_set_int(OBJECT(dev), slot, PC_DIMM_SLOT_PROP, > &local_err); > + hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev); > + hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err); > if (local_err) { > - goto out; > + pc_dimm_memory_unplug(dev, &pcms->hotplug_memory, mr); > } > - trace_mhp_pc_dimm_assigned_slot(slot); > > - if (!pcms->acpi_dev) { > - error_setg(&local_err, > - "memory hotplug is not enabled: missing acpi > device"); > - goto out; > - } > - > - if (kvm_enabled() && !kvm_has_free_slot(machine)) { > - error_setg(&local_err, "hypervisor has no free memory slots > left"); > - goto out; > - } > - > - memory_region_add_subregion(&pcms->hotplug_memory, > - addr - pcms->hotplug_memory_base, > mr); > - vmstate_register_ram(mr, dev); > - > - hhc = HOTPLUG_HANDLER_GET_CLASS(pcms->acpi_dev); > - hhc->plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err); > out: > error_propagate(errp, local_err); > } > @@ -1677,11 +1624,8 @@ static void pc_dimm_unplug(HotplugHandler > *hotplug_dev, goto out; > } > > - memory_region_del_subregion(&pcms->hotplug_memory, mr); > - vmstate_unregister_ram(mr, dev); > - > + pc_dimm_memory_unplug(dev, &pcms->hotplug_memory, mr); > object_unparent(OBJECT(dev)); > - > out: > error_propagate(errp, local_err); > } > @@ -1766,7 +1710,7 @@ > pc_machine_get_hotplug_memory_region_size(Object *obj, Visitor *v, > void *opaque, const char *name, Error **errp) { > PCMachineState *pcms = PC_MACHINE(obj); > - int64_t value = memory_region_size(&pcms->hotplug_memory); > + int64_t value = memory_region_size(&pcms->hotplug_memory.mr); > > visit_type_int(v, &value, name, errp); > } > diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c > index e70633d..98971b7 100644 > --- a/hw/mem/pc-dimm.c > +++ b/hw/mem/pc-dimm.c > @@ -23,12 +23,92 @@ > #include "qapi/visitor.h" > #include "qemu/range.h" > #include "sysemu/numa.h" > +#include "sysemu/kvm.h" > +#include "trace.h" > > typedef struct pc_dimms_capacity { > uint64_t size; > Error **errp; > } pc_dimms_capacity; > > +void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState *hpms, > + MemoryRegion *mr, uint64_t align, Error > **errp) +{ > + int slot; > + MachineState *machine = MACHINE(qdev_get_machine()); > + PCDIMMDevice *dimm = PC_DIMM(dev); > + Error *local_err = NULL; > + uint64_t existing_dimms_capacity = 0; > + uint64_t addr; > + > + addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP, > &local_err); > + if (local_err) { > + goto out; > + } > + > + addr = pc_dimm_get_free_addr(hpms->base, > + memory_region_size(&hpms->mr), > + !addr ? NULL : &addr, align, > + memory_region_size(mr), &local_err); > + if (local_err) { > + goto out; > + } > + > + existing_dimms_capacity = pc_existing_dimms_capacity(&local_err); > + if (local_err) { > + goto out; > + } > + > + if (existing_dimms_capacity + memory_region_size(mr) > > + machine->maxram_size - machine->ram_size) { > + error_setg(&local_err, "not enough space, currently 0x%" > PRIx64 > + " in use of total hot pluggable 0x" RAM_ADDR_FMT, > + existing_dimms_capacity, > + machine->maxram_size - machine->ram_size); > + goto out; > + } > + > + object_property_set_int(OBJECT(dev), addr, PC_DIMM_ADDR_PROP, > &local_err); > + if (local_err) { > + goto out; > + } > + trace_mhp_pc_dimm_assigned_address(addr); > + > + slot = object_property_get_int(OBJECT(dev), PC_DIMM_SLOT_PROP, > &local_err); > + if (local_err) { > + goto out; > + } > + > + slot = pc_dimm_get_free_slot(slot == PC_DIMM_UNASSIGNED_SLOT ? > NULL : &slot, > + machine->ram_slots, &local_err); > + if (local_err) { > + goto out; > + } > + object_property_set_int(OBJECT(dev), slot, PC_DIMM_SLOT_PROP, > &local_err); > + if (local_err) { > + goto out; > + } > + trace_mhp_pc_dimm_assigned_slot(slot); > + > + if (kvm_enabled() && !kvm_has_free_slot(machine)) { > + error_setg(&local_err, "hypervisor has no free memory slots > left"); > + goto out; > + } > + > + memory_region_add_subregion(&hpms->mr, addr - hpms->base, mr); > + vmstate_register_ram(mr, dev); > + > +out: > + error_propagate(errp, local_err); > +} > + > +void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState > *hpms, > + MemoryRegion *mr) > +{ > + memory_region_del_subregion(&hpms->mr, mr); > + vmstate_unregister_ram(mr, dev); > +} > + > static int pc_existing_dimms_capacity_internal(Object *obj, void > *opaque) { > pc_dimms_capacity *cap = opaque; > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 86c5651..6628791 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -15,6 +15,7 @@ > #include "hw/pci/pci.h" > #include "hw/boards.h" > #include "hw/compat.h" > +#include "hw/mem/pc-dimm.h" > > #define HPET_INTCAP "hpet-intcap" > > @@ -32,8 +33,7 @@ struct PCMachineState { > MachineState parent_obj; > > /* <public> */ > - ram_addr_t hotplug_memory_base; > - MemoryRegion hotplug_memory; > + MemoryHotplugState hotplug_memory; > > HotplugHandler *acpi_dev; > ISADevice *rtc; > diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h > index f7b80b4..9fbab16 100644 > --- a/include/hw/mem/pc-dimm.h > +++ b/include/hw/mem/pc-dimm.h > @@ -70,6 +70,11 @@ typedef struct PCDIMMDeviceClass { > MemoryRegion *(*get_memory_region)(PCDIMMDevice *dimm); > } PCDIMMDeviceClass; > > +typedef struct MemoryHotplugState { > + ram_addr_t 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, @@ -79,4 +84,8 @@ int pc_dimm_get_free_slot(const int > *hint, int max_slots, Error **errp); > int qmp_pc_dimm_device_list(Object *obj, void *opaque); > 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); > #endif
