On Tue, 5 Mar 2019 10:39:49 +0100
Auger Eric <eric.au...@redhat.com> wrote:
> Hi Shivaprasad,
>
> On 3/5/19 10:35 AM, Shivaprasad G Bhat wrote:
> >
> >
> > On 03/01/2019 07:36 PM, Eric Auger wrote:
> >> As NVDIMM support is looming for ARM and SPAPR, let's
> >> move the acpi_nvdimm_state to the generic machine struct
> > Name - "acpi_nvdimm_state" sounds very x86 specific.
> >
> > Could you please rename ?
> Sure, do you have any suggestion?
would "nvdimms_state" do?
>
> Thanks
>
> Eric
> >
> > Thanks,
> > Shivaprasad
> >> instead of duplicating the same code in several machines.
> >>
> >> nvdimm and nvdimm-persistence become generic machine options.
> >> We also add a description for those options.
> >>
> >> Signed-off-by: Eric Auger <eric.au...@redhat.com>
> >> Suggested-by: Igor Mammedov <imamm...@redhat.com>
> >> ---
> >> hw/core/machine.c | 57 ++++++++++++++++++++++++++++++++++++++++
> >> hw/i386/acpi-build.c | 6 ++---
> >> hw/i386/pc.c | 56 +++------------------------------------
> >> hw/i386/pc_piix.c | 4 +--
> >> hw/i386/pc_q35.c | 4 +--
> >> include/hw/boards.h | 2 ++
> >> include/hw/i386/pc.h | 4 ---
> >> include/hw/mem/pc-dimm.h | 1 -
> >> 8 files changed, 70 insertions(+), 64 deletions(-)
> >>
> >> diff --git a/hw/core/machine.c b/hw/core/machine.c
> >> index 766ca5899d..19a5ee7cd8 100644
> >> --- a/hw/core/machine.c
> >> +++ b/hw/core/machine.c
> >> @@ -481,6 +481,47 @@ static void machine_set_memory_encryption(Object
> >> *obj, const char *value,
> >> ms->memory_encryption = g_strdup(value);
> >> }
> >> +static bool machine_get_nvdimm(Object *obj, Error **errp)
> >> +{
> >> + MachineState *ms = MACHINE(obj);
> >> +
> >> + return ms->acpi_nvdimm_state.is_enabled;
> >> +}
> >> +
> >> +static void machine_set_nvdimm(Object *obj, bool value, Error **errp)
> >> +{
> >> + MachineState *ms = MACHINE(obj);
> >> +
> >> + ms->acpi_nvdimm_state.is_enabled = value;
> >> +}
> >> +
> >> +static char *machine_get_nvdimm_persistence(Object *obj, Error **errp)
> >> +{
> >> + MachineState *ms = MACHINE(obj);
> >> +
> >> + return g_strdup(ms->acpi_nvdimm_state.persistence_string);
> >> +}
> >> +
> >> +static void machine_set_nvdimm_persistence(Object *obj, const char
> >> *value,
> >> + Error **errp)
> >> +{
> >> + MachineState *ms = MACHINE(obj);
> >> + AcpiNVDIMMState *nvdimm_state = &ms->acpi_nvdimm_state;
> >> +
> >> + if (strcmp(value, "cpu") == 0)
> >> + nvdimm_state->persistence = 3;
> >> + else if (strcmp(value, "mem-ctrl") == 0)
> >> + nvdimm_state->persistence = 2;
> >> + else {
> >> + error_setg(errp, "-machine nvdimm-persistence=%s: unsupported
> >> option",
> >> + value);
> >> + return;
> >> + }
> >> +
> >> + g_free(nvdimm_state->persistence_string);
> >> + nvdimm_state->persistence_string = g_strdup(value);
> >> +}
> >> +
> >> void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const
> >> char *type)
> >> {
> >> strList *item = g_new0(strList, 1);
> >> @@ -765,6 +806,20 @@ static void machine_class_init(ObjectClass *oc,
> >> void *data)
> >> &error_abort);
> >> object_class_property_set_description(oc, "memory-encryption",
> >> "Set memory encryption object to use", &error_abort);
> >> +
> >> + object_class_property_add_bool(oc, "nvdimm",
> >> + machine_get_nvdimm, machine_set_nvdimm, &error_abort);
> >> + object_class_property_set_description(oc, "nvdimm",
> >> + "Set on/off to
> >> enable/disable NVDIMM "
> >> + "instantiation", NULL);
> >> +
> >> + object_class_property_add_str(oc, "nvdimm-persistence",
> >> + machine_get_nvdimm_persistence,
> >> + machine_set_nvdimm_persistence,
> >> &error_abort);
> >> + object_class_property_set_description(oc, "nvdimm-persistence",
> >> + "Set NVDIMM persistence"
> >> + "Valid values are cpu and
> >> mem-ctrl",
> >> + NULL);
> >> }
> >> static void machine_class_base_init(ObjectClass *oc, void *data)
> >> @@ -790,6 +845,8 @@ static void machine_initfn(Object *obj)
> >> ms->dump_guest_core = true;
> >> ms->mem_merge = true;
> >> ms->enable_graphics = true;
> >> + /* nvdimm is disabled on default. */
> >> + ms->acpi_nvdimm_state.is_enabled = false;
> >> /* Register notifier when init is done for sysbus sanity
> >> checks */
> >> ms->sysbus_notifier.notify = machine_init_notify;
> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >> index 9ecc96dcc7..622ccb9408 100644
> >> --- a/hw/i386/acpi-build.c
> >> +++ b/hw/i386/acpi-build.c
> >> @@ -1867,7 +1867,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >> aml_append(scope, method);
> >> }
> >> - if (pcms->acpi_nvdimm_state.is_enabled) {
> >> + if (machine->acpi_nvdimm_state.is_enabled) {
> >> method = aml_method("_E04", 0, AML_NOTSERIALIZED);
> >> aml_append(method, aml_notify(aml_name("\\_SB.NVDR"),
> >> aml_int(0x80)));
> >> @@ -2704,9 +2704,9 @@ void acpi_build(AcpiBuildTables *tables,
> >> MachineState *machine)
> >> build_dmar_q35(tables_blob, tables->linker);
> >> }
> >> }
> >> - if (pcms->acpi_nvdimm_state.is_enabled) {
> >> + if (machine->acpi_nvdimm_state.is_enabled) {
> >> nvdimm_build_acpi(table_offsets, tables_blob, tables->linker,
> >> - &pcms->acpi_nvdimm_state, machine->ram_slots);
> >> + &machine->acpi_nvdimm_state,
> >> machine->ram_slots);
> >> }
> >> /* Add tables supplied by user (if any) */
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index 3889eccdc3..356213e0b8 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -2096,6 +2096,7 @@ static void pc_memory_pre_plug(HotplugHandler
> >> *hotplug_dev, DeviceState *dev,
> >> {
> >> const PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> >> const PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> >> + const MachineState *ms = MACHINE(hotplug_dev);
> >> const bool is_nvdimm = object_dynamic_cast(OBJECT(dev),
> >> TYPE_NVDIMM);
> >> const uint64_t legacy_align = TARGET_PAGE_SIZE;
> >> @@ -2110,7 +2111,7 @@ static void pc_memory_pre_plug(HotplugHandler
> >> *hotplug_dev, DeviceState *dev,
> >> return;
> >> }
> >> - if (is_nvdimm && !pcms->acpi_nvdimm_state.is_enabled) {
> >> + if (is_nvdimm && !ms->acpi_nvdimm_state.is_enabled) {
> >> error_setg(errp, "nvdimm is not enabled: missing 'nvdimm' in
> >> '-M'");
> >> return;
> >> }
> >> @@ -2124,6 +2125,7 @@ static void pc_memory_plug(HotplugHandler
> >> *hotplug_dev,
> >> {
> >> Error *local_err = NULL;
> >> PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> >> + MachineState *ms = MACHINE(hotplug_dev);
> >> bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
> >> pc_dimm_plug(PC_DIMM(dev), MACHINE(pcms), &local_err);
> >> @@ -2132,7 +2134,7 @@ static void pc_memory_plug(HotplugHandler
> >> *hotplug_dev,
> >> }
> >> if (is_nvdimm) {
> >> - nvdimm_plug(&pcms->acpi_nvdimm_state);
> >> + nvdimm_plug(&ms->acpi_nvdimm_state);
> >> }
> >> hotplug_handler_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev,
> >> &error_abort);
> >> @@ -2574,47 +2576,6 @@ static void pc_machine_set_smm(Object *obj,
> >> Visitor *v, const char *name,
> >> visit_type_OnOffAuto(v, name, &pcms->smm, errp);
> >> }
> >> -static bool pc_machine_get_nvdimm(Object *obj, Error **errp)
> >> -{
> >> - PCMachineState *pcms = PC_MACHINE(obj);
> >> -
> >> - return pcms->acpi_nvdimm_state.is_enabled;
> >> -}
> >> -
> >> -static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp)
> >> -{
> >> - PCMachineState *pcms = PC_MACHINE(obj);
> >> -
> >> - pcms->acpi_nvdimm_state.is_enabled = value;
> >> -}
> >> -
> >> -static char *pc_machine_get_nvdimm_persistence(Object *obj, Error
> >> **errp)
> >> -{
> >> - PCMachineState *pcms = PC_MACHINE(obj);
> >> -
> >> - return g_strdup(pcms->acpi_nvdimm_state.persistence_string);
> >> -}
> >> -
> >> -static void pc_machine_set_nvdimm_persistence(Object *obj, const char
> >> *value,
> >> - Error **errp)
> >> -{
> >> - PCMachineState *pcms = PC_MACHINE(obj);
> >> - AcpiNVDIMMState *nvdimm_state = &pcms->acpi_nvdimm_state;
> >> -
> >> - if (strcmp(value, "cpu") == 0)
> >> - nvdimm_state->persistence = 3;
> >> - else if (strcmp(value, "mem-ctrl") == 0)
> >> - nvdimm_state->persistence = 2;
> >> - else {
> >> - error_setg(errp, "-machine nvdimm-persistence=%s: unsupported
> >> option",
> >> - value);
> >> - return;
> >> - }
> >> -
> >> - g_free(nvdimm_state->persistence_string);
> >> - nvdimm_state->persistence_string = g_strdup(value);
> >> -}
> >> -
> >> static bool pc_machine_get_smbus(Object *obj, Error **errp)
> >> {
> >> PCMachineState *pcms = PC_MACHINE(obj);
> >> @@ -2664,8 +2625,6 @@ static void pc_machine_initfn(Object *obj)
> >> pcms->max_ram_below_4g = 0; /* use default */
> >> pcms->smm = ON_OFF_AUTO_AUTO;
> >> pcms->vmport = ON_OFF_AUTO_AUTO;
> >> - /* nvdimm is disabled on default. */
> >> - pcms->acpi_nvdimm_state.is_enabled = false;
> >> /* acpi build is enabled by default if machine supports it */
> >> pcms->acpi_build_enabled =
> >> PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
> >> pcms->smbus_enabled = true;
> >> @@ -2826,13 +2785,6 @@ static void pc_machine_class_init(ObjectClass
> >> *oc, void *data)
> >> object_class_property_set_description(oc, PC_MACHINE_VMPORT,
> >> "Enable vmport (pc & q35)", &error_abort);
> >> - object_class_property_add_bool(oc, PC_MACHINE_NVDIMM,
> >> - pc_machine_get_nvdimm, pc_machine_set_nvdimm, &error_abort);
> >> -
> >> - object_class_property_add_str(oc, PC_MACHINE_NVDIMM_PERSIST,
> >> - pc_machine_get_nvdimm_persistence,
> >> - pc_machine_set_nvdimm_persistence, &error_abort);
> >> -
> >> object_class_property_add_bool(oc, PC_MACHINE_SMBUS,
> >> pc_machine_get_smbus, pc_machine_set_smbus, &error_abort);
> >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> >> index fd0f2c268f..8b94b70f14 100644
> >> --- a/hw/i386/pc_piix.c
> >> +++ b/hw/i386/pc_piix.c
> >> @@ -297,8 +297,8 @@ static void pc_init1(MachineState *machine,
> >> PC_MACHINE_ACPI_DEVICE_PROP,
> >> &error_abort);
> >> }
> >> - if (pcms->acpi_nvdimm_state.is_enabled) {
> >> - nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
> >> + if (machine->acpi_nvdimm_state.is_enabled) {
> >> + nvdimm_init_acpi_state(&machine->acpi_nvdimm_state, system_io,
> >> pcms->fw_cfg, OBJECT(pcms));
> >> }
> >> }
> >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> >> index 4a175ea50e..26727c964d 100644
> >> --- a/hw/i386/pc_q35.c
> >> +++ b/hw/i386/pc_q35.c
> >> @@ -329,8 +329,8 @@ static void pc_q35_init(MachineState *machine)
> >> pc_vga_init(isa_bus, host_bus);
> >> pc_nic_init(pcmc, isa_bus, host_bus);
> >> - if (pcms->acpi_nvdimm_state.is_enabled) {
> >> - nvdimm_init_acpi_state(&pcms->acpi_nvdimm_state, system_io,
> >> + if (machine->acpi_nvdimm_state.is_enabled) {
> >> + nvdimm_init_acpi_state(&machine->acpi_nvdimm_state, system_io,
> >> pcms->fw_cfg, OBJECT(pcms));
> >> }
> >> }
> >> diff --git a/include/hw/boards.h b/include/hw/boards.h
> >> index 05f9f45c3d..b55c9cc087 100644
> >> --- a/include/hw/boards.h
> >> +++ b/include/hw/boards.h
> >> @@ -8,6 +8,7 @@
> >> #include "hw/qdev.h"
> >> #include "qom/object.h"
> >> #include "qom/cpu.h"
> >> +#include "hw/mem/nvdimm.h"
> >> /**
> >> * memory_region_allocate_system_memory - Allocate a board's main
> >> memory
> >> @@ -268,6 +269,7 @@ struct MachineState {
> >> const char *cpu_type;
> >> AccelState *accelerator;
> >> CPUArchIdList *possible_cpus;
> >> + AcpiNVDIMMState acpi_nvdimm_state;
> >> };
> >> #define DEFINE_MACHINE(namestr, machine_initfn) \
> >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> >> index 3ff127ebd0..f7c59743e0 100644
> >> --- a/include/hw/i386/pc.h
> >> +++ b/include/hw/i386/pc.h
> >> @@ -45,8 +45,6 @@ struct PCMachineState {
> >> OnOffAuto vmport;
> >> OnOffAuto smm;
> >> - AcpiNVDIMMState acpi_nvdimm_state;
> >> -
> >> bool acpi_build_enabled;
> >> bool smbus_enabled;
> >> bool sata_enabled;
> >> @@ -74,8 +72,6 @@ struct PCMachineState {
> >> #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
> >> #define PC_MACHINE_VMPORT "vmport"
> >> #define PC_MACHINE_SMM "smm"
> >> -#define PC_MACHINE_NVDIMM "nvdimm"
> >> -#define PC_MACHINE_NVDIMM_PERSIST "nvdimm-persistence"
> >> #define PC_MACHINE_SMBUS "smbus"
> >> #define PC_MACHINE_SATA "sata"
> >> #define PC_MACHINE_PIT "pit"
> >> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> >> index 01436b9f50..3e5489d3a1 100644
> >> --- a/include/hw/mem/pc-dimm.h
> >> +++ b/include/hw/mem/pc-dimm.h
> >> @@ -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) \
> >
> >
