All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
To: <qemu-devel@nongnu.org>, <qemu-arm@nongnu.org>,
	<eric.auger@redhat.com>, <imammedo@redhat.com>
Cc: peter.maydell@linaro.org, xiaoguangrong.eric@gmail.com,
	mst@redhat.com, linuxarm@huawei.com, xuwei5@hisilicon.com,
	shannon.zhaosl@gmail.com, prime.zeng@hisilicon.com,
	lersek@redhat.com
Subject: [PATCH v4 1/7] hw/acpi/nvdimm: Fix for NVDIMM incorrect DSM output buffer length
Date: Tue, 21 Apr 2020 13:59:28 +0100	[thread overview]
Message-ID: <20200421125934.14952-2-shameerali.kolothum.thodi@huawei.com> (raw)
In-Reply-To: <20200421125934.14952-1-shameerali.kolothum.thodi@huawei.com>

As per ACPI spec 6.3, Table 19-419 Object Conversion Rules, if
the Buffer Field <= to the size of an Integer (in bits), it will
be treated as an integer. Moreover, the integer size depends on
DSDT tables revision number. If revision number is < 2, integer
size is 32 bits, otherwise it is 64 bits. Current NVDIMM common
DSM aml code (NCAL) uses CreateField() for creating DSM output
buffer. This creates an issue in arm/virt platform where DSDT
revision number is 2 and results in DSM buffer with a wrong
size(8 bytes) gets returned when actual length is < 8 bytes.
This causes guest kernel to report,

"nfit ACPI0012:00: found a zero length table '0' parsing nfit"

In order to fix this, aml code is now modified such that it builds
the DSM output buffer in a byte by byte fashion when length is
smaller than Integer size.

Suggested-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/acpi/nvdimm.c                            | 40 +++++++++++++++++++--
 tests/qtest/bios-tables-test-allowed-diff.h |  2 ++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index eb6a37b14e..df0790719a 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -938,6 +938,7 @@ static void nvdimm_build_common_dsm(Aml *dev)
     Aml *method, *ifctx, *function, *handle, *uuid, *dsm_mem, *elsectx2;
     Aml *elsectx, *unsupport, *unpatched, *expected_uuid, *uuid_invalid;
     Aml *pckg, *pckg_index, *pckg_buf, *field, *dsm_out_buf, *dsm_out_buf_size;
+    Aml *whilectx, *offset;
     uint8_t byte_list[1];
 
     method = aml_method(NVDIMM_COMMON_DSM, 5, AML_SERIALIZED);
@@ -1091,13 +1092,46 @@ static void nvdimm_build_common_dsm(Aml *dev)
     /* RLEN is not included in the payload returned to guest. */
     aml_append(method, aml_subtract(aml_name(NVDIMM_DSM_OUT_BUF_SIZE),
                aml_int(4), dsm_out_buf_size));
+
+    /*
+     * As per ACPI spec 6.3, Table 19-419 Object Conversion Rules, if
+     * the Buffer Field <= to the size of an Integer (in bits), it will
+     * be treated as an integer. Moreover, the integer size depends on
+     * DSDT tables revision number. If revision number is < 2, integer
+     * size is 32 bits, otherwise it is 64 bits.
+     * Because of this CreateField() canot be used if RLEN < Integer Size.
+     *
+     * Also please note that APCI ASL operator SizeOf() doesn't support
+     * Integer and there isn't any other way to figure out the Integer
+     * size. Hence we assume 8 byte as Integer size and if RLEN < 8 bytes,
+     * build dsm_out_buf byte by byte.
+     */
+    ifctx = aml_if(aml_lless(dsm_out_buf_size, aml_int(8)));
+    offset = aml_local(2);
+    aml_append(ifctx, aml_store(aml_int(0), offset));
+    aml_append(ifctx, aml_name_decl("TBUF", aml_buffer(1, NULL)));
+    aml_append(ifctx, aml_store(aml_buffer(0, NULL), dsm_out_buf));
+
+    whilectx = aml_while(aml_lless(offset, dsm_out_buf_size));
+    /* Copy 1 byte at offset from ODAT to temporary buffer(TBUF). */
+    aml_append(whilectx, aml_store(aml_derefof(aml_index(
+                                   aml_name(NVDIMM_DSM_OUT_BUF), offset)),
+                                   aml_index(aml_name("TBUF"), aml_int(0))));
+    aml_append(whilectx, aml_concatenate(dsm_out_buf, aml_name("TBUF"),
+                                         dsm_out_buf));
+    aml_append(whilectx, aml_increment(offset));
+    aml_append(ifctx, whilectx);
+
+    aml_append(ifctx, aml_return(dsm_out_buf));
+    aml_append(method, ifctx);
+
+    /* If RLEN >= Integer size, just use CreateField() operator */
     aml_append(method, aml_store(aml_shiftleft(dsm_out_buf_size, aml_int(3)),
                                  dsm_out_buf_size));
     aml_append(method, aml_create_field(aml_name(NVDIMM_DSM_OUT_BUF),
                aml_int(0), dsm_out_buf_size, "OBUF"));
-    aml_append(method, aml_concatenate(aml_buffer(0, NULL), aml_name("OBUF"),
-                                       dsm_out_buf));
-    aml_append(method, aml_return(dsm_out_buf));
+    aml_append(method, aml_return(aml_name("OBUF")));
+
     aml_append(dev, method);
 }
 
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..eb8bae1407 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,3 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/pc/SSDT.dimmpxm",
+"tests/data/acpi/q35/SSDT.dimmpxm",
-- 
2.17.1




  reply	other threads:[~2020-04-21 13:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-21 12:59 [PATCH v4 0/7] ARM virt: Add NVDIMM support Shameer Kolothum
2020-04-21 12:59 ` Shameer Kolothum [this message]
2020-04-21 12:59 ` [PATCH v4 2/7] nvdimm: Use configurable ACPI IO base and size Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 3/7] hw/arm/virt: Add nvdimm hot-plug infrastructure Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 4/7] hw/arm/virt: Add nvdimm hotplug support Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 5/7] tests: Update ACPI tables list for upcoming arm/virt test changes Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 6/7] bios-tables-test: test pc-dimm and nvdimm coldplug for arm/virt Shameer Kolothum
2020-04-21 12:59 ` [PATCH v4 7/7] tests/acpi: add expected tables for bios-tables-test Shameer Kolothum
2020-04-21 15:12 ` [PATCH v4 0/7] ARM virt: Add NVDIMM support no-reply
2020-05-04  9:57   ` Michael S. Tsirkin
2020-05-04 10:06     ` Peter Maydell
2020-05-04 11:10       ` Michael S. Tsirkin
2020-05-04  5:13 ` Michael S. Tsirkin
2020-05-04  9:29   ` Peter Maydell
2020-05-04  9:44     ` Auger Eric
2020-05-04  9:46     ` Michael S. Tsirkin
2020-05-04  9:57       ` Peter Maydell

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=20200421125934.14952-2-shameerali.kolothum.thodi@huawei.com \
    --to=shameerali.kolothum.thodi@huawei.com \
    --cc=eric.auger@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=lersek@redhat.com \
    --cc=linuxarm@huawei.com \
    --cc=mst@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=prime.zeng@hisilicon.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=shannon.zhaosl@gmail.com \
    --cc=xiaoguangrong.eric@gmail.com \
    --cc=xuwei5@hisilicon.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 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.