Re: [PATCH] acpi, nfit: fix acpi_check_dsm() vs zero functions implemented

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jul 19, 2016 at 11:52 AM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
> On Tue, Jul 19, 2016 at 11:50 AM, Linda Knippers <linda.knippers@xxxxxxx> wrote:
>> On 7/19/2016 1:11 PM, Jerry Hoemann wrote:
[..]
>>> As nfit_mem->family always equals NVDIMM_FAMILY_INTEL, no subsequent
>>> DSM call will succeed for NVDIMM_FAMILY_HPE1 or any other
>>> family.
>>>
>>> I don't have a fix as of yet, but wanted to make you aware of
>>> the problem.
>>
>> Could we try the all known UUIDs looking for one that returns a non-zero
>> value?
>>
>
> Yes, that seems like the way forward, and also make not finding a DSM
> family non-fatal.

Actually, all we need is that last bit...  Jerry, Xiao, can you try
the attached patch on top for v4.7-rc6 to see if it works in both HPe
and QEMU 2.6 environments respectively?
From 367f4468b349a9ed22fc0e6382a52c18c6775472 Mon Sep 17 00:00:00 2001
From: Dan Williams <dan.j.williams@xxxxxxxxx>
Date: Tue, 19 Jul 2016 12:32:39 -0700
Subject: [PATCH] nfit: make DIMM DSMs optional

Commit 4995734e973a "acpi, nfit: fix acpi_check_dsm() vs zero functions
implemented" attempted to fix a QEMU regression by supporting its usage
of a zero-mask as a valid response to a DSM-family probe request.
However, this behavior breaks HP platforms that return a zero-mask by
default causing the probe to misidentify the DSM-family.

Instead, the QEMU regression can be fixed by simply not requiring the DSM
family to be identified.

This effectively reverts commit 4995734e973a, and removes the DSM
requirement from the init path.

Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
Cc: Xiao Guangrong <guangrong.xiao@xxxxxxxxxxxxxxx>
Cc: Linda Knippers <linda.knippers@xxxxxxx>
Fixes: 4995734e973a ("acpi, nfit: fix acpi_check_dsm() vs zero functions implemented")
Reported-by: Jerry Hoemann <jerry.hoemann@xxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
---
 drivers/acpi/nfit.c  | 11 ++++++-----
 drivers/acpi/utils.c |  6 +++---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index ac6ddcc080d4..1f0e06065ae6 100644
--- a/drivers/acpi/nfit.c
+++ b/drivers/acpi/nfit.c
@@ -1131,11 +1131,11 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 
 	/*
 	 * Until standardization materializes we need to consider up to 3
-	 * different command sets.  Note, that checking for zero functions
-	 * tells us if any commands might be reachable through this uuid.
+	 * different command sets.  Note, that checking for function0 (bit0)
+	 * tells us if any commands are reachable through this uuid.
 	 */
 	for (i = NVDIMM_FAMILY_INTEL; i <= NVDIMM_FAMILY_HPE2; i++)
-		if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 0))
+		if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 1))
 			break;
 
 	/* limit the supported commands to those that are publicly documented */
@@ -1151,9 +1151,10 @@ static int acpi_nfit_add_dimm(struct acpi_nfit_desc *acpi_desc,
 		if (disable_vendor_specific)
 			dsm_mask &= ~(1 << 8);
 	} else {
-		dev_err(dev, "unknown dimm command family\n");
+		dev_dbg(dev, "unknown dimm command family\n");
 		nfit_mem->family = -1;
-		return force_enable_dimms ? 0 : -ENODEV;
+		/* DSMs are optional, continue loading the driver... */
+		return 0;
 	}
 
 	uuid = to_nfit_uuid(nfit_mem->family);
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index b4de130f2d57..22c09952e177 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -680,6 +680,9 @@ bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, u64 rev, u64 funcs)
 	u64 mask = 0;
 	union acpi_object *obj;
 
+	if (funcs == 0)
+		return false;
+
 	obj = acpi_evaluate_dsm(handle, uuid, rev, 0, NULL);
 	if (!obj)
 		return false;
@@ -692,9 +695,6 @@ bool acpi_check_dsm(acpi_handle handle, const u8 *uuid, u64 rev, u64 funcs)
 			mask |= (((u64)obj->buffer.pointer[i]) << (i * 8));
 	ACPI_FREE(obj);
 
-	if (funcs == 0)
-		return true;
-
 	/*
 	 * Bit 0 indicates whether there's support for any functions other than
 	 * function 0 for the specified UUID and revision.
-- 
2.5.5


[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux