[Qemu-devel] [RFC QEMU PATCH v4 09/10] nvdimm acpi: add compatibility for 64-bit integer in ACPI 2.0 and later

[Haozhong Zhang <haozhong.zhang@intel.com>]

When QEMU is used as Xen device model, the QEMU-built NVDIMM ACPI
tables (NFIT and SSDT) may be passed to Xen and merged with Xen-built
ACPI tables. However, different ACPI versions are used between QEMU
(ACPI 1.0) and Xen (ACPI 2.0), and different integer widths are used
between ACPI 1.0 (32 bits) and ACPI 2.0 (64 bits).

Due to the implicit type conversion between ACPI buffer field object
and ACPI integer object (ref. ACPI Spec 6.2, Sect 19.3.5.5, 19.3.5.7 &
19.3.5.8), the following AML in NVDIMM SSDT may behave differently in
ACPI 1.0 and ACPI 2.0:

    Method (NCAL, 5, Serialized)
    {
        Local6 = MEMA /* \MEMA */
        OperationRegion (NPIO, SystemIO, 0x0A18, 0x04)
        OperationRegion (NRAM, SystemMemory, Local6, 0x1000)
        Field (NPIO, DWordAcc, NoLock, Preserve)
        {
            NTFI,   32
        }

        ...

        Field (NRAM, DWordAcc, NoLock, Preserve)
        {
            RLEN,   32,
            ODAT,   32736
        }

        ...

        NTFI = Local6
        Local1 = (RLEN - 0x04)
        Local1 = (Local1 << 0x03)
        CreateField (ODAT, Zero, Local1, OBUF)
        Concatenate (Buffer (Zero){}, OBUF, Local7)
        Return (Local7)
    }

The C layout of the above ODAT is struct NvdimmFuncReadFitOut without
the length field:

    struct {
        uint32_t func_ret_status;
	uint8_t fit[0];
    }

When no error happens and no FIT data is needed to return,
nvdimm_dsm_func_read_fit() fills

    { .func_ret_status = 0 },

i.e., 4 bytes of 0's in ODAT. Because the length of ODAT is no larger
than an integer, OBUF is implicitly converted into an ACPI integer
object during the evaluation of CreateField. Later, when OBUF is
concatenated to another buffer, it needs to be converted to an ACPI
buffer object.  It's converted to a 4 bytes buffer in ACPI 1.0, but
it's converted to a 8 bytes buffer in ACPI 2.0. The extra 4 bytes in
ACPI 2.0 actually corresponds to the apparently incorrect case that

    { .func_ret_status = 0, fit = { 0, 0, 0, 0 } }

is filled in ODAT.

In order to mitigate this issue, we add a 32-bit reserved field after
func_ret_status and always fill it with 0. Therefore, the minimum
length of ODAT in both ACPI 1.0 and ACPI 2.0 is always 8 bytes, so no
extra bytes will be added accidentally by the implicit conversion.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
Cc: Xiao Guangrong <xiaoguangrong.eric@gmail.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
---
 hw/acpi/nvdimm.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 7b3062e001..bceb35e75a 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -493,6 +493,7 @@ struct NvdimmFuncReadFITOut {
     /* the size of buffer filled by QEMU. */
     uint32_t len;
     uint32_t func_ret_status; /* return status code. */
+    uint32_t reserved;
     uint8_t fit[0]; /* the FIT data. */
 } QEMU_PACKED;
 typedef struct NvdimmFuncReadFITOut NvdimmFuncReadFITOut;
@@ -597,6 +598,7 @@ exit:
 
     read_fit_out->len = cpu_to_le32(size);
     read_fit_out->func_ret_status = cpu_to_le32(func_ret_status);
+    read_fit_out->reserved = 0;
     memcpy(read_fit_out->fit, fit->data + read_fit->offset, read_len);
 
     nvdimm_copy_to_dsm_mem(dsm_mem_addr, read_fit_out, size);
@@ -1168,7 +1170,8 @@ static void nvdimm_build_fit(Aml *dev)
 
     aml_append(method, aml_store(aml_sizeof(buf), buf_size));
     aml_append(method, aml_subtract(buf_size,
-                                    aml_int(4) /* the size of "STAU" */,
+                                    aml_int(8) /* the size of "STAU" and the
+                                                  consequent reserved field */,
                                     buf_size));
 
     /* if we read the end of fit. */
@@ -1177,7 +1180,7 @@ static void nvdimm_build_fit(Aml *dev)
     aml_append(method, ifctx);
 
     aml_append(method, aml_create_field(buf,
-                            aml_int(4 * BITS_PER_BYTE), /* offset at byte 4.*/
+                            aml_int(8 * BITS_PER_BYTE), /* offset at byte 8. */
                             aml_shiftleft(buf_size, aml_int(3)), "BUFF"));
     aml_append(method, aml_return(aml_name("BUFF")));
     aml_append(dev, method);
-- 
2.15.1