[Qemu-devel] [PULL 2/2] nvdimm: make persistence option symbolic

[Michael S. Tsirkin]

From: Ross Zwisler <address@hidden>

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 <address@hidden>
Suggested-by: Michael S. Tsirkin <address@hidden>
Suggested-by: Dan Williams <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>
---
 docs/nvdimm.txt          | 31 ++++++++++++-------------------
 include/hw/i386/pc.h     |  2 +-
 include/hw/mem/nvdimm.h  |  3 ++-
 hw/acpi/nvdimm.c         |  4 ++--
 hw/i386/pc.c             | 33 ++++++++++++++++-----------------
 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/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/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/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"
-- 
MST