[PATCH v3 0/5] Support ACPI NVDIMM Label Methods

[PATCH v3 0/5] Support ACPI NVDIMM Label Methods

[Robert Hoo]

Originally NVDIMM Label methods was defined in Intel PMEM _DSM Interface
Spec [1], of function index 4, 5 and 6.
Recent ACPI spec [2] has deprecated those _DSM methods with ACPI NVDIMM
Label Methods _LS{I,R,W}. The essence of these functions has no changes.

This patch set is to update QEMU emulation on this, as well as update
bios-table-test golden binaries.

[1] Intel PMEM _DSM Interface Spec v2.0, 3.10 Deprecated Functions
https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf
[2] ACPI Spec v6.4, 6.5.10 NVDIMM Label Methods
https://uefi.org/sites/default/files/resources/ACPI_Spec_6_4_Jan22.pdf

---
Change Log:
v2 --> v3:
Patch of nvdimm_debug() --> qemu trace, has been separated and already
upstream'ed.
Patch of accepting _DSM rev.2 is dropped, as unnecessary.
Roll back implementation to the idea of simply wrapper _DSM.

v1 --> v2:
Almost rewritten
Separate Patch 2
Dance with tests/qtest/bios-table-tests
Add trace event

Robert Hoo (5):
  tests/acpi: allow SSDT changes
  acpi/ssdt: Fix aml_or() and aml_and() in if clause
  acpi/nvdimm: define macro for NVDIMM Device _DSM
  acpi/nvdimm: Implement ACPI NVDIMM Label Methods
  test/acpi/bios-tables-test: SSDT: update golden master binaries

 hw/acpi/nvdimm.c                 | 102 +++++++++++++++++++++++++++++--
 tests/data/acpi/pc/SSDT.dimmpxm  | Bin 734 -> 1893 bytes
 tests/data/acpi/q35/SSDT.dimmpxm | Bin 734 -> 1893 bytes
 3 files changed, 96 insertions(+), 6 deletions(-)

-- 
2.31.1

[PATCH v3 1/5] tests/acpi: allow SSDT changes

[Robert Hoo]

Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
 1 file changed, 2 insertions(+)

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.31.1

[PATCH v3 2/5] acpi/ssdt: Fix aml_or() and aml_and() in if clause

[Robert Hoo]

In If condition, using bitwise and/or, rather than logical and/or.

The result change in AML code:

If (((Local6 == Zero) | (Arg0 != Local0)))
==>
If (((Local6 == Zero) || (Arg0 != Local0)))

If (((ObjectType (Arg3) == 0x04) & (SizeOf (Arg3) == One)))
==>
If (((ObjectType (Arg3) == 0x04) && (SizeOf (Arg3) == One)))

