From 11c39b5cd966ddc067a1ca0c5392ec9b666c45b7 Mon Sep 17 00:00:00 2001 From: Ross Zwisler Date: Thu, 7 Jun 2018 16:31:11 -0600 Subject: [PATCH] nvdimm: make persistence option symbolic Replace the "nvdimm-cap" option which took numeric arguments such as "2" with a more user friendly "nvdimm-persistence" option which takes symbolic arguments "cpu" or "mem-ctrl". Signed-off-by: Ross Zwisler Suggested-by: Michael S. Tsirkin Suggested-by: Dan Williams Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- docs/nvdimm.txt | 31 ++++++++++++------------------- hw/acpi/nvdimm.c | 4 ++-- hw/i386/pc.c | 33 ++++++++++++++++----------------- include/hw/i386/pc.h | 2 +- include/hw/mem/nvdimm.h | 3 ++- tests/bios-tables-test.c | 2 +- 6 files changed, 34 insertions(+), 41 deletions(-) diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt index 8b48fb4633..24b443b655 100644 --- a/docs/nvdimm.txt +++ b/docs/nvdimm.txt @@ -154,29 +154,22 @@ guest software that this vNVDIMM device contains a region that cannot accept persistent writes. In result, for example, the guest Linux NVDIMM driver, marks such vNVDIMM device as read-only. -Platform Capabilities ---------------------- +NVDIMM Persistence +------------------ ACPI 6.2 Errata A added support for a new Platform Capabilities Structure which allows the platform to communicate what features it supports related to -NVDIMM data durability. Users can provide a capabilities value to a guest via -the optional "nvdimm-cap" machine command line option: +NVDIMM data persistence. Users can provide a persistence value to a guest via +the optional "nvdimm-persistence" machine command line option: - -machine pc,accel=kvm,nvdimm,nvdimm-cap=2 + -machine pc,accel=kvm,nvdimm,nvdimm-persistence=cpu -This "nvdimm-cap" field is an integer, and is the combined value of the -various capability bits defined in table 5-137 of the ACPI 6.2 Errata A spec. +There are currently two valid values for this option: -Here is a quick summary of the three bits that are defined as of that spec: +"mem-ctrl" - The platform supports flushing dirty data from the memory + controller to the NVDIMMs in the event of power loss. -Bit[0] - CPU Cache Flush to NVDIMM Durability on Power Loss Capable. -Bit[1] - Memory Controller Flush to NVDIMM Durability on Power Loss Capable. - Note: If bit 0 is set to 1 then this bit shall be set to 1 as well. -Bit[2] - Byte Addressable Persistent Memory Hardware Mirroring Capable. - -So, a "nvdimm-cap" value of 2 would mean that the platform supports Memory -Controller Flush on Power Loss, a value of 3 would mean that the platform -supports CPU Cache Flush and Memory Controller Flush on Power Loss, etc. - -For a complete list of the flags available and for more detailed descriptions, -please consult the ACPI spec. +"cpu" - The platform supports flushing dirty data from the CPU cache to + the NVDIMMs in the event of power loss. This implies that the + platform also supports flushing dirty data through the memory + controller on power loss. diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c index 87e4280c71..27eeb6609f 100644 --- a/hw/acpi/nvdimm.c +++ b/hw/acpi/nvdimm.c @@ -404,8 +404,8 @@ static GArray *nvdimm_build_device_structure(AcpiNVDIMMState *state) } g_slist_free(device_list); - if (state->capabilities) { - nvdimm_build_structure_caps(structures, state->capabilities); + if (state->persistence) { + nvdimm_build_structure_caps(structures, state->persistence); } return structures; diff --git a/hw/i386/pc.c b/hw/i386/pc.c index f3befe6721..5bba9dcf5a 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -2181,31 +2181,30 @@ static void pc_machine_set_nvdimm(Object *obj, bool value, Error **errp) pcms->acpi_nvdimm_state.is_enabled = value; } -static void pc_machine_get_nvdimm_capabilities(Object *obj, Visitor *v, - const char *name, void *opaque, - Error **errp) +static char *pc_machine_get_nvdimm_persistence(Object *obj, Error **errp) { PCMachineState *pcms = PC_MACHINE(obj); - uint32_t value = pcms->acpi_nvdimm_state.capabilities; - visit_type_uint32(v, name, &value, errp); + return g_strdup(pcms->acpi_nvdimm_state.persistence_string); } -static void pc_machine_set_nvdimm_capabilities(Object *obj, Visitor *v, - const char *name, void *opaque, +static void pc_machine_set_nvdimm_persistence(Object *obj, const char *value, Error **errp) { PCMachineState *pcms = PC_MACHINE(obj); - Error *error = NULL; - uint32_t value; + AcpiNVDIMMState *nvdimm_state = &pcms->acpi_nvdimm_state; - visit_type_uint32(v, name, &value, &error); - if (error) { - error_propagate(errp, error); - return; + if (strcmp(value, "cpu") == 0) + nvdimm_state->persistence = 3; + else if (strcmp(value, "mem-ctrl") == 0) + nvdimm_state->persistence = 2; + else { + error_report("-machine nvdimm-persistence=%s: unsupported option", value); + exit(EXIT_FAILURE); } - pcms->acpi_nvdimm_state.capabilities = value; + g_free(nvdimm_state->persistence_string); + nvdimm_state->persistence_string = g_strdup(value); } static bool pc_machine_get_smbus(Object *obj, Error **errp) @@ -2421,9 +2420,9 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) object_class_property_add_bool(oc, PC_MACHINE_NVDIMM, pc_machine_get_nvdimm, pc_machine_set_nvdimm, &error_abort); - object_class_property_add(oc, PC_MACHINE_NVDIMM_CAP, "uint32", - pc_machine_get_nvdimm_capabilities, - pc_machine_set_nvdimm_capabilities, NULL, NULL, &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/include/hw/i386/pc.h b/include/hw/i386/pc.h index 04d1f8c6c3..fc8dedca12 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -76,7 +76,7 @@ struct PCMachineState { #define PC_MACHINE_VMPORT "vmport" #define PC_MACHINE_SMM "smm" #define PC_MACHINE_NVDIMM "nvdimm" -#define PC_MACHINE_NVDIMM_CAP "nvdimm-cap" +#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/nvdimm.h b/include/hw/mem/nvdimm.h index 3c82751bab..9340631cfc 100644 --- a/include/hw/mem/nvdimm.h +++ b/include/hw/mem/nvdimm.h @@ -138,7 +138,8 @@ struct AcpiNVDIMMState { /* * Platform capabilities, section 5.2.25.9 of ACPI 6.2 Errata A */ - int32_t capabilities; + int32_t persistence; + char *persistence_string; }; typedef struct AcpiNVDIMMState AcpiNVDIMMState; diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 256d463cb8..4e24930c4b 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -830,7 +830,7 @@ static void test_acpi_tcg_dimm_pxm(const char *machine) memset(&data, 0, sizeof(data)); data.machine = machine; data.variant = ".dimmpxm"; - test_acpi_one(" -machine nvdimm=on,nvdimm-cap=3" + test_acpi_one(" -machine nvdimm=on,nvdimm-persistence=cpu" " -smp 4,sockets=4" " -m 128M,slots=3,maxmem=1G" " -numa node,mem=32M,nodeid=0"