On Wed, 2015-11-18 at 12:54 -0500, Linda Knippers wrote:
> Since commit 209851649dc4f7900a6bfe1de5e2640ab2c7d931, we no longer
> see NVDIMM devices on our systems. The NFIT/_FIT processing at
> initialization gets a table from _FIT but doesn't like it.
>
> When support for _FIT was added, the code presumed that the data
> returned by the _FIT method is identical to the NFIT table, which
> starts with an acpi_table_header. However, the _FIT is defined
> to return a data in the format of a series of NFIT type structure
> entries and as a method, has an acpi_object header rather tahn
> an acpi_table_header.
Hm, I couldn't find any reference to this in the spec - that NFIT will
have the acpi_table_header but _FIT will have a different header - but
I'm no ACPI expert - is this usual convention? Any pointers where I
could look at?
>
> To address the differences, explicitly save the acpi_table_header
> from the NFIT, since it is accessible through /sys, and change
> the nfit pointer in the acpi_desc structure to point to the
> table entries rather than the headers.
>
> This is an RFC patch for several reasons.
> 1) I've only tested the boot path, not the code path gets
> gets a _FIT later.
> 2) There is some debug information that we probably don't
> want to keep in there.
> 3) I'm not even sure we should be checking _FIT at boot time
I think there is good reason to. If the driver is reloaded after a
hotplug event but prior to a full system reboot, if we don't check _FIT,
we will get the stale NFIT.
> 4) While this fixes my platform, it probably breaks the tests
> that were used to test the original commit.
>
> If we need to have a long discussion about whether our firmware
> is correct, then perhaps we can remove the _FIT code from
> acpi_nfit_add()
> while we sort it out.
>
> Reported-by: Jeff Moyer (jmoyer@xxxxxxxxxx>
> Signed-off-by: Linda Knippers <linda.knippers@xxxxxx>
> ---
> drivers/acpi/nfit.c | 55 +++++++++++++++++++++++++++++++++++++++++---
> ---------
> drivers/acpi/nfit.h | 3 ++-
> 2 files changed, 45 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c
> index f7dab53..ad95113 100644
> --- a/drivers/acpi/nfit.c
> +++ b/drivers/acpi/nfit.c
> @@ -655,7 +655,7 @@ static ssize_t revision_show(struct device *dev,
> struct nvdimm_bus_descriptor *nd_desc =
> to_nd_desc(nvdimm_bus);
> struct acpi_nfit_desc *acpi_desc = to_acpi_desc(nd_desc);
>
> - return sprintf(buf, "%d\n", acpi_desc->nfit-
> >header.revision);
> + return sprintf(buf, "%d\n", acpi_desc->acpi_header.revision);
> }
> static DEVICE_ATTR_RO(revision);
>
> @@ -1652,7 +1652,6 @@ int acpi_nfit_init(struct acpi_nfit_desc
> *acpi_desc, acpi_size sz)
>
> data = (u8 *) acpi_desc->nfit;
> end = data + sz;
> - data += sizeof(struct acpi_table_nfit);
> while (!IS_ERR_OR_NULL(data))
> data = add_table(acpi_desc, &prev, data, end);
>
> @@ -1748,13 +1747,34 @@ static int acpi_nfit_add(struct acpi_device
> *adev)
> return PTR_ERR(acpi_desc);
> }
>
> - acpi_desc->nfit = (struct acpi_table_nfit *) tbl;
> + /*
> + * Save the acpi header for later and then skip it, make
> + * nfit point to the first nfit table header.
> + */
> + acpi_desc->acpi_header = *tbl;
> + acpi_desc->nfit = (void *) tbl + sizeof(struct
> acpi_table_nfit);
> + sz -= sizeof(struct acpi_table_nfit);
>
> /* Evaluate _FIT and override with that if present */
> status = acpi_evaluate_object(adev->handle, "_FIT", NULL,
> &buf);
> if (ACPI_SUCCESS(status) && buf.length > 0) {
> - acpi_desc->nfit = (struct acpi_table_nfit
> *)buf.pointer;
> - sz = buf.length;
> + union acpi_object *obj;
> +
> + dev_dbg(dev, "%s _FIT ptr %p, length: %d\n",
> + __func__, buf.pointer, (int)buf.length);
> + print_hex_dump_debug("_FIT: ", DUMP_PREFIX_OFFSET,
> 16, 1,
> + buf.pointer, buf.length, true);
> +
> + /*
> + * Adjust for the acpi_object header of the _FIT
> + */
> + obj = buf.pointer;
> + if (obj->type == ACPI_TYPE_BUFFER) {
> + acpi_desc->nfit = (struct acpi_nfit_header
> *)obj->buffer.pointer;
> + sz = obj->buffer.length;
> + } else
> + dev_dbg(dev, "%s invalid type %d, ignoring
> _FIT\n",
> + __func__, (int) obj->type);
> }
>
> rc = acpi_nfit_init(acpi_desc, sz);
> @@ -1777,8 +1796,9 @@ static void acpi_nfit_notify(struct acpi_device
> *adev, u32 event)
> {
> struct acpi_nfit_desc *acpi_desc = dev_get_drvdata(&adev-
> >dev);
> struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> - struct acpi_table_nfit *nfit_saved;
> + struct acpi_nfit_header *nfit_saved;
> struct device *dev = &adev->dev;
> + union acpi_object *obj;
> acpi_status status;
> int ret;
>
> @@ -1807,13 +1827,24 @@ static void acpi_nfit_notify(struct
> acpi_device *adev, u32 event)
> goto out_unlock;
> }
>
> + dev_dbg(dev, "%s _FIT ptr %p, length: %d\n",
> + __func__, buf.pointer, (int)buf.length);
> + print_hex_dump_debug("_FIT: ", DUMP_PREFIX_OFFSET, 16, 1,
> + buf.pointer, buf.length, true);
> +
> nfit_saved = acpi_desc->nfit;
> - acpi_desc->nfit = (struct acpi_table_nfit *)buf.pointer;
> - ret = acpi_nfit_init(acpi_desc, buf.length);
> - if (!ret) {
> - /* Merge failed, restore old nfit, and exit */
> - acpi_desc->nfit = nfit_saved;
> - dev_err(dev, "failed to merge updated NFIT\n");
> + obj = buf.pointer;
> + if (obj->type == ACPI_TYPE_BUFFER) {
> + acpi_desc->nfit = (struct acpi_nfit_header *)obj-
> >buffer.pointer;
> + ret = acpi_nfit_init(acpi_desc, obj->buffer.length);
> + if (!ret) {
> + /* Merge failed, restore old nfit, and exit
> */
> + acpi_desc->nfit = nfit_saved;
> + dev_err(dev, "failed to merge updated
> NFIT\n");
> + }
> + } else {
> + /* Bad _FIT, restore old nfit */
> + dev_err(dev, "Invalid _FIT\n");
> }
> kfree(buf.pointer);
>
> diff --git a/drivers/acpi/nfit.h b/drivers/acpi/nfit.h
> index 2ea5c07..3d549a3 100644
> --- a/drivers/acpi/nfit.h
> +++ b/drivers/acpi/nfit.h
> @@ -96,7 +96,8 @@ struct nfit_mem {
>
> struct acpi_nfit_desc {
> struct nvdimm_bus_descriptor nd_desc;
> - struct acpi_table_nfit *nfit;
> + struct acpi_table_header acpi_header;
> + struct acpi_nfit_header *nfit;
> struct mutex spa_map_mutex;
> struct mutex init_mutex;
> struct list_head spa_maps;��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