Fixes: 90623ebf603 ("nvdimm acpi: check UUID")
Fixes: 4568c948066 ("nvdimm acpi: save arg3 of _DSM method")
Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/acpi/nvdimm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 31e46df0bd..201317c611 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -1037,7 +1037,7 @@ static void nvdimm_build_common_dsm(Aml *dev,
 
     uuid_invalid = aml_lnot(aml_equal(uuid, expected_uuid));
 
-    unsupport = aml_if(aml_or(unpatched, uuid_invalid, NULL));
+    unsupport = aml_if(aml_lor(unpatched, uuid_invalid));
 
     /*
      * function 0 is called to inquire what functions are supported by
@@ -1069,10 +1069,9 @@ static void nvdimm_build_common_dsm(Aml *dev,
      * in the DSM Spec.
      */
     pckg = aml_arg(3);
-    ifctx = aml_if(aml_and(aml_equal(aml_object_type(pckg),
+    ifctx = aml_if(aml_land(aml_equal(aml_object_type(pckg),
                    aml_int(4 /* Package */)) /* It is a Package? */,
-                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */,
-                   NULL));
+                   aml_equal(aml_sizeof(pckg), aml_int(1)) /* 1 element? */));
 
     pckg_index = aml_local(2);
     pckg_buf = aml_local(3);
-- 
2.31.1

[PATCH v3 3/5] acpi/nvdimm: define macro for NVDIMM Device _DSM

[Robert Hoo]

Since it will be heavily used in next patch, define macro
NVDIMM_DEVICE_DSM_UUID for "4309AC30-0D11-11E4-9191-0800200C9A66", which is
NVDIMM device specific method uuid defined in NVDIMM _DSM interface spec,
Section 3. [1]

No functional changes in this patch.

[1] https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf

Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
---
 hw/acpi/nvdimm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 201317c611..afff911c1e 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -922,6 +922,7 @@ void nvdimm_init_acpi_state(NVDIMMState *state, MemoryRegion *io,
 #define NVDIMM_DSM_RFIT_STATUS  "RSTA"
 
 #define NVDIMM_QEMU_RSVD_UUID   "648B9CF2-CDA1-4312-8AD9-49C4AF32BD62"
+#define NVDIMM_DEVICE_DSM_UUID  "4309AC30-0D11-11E4-9191-0800200C9A66"
 
 static void nvdimm_build_common_dsm(Aml *dev,
                                     NVDIMMState *nvdimm_state)
@@ -1029,8 +1030,7 @@ static void nvdimm_build_common_dsm(Aml *dev,
                /* UUID for QEMU internal use */), expected_uuid));
     aml_append(elsectx, ifctx);
     elsectx2 = aml_else();
-    aml_append(elsectx2, aml_store(
-               aml_touuid("4309AC30-0D11-11E4-9191-0800200C9A66")
+    aml_append(elsectx2, aml_store(aml_touuid(NVDIMM_DEVICE_DSM_UUID)
                /* UUID for NVDIMM Devices */, expected_uuid));
     aml_append(elsectx, elsectx2);
     aml_append(method, elsectx);
-- 
2.31.1

[PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Robert Hoo]

Recent ACPI spec [1] has defined NVDIMM Label Methods _LS{I,R,W}, which
deprecates corresponding _DSM Functions defined by PMEM _DSM Interface spec
[2].

Since the semantics of the new Label Methods are same as old _DSM
methods, the implementations here simply wrapper old ones.

ASL form diff can be found in next patch of updating golden master
binaries.

[1] ACPI Spec v6.4, 6.5.10 NVDIMM Label Methods
https://uefi.org/sites/default/files/resources/ACPI_Spec_6_4_Jan22.pdf
[2] Intel PMEM _DSM Interface Spec v2.0, 3.10 Deprecated Functions
https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf

Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
---
 hw/acpi/nvdimm.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index afff911c1e..516acfe53b 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -1243,6 +1243,7 @@ static void nvdimm_build_fit(Aml *dev)
 static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots)
 {
     uint32_t slot;
+    Aml *method, *pkg, *field, *com_call;
 
     for (slot = 0; slot < ram_slots; slot++) {
         uint32_t handle = nvdimm_slot_to_handle(slot);
@@ -1260,6 +1261,96 @@ static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots)
          */
         aml_append(nvdimm_dev, aml_name_decl("_ADR", aml_int(handle)));
 
+        /*
+         * ACPI v6.4: Section 6.5.10 NVDIMM Label Methods
+         */
+        /* _LSI */
+        method = aml_method("_LSI", 0, AML_SERIALIZED);
+        com_call = aml_call5(NVDIMM_COMMON_DSM,
+                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
+                            aml_int(1), aml_int(4), aml_int(0),
+                            aml_int(handle));
+        aml_append(method, aml_store(com_call, aml_local(0)));
+
+        aml_append(method, aml_create_dword_field(aml_local(0),
+                                                  aml_int(0), "STTS"));
+        aml_append(method, aml_create_dword_field(aml_local(0), aml_int(4),
+                                                  "SLSA"));
+        aml_append(method, aml_create_dword_field(aml_local(0), aml_int(8),
+                                                  "MAXT"));
+
+        pkg = aml_package(3);
+        aml_append(pkg, aml_name("STTS"));
+        aml_append(pkg, aml_name("SLSA"));
+        aml_append(pkg, aml_name("MAXT"));
+        aml_append(method, aml_name_decl("RET", pkg));
+        aml_append(method, aml_return(aml_name("RET")));
+
+        aml_append(nvdimm_dev, method);
+
+        /* _LSR */
+        method = aml_method("_LSR", 2, AML_SERIALIZED);
+        aml_append(method, aml_name_decl("INPT", aml_buffer(8, NULL)));
+
+        aml_append(method, aml_create_dword_field(aml_name("INPT"),
+                                                  aml_int(0), "OFST"));
+        aml_append(method, aml_create_dword_field(aml_name("INPT"),
+                                                  aml_int(4), "LEN"));
+        aml_append(method, aml_store(aml_arg(0), aml_name("OFST")));
+        aml_append(method, aml_store(aml_arg(1), aml_name("LEN")));
+
+        pkg = aml_package(1);
+        aml_append(pkg, aml_name("INPT"));
+        aml_append(method, aml_name_decl("PKG1", pkg));
+
+        com_call = aml_call5(NVDIMM_COMMON_DSM,
+                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
+                            aml_int(1), aml_int(5), aml_name("PKG1"),
+                            aml_int(handle));
+        aml_append(method, aml_store(com_call, aml_local(3)));
+        field = aml_create_dword_field(aml_local(3), aml_int(0), "STTS");
+        aml_append(method, field);
+        field = aml_create_field(aml_local(3), aml_int(32),
+                                 aml_shiftleft(aml_name("LEN"), aml_int(3)),
+                                 "LDAT");
+        aml_append(method, field);
+        aml_append(method, aml_name_decl("LSA", aml_buffer(0, NULL)));
+        aml_append(method, aml_to_buffer(aml_name("LDAT"), aml_name("LSA")));
+        pkg = aml_package(2);
+        aml_append(pkg, aml_name("STTS"));
+        aml_append(pkg, aml_name("LSA"));
+        aml_append(method, aml_name_decl("RET", pkg));
+        aml_append(method, aml_return(aml_name("RET")));
+        aml_append(nvdimm_dev, method);
+
+        /* _LSW */
+        method = aml_method("_LSW", 3, AML_SERIALIZED);
+        aml_append(method, aml_store(aml_arg(2), aml_local(2)));
+        aml_append(method, aml_name_decl("INPT", aml_buffer(8, NULL)));
+        field = aml_create_dword_field(aml_name("INPT"),
+                                                  aml_int(0), "OFST");
+        aml_append(method, field);
+        field = aml_create_dword_field(aml_name("INPT"),
+                                                  aml_int(4), "TLEN");
+        aml_append(method, field);
+        aml_append(method, aml_store(aml_arg(0), aml_name("OFST")));
+        aml_append(method, aml_store(aml_arg(1), aml_name("TLEN")));
+
+        aml_append(method, aml_concatenate(aml_name("INPT"), aml_local(2),
+                                            aml_name("INPT")));
+        pkg = aml_package(1);
+        aml_append(pkg, aml_name("INPT"));
+        aml_append(method, aml_name_decl("PKG1", pkg));
+        com_call = aml_call5(NVDIMM_COMMON_DSM,
+                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
+                            aml_int(1), aml_int(6), aml_name("PKG1"),
+                            aml_int(handle));
+        aml_append(method, aml_store(com_call, aml_local(3)));
+        field = aml_create_dword_field(aml_local(3), aml_int(0), "STTS");
+        aml_append(method, field);
+        aml_append(method, aml_return(aml_to_integer(aml_name("STTS"))));
+        aml_append(nvdimm_dev, method);
+
         nvdimm_build_device_dsm(nvdimm_dev, handle);
         aml_append(root_dev, nvdimm_dev);
     }
-- 
2.31.1

[PATCH v3 5/5] test/acpi/bios-tables-test: SSDT: update golden master binaries

[Robert Hoo]

And empty bios-tables-test-allowed-diff.h.

Diff of ASL form, cited from qtest testlog.txt:

--- /tmp/asl-0WHMR1.dsl	2022-08-30 11:38:09.406635934 +0800
+++ /tmp/asl-APDMR1.dsl	2022-08-30 11:38:09.403635663 +0800
@@ -1,30 +1,30 @@
 /*
  * Intel ACPI Component Architecture
  * AML/ASL+ Disassembler version 20180629 (64-bit version)
  * Copyright (c) 2000 - 2018 Intel Corporation
  *
  * Disassembling to symbolic ASL+ operators
  *
- * Disassembly of tests/data/acpi/pc/SSDT.dimmpxm, Tue Aug 30 11:38:09 2022
+ * Disassembly of /tmp/aml-1AEMR1, Tue Aug 30 11:38:09 2022
  *
  * Original Table Header:
  *     Signature        "SSDT"
- *     Length           0x000002DE (734)
+ *     Length           0x00000765 (1893)
  *     Revision         0x01
- *     Checksum         0x56
+ *     Checksum         0x36
  *     OEM ID           "BOCHS "
  *     OEM Table ID     "NVDIMM"
  *     OEM Revision     0x00000001 (1)
  *     Compiler ID      "BXPC"
  *     Compiler Version 0x00000001 (1)
  */
 DefinitionBlock ("", "SSDT", 1, "BOCHS ", "NVDIMM", 0x00000001)
 {
     Scope (\_SB)
     {
         Device (NVDR)
         {
             Name (_HID, "ACPI0012" /* NVDIMM Root Device */)  // _HID: Hardware ID
             Method (NCAL, 5, Serialized)
             {
                 Local6 = MEMA /* \MEMA */
@@ -49,52 +49,52 @@
                     ODAT,   32736
                 }

                 If ((Arg4 == Zero))
                 {
                     Local0 = ToUUID ("2f10e7a4-9e91-11e4-89d3-123b93f75cba")
                 }
                 ElseIf ((Arg4 == 0x00010000))
                 {
                     Local0 = ToUUID ("648b9cf2-cda1-4312-8ad9-49c4af32bd62")
                 }
                 Else
                 {
                     Local0 = ToUUID ("4309ac30-0d11-11e4-9191-0800200c9a66")
                 }

-                If (((Local6 == Zero) | (Arg0 != Local0)))
+                If (((Local6 == Zero) || (Arg0 != Local0)))
                 {
                     If ((Arg2 == Zero))
                     {
                         Return (Buffer (One)
                         {
                              0x00                                             // .
                         })
                     }

                     Return (Buffer (One)
                     {
                          0x01                                             // .
                     })
                 }

                 HDLE = Arg4
                 REVS = Arg1
                 FUNC = Arg2
-                If (((ObjectType (Arg3) == 0x04) & (SizeOf (Arg3) == One)))
+                If (((ObjectType (Arg3) == 0x04) && (SizeOf (Arg3) == One)))
                 {
                     Local2 = Arg3 [Zero]
                     Local3 = DerefOf (Local2)
                     FARG = Local3
                 }

                 NTFI = Local6
                 Local1 = (RLEN - 0x04)
                 If ((Local1 < 0x08))
                 {
                     Local2 = Zero
                     Name (TBUF, Buffer (One)
                     {
                          0x00                                             // .
                     })
                     Local7 = Buffer (Zero){}
@@ -161,45 +161,234 @@
                     Else
                     {
                         If ((Local1 == Zero))
                         {
                             Return (Local2)
                         }

                         Local3 += Local1
                         Concatenate (Local2, Local0, Local2)
                     }
                 }
             }

             Device (NV00)
             {
                 Name (_ADR, One)  // _ADR: Address
+                Method (_LSI, 0, Serialized)  // _LSI: Label Storage Information
+                {
+                    Local0 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-0800200c9a66"), One, 0x04, Zero, One)
+                    CreateDWordField (Local0, Zero, STTS)
+                    CreateDWordField (Local0, 0x04, SLSA)
+                    CreateDWordField (Local0, 0x08, MAXT)
+                    Name (RET, Package (0x03)
+                    {
+                        STTS,
+                        SLSA,
+                        MAXT
+                    })
+                    Return (RET) /* \_SB_.NVDR.NV00._LSI.RET_ */
+                }
+
+                Method (_LSR, 2, Serialized)  // _LSR: Label Storage Read
+                {
+                    Name (INPT, Buffer (0x08)
+                    {
+                         0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00   // ........
+                    })
+                    CreateDWordField (INPT, Zero, OFST)
+                    CreateDWordField (INPT, 0x04, LEN)
+                    OFST = Arg0
+                    LEN = Arg1
+                    Name (PKG1, Package (0x01)
+                    {
+                        INPT
+                    })
+                    Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-0800200c9a66"), One, 0x05, PKG1, One)
+                    CreateDWordField (Local3, Zero, STTS)
+                    CreateField (Local3, 0x20, (LEN << 0x03), LDAT)
+                    Name (LSA, Buffer (Zero){})
+                    ToBuffer (LDAT, LSA) /* \_SB_.NVDR.NV00._LSR.LSA_ */
+                    Name (RET, Package (0x02)
+                    {
+                        STTS,
+                        LSA
+                    })
+                    Return (RET) /* \_SB_.NVDR.NV00._LSR.RET_ */
+                }
+
+                Method (_LSW, 3, Serialized)  // _LSW: Label Storage Write
+                {
+                    Local2 = Arg2
+                    Name (INPT, Buffer (0x08)
+                    {
+                         0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00   // ........
+                    })
+                    CreateDWordField (INPT, Zero, OFST)
+                    CreateDWordField (INPT, 0x04, TLEN)
+                    OFST = Arg0
+                    TLEN = Arg1
+                    Concatenate (INPT, Local2, INPT) /* \_SB_.NVDR.NV00._LSW.INPT */
+                    Name (PKG1, Package (0x01)
+                    {
+                        INPT
+                    })
+                    Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-0800200c9a66"), One, 0x06, PKG1, One)
+                    CreateDWordField (Local3, Zero, STTS)
+                    Return (ToInteger (STTS))
+                }
+
                 Method (_DSM, 4, NotSerialized)  // _DSM: Device-Specific Method
                 {
                     Return (NCAL (Arg0, Arg1, Arg2, Arg3, One))
                 }
             }
	     ... // iterates in each NV devices

Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
---
 tests/data/acpi/pc/SSDT.dimmpxm             | Bin 734 -> 1893 bytes
 tests/data/acpi/q35/SSDT.dimmpxm            | Bin 734 -> 1893 bytes
 tests/qtest/bios-tables-test-allowed-diff.h |   2 --
 3 files changed, 2 deletions(-)

diff --git a/tests/data/acpi/pc/SSDT.dimmpxm b/tests/data/acpi/pc/SSDT.dimmpxm
index ac55387d57e48adb99eb738a102308688a262fb8..2316d8f959267a178b12379dbcb439a6306b0db5 100644
GIT binary patch
literal 1893
zcmdUw&ubGw6vt<?X)~Rql1(Dk`h)!sOnWGT2ig4Cc4;;VyW3jW!v4~vO?nubdhpgC
zht>#Ux>Qg^Hf4{3cfp%iPoBJaD0ufGcu?nUe@F{zMJT$5nKy6VzWvPiJ0!YZGVUZ0
z;wB2U;*>5{XG@BzvNb}eFjp_aoR&NDmR_*Tb#<BTYuK7nO2bmIuH^G$<0v<MzFL$j
z!&EJ+Qo~%W)|DEU93dpmVog#}BZ<=HS`zYn)sPj@T)PY#{8Xt@7Pa!MF3L02q9{w+
z<m#7%xt57`wMw}v)=HAG`ZW=Z`b&rkS&|Mvrmyv$?+N$cWp4PN=U>_V>%mojDFw(;
z!KY^rZuj42irx3ho0sFSUAuoF%I9AU6@}qFq1VGmOg(Mb!AQ?<plyG_4u<>|3I(2v
z297QP7+nN1+5)uT(j)(2o5cOiHb=MG2)T5_^{5BLv;k*Y0a_Awf{7i6V2WI96lP>-
zNrf0{?q1n(9lEgv$8pgX>><=?!rZ*;hE00UAv|)-EEFK8#_}NyxwKW%)p!U3FD$m&
z2y40_p`4n~WW^Z5HCZ<hU4V%~4JdR{0FKKHa!?UzWOr@ETFg)wpjnXG_M=S5K<TFK
ztT=EE>^n@27&5aq5hFT=BZn~LZd&Z)7!KxA<n87;3N)S#ZwG=8U6Y7A-0!D@L1^YK
zdydy}ZI|7ni`ChD9$QjL<8tzVRnZlE#DCH#j>HkSk8S8(o5b~sF0mP_(wu>{vSDS;
z@w#E?kcV6dxm=mBwi>}owQdMWg4somo-QO1=n+LE&Wy7TBwU0T*QY1Pm}F4*3#b7o
z<GilN#4~g=>oJ+&b>?G*nTQ#T#T0rB3yAkgNerrCy-!)djPh2Jw%p%?8m#>xD8wBg
zbYVCp^58M#@Cl%xlUIy0*9)V}p0UUt63b1V!XFa*bA#lwFo?wd)}i8=JM@G3&?4dZ
luK<Z1cr?E65q~<5_<z+S{y+5Si1}w8@fiLOQ|QCD#CMx$@elw2

delta 150
zcmaFLcaN1TIM^lR9uortW7tG4X>Nb5nD}6)_~<4#t%(LAjJ^|Hw{uC>PEKQ(G&v)I
zVKOVD5|2#v<i2b!mdWkej0~HN7+n~>Wc<Pm3^?K)U4j@z1mazSeOZ?HIXn7fWM*YE
eMmNa;WevfyTudT@sM1_a5P2hrJo98vb{PO-TPR%s

diff --git a/tests/data/acpi/q35/SSDT.dimmpxm b/tests/data/acpi/q35/SSDT.dimmpxm
index 98e6f0e3f3bb02dd419e36bdd1db9b94c728c406..cfb1f0515293f2e3630b755da26d558f43c91bc4 100644
GIT binary patch
literal 1893
zcmdUwO=uHA6vt<?X)~R)l1(Dk`aw=!O?wc*gKRe0c4;;VyW3jW!hUJeCOr&IJ$UQK
zp*4b-E)^7!P1&R1UGV1BlPB*5ui{1Upw8R0l@`>BP;?J7Z{ECp`<wrNNOZku+({tB
zT`5$hDLq%2Eh?(Y)(D}(Tup*GCAa4-y<nN^>N4Bcur;L=M?|Tn<n!YbC_8<&B8lY@
zs+Lr-Zmw2pN|j5F5)x;zCaS2OL@9GE33<L^$V$#!y9gWnRIyyjX{A{`C(l%*oGfaS
zt6vi4S~}X*%B4!KS`>}duZdvHUqV#KkW}~~b+!9^Pq_aseZ&7e|H|%N4=(#l!+;zW
ze0uiacJFOk>bzIpyqtLL+P!m8KL27!5=QofZWp86@YA{-jCTDx+V&UhV90NxP~f>}
z;OG*7(M2GmO+X7SJn|p5NZj9SadfMNkV`jQk9t5%>u{zOpe1o9xVGaEOp%L?!i)?p
zsSrbr-Ag;JLpQeII4;_PJ%ri~m>YN9utD!Rgh!5<fdb@ISw4gzo3e_!8V^C>g~etI
zVNEwglvNX&tQbSMD(eQK3oucr0fmkWz;Wq84k{vz?2e6Cix~<7Gz*5e{U{SMP`YV5
zD-N6k`wmkhhRlpjauFTFkwX}=H!b#Y3<q;5@^<qa1sczWw*$e4u1Ull?DtZ^AT)EA
zJ;!Uiw#)9(`O0iQk1esNaano5D(eb9<Uei{N8*U<V;g$MCUL#2i)_ZKIA>t3WLW7l
zylxme<RKSAE?Xk3twyj?sTo3&V0MwCrvnKDdPEV4Gvlm%2^S&8wdu)mCK=TI0%`!t
zIIrU|@eJL}cuXdEo%vX1CSnF-F@+w(0^&X?i9t22`;-ODC~qZc)BQcD!P*~yLfi&I
z7lsoe4<11dp8yIve#JO*y)ee?8ISBCvFzjt{2{SFH%LAWgGlUe9g<Gnp&!f#770gx
l1xRe)qlvyp{K-7x|5cCp|InjD=AU`QWB5NDMjyW=z5^xL@elw2

delta 150
zcmaFLcaN1TIM^lR9uortquWF-X>Nb5nD}6)_~<4#t%(LAjJ^|Hw{uC>PEKQ(G&v)I
zVKOVD5|2#v<i2b!mdWkej0~HN7+n~>Wc<Pm3^?K)U4j@z1mazSeOZ?HIXn7fWM*YE
eMmNa;WevfyTudT@sM1_a5P2hrJo98vb{PO!+bB%{

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index eb8bae1407..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,3 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/pc/SSDT.dimmpxm",
-"tests/data/acpi/q35/SSDT.dimmpxm",
-- 
2.31.1

Re: [PATCH v3 3/5] acpi/nvdimm: define macro for NVDIMM Device _DSM

[Igor Mammedov]

On Thu,  1 Sep 2022 11:27:19 +0800
Robert Hoo <robert.hu@linux.intel.com> wrote:

> Since it will be heavily used in next patch, define macro
> NVDIMM_DEVICE_DSM_UUID for "4309AC30-0D11-11E4-9191-0800200C9A66", which is
> NVDIMM device specific method uuid defined in NVDIMM _DSM interface spec,
> Section 3. [1]
> 
> No functional changes in this patch.
> 
> [1] https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf
> 
> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/acpi/nvdimm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 201317c611..afff911c1e 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -922,6 +922,7 @@ void nvdimm_init_acpi_state(NVDIMMState *state, MemoryRegion *io,
>  #define NVDIMM_DSM_RFIT_STATUS  "RSTA"
>  
>  #define NVDIMM_QEMU_RSVD_UUID   "648B9CF2-CDA1-4312-8AD9-49C4AF32BD62"
> +#define NVDIMM_DEVICE_DSM_UUID  "4309AC30-0D11-11E4-9191-0800200C9A66"
>  
>  static void nvdimm_build_common_dsm(Aml *dev,
>                                      NVDIMMState *nvdimm_state)
> @@ -1029,8 +1030,7 @@ static void nvdimm_build_common_dsm(Aml *dev,
>                 /* UUID for QEMU internal use */), expected_uuid));
>      aml_append(elsectx, ifctx);
>      elsectx2 = aml_else();
> -    aml_append(elsectx2, aml_store(
> -               aml_touuid("4309AC30-0D11-11E4-9191-0800200C9A66")
> +    aml_append(elsectx2, aml_store(aml_touuid(NVDIMM_DEVICE_DSM_UUID)
>                 /* UUID for NVDIMM Devices */, expected_uuid));
>      aml_append(elsectx, elsectx2);
>      aml_append(method, elsectx);

Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Igor Mammedov]

On Thu,  1 Sep 2022 11:27:20 +0800
Robert Hoo <robert.hu@linux.intel.com> wrote:

> Recent ACPI spec [1] has defined NVDIMM Label Methods _LS{I,R,W}, which
> deprecates corresponding _DSM Functions defined by PMEM _DSM Interface spec
> [2].
> 
> Since the semantics of the new Label Methods are same as old _DSM
> methods, the implementations here simply wrapper old ones.
> 
> ASL form diff can be found in next patch of updating golden master
> binaries.
> 
> [1] ACPI Spec v6.4, 6.5.10 NVDIMM Label Methods
> https://uefi.org/sites/default/files/resources/ACPI_Spec_6_4_Jan22.pdf
> [2] Intel PMEM _DSM Interface Spec v2.0, 3.10 Deprecated Functions
> https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf
> 
> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>

looks more or less fine except of excessive use of named variables
which creates global scope variables.

I'd suggest to store temporary buffers/packages in LocalX variales,
you should be able to do that for everything modulo aml_create_dword_field().

see an example below

> ---
>  hw/acpi/nvdimm.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
> 
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index afff911c1e..516acfe53b 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -1243,6 +1243,7 @@ static void nvdimm_build_fit(Aml *dev)
>  static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots)
>  {
>      uint32_t slot;
> +    Aml *method, *pkg, *field, *com_call;
>  
>      for (slot = 0; slot < ram_slots; slot++) {
>          uint32_t handle = nvdimm_slot_to_handle(slot);
> @@ -1260,6 +1261,96 @@ static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t ram_slots)
>           */
>          aml_append(nvdimm_dev, aml_name_decl("_ADR", aml_int(handle)));
>  
> +        /*
> +         * ACPI v6.4: Section 6.5.10 NVDIMM Label Methods
> +         */
> +        /* _LSI */
> +        method = aml_method("_LSI", 0, AML_SERIALIZED);
> +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> +                            aml_int(1), aml_int(4), aml_int(0),
> +                            aml_int(handle));
> +        aml_append(method, aml_store(com_call, aml_local(0)));
> +
> +        aml_append(method, aml_create_dword_field(aml_local(0),
> +                                                  aml_int(0), "STTS"));
> +        aml_append(method, aml_create_dword_field(aml_local(0), aml_int(4),
> +                                                  "SLSA"));
> +        aml_append(method, aml_create_dword_field(aml_local(0), aml_int(8),
> +                                                  "MAXT"));
> +
> +        pkg = aml_package(3);
> +        aml_append(pkg, aml_name("STTS"));
> +        aml_append(pkg, aml_name("SLSA"));
> +        aml_append(pkg, aml_name("MAXT"));
> +        aml_append(method, aml_name_decl("RET", pkg));
ex: put it in local instead of named variable and return that
the same applies to other named temporary named variables.

> +        aml_append(method, aml_return(aml_name("RET")));
> +
> +        aml_append(nvdimm_dev, method);
> +
> +        /* _LSR */
> +        method = aml_method("_LSR", 2, AML_SERIALIZED);
> +        aml_append(method, aml_name_decl("INPT", aml_buffer(8, NULL)));
> +
> +        aml_append(method, aml_create_dword_field(aml_name("INPT"),
> +                                                  aml_int(0), "OFST"));
> +        aml_append(method, aml_create_dword_field(aml_name("INPT"),
> +                                                  aml_int(4), "LEN"));
> +        aml_append(method, aml_store(aml_arg(0), aml_name("OFST")));
> +        aml_append(method, aml_store(aml_arg(1), aml_name("LEN")));
> +
> +        pkg = aml_package(1);
> +        aml_append(pkg, aml_name("INPT"));
> +        aml_append(method, aml_name_decl("PKG1", pkg));
> +
> +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> +                            aml_int(1), aml_int(5), aml_name("PKG1"),
> +                            aml_int(handle));
> +        aml_append(method, aml_store(com_call, aml_local(3)));
> +        field = aml_create_dword_field(aml_local(3), aml_int(0), "STTS");
> +        aml_append(method, field);
> +        field = aml_create_field(aml_local(3), aml_int(32),
> +                                 aml_shiftleft(aml_name("LEN"), aml_int(3)),
> +                                 "LDAT");
> +        aml_append(method, field);
> +        aml_append(method, aml_name_decl("LSA", aml_buffer(0, NULL)));
> +        aml_append(method, aml_to_buffer(aml_name("LDAT"), aml_name("LSA")));
> +        pkg = aml_package(2);
> +        aml_append(pkg, aml_name("STTS"));
> +        aml_append(pkg, aml_name("LSA"));
> +        aml_append(method, aml_name_decl("RET", pkg));
> +        aml_append(method, aml_return(aml_name("RET")));
> +        aml_append(nvdimm_dev, method);
> +
> +        /* _LSW */
> +        method = aml_method("_LSW", 3, AML_SERIALIZED);
> +        aml_append(method, aml_store(aml_arg(2), aml_local(2)));
> +        aml_append(method, aml_name_decl("INPT", aml_buffer(8, NULL)));
> +        field = aml_create_dword_field(aml_name("INPT"),
> +                                                  aml_int(0), "OFST");
> +        aml_append(method, field);
> +        field = aml_create_dword_field(aml_name("INPT"),
> +                                                  aml_int(4), "TLEN");
> +        aml_append(method, field);
> +        aml_append(method, aml_store(aml_arg(0), aml_name("OFST")));
> +        aml_append(method, aml_store(aml_arg(1), aml_name("TLEN")));
> +
> +        aml_append(method, aml_concatenate(aml_name("INPT"), aml_local(2),
> +                                            aml_name("INPT")));
> +        pkg = aml_package(1);
> +        aml_append(pkg, aml_name("INPT"));
> +        aml_append(method, aml_name_decl("PKG1", pkg));
> +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> +                            aml_int(1), aml_int(6), aml_name("PKG1"),
> +                            aml_int(handle));
> +        aml_append(method, aml_store(com_call, aml_local(3)));
> +        field = aml_create_dword_field(aml_local(3), aml_int(0), "STTS");
> +        aml_append(method, field);
> +        aml_append(method, aml_return(aml_to_integer(aml_name("STTS"))));
why do you need explicitly convert DWORD field to integer?
it should be fine to return STTS directly (implicit conversion should take care of the rest)

> +        aml_append(nvdimm_dev, method);
> +
>          nvdimm_build_device_dsm(nvdimm_dev, handle);
>          aml_append(root_dev, nvdimm_dev);
>      }

Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Robert Hoo]

On Fri, 2022-09-09 at 15:39 +0200, Igor Mammedov wrote:
> On Thu,  1 Sep 2022 11:27:20 +0800
> Robert Hoo <robert.hu@linux.intel.com> wrote:
> 
> > Recent ACPI spec [1] has defined NVDIMM Label Methods _LS{I,R,W},
> > which
> > deprecates corresponding _DSM Functions defined by PMEM _DSM
> > Interface spec
> > [2].
> > 
> > Since the semantics of the new Label Methods are same as old _DSM
> > methods, the implementations here simply wrapper old ones.
> > 
> > ASL form diff can be found in next patch of updating golden master
> > binaries.
> > 
> > [1] ACPI Spec v6.4, 6.5.10 NVDIMM Label Methods
> > https://uefi.org/sites/default/files/resources/ACPI_Spec_6_4_Jan22.pdf
> > [2] Intel PMEM _DSM Interface Spec v2.0, 3.10 Deprecated Functions
> > https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf
> > 
> > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> > Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
> 
> looks more or less fine except of excessive use of named variables
> which creates global scope variables.
> 
> I'd suggest to store temporary buffers/packages in LocalX variales,
> you should be able to do that for everything modulo
> aml_create_dword_field().
> 
> see an example below
> 
> > ---
> >  hw/acpi/nvdimm.c | 91
> > ++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 91 insertions(+)
> > 
> > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> > index afff911c1e..516acfe53b 100644
> > --- a/hw/acpi/nvdimm.c
> > +++ b/hw/acpi/nvdimm.c
> > @@ -1243,6 +1243,7 @@ static void nvdimm_build_fit(Aml *dev)
> >  static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t
> > ram_slots)
> >  {
> >      uint32_t slot;
> > +    Aml *method, *pkg, *field, *com_call;
> >  
> >      for (slot = 0; slot < ram_slots; slot++) {
> >          uint32_t handle = nvdimm_slot_to_handle(slot);
> > @@ -1260,6 +1261,96 @@ static void nvdimm_build_nvdimm_devices(Aml
> > *root_dev, uint32_t ram_slots)
> >           */
> >          aml_append(nvdimm_dev, aml_name_decl("_ADR",
> > aml_int(handle)));
> >  
> > +        /*
> > +         * ACPI v6.4: Section 6.5.10 NVDIMM Label Methods
> > +         */
> > +        /* _LSI */
> > +        method = aml_method("_LSI", 0, AML_SERIALIZED);
> > +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> > +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> > +                            aml_int(1), aml_int(4), aml_int(0),
> > +                            aml_int(handle));
> > +        aml_append(method, aml_store(com_call, aml_local(0)));
> > +
> > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > +                                                  aml_int(0),
> > "STTS"));
> > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > aml_int(4),
> > +                                                  "SLSA"));
> > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > aml_int(8),
> > +                                                  "MAXT"));
> > +
> > +        pkg = aml_package(3);
> > +        aml_append(pkg, aml_name("STTS"));
> > +        aml_append(pkg, aml_name("SLSA"));
> > +        aml_append(pkg, aml_name("MAXT"));
> > +        aml_append(method, aml_name_decl("RET", pkg));
> 
> ex: put it in local instead of named variable and return that
> the same applies to other named temporary named variables.

Could you help provide the example in form of ASL?
Thanks.
> 
> > +        aml_append(method, aml_return(aml_name("RET")));
> > +
> > +        aml_append(nvdimm_dev, method);
> > +
> > +        /* _LSR */
> > +        method = aml_method("_LSR", 2, AML_SERIALIZED);
> > +        aml_append(method, aml_name_decl("INPT", aml_buffer(8,
> > NULL)));
> > +
> > +        aml_append(method,
> > aml_create_dword_field(aml_name("INPT"),
> > +                                                  aml_int(0),
> > "OFST"));
> > +        aml_append(method,
> > aml_create_dword_field(aml_name("INPT"),
> > +                                                  aml_int(4),
> > "LEN"));
> > +        aml_append(method, aml_store(aml_arg(0),
> > aml_name("OFST")));
> > +        aml_append(method, aml_store(aml_arg(1),
> > aml_name("LEN")));
> > +
> > +        pkg = aml_package(1);
> > +        aml_append(pkg, aml_name("INPT"));
> > +        aml_append(method, aml_name_decl("PKG1", pkg));
> > +
> > +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> > +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> > +                            aml_int(1), aml_int(5),
> > aml_name("PKG1"),
> > +                            aml_int(handle));
> > +        aml_append(method, aml_store(com_call, aml_local(3)));
> > +        field = aml_create_dword_field(aml_local(3), aml_int(0),
> > "STTS");
> > +        aml_append(method, field);
> > +        field = aml_create_field(aml_local(3), aml_int(32),
> > +                                 aml_shiftleft(aml_name("LEN"),
> > aml_int(3)),
> > +                                 "LDAT");
> > +        aml_append(method, field);
> > +        aml_append(method, aml_name_decl("LSA", aml_buffer(0,
> > NULL)));
> > +        aml_append(method, aml_to_buffer(aml_name("LDAT"),
> > aml_name("LSA")));
> > +        pkg = aml_package(2);
> > +        aml_append(pkg, aml_name("STTS"));
> > +        aml_append(pkg, aml_name("LSA"));
> > +        aml_append(method, aml_name_decl("RET", pkg));
> > +        aml_append(method, aml_return(aml_name("RET")));
> > +        aml_append(nvdimm_dev, method);
> > +
> > +        /* _LSW */
> > +        method = aml_method("_LSW", 3, AML_SERIALIZED);
> > +        aml_append(method, aml_store(aml_arg(2), aml_local(2)));
> > +        aml_append(method, aml_name_decl("INPT", aml_buffer(8,
> > NULL)));
> > +        field = aml_create_dword_field(aml_name("INPT"),
> > +                                                  aml_int(0),
> > "OFST");
> > +        aml_append(method, field);
> > +        field = aml_create_dword_field(aml_name("INPT"),
> > +                                                  aml_int(4),
> > "TLEN");
> > +        aml_append(method, field);
> > +        aml_append(method, aml_store(aml_arg(0),
> > aml_name("OFST")));
> > +        aml_append(method, aml_store(aml_arg(1),
> > aml_name("TLEN")));
> > +
> > +        aml_append(method, aml_concatenate(aml_name("INPT"),
> > aml_local(2),
> > +                                            aml_name("INPT")));
> > +        pkg = aml_package(1);
> > +        aml_append(pkg, aml_name("INPT"));
> > +        aml_append(method, aml_name_decl("PKG1", pkg));
> > +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> > +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> > +                            aml_int(1), aml_int(6),
> > aml_name("PKG1"),
> > +                            aml_int(handle));
> > +        aml_append(method, aml_store(com_call, aml_local(3)));
> > +        field = aml_create_dword_field(aml_local(3), aml_int(0),
> > "STTS");
> > +        aml_append(method, field);
> > +        aml_append(method,
> > aml_return(aml_to_integer(aml_name("STTS"))));
> 
> why do you need explicitly convert DWORD field to integer?
> it should be fine to return STTS directly (implicit conversion should
> take care of the rest)
> 
> > +        aml_append(nvdimm_dev, method);
> > +
> >          nvdimm_build_device_dsm(nvdimm_dev, handle);
> >          aml_append(root_dev, nvdimm_dev);
> >      }
> 
> 

Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Igor Mammedov]

On Fri, 09 Sep 2022 22:02:34 +0800
Robert Hoo <robert.hu@linux.intel.com> wrote:

> On Fri, 2022-09-09 at 15:39 +0200, Igor Mammedov wrote:
> > On Thu,  1 Sep 2022 11:27:20 +0800
> > Robert Hoo <robert.hu@linux.intel.com> wrote:
> >   
> > > Recent ACPI spec [1] has defined NVDIMM Label Methods _LS{I,R,W},
> > > which
> > > deprecates corresponding _DSM Functions defined by PMEM _DSM
> > > Interface spec
> > > [2].
> > > 
> > > Since the semantics of the new Label Methods are same as old _DSM
> > > methods, the implementations here simply wrapper old ones.
> > > 
> > > ASL form diff can be found in next patch of updating golden master
> > > binaries.
> > > 
> > > [1] ACPI Spec v6.4, 6.5.10 NVDIMM Label Methods
> > > https://uefi.org/sites/default/files/resources/ACPI_Spec_6_4_Jan22.pdf
> > > [2] Intel PMEM _DSM Interface Spec v2.0, 3.10 Deprecated Functions
> > > https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf
> > > 
> > > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> > > Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>  
> > 
> > looks more or less fine except of excessive use of named variables
> > which creates global scope variables.
> > 
> > I'd suggest to store temporary buffers/packages in LocalX variales,
> > you should be able to do that for everything modulo
> > aml_create_dword_field().
> > 
> > see an example below
> >   
> > > ---
> > >  hw/acpi/nvdimm.c | 91
> > > ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 91 insertions(+)
> > > 
> > > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> > > index afff911c1e..516acfe53b 100644
> > > --- a/hw/acpi/nvdimm.c
> > > +++ b/hw/acpi/nvdimm.c
> > > @@ -1243,6 +1243,7 @@ static void nvdimm_build_fit(Aml *dev)
> > >  static void nvdimm_build_nvdimm_devices(Aml *root_dev, uint32_t
> > > ram_slots)
> > >  {
> > >      uint32_t slot;
> > > +    Aml *method, *pkg, *field, *com_call;
> > >  
> > >      for (slot = 0; slot < ram_slots; slot++) {
> > >          uint32_t handle = nvdimm_slot_to_handle(slot);
> > > @@ -1260,6 +1261,96 @@ static void nvdimm_build_nvdimm_devices(Aml
> > > *root_dev, uint32_t ram_slots)
> > >           */
> > >          aml_append(nvdimm_dev, aml_name_decl("_ADR",
> > > aml_int(handle)));
> > >  
> > > +        /*
> > > +         * ACPI v6.4: Section 6.5.10 NVDIMM Label Methods
> > > +         */
> > > +        /* _LSI */
> > > +        method = aml_method("_LSI", 0, AML_SERIALIZED);
> > > +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> > > +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> > > +                            aml_int(1), aml_int(4), aml_int(0),
> > > +                            aml_int(handle));
> > > +        aml_append(method, aml_store(com_call, aml_local(0)));
> > > +
> > > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > > +                                                  aml_int(0),
> > > "STTS"));
> > > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > > aml_int(4),
> > > +                                                  "SLSA"));
> > > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > > aml_int(8),
> > > +                                                  "MAXT"));
> > > +
> > > +        pkg = aml_package(3);
> > > +        aml_append(pkg, aml_name("STTS"));
> > > +        aml_append(pkg, aml_name("SLSA"));
> > > +        aml_append(pkg, aml_name("MAXT"));
> > > +        aml_append(method, aml_name_decl("RET", pkg));  
> > 
> > ex: put it in local instead of named variable and return that
> > the same applies to other named temporary named variables.  
> 
> Could you help provide the example in form of ASL?

see hw/i386/acpi-build.c: build_prt() or aml_pci_device_dsm()

> Thanks.
> >   
> > > +        aml_append(method, aml_return(aml_name("RET")));
> > > +
> > > +        aml_append(nvdimm_dev, method);
> > > +
> > > +        /* _LSR */
> > > +        method = aml_method("_LSR", 2, AML_SERIALIZED);
> > > +        aml_append(method, aml_name_decl("INPT", aml_buffer(8,
> > > NULL)));
> > > +
> > > +        aml_append(method,
> > > aml_create_dword_field(aml_name("INPT"),
> > > +                                                  aml_int(0),
> > > "OFST"));
> > > +        aml_append(method,
> > > aml_create_dword_field(aml_name("INPT"),
> > > +                                                  aml_int(4),
> > > "LEN"));
> > > +        aml_append(method, aml_store(aml_arg(0),
> > > aml_name("OFST")));
> > > +        aml_append(method, aml_store(aml_arg(1),
> > > aml_name("LEN")));
> > > +
> > > +        pkg = aml_package(1);
> > > +        aml_append(pkg, aml_name("INPT"));
> > > +        aml_append(method, aml_name_decl("PKG1", pkg));
> > > +
> > > +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> > > +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> > > +                            aml_int(1), aml_int(5),
> > > aml_name("PKG1"),
> > > +                            aml_int(handle));
> > > +        aml_append(method, aml_store(com_call, aml_local(3)));
> > > +        field = aml_create_dword_field(aml_local(3), aml_int(0),
> > > "STTS");
> > > +        aml_append(method, field);
> > > +        field = aml_create_field(aml_local(3), aml_int(32),
> > > +                                 aml_shiftleft(aml_name("LEN"),
> > > aml_int(3)),
> > > +                                 "LDAT");
> > > +        aml_append(method, field);
> > > +        aml_append(method, aml_name_decl("LSA", aml_buffer(0,
> > > NULL)));
> > > +        aml_append(method, aml_to_buffer(aml_name("LDAT"),
> > > aml_name("LSA")));
> > > +        pkg = aml_package(2);
> > > +        aml_append(pkg, aml_name("STTS"));
> > > +        aml_append(pkg, aml_name("LSA"));
> > > +        aml_append(method, aml_name_decl("RET", pkg));
> > > +        aml_append(method, aml_return(aml_name("RET")));
> > > +        aml_append(nvdimm_dev, method);
> > > +
> > > +        /* _LSW */
> > > +        method = aml_method("_LSW", 3, AML_SERIALIZED);
> > > +        aml_append(method, aml_store(aml_arg(2), aml_local(2)));
> > > +        aml_append(method, aml_name_decl("INPT", aml_buffer(8,
> > > NULL)));
> > > +        field = aml_create_dword_field(aml_name("INPT"),
> > > +                                                  aml_int(0),
> > > "OFST");
> > > +        aml_append(method, field);
> > > +        field = aml_create_dword_field(aml_name("INPT"),
> > > +                                                  aml_int(4),
> > > "TLEN");
> > > +        aml_append(method, field);
> > > +        aml_append(method, aml_store(aml_arg(0),
> > > aml_name("OFST")));
> > > +        aml_append(method, aml_store(aml_arg(1),
> > > aml_name("TLEN")));
> > > +
> > > +        aml_append(method, aml_concatenate(aml_name("INPT"),
> > > aml_local(2),
> > > +                                            aml_name("INPT")));
> > > +        pkg = aml_package(1);
> > > +        aml_append(pkg, aml_name("INPT"));
> > > +        aml_append(method, aml_name_decl("PKG1", pkg));
> > > +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> > > +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> > > +                            aml_int(1), aml_int(6),
> > > aml_name("PKG1"),
> > > +                            aml_int(handle));
> > > +        aml_append(method, aml_store(com_call, aml_local(3)));
> > > +        field = aml_create_dword_field(aml_local(3), aml_int(0),
> > > "STTS");
> > > +        aml_append(method, field);
> > > +        aml_append(method,
> > > aml_return(aml_to_integer(aml_name("STTS"))));  
> > 
> > why do you need explicitly convert DWORD field to integer?
> > it should be fine to return STTS directly (implicit conversion should
> > take care of the rest)
> >   
> > > +        aml_append(nvdimm_dev, method);
> > > +
> > >          nvdimm_build_device_dsm(nvdimm_dev, handle);
> > >          aml_append(root_dev, nvdimm_dev);
> > >      }  
> > 
> >   
> 

Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Robert Hoo]

On Fri, 2022-09-09 at 15:39 +0200, Igor Mammedov wrote:
...
> looks more or less fine except of excessive use of named variables
> which creates global scope variables.
> 
> I'd suggest to store temporary buffers/packages in LocalX variales,
> you should be able to do that for everything modulo
> aml_create_dword_field().
> 
> see an example below
> 
...
> >  
> > +        /*
> > +         * ACPI v6.4: Section 6.5.10 NVDIMM Label Methods
> > +         */
> > +        /* _LSI */
> > +        method = aml_method("_LSI", 0, AML_SERIALIZED);
> > +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> > +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> > +                            aml_int(1), aml_int(4), aml_int(0),
> > +                            aml_int(handle));
> > +        aml_append(method, aml_store(com_call, aml_local(0)));
> > +
> > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > +                                                  aml_int(0),
> > "STTS"));
> > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > aml_int(4),
> > +                                                  "SLSA"));
> > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > aml_int(8),
> > +                                                  "MAXT"));
> > +
> > +        pkg = aml_package(3);
> > +        aml_append(pkg, aml_name("STTS"));
> > +        aml_append(pkg, aml_name("SLSA"));
> > +        aml_append(pkg, aml_name("MAXT"));
> > +        aml_append(method, aml_name_decl("RET", pkg));
> 
> ex: put it in local instead of named variable and return that
> the same applies to other named temporary named variables.
> 
Fine, get your point now.
In ASL it will look like this:
                    Local1 = Package (0x3) {STTS, SLSA, MAXT}
                    Return (Local1)

But as for 
                    CreateDWordField (Local0, Zero, STTS)  // Status
                    CreateDWordField (Local0, 0x04, SLSA)  // SizeofLSA
                    CreateDWordField (Local0, 0x08, MAXT)  // Max Trans

I cannot figure out how to substitute with LocalX. Can you shed more
light?

CreateQWordFieldTerm :=
CreateQWordField (
SourceBuffer, // TermArg => Buffer
ByteIndex, // TermArg => Integer
QWordFieldName // NameString
)
NameString :=
<RootChar NamePath> | <ParentPrefixChar PrefixPath NamePath> |
NonEmptyNamePath

> > +        aml_append(method, aml_return(aml_name("RET")));
> > +
...
> > +        field = aml_create_dword_field(aml_local(3), aml_int(0),
> > "STTS");
> > +        aml_append(method, field);
> > +        aml_append(method,
> > aml_return(aml_to_integer(aml_name("STTS"))));
> 
> why do you need explicitly convert DWORD field to integer?
> it should be fine to return STTS directly (implicit conversion should
> take care of the rest)

Explicit convert eases my anxiety on uncertainty. ;)

> 
> > +        aml_append(nvdimm_dev, method);
> > +
...

Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Igor Mammedov]

On Fri, 16 Sep 2022 10:27:08 +0800
Robert Hoo <robert.hu@linux.intel.com> wrote:

> On Fri, 2022-09-09 at 15:39 +0200, Igor Mammedov wrote:
> ...
> > looks more or less fine except of excessive use of named variables
> > which creates global scope variables.
> > 
> > I'd suggest to store temporary buffers/packages in LocalX variales,
> > you should be able to do that for everything modulo
> > aml_create_dword_field().
> > 
> > see an example below
> >   
> ...
> > >  
> > > +        /*
> > > +         * ACPI v6.4: Section 6.5.10 NVDIMM Label Methods
> > > +         */
> > > +        /* _LSI */
> > > +        method = aml_method("_LSI", 0, AML_SERIALIZED);
> > > +        com_call = aml_call5(NVDIMM_COMMON_DSM,
> > > +                            aml_touuid(NVDIMM_DEVICE_DSM_UUID),
> > > +                            aml_int(1), aml_int(4), aml_int(0),
> > > +                            aml_int(handle));
> > > +        aml_append(method, aml_store(com_call, aml_local(0)));
> > > +
> > > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > > +                                                  aml_int(0),
> > > "STTS"));
> > > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > > aml_int(4),
> > > +                                                  "SLSA"));
> > > +        aml_append(method, aml_create_dword_field(aml_local(0),
> > > aml_int(8),
> > > +                                                  "MAXT"));
> > > +
> > > +        pkg = aml_package(3);
> > > +        aml_append(pkg, aml_name("STTS"));
> > > +        aml_append(pkg, aml_name("SLSA"));
> > > +        aml_append(pkg, aml_name("MAXT"));
> > > +        aml_append(method, aml_name_decl("RET", pkg));  
> > 
> > ex: put it in local instead of named variable and return that
> > the same applies to other named temporary named variables.
> >   
> Fine, get your point now.
> In ASL it will look like this:
>                     Local1 = Package (0x3) {STTS, SLSA, MAXT}
>                     Return (Local1)


> 
> But as for 
>                     CreateDWordField (Local0, Zero, STTS)  // Status
>                     CreateDWordField (Local0, 0x04, SLSA)  // SizeofLSA
>                     CreateDWordField (Local0, 0x08, MAXT)  // Max Trans
> 
> I cannot figure out how to substitute with LocalX. Can you shed more
> light?

Leave this as is, there is no way to make it anonymous/local with FooField.

(well one might try to use Index and copy field's bytes into a buffer and
then explicitly convert to Integer, but that's a rather convoluted way
around limitation so I'd not go this route)

> 
> CreateQWordFieldTerm :=
> CreateQWordField (
> SourceBuffer, // TermArg => Buffer
> ByteIndex, // TermArg => Integer
> QWordFieldName // NameString
> )
> NameString :=
> <RootChar NamePath> | <ParentPrefixChar PrefixPath NamePath> |
> NonEmptyNamePath
> 
> > > +        aml_append(method, aml_return(aml_name("RET")));
> > > +  
> ...
> > > +        field = aml_create_dword_field(aml_local(3), aml_int(0),
> > > "STTS");
> > > +        aml_append(method, field);
> > > +        aml_append(method,
> > > aml_return(aml_to_integer(aml_name("STTS"))));  
> > 
> > why do you need explicitly convert DWORD field to integer?
> > it should be fine to return STTS directly (implicit conversion should
> > take care of the rest)  
> 
> Explicit convert eases my anxiety on uncertainty. ;)
> 
> >   
> > > +        aml_append(nvdimm_dev, method);
> > > +  
> ...
> 

Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Robert Hoo]

On Fri, 2022-09-16 at 09:37 +0200, Igor Mammedov wrote:

> > Fine, get your point now.
> > In ASL it will look like this:
> >                     Local1 = Package (0x3) {STTS, SLSA, MAXT}
> >                     Return (Local1)
> 
> 
> > 
> > But as for 
> >                     CreateDWordField (Local0, Zero, STTS)  //
> > Status
> >                     CreateDWordField (Local0, 0x04, SLSA)  //
> > SizeofLSA
> >                     CreateDWordField (Local0, 0x08, MAXT)  // Max
> > Trans
> > 
> > I cannot figure out how to substitute with LocalX. Can you shed
> > more
> > light?
> 
> Leave this as is, there is no way to make it anonymous/local with
> FooField.
> 
> (well one might try to use Index and copy field's bytes into a buffer
> and
> then explicitly convert to Integer, but that's a rather convoluted
> way
> around limitation so I'd not go this route)
> 
OK, pls. take a look, how about this?

Method (_LSI, 0, Serialized)  // _LSI: Label Storage Information
{   
    Local0 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-0800200c9a66"),
0x02, 0x04, Zero, One)    // Buffer
    CreateDWordField (Local0, Zero, STTS)  // Status
    CreateDWordField (Local0, 0x04, SLSA)  // Size of LSA
    CreateDWordField (Local0, 0x08, MAXT)  // Max Transfer Size
    Local1 = Package (0x3) {STTS, SLSA, MAXT}
    Return (Local1)
}

Method (_LSR, 2, Serialized)  // _LSR: Label Storage Read
{
    Name (INPT, Buffer(8) {})
    CreateDWordField (INPT, Zero, OFST);
    CreateDWordField (INPT, 4, LEN);
    OFST = Arg0
    LEN = Arg1
    Local0 = Package (0x01) {INPT}
    Local3 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-0800200c9a66"),
0x02, 0x05, Local0, One)
    CreateDWordField (Local3, Zero, STTS)
    CreateField (Local3, 32, LEN << 3, LDAT)
    Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
    Return (Local1)
}

Method (_LSW, 3, Serialized)  // _LSW: Label Storage Write
{
    Local2 = Arg2
    Name (INPT, Buffer(8) {})
    CreateDWordField (INPT, Zero, OFST);
    CreateDWordField (INPT, 4, TLEN);
    OFST = Arg0
    TLEN = Arg1
    Concatenate(INPT, Local2, INPT)
    Local0 = Package (0x01)
    {
        INPT
    }
    Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-0800200c9a66"),
0x02, 0x06, Local0, One)
    CreateDWordField (Local3, 0, STTS)
    Return (STTS)
}

Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Igor Mammedov]

On Fri, 16 Sep 2022 21:15:35 +0800
Robert Hoo <robert.hu@linux.intel.com> wrote:

> On Fri, 2022-09-16 at 09:37 +0200, Igor Mammedov wrote:
> 
> > > Fine, get your point now.
> > > In ASL it will look like this:
> > >                     Local1 = Package (0x3) {STTS, SLSA, MAXT}
> > >                     Return (Local1)  
> > 
> >   
> > > 
> > > But as for 
> > >                     CreateDWordField (Local0, Zero, STTS)  //
> > > Status
> > >                     CreateDWordField (Local0, 0x04, SLSA)  //
> > > SizeofLSA
> > >                     CreateDWordField (Local0, 0x08, MAXT)  // Max
> > > Trans
> > > 
> > > I cannot figure out how to substitute with LocalX. Can you shed
> > > more
> > > light?  
> > 
> > Leave this as is, there is no way to make it anonymous/local with
> > FooField.
> > 
> > (well one might try to use Index and copy field's bytes into a buffer
> > and
> > then explicitly convert to Integer, but that's a rather convoluted
> > way
> > around limitation so I'd not go this route)
> >   
> OK, pls. take a look, how about this?
> 
> Method (_LSI, 0, Serialized)  // _LSI: Label Storage Information
> {   
>     Local0 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-0800200c9a66"),
> 0x02, 0x04, Zero, One)    // Buffer
>     CreateDWordField (Local0, Zero, STTS)  // Status
>     CreateDWordField (Local0, 0x04, SLSA)  // Size of LSA
>     CreateDWordField (Local0, 0x08, MAXT)  // Max Transfer Size
>     Local1 = Package (0x3) {STTS, SLSA, MAXT}
>     Return (Local1)
> }
> 
> Method (_LSR, 2, Serialized)  // _LSR: Label Storage Read
> {
>     Name (INPT, Buffer(8) {})
>     CreateDWordField (INPT, Zero, OFST);
>     CreateDWordField (INPT, 4, LEN);
why do you have to create and use INPT, wouldn't local be enough to keep the buffer?

>     OFST = Arg0
>     LEN = Arg1
>     Local0 = Package (0x01) {INPT}
>     Local3 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-0800200c9a66"),
> 0x02, 0x05, Local0, One)
>     CreateDWordField (Local3, Zero, STTS)
>     CreateField (Local3, 32, LEN << 3, LDAT)
>     Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
>     Return (Local1)
> }
> 
> Method (_LSW, 3, Serialized)  // _LSW: Label Storage Write
> {
>     Local2 = Arg2
>     Name (INPT, Buffer(8) {})
ditto

>     CreateDWordField (INPT, Zero, OFST);
>     CreateDWordField (INPT, 4, TLEN);
>     OFST = Arg0
>     TLEN = Arg1
>     Concatenate(INPT, Local2, INPT)
>     Local0 = Package (0x01)
>     {
>         INPT
>     }
>     Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-0800200c9a66"),
> 0x02, 0x06, Local0, One)
>     CreateDWordField (Local3, 0, STTS)
>     Return (STTS)
> }
> 
> 
> 

Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Robert Hoo]

On Tue, 2022-09-20 at 11:13 +0200, Igor Mammedov wrote:
> On Fri, 16 Sep 2022 21:15:35 +0800
> Robert Hoo <robert.hu@linux.intel.com> wrote:
> 
> > On Fri, 2022-09-16 at 09:37 +0200, Igor Mammedov wrote:
> > 
> > > > Fine, get your point now.
> > > > In ASL it will look like this:
> > > >                     Local1 = Package (0x3) {STTS, SLSA, MAXT}
> > > >                     Return (Local1)  
> > > 
> > >   
> > > > 
> > > > But as for 
> > > >                     CreateDWordField (Local0, Zero, STTS)  //
> > > > Status
> > > >                     CreateDWordField (Local0, 0x04, SLSA)  //
> > > > SizeofLSA
> > > >                     CreateDWordField (Local0, 0x08, MAXT)  //
> > > > Max
> > > > Trans
> > > > 
> > > > I cannot figure out how to substitute with LocalX. Can you shed
> > > > more
> > > > light?  
> > > 
> > > Leave this as is, there is no way to make it anonymous/local with
> > > FooField.
> > > 
> > > (well one might try to use Index and copy field's bytes into a
> > > buffer
> > > and
> > > then explicitly convert to Integer, but that's a rather
> > > convoluted
> > > way
> > > around limitation so I'd not go this route)
> > >   
> > 
> > OK, pls. take a look, how about this?
> > 
> > Method (_LSI, 0, Serialized)  // _LSI: Label Storage Information
> > {   
> >     Local0 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-0800200c9a66"),
> > 0x02, 0x04, Zero, One)    // Buffer
> >     CreateDWordField (Local0, Zero, STTS)  // Status
> >     CreateDWordField (Local0, 0x04, SLSA)  // Size of LSA
> >     CreateDWordField (Local0, 0x08, MAXT)  // Max Transfer Size
> >     Local1 = Package (0x3) {STTS, SLSA, MAXT}
> >     Return (Local1)
> > }
> > 
> > Method (_LSR, 2, Serialized)  // _LSR: Label Storage Read
> > {
> >     Name (INPT, Buffer(8) {})
> >     CreateDWordField (INPT, Zero, OFST);
> >     CreateDWordField (INPT, 4, LEN);
> 
> why do you have to create and use INPT, wouldn't local be enough to
> keep the buffer?

If substitute INPT with LocalX, then later
Local0 = Package (0x01) {LocalX} isn't accepted.

PackageElement :=
DataObject | NameString
> 
> >     OFST = Arg0
> >     LEN = Arg1
> >     Local0 = Package (0x01) {INPT}
> >     Local3 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-0800200c9a66"),
> > 0x02, 0x05, Local0, One)
> >     CreateDWordField (Local3, Zero, STTS)
> >     CreateField (Local3, 32, LEN << 3, LDAT)
> >     Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> >     Return (Local1)
> > }
> > 
> > Method (_LSW, 3, Serialized)  // _LSW: Label Storage Write
> > {
> >     Local2 = Arg2
> >     Name (INPT, Buffer(8) {})
> 
> ditto
> 
> >     CreateDWordField (INPT, Zero, OFST);
> >     CreateDWordField (INPT, 4, TLEN);
> >     OFST = Arg0
> >     TLEN = Arg1
> >     Concatenate(INPT, Local2, INPT)
> >     Local0 = Package (0x01)
> >     {
> >         INPT
> >     }
> >     Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-0800200c9a66"),
> > 0x02, 0x06, Local0, One)
> >     CreateDWordField (Local3, 0, STTS)
> >     Return (STTS)
> > }

Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Igor Mammedov]

On Tue, 20 Sep 2022 20:28:31 +0800
Robert Hoo <robert.hu@linux.intel.com> wrote:

> On Tue, 2022-09-20 at 11:13 +0200, Igor Mammedov wrote:
> > On Fri, 16 Sep 2022 21:15:35 +0800
> > Robert Hoo <robert.hu@linux.intel.com> wrote:
> >   
> > > On Fri, 2022-09-16 at 09:37 +0200, Igor Mammedov wrote:
> > >   
> > > > > Fine, get your point now.
> > > > > In ASL it will look like this:
> > > > >                     Local1 = Package (0x3) {STTS, SLSA, MAXT}
> > > > >                     Return (Local1)    
> > > > 
> > > >     
> > > > > 
> > > > > But as for 
> > > > >                     CreateDWordField (Local0, Zero, STTS)  //
> > > > > Status
> > > > >                     CreateDWordField (Local0, 0x04, SLSA)  //
> > > > > SizeofLSA
> > > > >                     CreateDWordField (Local0, 0x08, MAXT)  //
> > > > > Max
> > > > > Trans
> > > > > 
> > > > > I cannot figure out how to substitute with LocalX. Can you shed
> > > > > more
> > > > > light?    
> > > > 
> > > > Leave this as is, there is no way to make it anonymous/local with
> > > > FooField.
> > > > 
> > > > (well one might try to use Index and copy field's bytes into a
> > > > buffer
> > > > and
> > > > then explicitly convert to Integer, but that's a rather
> > > > convoluted
> > > > way
> > > > around limitation so I'd not go this route)
> > > >     
> > > 
> > > OK, pls. take a look, how about this?
> > > 
> > > Method (_LSI, 0, Serialized)  // _LSI: Label Storage Information
> > > {   
> > >     Local0 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-0800200c9a66"),
> > > 0x02, 0x04, Zero, One)    // Buffer
> > >     CreateDWordField (Local0, Zero, STTS)  // Status
> > >     CreateDWordField (Local0, 0x04, SLSA)  // Size of LSA
> > >     CreateDWordField (Local0, 0x08, MAXT)  // Max Transfer Size
> > >     Local1 = Package (0x3) {STTS, SLSA, MAXT}
> > >     Return (Local1)
> > > }
> > > 
> > > Method (_LSR, 2, Serialized)  // _LSR: Label Storage Read
> > > {
> > >     Name (INPT, Buffer(8) {})
> > >     CreateDWordField (INPT, Zero, OFST);
> > >     CreateDWordField (INPT, 4, LEN);  
> > 
> > why do you have to create and use INPT, wouldn't local be enough to
> > keep the buffer?  
> 
> If substitute INPT with LocalX, then later
> Local0 = Package (0x01) {LocalX} isn't accepted.
> 
> PackageElement :=
> DataObject | NameString

ok, then respin series and lets get it some testing.

BTW:
it looks like Windows Server starting from v2019 has support for
NVDIMM-P devices which came with 'Optane DC Persistent Memory Modules'
but it fails to recognize NVDIMMs in QEMU (complaining something about
wrong target). Perhaps you can reach someone with Optane/ACPI
expertise within your org and try to fix QEMU side.

> >   
> > >     OFST = Arg0
> > >     LEN = Arg1
> > >     Local0 = Package (0x01) {INPT}
> > >     Local3 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-0800200c9a66"),
> > > 0x02, 0x05, Local0, One)
> > >     CreateDWordField (Local3, Zero, STTS)
> > >     CreateField (Local3, 32, LEN << 3, LDAT)
> > >     Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> > >     Return (Local1)
> > > }
> > > 
> > > Method (_LSW, 3, Serialized)  // _LSW: Label Storage Write
> > > {
> > >     Local2 = Arg2
> > >     Name (INPT, Buffer(8) {})  
> > 
> > ditto
> >   
> > >     CreateDWordField (INPT, Zero, OFST);
> > >     CreateDWordField (INPT, 4, TLEN);
> > >     OFST = Arg0
> > >     TLEN = Arg1
> > >     Concatenate(INPT, Local2, INPT)
> > >     Local0 = Package (0x01)
> > >     {
> > >         INPT
> > >     }
> > >     Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-0800200c9a66"),
> > > 0x02, 0x06, Local0, One)
> > >     CreateDWordField (Local3, 0, STTS)
> > >     Return (STTS)
> > > }  
> 
> 

Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods

[Robert Hoo]

On Wed, 2022-09-21 at 15:29 +0200, Igor Mammedov wrote:
> On Tue, 20 Sep 2022 20:28:31 +0800
> Robert Hoo <robert.hu@linux.intel.com> wrote:
> 
> > On Tue, 2022-09-20 at 11:13 +0200, Igor Mammedov wrote:
> > > On Fri, 16 Sep 2022 21:15:35 +0800
> > > Robert Hoo <robert.hu@linux.intel.com> wrote:
> > >   
> > > > On Fri, 2022-09-16 at 09:37 +0200, Igor Mammedov wrote:
> > > >   
> > > > > > Fine, get your point now.
> > > > > > In ASL it will look like this:
> > > > > >                     Local1 = Package (0x3) {STTS, SLSA,
> > > > > > MAXT}
> > > > > >                     Return (Local1)    
> > > > > 
> > > > >     
> > > > > > 
> > > > > > But as for 
> > > > > >                     CreateDWordField (Local0, Zero,
> > > > > > STTS)  //
> > > > > > Status
> > > > > >                     CreateDWordField (Local0, 0x04,
> > > > > > SLSA)  //
> > > > > > SizeofLSA
> > > > > >                     CreateDWordField (Local0, 0x08,
> > > > > > MAXT)  //
> > > > > > Max
> > > > > > Trans
> > > > > > 
> > > > > > I cannot figure out how to substitute with LocalX. Can you
> > > > > > shed
> > > > > > more
> > > > > > light?    
> > > > > 
> > > > > Leave this as is, there is no way to make it anonymous/local
> > > > > with
> > > > > FooField.
> > > > > 
> > > > > (well one might try to use Index and copy field's bytes into
> > > > > a
> > > > > buffer
> > > > > and
> > > > > then explicitly convert to Integer, but that's a rather
> > > > > convoluted
> > > > > way
> > > > > around limitation so I'd not go this route)
> > > > >     
> > > > 
> > > > OK, pls. take a look, how about this?
> > > > 
> > > > Method (_LSI, 0, Serialized)  // _LSI: Label Storage
> > > > Information
> > > > {   
> > > >     Local0 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-
> > > > 0800200c9a66"),
> > > > 0x02, 0x04, Zero, One)    // Buffer
> > > >     CreateDWordField (Local0, Zero, STTS)  // Status
> > > >     CreateDWordField (Local0, 0x04, SLSA)  // Size of LSA
> > > >     CreateDWordField (Local0, 0x08, MAXT)  // Max Transfer Size
> > > >     Local1 = Package (0x3) {STTS, SLSA, MAXT}
> > > >     Return (Local1)
> > > > }
> > > > 
> > > > Method (_LSR, 2, Serialized)  // _LSR: Label Storage Read
> > > > {
> > > >     Name (INPT, Buffer(8) {})
> > > >     CreateDWordField (INPT, Zero, OFST);
> > > >     CreateDWordField (INPT, 4, LEN);  
> > > 
> > > why do you have to create and use INPT, wouldn't local be enough
> > > to
> > > keep the buffer?  
> > 
> > If substitute INPT with LocalX, then later
> > Local0 = Package (0x01) {LocalX} isn't accepted.
> > 
> > PackageElement :=
> > DataObject | NameString
> 
> ok, then respin series and lets get it some testing.

Sure. I'd also like it to go through your tests, though I had done some
ordinary tests like guest create/delete/init-labels on vNVDIMM.

> 
> BTW:
> it looks like Windows Server starting from v2019 has support for
> NVDIMM-P devices which came with 'Optane DC Persistent Memory
> Modules'
> but it fails to recognize NVDIMMs in QEMU (complaining something
> about
> wrong target). Perhaps you can reach someone with Optane/ACPI
> expertise within your org and try to fix QEMU side.

Yes, it's a known gap there. I will try that once I had some time and
resource.
> 
> > >   
> > > >     OFST = Arg0
> > > >     LEN = Arg1
> > > >     Local0 = Package (0x01) {INPT}
> > > >     Local3 = NCAL (ToUUID("4309ac30-0d11-11e4-9191-
> > > > 0800200c9a66"),
> > > > 0x02, 0x05, Local0, One)
> > > >     CreateDWordField (Local3, Zero, STTS)
> > > >     CreateField (Local3, 32, LEN << 3, LDAT)
> > > >     Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> > > >     Return (Local1)
> > > > }
> > > > 
> > > > Method (_LSW, 3, Serialized)  // _LSW: Label Storage Write
> > > > {
> > > >     Local2 = Arg2
> > > >     Name (INPT, Buffer(8) {})  
> > > 
> > > ditto
> > >   
> > > >     CreateDWordField (INPT, Zero, OFST);
> > > >     CreateDWordField (INPT, 4, TLEN);
> > > >     OFST = Arg0
> > > >     TLEN = Arg1
> > > >     Concatenate(INPT, Local2, INPT)
> > > >     Local0 = Package (0x01)
> > > >     {
> > > >         INPT
> > > >     }
> > > >     Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-
> > > > 0800200c9a66"),
> > > > 0x02, 0x06, Local0, One)
> > > >     CreateDWordField (Local3, 0, STTS)
> > > >     Return (STTS)
> > > > }  
> > 
> > 
> 
>