[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/4] pc, pc-dimm: Factor out reusable parts i
From: |
Bharata B Rao |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/4] pc, pc-dimm: Factor out reusable parts in pc_dimm_plug to a separate routine |
Date: |
Wed, 24 Jun 2015 16:41:54 +0530 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Fri, Jun 19, 2015 at 10:40:21AM +0530, Bharata B Rao 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 <address@hidden>
> Reviewed-by: David Gibson <address@hidden>
> ---
> 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 =
> 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;
> }
Shouldn't be calling pc_dimm_memory_unplug() here because when
pc_dimm_memory_plug() fails, it wouldn't have done
memory_region_add_subregion() and vmstate_register_ram().
Will fix this in the next post.
Regards,
Bharata.
- [Qemu-devel] [PATCH v2 0/4] Refactoring pc_dimm_plug and NUMA node lookup API, Bharata B Rao, 2015/06/19
- [Qemu-devel] [PATCH v2 1/4] pc, pc-dimm: Factor out reusable parts in pc_dimm_plug to a separate routine, Bharata B Rao, 2015/06/19
- [Qemu-devel] [PATCH v2 3/4] numa: Store boot memory address range in node_info, Bharata B Rao, 2015/06/19
- [Qemu-devel] [PATCH v2 2/4] numa, pc-dimm: Store pc-dimm memory information in numa_info, Bharata B Rao, 2015/06/19
- [Qemu-devel] [PATCH v2 4/4] numa: API to lookup NUMA node by address, Bharata B Rao, 2015/06/19
- Re: [Qemu-devel] [PATCH v2 0/4] Refactoring pc_dimm_plug and NUMA node lookup API, Bharata B Rao, 2015/06/23