On Thu, Mar 07, 2019 at 10:06:39AM +0100, Eric Auger wrote: > As NVDIMM support is looming for ARM and SPAPR, let's > move the acpi_nvdimm_state to the generic machine struct > instead of duplicating the same code in several machines. > It is also renamed into nvdimms_state. > > nvdimm and nvdimm-persistence become generic machine options. > We also add a description for those options. > > We also remove the nvdimms_state.is_enabled initialization to > false as objects are guaranteed to be zero initialized. > > Signed-off-by: Eric Auger <eric.au...@redhat.com> > Suggested-by: Igor Mammedov <imamm...@redhat.com> > > --- > > v1 -> v2: > - s/acpi_nvdimm_state/nvdimms_state > - remove ms->nvdimms_state.is_enabled initialization to false. [...] > @@ -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);
As noted in another reply, I don't mind adding new MachineState fields, but now I noticed you are adding new user-visible options, which requires more care. This patch seems to make all machines except PC silently ignore the new nvdimm options. If the current machine doesn't support nvdimm, "nvdimm-persistence=..." and "nvdimm=on" should be rejected by QEMU instead of silently ignored. Probably the simplest way to do that is by making the registration of those QOM properties conditional. We could add a simple bool MachineClass::nvdimm_supported field, or we could add a static void nvdimm_machine_class_init(MachineClass *mc); helper that would enable nvdimm support on the machine type. -- Eduardo
