From f596c8844fe1d0022007ae6c7a377361fb653eff Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 29 Jan 2019 22:06:41 -0800 Subject: nfit: Fix nfit_intel_shutdown_status() command submission The implementation is broken in all the ways the unit test did not touch: 1/ The local definition of in_buf and in_obj violated C99 initializer expectations for zeroing. By only initializing 2 out of the three struct members the compiler was free to zero-initialize the remaining entry even though the aliased location in the union was initialized. 2/ The implementation made assumptions about the state of the 'smart' payload after command execution that are satisfied by acpi_nfit_ctl(), but not acpi_evaluate_dsm(). 3/ populate_shutdown_status() is skipped on Intel NVDIMMs due to the early return for skipping the common _LS{I,R,W} enabling. 4/ The input length should be zero. This breakage was missed due to the unit test implementation only testing the case where nfit_intel_shutdown_status() returns a valid payload. Much of this complexity would be saved if acpi_nfit_ctl() could be used, but that currently requires a 'struct nvdimm *' argument and one is not created until later in the init process. The health result is needed before the device is created because the payload gates whether the nmemX/nfit/dirty_shutdown property is visible in sysfs. Cc: Fixes: 0ead11181fe0 ("acpi, nfit: Collect shutdown status") Reported-by: Dexuan Cui Reviewed-by: Dexuan Cui Signed-off-by: Dan Williams --- drivers/acpi/nfit/core.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 011d3db19c80..95db6a2d0d6b 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -1738,14 +1738,14 @@ static bool acpi_nvdimm_has_method(struct acpi_device *adev, char *method) __weak void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem) { + struct device *dev = &nfit_mem->adev->dev; struct nd_intel_smart smart = { 0 }; union acpi_object in_buf = { - .type = ACPI_TYPE_BUFFER, - .buffer.pointer = (char *) &smart, - .buffer.length = sizeof(smart), + .buffer.type = ACPI_TYPE_BUFFER, + .buffer.length = 0, }; union acpi_object in_obj = { - .type = ACPI_TYPE_PACKAGE, + .package.type = ACPI_TYPE_PACKAGE, .package.count = 1, .package.elements = &in_buf, }; @@ -1760,8 +1760,15 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem) return; out_obj = acpi_evaluate_dsm(handle, guid, revid, func, &in_obj); - if (!out_obj) + if (!out_obj || out_obj->type != ACPI_TYPE_BUFFER + || out_obj->buffer.length < sizeof(smart)) { + dev_dbg(dev->parent, "%s: failed to retrieve initial health\n", + dev_name(dev)); + ACPI_FREE(out_obj); return; + } + memcpy(&smart, out_obj->buffer.pointer, sizeof(smart)); + ACPI_FREE(out_obj); if (smart.flags & ND_INTEL_SMART_SHUTDOWN_VALID) { if (smart.shutdown_state) @@ -1772,7 +1779,6 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem) set_bit(NFIT_MEM_DIRTY_COUNT, &nfit_mem->flags); nfit_mem->dirty_shutdown = smart.shutdown_count; } - ACPI_FREE(out_obj); } static void populate_shutdown_status(struct nfit_mem *nfit_mem) @@ -1887,18 +1893,19 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, | 1 << ND_CMD_SET_CONFIG_DATA; if (family == NVDIMM_FAMILY_INTEL && (dsm_mask & label_mask) == label_mask) - return 0; - - if (acpi_nvdimm_has_method(adev_dimm, "_LSI") - && acpi_nvdimm_has_method(adev_dimm, "_LSR")) { - dev_dbg(dev, "%s: has _LSR\n", dev_name(&adev_dimm->dev)); - set_bit(NFIT_MEM_LSR, &nfit_mem->flags); - } + /* skip _LS{I,R,W} enabling */; + else { + if (acpi_nvdimm_has_method(adev_dimm, "_LSI") + && acpi_nvdimm_has_method(adev_dimm, "_LSR")) { + dev_dbg(dev, "%s: has _LSR\n", dev_name(&adev_dimm->dev)); + set_bit(NFIT_MEM_LSR, &nfit_mem->flags); + } - if (test_bit(NFIT_MEM_LSR, &nfit_mem->flags) - && acpi_nvdimm_has_method(adev_dimm, "_LSW")) { - dev_dbg(dev, "%s: has _LSW\n", dev_name(&adev_dimm->dev)); - set_bit(NFIT_MEM_LSW, &nfit_mem->flags); + if (test_bit(NFIT_MEM_LSR, &nfit_mem->flags) + && acpi_nvdimm_has_method(adev_dimm, "_LSW")) { + dev_dbg(dev, "%s: has _LSW\n", dev_name(&adev_dimm->dev)); + set_bit(NFIT_MEM_LSW, &nfit_mem->flags); + } } populate_shutdown_status(nfit_mem); -- cgit v1.2.3 From 43f89877f26671c6309cd87d7364b1a3e66e71cf Mon Sep 17 00:00:00 2001 From: Dexuan Cui Date: Wed, 30 Jan 2019 01:23:01 +0000 Subject: nfit: acpi_nfit_ctl(): Check out_obj->type in the right place In the case of ND_CMD_CALL, we should also check out_obj->type. The patch uses out_obj->type, which is a short alias to out_obj->package.type. Fixes: 31eca76ba2fc ("nfit, libnvdimm: limited/whitelisted dimm command marshaling mechanism") Cc: Signed-off-by: Dexuan Cui Signed-off-by: Dan Williams --- drivers/acpi/nfit/core.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 95db6a2d0d6b..1598e3a121a6 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -535,6 +535,13 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, return -EINVAL; } + if (out_obj->type != ACPI_TYPE_BUFFER) { + dev_dbg(dev, "%s unexpected output object type cmd: %s type: %d\n", + dimm_name, cmd_name, out_obj->type); + rc = -EINVAL; + goto out; + } + if (call_pkg) { call_pkg->nd_fw_size = out_obj->buffer.length; memcpy(call_pkg->nd_payload + call_pkg->nd_size_in, @@ -553,13 +560,6 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm, return 0; } - if (out_obj->package.type != ACPI_TYPE_BUFFER) { - dev_dbg(dev, "%s unexpected output object type cmd: %s type: %d\n", - dimm_name, cmd_name, out_obj->type); - rc = -EINVAL; - goto out; - } - dev_dbg(dev, "%s cmd: %s output length: %d\n", dimm_name, cmd_name, out_obj->buffer.length); print_hex_dump_debug(cmd_name, DUMP_PREFIX_OFFSET, 4, 4, -- cgit v1.2.3 From 1194c4133195dfcb6c5fc0935d54bbed872a5285 Mon Sep 17 00:00:00 2001 From: Dexuan Cui Date: Tue, 29 Jan 2019 00:56:17 +0000 Subject: nfit: Add Hyper-V NVDIMM DSM command set to white list Add the Hyper-V _DSM command set to the white list of NVDIMM command sets. This command set is documented at http://www.uefi.org/RFIC_LIST (see "Virtual NVDIMM 0x1901"). Signed-off-by: Dexuan Cui Reviewed-by: Michael Kelley Signed-off-by: Dan Williams --- drivers/acpi/nfit/core.c | 17 ++++++++++++++--- drivers/acpi/nfit/nfit.h | 6 +++++- include/uapi/linux/ndctl.h | 1 + 3 files changed, 20 insertions(+), 4 deletions(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 1598e3a121a6..4a7e8b1fa43b 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -1846,9 +1846,17 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, dev_set_drvdata(&adev_dimm->dev, nfit_mem); /* - * Until standardization materializes we need to consider 4 - * different command sets. Note, that checking for function0 (bit0) - * tells us if any commands are reachable through this GUID. + * There are 4 "legacy" NVDIMM command sets + * (NVDIMM_FAMILY_{INTEL,MSFT,HPE1,HPE2}) that were created before + * an EFI working group was established to constrain this + * proliferation. The nfit driver probes for the supported command + * set by GUID. Note, if you're a platform developer looking to add + * a new command set to this probe, consider using an existing set, + * or otherwise seek approval to publish the command set at + * http://www.uefi.org/RFIC_LIST. + * + * Note, that checking for function0 (bit0) tells us if any commands + * are reachable through this GUID. */ for (i = 0; i <= NVDIMM_FAMILY_MAX; i++) if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 1)) @@ -1871,6 +1879,8 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, dsm_mask &= ~(1 << 8); } else if (nfit_mem->family == NVDIMM_FAMILY_MSFT) { dsm_mask = 0xffffffff; + } else if (nfit_mem->family == NVDIMM_FAMILY_HYPERV) { + dsm_mask = 0x1f; } else { dev_dbg(dev, "unknown dimm command family\n"); nfit_mem->family = -1; @@ -3714,6 +3724,7 @@ static __init int nfit_init(void) guid_parse(UUID_NFIT_DIMM_N_HPE1, &nfit_uuid[NFIT_DEV_DIMM_N_HPE1]); guid_parse(UUID_NFIT_DIMM_N_HPE2, &nfit_uuid[NFIT_DEV_DIMM_N_HPE2]); guid_parse(UUID_NFIT_DIMM_N_MSFT, &nfit_uuid[NFIT_DEV_DIMM_N_MSFT]); + guid_parse(UUID_NFIT_DIMM_N_HYPERV, &nfit_uuid[NFIT_DEV_DIMM_N_HYPERV]); nfit_wq = create_singlethread_workqueue("nfit"); if (!nfit_wq) diff --git a/drivers/acpi/nfit/nfit.h b/drivers/acpi/nfit/nfit.h index 33691aecfcee..4de167b4f76f 100644 --- a/drivers/acpi/nfit/nfit.h +++ b/drivers/acpi/nfit/nfit.h @@ -34,11 +34,14 @@ /* https://msdn.microsoft.com/library/windows/hardware/mt604741 */ #define UUID_NFIT_DIMM_N_MSFT "1ee68b36-d4bd-4a1a-9a16-4f8e53d46e05" +/* http://www.uefi.org/RFIC_LIST (see "Virtual NVDIMM 0x1901") */ +#define UUID_NFIT_DIMM_N_HYPERV "5746c5f2-a9a2-4264-ad0e-e4ddc9e09e80" + #define ACPI_NFIT_MEM_FAILED_MASK (ACPI_NFIT_MEM_SAVE_FAILED \ | ACPI_NFIT_MEM_RESTORE_FAILED | ACPI_NFIT_MEM_FLUSH_FAILED \ | ACPI_NFIT_MEM_NOT_ARMED | ACPI_NFIT_MEM_MAP_FAILED) -#define NVDIMM_FAMILY_MAX NVDIMM_FAMILY_MSFT +#define NVDIMM_FAMILY_MAX NVDIMM_FAMILY_HYPERV #define NVDIMM_STANDARD_CMDMASK \ (1 << ND_CMD_SMART | 1 << ND_CMD_SMART_THRESHOLD | 1 << ND_CMD_DIMM_FLAGS \ @@ -94,6 +97,7 @@ enum nfit_uuids { NFIT_DEV_DIMM_N_HPE1 = NVDIMM_FAMILY_HPE1, NFIT_DEV_DIMM_N_HPE2 = NVDIMM_FAMILY_HPE2, NFIT_DEV_DIMM_N_MSFT = NVDIMM_FAMILY_MSFT, + NFIT_DEV_DIMM_N_HYPERV = NVDIMM_FAMILY_HYPERV, NFIT_SPA_VOLATILE, NFIT_SPA_PM, NFIT_SPA_DCR, diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h index f57c9e434d2d..de5d90212409 100644 --- a/include/uapi/linux/ndctl.h +++ b/include/uapi/linux/ndctl.h @@ -243,6 +243,7 @@ struct nd_cmd_pkg { #define NVDIMM_FAMILY_HPE1 1 #define NVDIMM_FAMILY_HPE2 2 #define NVDIMM_FAMILY_MSFT 3 +#define NVDIMM_FAMILY_HYPERV 4 #define ND_IOCTL_CALL _IOWR(ND_IOCTL, ND_CMD_CALL,\ struct nd_cmd_pkg) -- cgit v1.2.3 From d5d30d5a5c60628de5e77e3f292a8f9012d51350 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sat, 2 Feb 2019 16:35:26 -0800 Subject: libnvdimm/dimm: Add a no-BLK quirk based on NVDIMM family As Dexuan reports the NVDIMM_FAMILY_HYPERV platform is incompatible with the existing Linux namespace implementation because it uses NSLABEL_FLAG_LOCAL for x1-width PMEM interleave sets. Quirk it as an platform / DIMM that does not provide BLK-aperture access. Allow the libnvdimm core to assume no potential for aliasing. In case other implementations make the same mistake, provide a "noblk" module parameter to force-enable the quirk. Link: https://lkml.kernel.org/r/PU1P153MB0169977604493B82B662A01CBF920@PU1P153MB0169.APCP153.PROD.OUTLOOK.COM Reported-by: Dexuan Cui Tested-by: Dexuan Cui Signed-off-by: Dan Williams --- drivers/acpi/nfit/core.c | 4 ++++ drivers/nvdimm/dimm_devs.c | 7 +++++++ drivers/nvdimm/label.c | 3 +++ drivers/nvdimm/namespace_devs.c | 6 ++++++ drivers/nvdimm/region_devs.c | 7 +++++++ include/linux/libnvdimm.h | 2 ++ 6 files changed, 29 insertions(+) (limited to 'drivers/acpi') diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 4a7e8b1fa43b..811c399a3a76 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -2016,6 +2016,10 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc) cmd_mask |= nfit_mem->dsm_mask & NVDIMM_STANDARD_CMDMASK; } + /* Quirk to ignore LOCAL for labels on HYPERV DIMMs */ + if (nfit_mem->family == NVDIMM_FAMILY_HYPERV) + set_bit(NDD_NOBLK, &flags); + if (test_bit(NFIT_MEM_LSR, &nfit_mem->flags)) { set_bit(ND_CMD_GET_CONFIG_SIZE, &cmd_mask); set_bit(ND_CMD_GET_CONFIG_DATA, &cmd_mask); diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c index 4890310df874..553aa78abeee 100644 --- a/drivers/nvdimm/dimm_devs.c +++ b/drivers/nvdimm/dimm_devs.c @@ -11,6 +11,7 @@ * General Public License for more details. */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include #include #include #include @@ -25,6 +26,10 @@ static DEFINE_IDA(dimm_ida); +static bool noblk; +module_param(noblk, bool, 0444); +MODULE_PARM_DESC(noblk, "force disable BLK / local alias support"); + /* * Retrieve bus and dimm handle and return if this bus supports * get_config_data commands @@ -551,6 +556,8 @@ struct nvdimm *__nvdimm_create(struct nvdimm_bus *nvdimm_bus, nvdimm->dimm_id = dimm_id; nvdimm->provider_data = provider_data; + if (noblk) + flags |= 1 << NDD_NOBLK; nvdimm->flags = flags; nvdimm->cmd_mask = cmd_mask; nvdimm->num_flush = num_flush; diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c index 6d6e9a12150b..f3d753d3169c 100644 --- a/drivers/nvdimm/label.c +++ b/drivers/nvdimm/label.c @@ -392,6 +392,7 @@ int nd_label_reserve_dpa(struct nvdimm_drvdata *ndd) return 0; /* no label, nothing to reserve */ for_each_clear_bit_le(slot, free, nslot) { + struct nvdimm *nvdimm = to_nvdimm(ndd->dev); struct nd_namespace_label *nd_label; struct nd_region *nd_region = NULL; u8 label_uuid[NSLABEL_UUID_LEN]; @@ -406,6 +407,8 @@ int nd_label_reserve_dpa(struct nvdimm_drvdata *ndd) memcpy(label_uuid, nd_label->uuid, NSLABEL_UUID_LEN); flags = __le32_to_cpu(nd_label->flags); + if (test_bit(NDD_NOBLK, &nvdimm->flags)) + flags &= ~NSLABEL_FLAG_LOCAL; nd_label_gen_id(&label_id, label_uuid, flags); res = nvdimm_allocate_dpa(ndd, &label_id, __le64_to_cpu(nd_label->dpa), diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c index 4b077555ac70..3677b0c4a33d 100644 --- a/drivers/nvdimm/namespace_devs.c +++ b/drivers/nvdimm/namespace_devs.c @@ -2492,6 +2492,12 @@ static int init_active_labels(struct nd_region *nd_region) if (!label_ent) break; label = nd_label_active(ndd, j); + if (test_bit(NDD_NOBLK, &nvdimm->flags)) { + u32 flags = __le32_to_cpu(label->flags); + + flags &= ~NSLABEL_FLAG_LOCAL; + label->flags = __cpu_to_le32(flags); + } label_ent->label = label; mutex_lock(&nd_mapping->lock); diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c index e2818f94f292..3b58baa44b5c 100644 --- a/drivers/nvdimm/region_devs.c +++ b/drivers/nvdimm/region_devs.c @@ -1003,6 +1003,13 @@ static struct nd_region *nd_region_create(struct nvdimm_bus *nvdimm_bus, if (test_bit(NDD_UNARMED, &nvdimm->flags)) ro = 1; + + if (test_bit(NDD_NOBLK, &nvdimm->flags) + && dev_type == &nd_blk_device_type) { + dev_err(&nvdimm_bus->dev, "%s: %s mapping%d is not BLK capable\n", + caller, dev_name(&nvdimm->dev), i); + return NULL; + } } if (dev_type == &nd_blk_device_type) { diff --git a/include/linux/libnvdimm.h b/include/linux/libnvdimm.h index 5440f11b0907..7da406ae3a2b 100644 --- a/include/linux/libnvdimm.h +++ b/include/linux/libnvdimm.h @@ -42,6 +42,8 @@ enum { NDD_SECURITY_OVERWRITE = 3, /* tracking whether or not there is a pending device reference */ NDD_WORK_PENDING = 4, + /* ignore / filter NSLABEL_FLAG_LOCAL for this DIMM, i.e. no aliasing */ + NDD_NOBLK = 5, /* need to set a limit somewhere, but yes, this is likely overkill */ ND_IOCTL_MAX_BUFLEN = SZ_4M, -- cgit v1.2.3 From 0171b6b78131110a6870d4b7296bc9dfc392116a Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Sun, 3 Feb 2019 11:17:27 -0800 Subject: acpi/nfit: Require opt-in for read-only label configurations Recent fixes to command handling enabled Linux to read label configurations that it could not before. Unfortunately that means that configurations that were operating in label-less mode will be broken as the kernel ignores the existing namespace configuration and tries to honor the new found labels. Fortunately this seems limited to a case where Linux can quirk the behavior and maintain the existing label-less semantics by default. When the platform does not emit an _LSW method, disable all label access methods. Provide a 'force_labels' module parameter to allow read-only label operation. Fixes: 11189c1089da ("acpi/nfit: Fix command-supported detection") Reported-by: Dexuan Cui Reviewed-by: Dexuan Cui Signed-off-by: Dan Williams --- drivers/acpi/nfit/core.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'drivers/acpi') diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 811c399a3a76..5b5e802de7b8 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -56,6 +56,10 @@ static bool no_init_ars; module_param(no_init_ars, bool, 0644); MODULE_PARM_DESC(no_init_ars, "Skip ARS run at nfit init time"); +static bool force_labels; +module_param(force_labels, bool, 0444); +MODULE_PARM_DESC(force_labels, "Opt-in to labels despite missing methods"); + LIST_HEAD(acpi_descs); DEFINE_MUTEX(acpi_desc_lock); @@ -1916,6 +1920,19 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc, dev_dbg(dev, "%s: has _LSW\n", dev_name(&adev_dimm->dev)); set_bit(NFIT_MEM_LSW, &nfit_mem->flags); } + + /* + * Quirk read-only label configurations to preserve + * access to label-less namespaces by default. + */ + if (!test_bit(NFIT_MEM_LSW, &nfit_mem->flags) + && !force_labels) { + dev_dbg(dev, "%s: No _LSW, disable labels\n", + dev_name(&adev_dimm->dev)); + clear_bit(NFIT_MEM_LSR, &nfit_mem->flags); + } else + dev_dbg(dev, "%s: Force enable labels\n", + dev_name(&adev_dimm->dev)); } populate_shutdown_status(nfit_mem); -- cgit v1.2.3 From 5c9d62d0026a439a785e473859cb3ec9b11a69e9 Mon Sep 17 00:00:00 2001 From: Toshi Kani Date: Thu, 28 Feb 2019 13:12:18 -0700 Subject: acpi/nfit: Update NFIT flags error message ACPI NFIT flags field reports major errors on NVDIMM, which need user's attention. Update the current log to a proper error message with dev_err(). The current message string is kept for grep-compatibility. Signed-off-by: Toshi Kani Cc: Dan Williams Cc: "Rafael J. Wysocki" Cc: Robert Elliott Signed-off-by: Dan Williams --- drivers/acpi/nfit/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/acpi') diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 5b5e802de7b8..a22e2f2bbb75 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -2060,7 +2060,7 @@ static int acpi_nfit_register_dimms(struct acpi_nfit_desc *acpi_desc) if ((mem_flags & ACPI_NFIT_MEM_FAILED_MASK) == 0) continue; - dev_info(acpi_desc->dev, "%s flags:%s%s%s%s%s\n", + dev_err(acpi_desc->dev, "Error found in NVDIMM %s flags:%s%s%s%s%s\n", nvdimm_name(nvdimm), mem_flags & ACPI_NFIT_MEM_SAVE_FAILED ? " save_fail" : "", mem_flags & ACPI_NFIT_MEM_RESTORE_FAILED ? " restore_fail":"", -- cgit v1.2.3