| Message ID | 03fba073944c91810f89e321cb7fa024f8e9e2c1.1448045168.git.linda.knippers@hpe.com |
|---|---|
| State | New |
| Headers | show |
On 11/23/2015 6:26 PM, Verma, Vishal L wrote: > On Fri, 2015-11-20 at 19:05 -0500, Linda Knippers wrote: >> 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. >> >> 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. >> >> Reported-by: Jeff Moyer (jmoyer@redhat.com> >> Signed-off-by: Linda Knippers <linda.knippers@hpe.com> >> --- >> drivers/acpi/nfit.c | 47 +++++++++++++++++++++++++++++++++++--------- >> --- >> drivers/acpi/nfit.h | 3 ++- >> 2 files changed, 37 insertions(+), 13 deletions(-) >> > > This patch looks good, but the fact that we're doing the header > processing earlier (before acpi_nfit_init) means that the nfit supplied > by the nfit_test framework (tools/testing/nvdimm/test) can no longer > have a header. The following patch fixes this. > > Apart from that, and the open question for patch 1, for the series, > > Reviewed-by: Vishal Verma <vishal.l.verma@intel.com> Thanks Vishal. I hope I've addressed your question on patch 1. I think patch 1 works as well as the original code did so if you have concerns about making the code more bullet proof in the face of buggy firmware, perhaps we can address that separately from this series. I would really like to see this series go in soon because since the hotplug code went in I can't run an upstream kernel on my hardware. -- ljk > > > > 8<----- > > From da2fd5730bc5125c72101a6b55b85fce97663e40 Mon Sep 17 00:00:00 2001 > From: Vishal Verma <vishal.l.verma@intel.com> > Date: Mon, 23 Nov 2015 15:41:00 -0700 > Subject: [PATCH] nfit_test: Fix nfit generation for test framework > > The hotplug fixes move header parsing to happen earlier, > and the way nfit_test calls into the nfit initialization code, > a header is no longer needed. Remove the header from the table > generated by nfit_test entirely. > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > tools/testing/nvdimm/test/nfit.c | 46 ++++++++++++---------------------------- > 1 file changed, 14 insertions(+), 32 deletions(-) > > diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c > index 40ab447..3f92d7f 100644 > --- a/tools/testing/nvdimm/test/nfit.c > +++ b/tools/testing/nvdimm/test/nfit.c > @@ -420,8 +420,7 @@ static struct nfit_test_resource *nfit_test_lookup(resource_size_t addr) > > static int nfit_test0_alloc(struct nfit_test *t) > { > - size_t nfit_size = sizeof(struct acpi_table_nfit) > - + sizeof(struct acpi_nfit_system_address) * NUM_SPA > + size_t nfit_size = sizeof(struct acpi_nfit_system_address) * NUM_SPA > + sizeof(struct acpi_nfit_memory_map) * NUM_MEM > + sizeof(struct acpi_nfit_control_region) * NUM_DCR > + sizeof(struct acpi_nfit_data_region) * NUM_BDW > @@ -471,8 +470,7 @@ static int nfit_test0_alloc(struct nfit_test *t) > > static int nfit_test1_alloc(struct nfit_test *t) > { > - size_t nfit_size = sizeof(struct acpi_table_nfit) > - + sizeof(struct acpi_nfit_system_address) > + size_t nfit_size = sizeof(struct acpi_nfit_system_address) > + sizeof(struct acpi_nfit_memory_map) > + sizeof(struct acpi_nfit_control_region); > > @@ -488,18 +486,6 @@ static int nfit_test1_alloc(struct nfit_test *t) > return 0; > } > > -static void nfit_test_init_header(struct acpi_table_nfit *nfit, size_t size) > -{ > - memcpy(nfit->header.signature, ACPI_SIG_NFIT, 4); > - nfit->header.length = size; > - nfit->header.revision = 1; > - memcpy(nfit->header.oem_id, "LIBND", 6); > - memcpy(nfit->header.oem_table_id, "TEST", 5); > - nfit->header.oem_revision = 1; > - memcpy(nfit->header.asl_compiler_id, "TST", 4); > - nfit->header.asl_compiler_revision = 1; > -} > - > static void nfit_test0_setup(struct nfit_test *t) > { > struct nvdimm_bus_descriptor *nd_desc; > @@ -513,14 +499,12 @@ static void nfit_test0_setup(struct nfit_test *t) > struct acpi_nfit_flush_address *flush; > unsigned int offset; > > - nfit_test_init_header(nfit_buf, size); > - > /* > * spa0 (interleave first half of dimm0 and dimm1, note storage > * does not actually alias the related block-data-window > * regions) > */ > - spa = nfit_buf + sizeof(struct acpi_table_nfit); > + spa = nfit_buf; > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > spa->header.length = sizeof(*spa); > memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_PM), 16); > @@ -533,7 +517,7 @@ static void nfit_test0_setup(struct nfit_test *t) > * does not actually alias the related block-data-window > * regions) > */ > - spa = nfit_buf + sizeof(struct acpi_table_nfit) + sizeof(*spa); > + spa = nfit_buf + sizeof(*spa); > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > spa->header.length = sizeof(*spa); > memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_PM), 16); > @@ -542,7 +526,7 @@ static void nfit_test0_setup(struct nfit_test *t) > spa->length = SPA1_SIZE; > > /* spa2 (dcr0) dimm0 */ > - spa = nfit_buf + sizeof(struct acpi_table_nfit) + sizeof(*spa) * 2; > + spa = nfit_buf + sizeof(*spa) * 2; > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > spa->header.length = sizeof(*spa); > memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_DCR), 16); > @@ -551,7 +535,7 @@ static void nfit_test0_setup(struct nfit_test *t) > spa->length = DCR_SIZE; > > /* spa3 (dcr1) dimm1 */ > - spa = nfit_buf + sizeof(struct acpi_table_nfit) + sizeof(*spa) * 3; > + spa = nfit_buf + sizeof(*spa) * 3; > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > spa->header.length = sizeof(*spa); > memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_DCR), 16); > @@ -560,7 +544,7 @@ static void nfit_test0_setup(struct nfit_test *t) > spa->length = DCR_SIZE; > > /* spa4 (dcr2) dimm2 */ > - spa = nfit_buf + sizeof(struct acpi_table_nfit) + sizeof(*spa) * 4; > + spa = nfit_buf + sizeof(*spa) * 4; > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > spa->header.length = sizeof(*spa); > memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_DCR), 16); > @@ -569,7 +553,7 @@ static void nfit_test0_setup(struct nfit_test *t) > spa->length = DCR_SIZE; > > /* spa5 (dcr3) dimm3 */ > - spa = nfit_buf + sizeof(struct acpi_table_nfit) + sizeof(*spa) * 5; > + spa = nfit_buf + sizeof(*spa) * 5; > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > spa->header.length = sizeof(*spa); > memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_DCR), 16); > @@ -578,7 +562,7 @@ static void nfit_test0_setup(struct nfit_test *t) > spa->length = DCR_SIZE; > > /* spa6 (bdw for dcr0) dimm0 */ > - spa = nfit_buf + sizeof(struct acpi_table_nfit) + sizeof(*spa) * 6; > + spa = nfit_buf + sizeof(*spa) * 6; > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > spa->header.length = sizeof(*spa); > memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_BDW), 16); > @@ -587,7 +571,7 @@ static void nfit_test0_setup(struct nfit_test *t) > spa->length = DIMM_SIZE; > > /* spa7 (bdw for dcr1) dimm1 */ > - spa = nfit_buf + sizeof(struct acpi_table_nfit) + sizeof(*spa) * 7; > + spa = nfit_buf + sizeof(*spa) * 7; > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > spa->header.length = sizeof(*spa); > memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_BDW), 16); > @@ -596,7 +580,7 @@ static void nfit_test0_setup(struct nfit_test *t) > spa->length = DIMM_SIZE; > > /* spa8 (bdw for dcr2) dimm2 */ > - spa = nfit_buf + sizeof(struct acpi_table_nfit) + sizeof(*spa) * 8; > + spa = nfit_buf + sizeof(*spa) * 8; > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > spa->header.length = sizeof(*spa); > memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_BDW), 16); > @@ -605,7 +589,7 @@ static void nfit_test0_setup(struct nfit_test *t) > spa->length = DIMM_SIZE; > > /* spa9 (bdw for dcr3) dimm3 */ > - spa = nfit_buf + sizeof(struct acpi_table_nfit) + sizeof(*spa) * 9; > + spa = nfit_buf + sizeof(*spa) * 9; > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > spa->header.length = sizeof(*spa); > memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_BDW), 16); > @@ -613,7 +597,7 @@ static void nfit_test0_setup(struct nfit_test *t) > spa->address = t->dimm_dma[3]; > spa->length = DIMM_SIZE; > > - offset = sizeof(struct acpi_table_nfit) + sizeof(*spa) * 10; > + offset = sizeof(*spa) * 10; > /* mem-region0 (spa0, dimm0) */ > memdev = nfit_buf + offset; > memdev->header.type = ACPI_NFIT_TYPE_MEMORY_MAP; > @@ -1106,9 +1090,7 @@ static void nfit_test1_setup(struct nfit_test *t) > struct acpi_nfit_control_region *dcr; > struct acpi_nfit_system_address *spa; > > - nfit_test_init_header(nfit_buf, size); > - > - offset = sizeof(struct acpi_table_nfit); > + offset = 0; > /* spa0 (flat range with no bdw aliasing) */ > spa = nfit_buf + offset; > spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS; > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/nfit.c b/drivers/acpi/nfit.c index c140194d..b1fae13 100644 --- a/drivers/acpi/nfit.c +++ b/drivers/acpi/nfit.c @@ -679,7 +679,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); @@ -1676,7 +1676,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); @@ -1772,13 +1771,29 @@ 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, + * making 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; + /* + * 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); @@ -1801,7 +1816,8 @@ 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; + union acpi_object *obj; struct device *dev = &adev->dev; acpi_status status; int ret; @@ -1832,12 +1848,19 @@ static void acpi_nfit_notify(struct acpi_device *adev, u32 event) } 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;
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. 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. Reported-by: Jeff Moyer (jmoyer@redhat.com> Signed-off-by: Linda Knippers <linda.knippers@hpe.com> --- drivers/acpi/nfit.c | 47 +++++++++++++++++++++++++++++++++++------------ drivers/acpi/nfit.h | 3 ++- 2 files changed, 37 insertions(+), 13 deletions(-) -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html