All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: linux-nvdimm@lists.01.org
Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com>,
	linux-acpi@vger.kernel.org,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>
Subject: [PATCH] acpi, nfit: fix acpi_check_dsm() vs zero functions implemented
Date: Fri, 24 Jun 2016 10:44:25 -0700	[thread overview]
Message-ID: <146679026571.24395.11569929364936343871.stgit@dwillia2-desk3.amr.corp.intel.com> (raw)

QEMU 2.6 implements nascent support for nvdimm DSMs. Depending on
configuration it may only implement the function0 dsm to indicate that
no other DSMs are available. Commit 31eca76ba2fc "nfit, libnvdimm:
limited/whitelisted dimm command marshaling mechanism" breaks QEMU, but
QEMU is spec compliant.  Per the spec the way to indicate that no
functions are supported is:

    If Function Index is zero, the return is a buffer containing one bit
    for each function index, starting with zero. Bit 0 indicates whether
    there is support for any functions other than function 0 for the
    specified UUID and Revision ID. If set to zero, no functions are
    supported (other than function zero) for the specified UUID and
    Revision ID.

Update the nfit driver to determine the family (interface UUID) without
requiring the implementation to define any other functions, i.e.
short-circuit acpi_check_dsm() to succeed per the spec.  The nfit driver
appears to be the only user passing funcs==0 to acpi_check_dsm(), so
this behavior change of the common routine should be limited to the
probing done by the nfit driver.

Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Len Brown <lenb@kernel.org>
Cc: Jerry Hoemann <jerry.hoemann@hpe.com>
Fixes: 31eca76ba2fc ("nfit, libnvdimm: limited/whitelisted dimm command marshaling mechanism")
Reported-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Tested-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/acpi/nfit.c  |    6 +++---
 drivers/acpi/utils.c |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index 2215fc847fa9..32579a7b71d5 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 function0 (bit0)
-	 * tells us if any commands are reachable through this uuid.
+	 * different command sets.  Note, that checking for zero functions
+	 * tells us if any commands might be 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, 1))
+		if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 0))
 			break;
 
 	/* limit the supported commands to those that are publicly documented */
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 22c09952e177..b4de130f2d57 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -680,9 +680,6 @@ 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;
@@ -695,6 +692,9 @@ 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.

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org
Cc: Xiao Guangrong
	<guangrong.xiao-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"Rafael J. Wysocki" <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>,
	Len Brown <lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Subject: [PATCH] acpi, nfit: fix acpi_check_dsm() vs zero functions implemented
Date: Fri, 24 Jun 2016 10:44:25 -0700	[thread overview]
Message-ID: <146679026571.24395.11569929364936343871.stgit@dwillia2-desk3.amr.corp.intel.com> (raw)

QEMU 2.6 implements nascent support for nvdimm DSMs. Depending on
configuration it may only implement the function0 dsm to indicate that
no other DSMs are available. Commit 31eca76ba2fc "nfit, libnvdimm:
limited/whitelisted dimm command marshaling mechanism" breaks QEMU, but
QEMU is spec compliant.  Per the spec the way to indicate that no
functions are supported is:

    If Function Index is zero, the return is a buffer containing one bit
    for each function index, starting with zero. Bit 0 indicates whether
    there is support for any functions other than function 0 for the
    specified UUID and Revision ID. If set to zero, no functions are
    supported (other than function zero) for the specified UUID and
    Revision ID.

Update the nfit driver to determine the family (interface UUID) without
requiring the implementation to define any other functions, i.e.
short-circuit acpi_check_dsm() to succeed per the spec.  The nfit driver
appears to be the only user passing funcs==0 to acpi_check_dsm(), so
this behavior change of the common routine should be limited to the
probing done by the nfit driver.

Cc: "Rafael J. Wysocki" <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org>
Cc: Len Brown <lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Jerry Hoemann <jerry.hoemann-ZPxbGqLxI0U@public.gmane.org>
Fixes: 31eca76ba2fc ("nfit, libnvdimm: limited/whitelisted dimm command marshaling mechanism")
Reported-by: Xiao Guangrong <guangrong.xiao-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Tested-by: Xiao Guangrong <guangrong.xiao-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Signed-off-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/acpi/nfit.c  |    6 +++---
 drivers/acpi/utils.c |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
index 2215fc847fa9..32579a7b71d5 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 function0 (bit0)
-	 * tells us if any commands are reachable through this uuid.
+	 * different command sets.  Note, that checking for zero functions
+	 * tells us if any commands might be 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, 1))
+		if (acpi_check_dsm(adev_dimm->handle, to_nfit_uuid(i), 1, 0))
 			break;
 
 	/* limit the supported commands to those that are publicly documented */
diff --git a/drivers/acpi/utils.c b/drivers/acpi/utils.c
index 22c09952e177..b4de130f2d57 100644
--- a/drivers/acpi/utils.c
+++ b/drivers/acpi/utils.c
@@ -680,9 +680,6 @@ 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;
@@ -695,6 +692,9 @@ 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.

             reply	other threads:[~2016-06-24 17:45 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-24 17:44 Dan Williams [this message]
2016-06-24 17:44 ` [PATCH] acpi, nfit: fix acpi_check_dsm() vs zero functions implemented Dan Williams
2016-06-24 22:15 ` Rafael J. Wysocki
2016-06-24 22:15   ` Rafael J. Wysocki
2016-06-27 17:40 ` Linda Knippers
2016-06-27 17:40   ` Linda Knippers
2016-06-27 18:06   ` Dan Williams
2016-06-27 18:06     ` Dan Williams
2016-06-27 18:47     ` Linda Knippers
2016-06-27 18:47       ` Linda Knippers
2016-06-27 18:58       ` Dan Williams
2016-06-27 18:58         ` Dan Williams
2016-06-27 19:03         ` Linda Knippers
2016-06-27 19:03           ` Linda Knippers
2016-06-28  9:37         ` Xiao Guangrong
2016-06-28  9:37           ` Xiao Guangrong
2016-06-28 15:31           ` Jerry Hoemann
2016-06-28 15:31             ` Jerry Hoemann
2016-07-19 17:11 ` Jerry Hoemann
2016-07-19 17:11   ` Jerry Hoemann
2016-07-19 18:50   ` Linda Knippers
2016-07-19 18:50     ` Linda Knippers
2016-07-19 18:52     ` Dan Williams
2016-07-19 18:52       ` Dan Williams
2016-07-19 19:00       ` Jerry Hoemann
2016-07-19 19:00         ` Jerry Hoemann
2016-07-19 20:01       ` Dan Williams
2016-07-19 20:01         ` Dan Williams
2016-07-19 22:46         ` Jerry Hoemann
2016-07-19 22:46           ` Jerry Hoemann
2016-07-19 22:53           ` Dan Williams
2016-07-19 22:53             ` Dan Williams
2016-07-20 22:49             ` Dan Williams
2016-07-20 22:49               ` Dan Williams
2016-07-21  5:40               ` Xiao Guangrong
2016-07-21  5:40                 ` Xiao Guangrong
2016-07-22  8:14         ` Johannes Thumshirn
2016-07-22  8:14           ` Johannes Thumshirn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=146679026571.24395.11569929364936343871.stgit@dwillia2-desk3.amr.corp.intel.com \
    --to=dan.j.williams@intel.com \
    --cc=guangrong.xiao@linux.intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-nvdimm@lists.01.org \
    --cc=rjw@rjwysocki.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.