On Tue, Jul 19, 2016 at 01:01:16PM -0700, Dan Williams wrote:
> 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?
Dan,
I applied this patch on top of the SLES 12 sp2 kernel I was testing
with last night.
The proposed patch below works for HPE nvdimms.
Jerry
> 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
>
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
