On Mon, May 21, 2018 at 10:32:02AM -0600, Ross Zwisler wrote:
> Add a machine command line option to allow the user to control the Platform
> Capabilities Structure in the virtualized NFIT. This Platform Capabilities
> Structure was added in ACPI 6.2 Errata A.
>
> Signed-off-by: Ross Zwisler <ross.zwis...@linux.intel.com>
I tried playing with it and encoding the capabilities is
quite awkward.
Can we add bits for specific capabilities instead of nvdimm-cap?
How about:
"cpu-flush-on-power-loss-cap"
"memory-flush-on-power-loss-cap"
"byte-addressable-mirroring-cap"
?
> ---
> docs/nvdimm.txt | 27 +++++++++++++++++++++++++++
> hw/acpi/nvdimm.c | 45 +++++++++++++++++++++++++++++++++++++++++----
> hw/i386/pc.c | 31 +++++++++++++++++++++++++++++++
> include/hw/i386/pc.h | 1 +
> include/hw/mem/nvdimm.h | 5 +++++
> 5 files changed, 105 insertions(+), 4 deletions(-)
>
> diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
> index e903d8bb09..8b48fb4633 100644
> --- a/docs/nvdimm.txt
> +++ b/docs/nvdimm.txt
> @@ -153,3 +153,30 @@ guest NVDIMM region mapping structure. This unarmed
> flag indicates
> 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
> +---------------------
> +
> +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:
> +
> + -machine pc,accel=kvm,nvdimm,nvdimm-cap=2
> +
> +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.
> +
> +Here is a quick summary of the three bits that are defined as of that spec:
> +
> +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.
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 59d6e4254c..87e4280c71 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -169,6 +169,21 @@ struct NvdimmNfitControlRegion {
> } QEMU_PACKED;
> typedef struct NvdimmNfitControlRegion NvdimmNfitControlRegion;
>
> +/*
> + * NVDIMM Platform Capabilities Structure
> + *
> + * Defined in section 5.2.25.9 of ACPI 6.2 Errata A, September 2017
> + */
> +struct NvdimmNfitPlatformCaps {
> + uint16_t type;
> + uint16_t length;
> + uint8_t highest_cap;
> + uint8_t reserved[3];
> + uint32_t capabilities;
> + uint8_t reserved2[4];
> +} QEMU_PACKED;
> +typedef struct NvdimmNfitPlatformCaps NvdimmNfitPlatformCaps;
> +
> /*
> * Module serial number is a unique number for each device. We use the
> * slot id of NVDIMM device to generate this number so that each device
> @@ -351,7 +366,23 @@ static void nvdimm_build_structure_dcr(GArray
> *structures, DeviceState *dev)
> JEDEC Annex L Release 3. */);
> }
>
> -static GArray *nvdimm_build_device_structure(void)
> +/*
> + * ACPI 6.2 Errata A: 5.2.25.9 NVDIMM Platform Capabilities Structure
> + */
> +static void
> +nvdimm_build_structure_caps(GArray *structures, uint32_t capabilities)
> +{
> + NvdimmNfitPlatformCaps *nfit_caps;
> +
> + nfit_caps = acpi_data_push(structures, sizeof(*nfit_caps));
> +
> + nfit_caps->type = cpu_to_le16(7 /* NVDIMM Platform Capabilities */);
> + nfit_caps->length = cpu_to_le16(sizeof(*nfit_caps));
> + nfit_caps->highest_cap = 31 - clz32(capabilities);
> + nfit_caps->capabilities = cpu_to_le32(capabilities);
> +}
> +
> +static GArray *nvdimm_build_device_structure(AcpiNVDIMMState *state)
> {
> GSList *device_list = nvdimm_get_device_list();
> GArray *structures = g_array_new(false, true /* clear */, 1);
> @@ -373,6 +404,10 @@ static GArray *nvdimm_build_device_structure(void)
> }
> g_slist_free(device_list);
>
> + if (state->capabilities) {
> + nvdimm_build_structure_caps(structures, state->capabilities);
> + }
> +
> return structures;
> }
>
> @@ -381,16 +416,18 @@ static void nvdimm_init_fit_buffer(NvdimmFitBuffer
> *fit_buf)
> fit_buf->fit = g_array_new(false, true /* clear */, 1);
> }
>
> -static void nvdimm_build_fit_buffer(NvdimmFitBuffer *fit_buf)
> +static void nvdimm_build_fit_buffer(AcpiNVDIMMState *state)
> {
> + NvdimmFitBuffer *fit_buf = &state->fit_buf;
> +
> g_array_free(fit_buf->fit, true);
> - fit_buf->fit = nvdimm_build_device_structure();
> + fit_buf->fit = nvdimm_build_device_structure(state);
> fit_buf->dirty = true;
> }
>
> void nvdimm_plug(AcpiNVDIMMState *state)
> {
> - nvdimm_build_fit_buffer(&state->fit_buf);
> + nvdimm_build_fit_buffer(state);
> }
>
> static void nvdimm_build_nfit(AcpiNVDIMMState *state, GArray *table_offsets,
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index d768930d02..1b2684c549 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2182,6 +2182,33 @@ 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)
> +{
> + PCMachineState *pcms = PC_MACHINE(obj);
> + uint32_t value = pcms->acpi_nvdimm_state.capabilities;
> +
> + visit_type_uint32(v, name, &value, errp);
> +}
> +
> +static void pc_machine_set_nvdimm_capabilities(Object *obj, Visitor *v,
> + const char *name, void
> *opaque,
> + Error **errp)
> +{
> + PCMachineState *pcms = PC_MACHINE(obj);
> + Error *error = NULL;
> + uint32_t value;
> +
> + visit_type_uint32(v, name, &value, &error);
> + if (error) {
> + error_propagate(errp, error);
> + return;
> + }
> +
> + pcms->acpi_nvdimm_state.capabilities = value;
> +}
> +
> static bool pc_machine_get_smbus(Object *obj, Error **errp)
> {
> PCMachineState *pcms = PC_MACHINE(obj);
> @@ -2395,6 +2422,10 @@ 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_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 2a98e3ad68..e9b22f929c 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -76,6 +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_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 74c60332e1..3c82751bab 100644
> --- a/include/hw/mem/nvdimm.h
> +++ b/include/hw/mem/nvdimm.h
> @@ -134,6 +134,11 @@ struct AcpiNVDIMMState {
>
> /* the IO region used by OSPM to transfer control to QEMU. */
> MemoryRegion io_mr;
> +
> + /*
> + * Platform capabilities, section 5.2.25.9 of ACPI 6.2 Errata A
> + */
> + int32_t capabilities;
> };
> typedef struct AcpiNVDIMMState AcpiNVDIMMState;
>
> --
> 2.14.3
