From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Rafael Wysocki <rafael.j.wysocki@intel.com>,
Bob Moore <robert.moore@intel.com>,
Erik Kaneda <erik.kaneda@intel.com>,
Yi Zhang <yi.zhang@redhat.com>,
nvdimm@lists.linux.dev, linux-nvdimm <linux-nvdimm@lists.01.org>,
ACPI Devel Maling List <linux-acpi@vger.kernel.org>
Subject: Re: [PATCH] ACPI: NFIT: Fix support for variable 'SPA' structure size
Date: Fri, 7 May 2021 11:47:25 +0200 [thread overview]
Message-ID: <CAJZ5v0gBCxFQ1pC1PTRximwzGXOSQDCzOfjX497aqBq5GQ48tA@mail.gmail.com> (raw)
In-Reply-To: <162037273007.1195827.10907249070709169329.stgit@dwillia2-desk3.amr.corp.intel.com>
Hi Dan,
On Fri, May 7, 2021 at 9:33 AM Dan Williams <dan.j.williams@intel.com> wrote:
>
> ACPI 6.4 introduced the "SpaLocationCookie" to the NFIT "System Physical
> Address (SPA) Range Structure". The presence of that new field is
> indicated by the ACPI_NFIT_LOCATION_COOKIE_VALID flag. Pre-ACPI-6.4
> firmware implementations omit the flag and maintain the original size of
> the structure.
>
> Update the implementation to check that flag to determine the size
> rather than the ACPI 6.4 compliant definition of 'struct
> acpi_nfit_system_address' from the Linux ACPICA definitions.
>
> Update the test infrastructure for the new expectations as well, i.e.
> continue to emulate the ACPI 6.3 definition of that structure.
>
> Without this fix the kernel fails to validate 'SPA' structures and this
> leads to a crash in nfit_get_smbios_id() since that routine assumes that
> SPAs are valid if it finds valid SMBIOS tables.
>
> BUG: unable to handle page fault for address: ffffffffffffffa8
> [..]
> Call Trace:
> skx_get_nvdimm_info+0x56/0x130 [skx_edac]
> skx_get_dimm_config+0x1f5/0x213 [skx_edac]
> skx_register_mci+0x132/0x1c0 [skx_edac]
>
> Cc: Bob Moore <robert.moore@intel.com>
> Cc: Erik Kaneda <erik.kaneda@intel.com>
> Fixes: cf16b05c607b ("ACPICA: ACPI 6.4: NFIT: add Location Cookie field")
Do you want me to apply this (as the commit being fixed went in
through the ACPI tree)?
If you'd rather take care of it yourself:
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reported-by: Yi Zhang <yi.zhang@redhat.com>
> Tested-by: Yi Zhang <yi.zhang@redhat.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>
> Rafael, I can take this through nvdimm.git after -rc1, but if you are
> sending any fixes to Linus based on your merge baseline between now and
> Monday, please pick up this one.
>
> drivers/acpi/nfit/core.c | 15 ++++++++++----
> tools/testing/nvdimm/test/nfit.c | 42 +++++++++++++++++++++++---------------
> 2 files changed, 36 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
> index 958aaac869e8..23d9a09d7060 100644
> --- a/drivers/acpi/nfit/core.c
> +++ b/drivers/acpi/nfit/core.c
> @@ -686,6 +686,13 @@ int nfit_spa_type(struct acpi_nfit_system_address *spa)
> return -1;
> }
>
> +static size_t sizeof_spa(struct acpi_nfit_system_address *spa)
> +{
> + if (spa->flags & ACPI_NFIT_LOCATION_COOKIE_VALID)
> + return sizeof(*spa);
> + return sizeof(*spa) - 8;
> +}
> +
> static bool add_spa(struct acpi_nfit_desc *acpi_desc,
> struct nfit_table_prev *prev,
> struct acpi_nfit_system_address *spa)
> @@ -693,22 +700,22 @@ static bool add_spa(struct acpi_nfit_desc *acpi_desc,
> struct device *dev = acpi_desc->dev;
> struct nfit_spa *nfit_spa;
>
> - if (spa->header.length != sizeof(*spa))
> + if (spa->header.length != sizeof_spa(spa))
> return false;
>
> list_for_each_entry(nfit_spa, &prev->spas, list) {
> - if (memcmp(nfit_spa->spa, spa, sizeof(*spa)) == 0) {
> + if (memcmp(nfit_spa->spa, spa, sizeof_spa(spa)) == 0) {
> list_move_tail(&nfit_spa->list, &acpi_desc->spas);
> return true;
> }
> }
>
> - nfit_spa = devm_kzalloc(dev, sizeof(*nfit_spa) + sizeof(*spa),
> + nfit_spa = devm_kzalloc(dev, sizeof(*nfit_spa) + sizeof_spa(spa),
> GFP_KERNEL);
> if (!nfit_spa)
> return false;
> INIT_LIST_HEAD(&nfit_spa->list);
> - memcpy(nfit_spa->spa, spa, sizeof(*spa));
> + memcpy(nfit_spa->spa, spa, sizeof_spa(spa));
> list_add_tail(&nfit_spa->list, &acpi_desc->spas);
> dev_dbg(dev, "spa index: %d type: %s\n",
> spa->range_index,
> diff --git a/tools/testing/nvdimm/test/nfit.c b/tools/testing/nvdimm/test/nfit.c
> index 9b185bf82da8..54f367cbadae 100644
> --- a/tools/testing/nvdimm/test/nfit.c
> +++ b/tools/testing/nvdimm/test/nfit.c
> @@ -1871,9 +1871,16 @@ static void smart_init(struct nfit_test *t)
> }
> }
>
> +static size_t sizeof_spa(struct acpi_nfit_system_address *spa)
> +{
> + /* until spa location cookie support is added... */
> + return sizeof(*spa) - 8;
> +}
> +
> static int nfit_test0_alloc(struct nfit_test *t)
> {
> - size_t nfit_size = sizeof(struct acpi_nfit_system_address) * NUM_SPA
> + struct acpi_nfit_system_address *spa = NULL;
> + size_t nfit_size = sizeof_spa(spa) * NUM_SPA
> + sizeof(struct acpi_nfit_memory_map) * NUM_MEM
> + sizeof(struct acpi_nfit_control_region) * NUM_DCR
> + offsetof(struct acpi_nfit_control_region,
> @@ -1937,7 +1944,8 @@ 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_nfit_system_address) * 2
> + struct acpi_nfit_system_address *spa = NULL;
> + size_t nfit_size = sizeof_spa(spa) * 2
> + sizeof(struct acpi_nfit_memory_map) * 2
> + offsetof(struct acpi_nfit_control_region, window_size) * 2;
> int i;
> @@ -2000,7 +2008,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> */
> spa = nfit_buf;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_PM), 16);
> spa->range_index = 0+1;
> spa->address = t->spa_set_dma[0];
> @@ -2014,7 +2022,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_PM), 16);
> spa->range_index = 1+1;
> spa->address = t->spa_set_dma[1];
> @@ -2024,7 +2032,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> /* spa2 (dcr0) dimm0 */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_DCR), 16);
> spa->range_index = 2+1;
> spa->address = t->dcr_dma[0];
> @@ -2034,7 +2042,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> /* spa3 (dcr1) dimm1 */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_DCR), 16);
> spa->range_index = 3+1;
> spa->address = t->dcr_dma[1];
> @@ -2044,7 +2052,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> /* spa4 (dcr2) dimm2 */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_DCR), 16);
> spa->range_index = 4+1;
> spa->address = t->dcr_dma[2];
> @@ -2054,7 +2062,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> /* spa5 (dcr3) dimm3 */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_DCR), 16);
> spa->range_index = 5+1;
> spa->address = t->dcr_dma[3];
> @@ -2064,7 +2072,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> /* spa6 (bdw for dcr0) dimm0 */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_BDW), 16);
> spa->range_index = 6+1;
> spa->address = t->dimm_dma[0];
> @@ -2074,7 +2082,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> /* spa7 (bdw for dcr1) dimm1 */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_BDW), 16);
> spa->range_index = 7+1;
> spa->address = t->dimm_dma[1];
> @@ -2084,7 +2092,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> /* spa8 (bdw for dcr2) dimm2 */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_BDW), 16);
> spa->range_index = 8+1;
> spa->address = t->dimm_dma[2];
> @@ -2094,7 +2102,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> /* spa9 (bdw for dcr3) dimm3 */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_BDW), 16);
> spa->range_index = 9+1;
> spa->address = t->dimm_dma[3];
> @@ -2581,7 +2589,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> /* spa10 (dcr4) dimm4 */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_DCR), 16);
> spa->range_index = 10+1;
> spa->address = t->dcr_dma[4];
> @@ -2595,7 +2603,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_PM), 16);
> spa->range_index = 11+1;
> spa->address = t->spa_set_dma[2];
> @@ -2605,7 +2613,7 @@ static void nfit_test0_setup(struct nfit_test *t)
> /* spa12 (bdw for dcr4) dimm4 */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_BDW), 16);
> spa->range_index = 12+1;
> spa->address = t->dimm_dma[4];
> @@ -2739,7 +2747,7 @@ static void nfit_test1_setup(struct nfit_test *t)
> /* spa0 (flat range with no bdw aliasing) */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_PM), 16);
> spa->range_index = 0+1;
> spa->address = t->spa_set_dma[0];
> @@ -2749,7 +2757,7 @@ static void nfit_test1_setup(struct nfit_test *t)
> /* virtual cd region */
> spa = nfit_buf + offset;
> spa->header.type = ACPI_NFIT_TYPE_SYSTEM_ADDRESS;
> - spa->header.length = sizeof(*spa);
> + spa->header.length = sizeof_spa(spa);
> memcpy(spa->range_guid, to_nfit_uuid(NFIT_SPA_VCD), 16);
> spa->range_index = 0;
> spa->address = t->spa_set_dma[1];
>
next prev parent reply other threads:[~2021-05-07 9:47 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-07 7:33 [PATCH] ACPI: NFIT: Fix support for variable 'SPA' structure size Dan Williams
2021-05-07 9:47 ` Rafael J. Wysocki [this message]
2021-05-07 14:12 ` Dan Williams
2021-05-07 14:49 ` Rafael J. Wysocki
2021-05-07 23:01 ` Dan Williams
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=CAJZ5v0gBCxFQ1pC1PTRximwzGXOSQDCzOfjX497aqBq5GQ48tA@mail.gmail.com \
--to=rafael@kernel.org \
--cc=dan.j.williams@intel.com \
--cc=erik.kaneda@intel.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-nvdimm@lists.01.org \
--cc=nvdimm@lists.linux.dev \
--cc=rafael.j.wysocki@intel.com \
--cc=robert.moore@intel.com \
--cc=yi.zhang@redhat.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).