UbiquitousOS
/
OpenCloudOS-Kernel
mirror of https://gitee.com/OpenCloudOS/OpenCloudOS-Kernel.git
11
0
Fork 0
Code Issues Projects Releases Wiki Activity

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: <stable@vger.kernel.org>
Fixes: 0ead11181f ("acpi, nfit: Collect shutdown status")
Reported-by: Dexuan Cui <decui@microsoft.com>
Reviewed-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
This commit is contained in:
Dan Williams 2019-01-29 22:06:41 -08:00
parent 966d23a006
commit f596c8844f
1 changed files with 24 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
drivers/acpi/nfit/core.c
View File

@ -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) __weak void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem)
{ {
struct device *dev = &nfit_mem->adev->dev;
struct nd_intel_smart smart = { 0 }; struct nd_intel_smart smart = { 0 };
union acpi_object in_buf = { union acpi_object in_buf = {
.type = ACPI_TYPE_BUFFER, .buffer.type = ACPI_TYPE_BUFFER,
.buffer.pointer = (char *) &smart, .buffer.length = 0,
.buffer.length = sizeof(smart),
}; };
union acpi_object in_obj = { union acpi_object in_obj = {
.type = ACPI_TYPE_PACKAGE, .package.type = ACPI_TYPE_PACKAGE,
.package.count = 1, .package.count = 1,
.package.elements = &in_buf, .package.elements = &in_buf,
}; };
@ -1760,8 +1760,15 @@ __weak void nfit_intel_shutdown_status(struct nfit_mem *nfit_mem)
return; return;
out_obj = acpi_evaluate_dsm(handle, guid, revid, func, &in_obj); 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; return;
}
memcpy(&smart, out_obj->buffer.pointer, sizeof(smart));
ACPI_FREE(out_obj);
if (smart.flags & ND_INTEL_SMART_SHUTDOWN_VALID) { if (smart.flags & ND_INTEL_SMART_SHUTDOWN_VALID) {
if (smart.shutdown_state) 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); set_bit(NFIT_MEM_DIRTY_COUNT, &nfit_mem->flags);
nfit_mem->dirty_shutdown = smart.shutdown_count; nfit_mem->dirty_shutdown = smart.shutdown_count;
} }
ACPI_FREE(out_obj);
} }
static void populate_shutdown_status(struct nfit_mem *nfit_mem) 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; | 1 << ND_CMD_SET_CONFIG_DATA;
if (family == NVDIMM_FAMILY_INTEL if (family == NVDIMM_FAMILY_INTEL
&& (dsm_mask & label_mask) == label_mask) && (dsm_mask & label_mask) == label_mask)
return 0; /* 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 (acpi_nvdimm_has_method(adev_dimm, "_LSI") if (test_bit(NFIT_MEM_LSR, &nfit_mem->flags)
&& acpi_nvdimm_has_method(adev_dimm, "_LSR")) { && acpi_nvdimm_has_method(adev_dimm, "_LSW")) {
dev_dbg(dev, "%s: has _LSR\n", dev_name(&adev_dimm->dev)); dev_dbg(dev, "%s: has _LSW\n", dev_name(&adev_dimm->dev));
set_bit(NFIT_MEM_LSR, &nfit_mem->flags); 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); populate_shutdown_status(nfit_mem);