mbox series

[v4,0/5] Support ACPI NVDIMM Label Methods

Message ID 20220922122155.1326543-1-robert.hu@linux.intel.com
Headers show
Series Support ACPI NVDIMM Label Methods | expand

Message

Robert Hoo Sept. 22, 2022, 12:21 p.m. UTC
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:
v3 --> v4:
Use more LocalX in ASL functions, instead of global scope variables.

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                 | 106 +++++++++++++++++++++++++++++--
 tests/data/acpi/pc/SSDT.dimmpxm  | Bin 734 -> 1815 bytes
 tests/data/acpi/q35/SSDT.dimmpxm | Bin 734 -> 1815 bytes
 3 files changed, 100 insertions(+), 6 deletions(-)


base-commit: 6338c30111d596d955e6bc933a82184a0b910c43

Comments

Robert Hoo Sept. 22, 2022, 12:29 p.m. UTC | #1
On Thu, 2022-09-22 at 20:21 +0800, Robert Hoo wrote:
> And empty bios-tables-test-allowed-diff.h.
> 
> Diff of ASL form, from qtest testlog.txt:
> 
> --- /tmp/asl-RFWZS1.dsl	2022-09-22 18:25:06.191519589 +0800
> +++ /tmp/asl-B1ZZS1.dsl	2022-09-22 18:25:06.187519182 +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, Thu Sep 22
> 18:25:06 2022
> + * Disassembly of /tmp/aml-YYZZS1, Thu Sep 22 18:25:06 2022
>   *
>   * Original Table Header:
>   *     Signature        "SSDT"
> - *     Length           0x000002DE (734)
> + *     Length           0x00000717 (1815)
>   *     Revision         0x01
> - *     Checksum         0x56
> + *     Checksum         0xBC
>   *     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)
> +                    Local1 = Package (0x03)
> +                        {
> +                            STTS,
> +                            SLSA,
> +                            MAXT
> +                        }
> +                    Return (Local1)
> +                }
> +
> +                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
> +                    Local0 = Package (0x01)
> +                        {
> +                            INPT
> +                        }
> +                    Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-
> 0800200c9a66"), One, 0x05, Local0, One)
> +                    CreateDWordField (Local3, Zero, STTS)
> +                    CreateField (Local3, 0x20, (LEN << 0x03), LDAT)
> +                    Name (LSA, Buffer (Zero){})
> +                    ToBuffer (LDAT, LSA) /*
> \_SB_.NVDR.NV00._LSR.LSA_ */
> +                    Local1 = Package (0x02)
> +                        {
> +                            STTS,
> +                            LSA
> +                        }
Hi Igor,

Here is a little different from original proposal 
https://lore.kernel.org/qemu-devel/80b09055416c790922c7c3db60d2ba865792d1b0.camel@linux.intel.com/

   Local1 = Package (0x2) {STTS, toBuffer(LDAT)}

Because in my test, Linux guest complains:

[    3.884656] ACPI Error: AE_SUPPORT, Expressions within package
elements are not supported (20220331/dspkginit-172)
[    3.887104] ACPI Error: Aborting method \_SB.NVDR.NV00._LSR due to
previous error (AE_SUPPORT) (20220331/psparse-531)


So I have to move toBuffer() out of Package{} and name LSA to hold the
buffer. If you have better idea, pls. let me know.

> +                    Return (Local1)
> +                }
> +
> +                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 */
> +                    Local0 = Package (0x01)
> +                        {
> +                            INPT
> +                        }
> +                    Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-
> 0800200c9a66"), One, 0x06, Local0, One)
> +                    CreateDWordField (Local3, Zero, STTS)
> +                    Return (STTS) /* \_SB_.NVDR.NV00._LSW.STTS */
> +                }
> +
> (iterates in each NV)
> 
> Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> ---
>  tests/data/acpi/pc/SSDT.dimmpxm             | Bin 734 -> 1815 bytes
>  tests/data/acpi/q35/SSDT.dimmpxm            | Bin 734 -> 1815 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..70f133412f5e0aa128ab210245a
> 8de7304eeb843 100644
> GIT binary patch
> literal 1815
> zcmdUwyKmD_6vnUPv~g}y6emHgc**|(X$OSF0FILox3Lr1ZmHx-examI3c8|YVC!RO
> z2@)c;%774ZDvwC)2sTzGCN_pj>?}wOz&%bMqC!xRK#<|wbI(0K`Q7hx6kRVFqX~qV
> z7sa|%)dh8?Br6KtBZP{x4GGpv_2!(V7cFzGeuJKCoK=-eBcjxh3x)9sl%G1ON@8t<
> zC}l-#nk#BUt~04IjN>%dL<KcdC}Xaspw6mBMHbA}GjPCGOSQ6~m1lIJGObENMbxgY
> zd`g(B+2~ZOl~ti$5{;G5iQtsKhzOs<nect)eDBFFfA>xHlK*k;x!u1QobwmcfE+b^
> zczo}A|8-XCzLj4+n|SHk{n4mic$$>>kzKym<B*Vk)U<=Kp5H`U{=6L|{Wc1DmWcvG
> z76FVb02yfmT5$S-f4_s{{ziu(n;nE)vhI4s17gyIJ1qk(jyu7HZ3lA%xtvj)uE0pb
> z$53nM?6&KW^-Z{ri#Fj5p`{kAt=n$cB6l3jBFD@@19IxL9zw`xtdg$8LlAg=q1{28
> zrW+#4D+#S48%eHS(G5iAVIj~13LO=IVY0&vbVM52T^rF6(*yzx3({MDR0%04*|42u
> z2kyc74pk$D%$$vdh>qe^LJ0ZG7X5M#F6I*C?GzXSG@cDl2fPncQ;69=?`MKx80Oyc
> z9B;|BU2{zuQ)dbV&Js%+lfN=#)pVIV;6G{<gX4%9U>kbZ#&Nx-i*)4_an>N&6Rd6+
> zI@DnAgic;g(t#T0WVK=NDa_GVIQn#<fIx{T!*ObvwI|*}lvAOg$NmA!kj;2qj|yh!
> zX3nG1z=PDg8a0li5EfPCA#5NgSsa5-$boD!LCLPANZb86oIwZg!$H0TWG)1949wv}
> z%n%5UzDSI@Rs|ErBNK2eFCN1M9Qzd;CjYDrIQIKKO#MY44mk%#@ZbKTacs|tiGdUB
> c@tk1)B`4Vb#EApW?>oVA@DG+o@4h6y0>WY85C8xG
> 
> delta 135
> zcmbQvcaN1TIM^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
> dMmNa;WevfyTuhS-Sw(n20!9!4=E=X=WB}s5Bn$um
> 
> diff --git a/tests/data/acpi/q35/SSDT.dimmpxm
> b/tests/data/acpi/q35/SSDT.dimmpxm
> index
> 98e6f0e3f3bb02dd419e36bdd1db9b94c728c406..9ea4e0d0ceaa8a5cbd706afb6d4
> 9de853fafe654 100644
> GIT binary patch
> literal 1815
> zcmdUw%WD%+6vppl(q?j#N+yZ4_+tJ8(=J4Cp_55srp-*k%rq9JFfU2kq{_wCg}Xi$
> ztr5g@s0$I9lvx(s3+~*ya^=3@R@?|K)O)5cETUCVG>dc3J@?GX?|$Ee=z7T*O(4YV
> z6zft|7u04+RusBN2o+}<60Di(?O97NTIOo~7CqNEt16d9M5!Sc3gZ(fKXqD_#M%f^
> z%8J-Bm(_+`XHsJr$7!yK3TmZL##~83omGvBESM|j;DD``YGpwyH+7*htx7^g)UGdo
> zN|-Cz=v1qfRiR!IjpfgY;Ecb32%p25@LlF&|Jg2o|4sIa|8e$(J-8fP@E6j695sA+
> z`rzi^byn)Vm0vxdc<I`M(WqY7NlN_4F5mBSNJu|v*}+)fZ=p?p&JL1(2ZcP#M1dg-
> z07mA4jC24kIQz(d*u`;wy~~h|E<!F@b3Nh#F=@e_mVg$=o#4`zgE@j+&L~b-U?kyV
> zsJ(rD%XP@w23*HQ8*qluVjI@>T{mo#dk$uiW9Fa%IdxhOA>=bwNmt?_2s}66=^{?k
> z4H4y)gjSJ_Bv-HK1|oB?5a<Dgjtjsr*<l7cB8}{xjp&GJ0s)2v=}kYXgcOvl+s={$
> z_uzbosu4qG&c;GSM{z14g#1;DemMsha|!ac3k(4o&xWT1-iN6v#2lOtGQltmbMJVL
> zx9HlgxhjpR%|d~*#FED3uMJr>UFHt?j~m6{IAZp&4ZUaMxL(smx^jv*W034ARyPbC
> zYOr@gCod=IKn-)U+A#PO=IARNeR@zpphT46IJL~$7jHhwsZh{k|A1x4X1tz91v7Lr
> z=TT|kLF!$N8plxxi>mS%HjtAnjzK5nKsK46WH(Zz<Nh4Zpo0(KAYTMB7Xmo}=I{|_
> z2n5GpB*t8=f(hf12{@J)Pv8QM{fZ5ff7S*Z`+Xm#{-O^@oO?#_-~OU;Y~P8AJtx?c
> bIl=x*PO$%p6NjANcY@{MA1saDe@T1=QL*6=
> 
> delta 135
> zcmbQvcaN1TIM^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
> dMmNa;WevfyTuhS-Sw(n20!9!4=E=X=WB}VOBm4jW
> 
> 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",
Igor Mammedov Sept. 26, 2022, 1:22 p.m. UTC | #2
On Thu, 22 Sep 2022 20:29:12 +0800
Robert Hoo <robert.hu@linux.intel.com> wrote:

> On Thu, 2022-09-22 at 20:21 +0800, Robert Hoo wrote:
> > And empty bios-tables-test-allowed-diff.h.
> > 
> > Diff of ASL form, from qtest testlog.txt:
> > 
> > --- /tmp/asl-RFWZS1.dsl	2022-09-22 18:25:06.191519589 +0800
> > +++ /tmp/asl-B1ZZS1.dsl	2022-09-22 18:25:06.187519182 +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, Thu Sep 22
> > 18:25:06 2022
> > + * Disassembly of /tmp/aml-YYZZS1, Thu Sep 22 18:25:06 2022
> >   *
> >   * Original Table Header:
> >   *     Signature        "SSDT"
> > - *     Length           0x000002DE (734)
> > + *     Length           0x00000717 (1815)
> >   *     Revision         0x01
> > - *     Checksum         0x56
> > + *     Checksum         0xBC
> >   *     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)
> > +                    Local1 = Package (0x03)
> > +                        {
> > +                            STTS,
> > +                            SLSA,
> > +                            MAXT
> > +                        }
> > +                    Return (Local1)
> > +                }
> > +
> > +                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
> > +                    Local0 = Package (0x01)
> > +                        {
> > +                            INPT
> > +                        }
> > +                    Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-
> > 0800200c9a66"), One, 0x05, Local0, One)
> > +                    CreateDWordField (Local3, Zero, STTS)
> > +                    CreateField (Local3, 0x20, (LEN << 0x03), LDAT)
> > +                    Name (LSA, Buffer (Zero){})
> > +                    ToBuffer (LDAT, LSA) /*
> > \_SB_.NVDR.NV00._LSR.LSA_ */
> > +                    Local1 = Package (0x02)
> > +                        {
> > +                            STTS,
> > +                            LSA
> > +                        }  
> Hi Igor,
> 
> Here is a little different from original proposal 
> https://lore.kernel.org/qemu-devel/80b09055416c790922c7c3db60d2ba865792d1b0.camel@linux.intel.com/
> 
>    Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> 
> Because in my test, Linux guest complains:
> 
> [    3.884656] ACPI Error: AE_SUPPORT, Expressions within package
> elements are not supported (20220331/dspkginit-172)
> [    3.887104] ACPI Error: Aborting method \_SB.NVDR.NV00._LSR due to
> previous error (AE_SUPPORT) (20220331/psparse-531)
> 
> 
> So I have to move toBuffer() out of Package{} and name LSA to hold the
> buffer. If you have better idea, pls. let me know.

Would something like following work?

LocalX =  Buffer (Zero){}
LocalY = Package (0x01) { LocalX }

> 
> > +                    Return (Local1)
> > +                }
> > +
> > +                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 */
> > +                    Local0 = Package (0x01)
> > +                        {
> > +                            INPT
> > +                        }
> > +                    Local3 = NCAL (ToUUID ("4309ac30-0d11-11e4-9191-
> > 0800200c9a66"), One, 0x06, Local0, One)
> > +                    CreateDWordField (Local3, Zero, STTS)
> > +                    Return (STTS) /* \_SB_.NVDR.NV00._LSW.STTS */
> > +                }
> > +
> > (iterates in each NV)
> > 
> > Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
> > ---
> >  tests/data/acpi/pc/SSDT.dimmpxm             | Bin 734 -> 1815 bytes
> >  tests/data/acpi/q35/SSDT.dimmpxm            | Bin 734 -> 1815 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..70f133412f5e0aa128ab210245a
> > 8de7304eeb843 100644
> > GIT binary patch
> > literal 1815
> > zcmdUwyKmD_6vnUPv~g}y6emHgc**|(X$OSF0FILox3Lr1ZmHx-examI3c8|YVC!RO  
> > z2@)c;%774ZDvwC)2sTzGCN_pj>?}wOz&%bMqC!xRK#<|wbI(0K`Q7hx6kRVFqX~qV  
> > z7sa|%)dh8?Br6KtBZP{x4GGpv_2!(V7cFzGeuJKCoK=-eBcjxh3x)9sl%G1ON@8t<
> > zC}l-#nk#BUt~04IjN>%dL<KcdC}Xaspw6mBMHbA}GjPCGOSQ6~m1lIJGObENMbxgY
> > zd`g(B+2~ZOl~ti$5{;G5iQtsKhzOs<nect)eDBFFfA>xHlK*k;x!u1QobwmcfE+b^
> > zczo}A|8-XCzLj4+n|SHk{n4mic$$>>kzKym<B*Vk)U<=Kp5H`U{=6L|{Wc1DmWcvG
> > z76FVb02yfmT5$S-f4_s{{ziu(n;nE)vhI4s17gyIJ1qk(jyu7HZ3lA%xtvj)uE0pb
> > z$53nM?6&KW^-Z{ri#Fj5p`{kAt=n$cB6l3jBFD@@19IxL9zw`xtdg$8LlAg=q1{28
> > zrW+#4D+#S48%eHS(G5iAVIj~13LO=IVY0&vbVM52T^rF6(*yzx3({MDR0%04*|42u  
> > z2kyc74pk$D%$$vdh>qe^LJ0ZG7X5M#F6I*C?GzXSG@cDl2fPncQ;69=?`MKx80Oyc  
> > z9B;|BU2{zuQ)dbV&Js%+lfN=#)pVIV;6G{<gX4%9U>kbZ#&Nx-i*)4_an>N&6Rd6+
> > zI@DnAgic;g(t#T0WVK=NDa_GVIQn#<fIx{T!*ObvwI|*}lvAOg$NmA!kj;2qj|yh!
> > zX3nG1z=PDg8a0li5EfPCA#5NgSsa5-$boD!LCLPANZb86oIwZg!$H0TWG)1949wv}
> > z%n%5UzDSI@Rs|ErBNK2eFCN1M9Qzd;CjYDrIQIKKO#MY44mk%#@ZbKTacs|tiGdUB  
> > c@tk1)B`4Vb#EApW?>oVA@DG+o@4h6y0>WY85C8xG  
> > 
> > delta 135  
> > zcmbQvcaN1TIM^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
> > dMmNa;WevfyTuhS-Sw(n20!9!4=E=X=WB}s5Bn$um
> > 
> > diff --git a/tests/data/acpi/q35/SSDT.dimmpxm
> > b/tests/data/acpi/q35/SSDT.dimmpxm
> > index
> > 98e6f0e3f3bb02dd419e36bdd1db9b94c728c406..9ea4e0d0ceaa8a5cbd706afb6d4
> > 9de853fafe654 100644
> > GIT binary patch
> > literal 1815
> > zcmdUw%WD%+6vppl(q?j#N+yZ4_+tJ8(=J4Cp_55srp-*k%rq9JFfU2kq{_wCg}Xi$  
> > ztr5g@s0$I9lvx(s3+~*ya^=3@R@?|K)O)5cETUCVG>dc3J@?GX?|$Ee=z7T*O(4YV  
> > z6zft|7u04+RusBN2o+}<60Di(?O97NTIOo~7CqNEt16d9M5!Sc3gZ(fKXqD_#M%f^
> > z%8J-Bm(_+`XHsJr$7!yK3TmZL##~83omGvBESM|j;DD``YGpwyH+7*htx7^g)UGdo
> > zN|-Cz=v1qfRiR!IjpfgY;Ecb32%p25@LlF&|Jg2o|4sIa|8e$(J-8fP@E6j695sA+
> > z`rzi^byn)Vm0vxdc<I`M(WqY7NlN_4F5mBSNJu|v*}+)fZ=p?p&JL1(2ZcP#M1dg-
> > z07mA4jC24kIQz(d*u`;wy~~h|E<!F@b3Nh#F=@e_mVg$=o#4`zgE@j+&L~b-U?kyV  
> > zsJ(rD%XP@w23*HQ8*qluVjI@>T{mo#dk$uiW9Fa%IdxhOA>=bwNmt?_2s}66=^{?k  
> > z4H4y)gjSJ_Bv-HK1|oB?5a<Dgjtjsr*<l7cB8}{xjp&GJ0s)2v=}kYXgcOvl+s={$
> > z_uzbosu4qG&c;GSM{z14g#1;DemMsha|!ac3k(4o&xWT1-iN6v#2lOtGQltmbMJVL  
> > zx9HlgxhjpR%|d~*#FED3uMJr>UFHt?j~m6{IAZp&4ZUaMxL(smx^jv*W034ARyPbC  
> > zYOr@gCod=IKn-)U+A#PO=IARNeR@zpphT46IJL~$7jHhwsZh{k|A1x4X1tz91v7Lr  
> > z=TT|kLF!$N8plxxi>mS%HjtAnjzK5nKsK46WH(Zz<Nh4Zpo0(KAYTMB7Xmo}=I{|_  
> > z2n5GpB*t8=f(hf12{@J)Pv8QM{fZ5ff7S*Z`+Xm#{-O^@oO?#_-~OU;Y~P8AJtx?c
> > bIl=x*PO$%p6NjANcY@{MA1saDe@T1=QL*6=
> > 
> > delta 135
> > zcmbQvcaN1TIM^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
> > dMmNa;WevfyTuhS-Sw(n20!9!4=E=X=WB}VOBm4jW
> > 
> > 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",  
>
Robert Hoo Sept. 27, 2022, 12:30 a.m. UTC | #3
On Mon, 2022-09-26 at 15:22 +0200, Igor Mammedov wrote:
> > > 0800200c9a66"), One, 0x05, Local0, One)
> > > +                    CreateDWordField (Local3, Zero, STTS)
> > > +                    CreateField (Local3, 0x20, (LEN << 0x03),
> > > LDAT)
> > > +                    Name (LSA, Buffer (Zero){})
> > > +                    ToBuffer (LDAT, LSA) /*
> > > \_SB_.NVDR.NV00._LSR.LSA_ */
> > > +                    Local1 = Package (0x02)
> > > +                        {
> > > +                            STTS,
> > > +                            LSA
> > > +                        }  
> > 
> > Hi Igor,
> > 
> > Here is a little different from original proposal 
> > https://lore.kernel.org/qemu-devel/80b09055416c790922c7c3db60d2ba865792d1b0.camel@linux.intel.com/
> > 
> >    Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> > 
> > Because in my test, Linux guest complains:
> > 
> > [    3.884656] ACPI Error: AE_SUPPORT, Expressions within package
> > elements are not supported (20220331/dspkginit-172)
> > [    3.887104] ACPI Error: Aborting method \_SB.NVDR.NV00._LSR due
> > to
> > previous error (AE_SUPPORT) (20220331/psparse-531)
> > 
> > 
> > So I have to move toBuffer() out of Package{} and name LSA to hold
> > the
> > buffer. If you have better idea, pls. let me know.
> 
> Would something like following work?
> 
> LocalX =  Buffer (Zero){}
> LocalY = Package (0x01) { LocalX }


No, Package{} doesn't accept LocalX as elements.

PackageTerm :=
Package (
NumElements // Nothing | ByteConstExpr | TermArg => Integer
) {PackageList} => Package

PackageList :=
Nothing | <PackageElement PackageListTail>

PackageElement :=
DataObject | NameString
Robert Hoo Oct. 7, 2022, 1:27 p.m. UTC | #4
Ping...
On Tue, 2022-09-27 at 08:30 +0800, Robert Hoo wrote:
> On Mon, 2022-09-26 at 15:22 +0200, Igor Mammedov wrote:
> > > > 0800200c9a66"), One, 0x05, Local0, One)
> > > > +                    CreateDWordField (Local3, Zero, STTS)
> > > > +                    CreateField (Local3, 0x20, (LEN << 0x03),
> > > > LDAT)
> > > > +                    Name (LSA, Buffer (Zero){})
> > > > +                    ToBuffer (LDAT, LSA) /*
> > > > \_SB_.NVDR.NV00._LSR.LSA_ */
> > > > +                    Local1 = Package (0x02)
> > > > +                        {
> > > > +                            STTS,
> > > > +                            LSA
> > > > +                        }  
> > > 
> > > Hi Igor,
> > > 
> > > Here is a little different from original proposal 
> > > https://lore.kernel.org/qemu-devel/80b09055416c790922c7c3db60d2ba865792d1b0.camel@linux.intel.com/
> > > 
> > >    Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> > > 
> > > Because in my test, Linux guest complains:
> > > 
> > > [    3.884656] ACPI Error: AE_SUPPORT, Expressions within package
> > > elements are not supported (20220331/dspkginit-172)
> > > [    3.887104] ACPI Error: Aborting method \_SB.NVDR.NV00._LSR
> > > due
> > > to
> > > previous error (AE_SUPPORT) (20220331/psparse-531)
> > > 
> > > 
> > > So I have to move toBuffer() out of Package{} and name LSA to
> > > hold
> > > the
> > > buffer. If you have better idea, pls. let me know.
> > 
> > Would something like following work?
> > 
> > LocalX =  Buffer (Zero){}
> > LocalY = Package (0x01) { LocalX }
> 
> 
> No, Package{} doesn't accept LocalX as elements.
> 
> PackageTerm :=
> Package (
> NumElements // Nothing | ByteConstExpr | TermArg => Integer
> ) {PackageList} => Package
> 
> PackageList :=
> Nothing | <PackageElement PackageListTail>
> 
> PackageElement :=
> DataObject | NameString
Robert Hoo Oct. 20, 2022, 12:48 a.m. UTC | #5
Ping...
On Fri, 2022-10-07 at 21:27 +0800, Robert Hoo wrote:
> Ping...
> On Tue, 2022-09-27 at 08:30 +0800, Robert Hoo wrote:
> > On Mon, 2022-09-26 at 15:22 +0200, Igor Mammedov wrote:
> > > > > 0800200c9a66"), One, 0x05, Local0, One)
> > > > > +                    CreateDWordField (Local3, Zero, STTS)
> > > > > +                    CreateField (Local3, 0x20, (LEN <<
> > > > > 0x03),
> > > > > LDAT)
> > > > > +                    Name (LSA, Buffer (Zero){})
> > > > > +                    ToBuffer (LDAT, LSA) /*
> > > > > \_SB_.NVDR.NV00._LSR.LSA_ */
> > > > > +                    Local1 = Package (0x02)
> > > > > +                        {
> > > > > +                            STTS,
> > > > > +                            LSA
> > > > > +                        }  
> > > > 
> > > > Hi Igor,
> > > > 
> > > > Here is a little different from original proposal 
> > > > https://lore.kernel.org/qemu-devel/80b09055416c790922c7c3db60d2ba865792d1b0.camel@linux.intel.com/
> > > > 
> > > >    Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> > > > 
> > > > Because in my test, Linux guest complains:
> > > > 
> > > > [    3.884656] ACPI Error: AE_SUPPORT, Expressions within
> > > > package
> > > > elements are not supported (20220331/dspkginit-172)
> > > > [    3.887104] ACPI Error: Aborting method \_SB.NVDR.NV00._LSR
> > > > due
> > > > to
> > > > previous error (AE_SUPPORT) (20220331/psparse-531)
> > > > 
> > > > 
> > > > So I have to move toBuffer() out of Package{} and name LSA to
> > > > hold
> > > > the
> > > > buffer. If you have better idea, pls. let me know.
> > > 
> > > Would something like following work?
> > > 
> > > LocalX =  Buffer (Zero){}
> > > LocalY = Package (0x01) { LocalX }
> > 
> > 
> > No, Package{} doesn't accept LocalX as elements.
> > 
> > PackageTerm :=
> > Package (
> > NumElements // Nothing | ByteConstExpr | TermArg => Integer
> > ) {PackageList} => Package
> > 
> > PackageList :=
> > Nothing | <PackageElement PackageListTail>
> > 
> > PackageElement :=
> > DataObject | NameString
Igor Mammedov Oct. 20, 2022, 11:48 a.m. UTC | #6
On Thu, 20 Oct 2022 08:48:36 +0800
Robert Hoo <robert.hu@linux.intel.com> wrote:

> Ping...

sorry, series got lost among the rest.
I've just acked 4/5, but this patch doesn't apply anymore with
 error: corrupt patch at line 172

> On Fri, 2022-10-07 at 21:27 +0800, Robert Hoo wrote:
> > Ping...
> > On Tue, 2022-09-27 at 08:30 +0800, Robert Hoo wrote:  
> > > On Mon, 2022-09-26 at 15:22 +0200, Igor Mammedov wrote:  
> > > > > > 0800200c9a66"), One, 0x05, Local0, One)
> > > > > > +                    CreateDWordField (Local3, Zero, STTS)
> > > > > > +                    CreateField (Local3, 0x20, (LEN <<
> > > > > > 0x03),
> > > > > > LDAT)
> > > > > > +                    Name (LSA, Buffer (Zero){})
> > > > > > +                    ToBuffer (LDAT, LSA) /*
> > > > > > \_SB_.NVDR.NV00._LSR.LSA_ */
> > > > > > +                    Local1 = Package (0x02)
> > > > > > +                        {
> > > > > > +                            STTS,
> > > > > > +                            LSA
> > > > > > +                        }    
> > > > > 
> > > > > Hi Igor,
> > > > > 
> > > > > Here is a little different from original proposal 
> > > > > https://lore.kernel.org/qemu-devel/80b09055416c790922c7c3db60d2ba865792d1b0.camel@linux.intel.com/
> > > > > 
> > > > >    Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> > > > > 
> > > > > Because in my test, Linux guest complains:
> > > > > 
> > > > > [    3.884656] ACPI Error: AE_SUPPORT, Expressions within
> > > > > package
> > > > > elements are not supported (20220331/dspkginit-172)
> > > > > [    3.887104] ACPI Error: Aborting method \_SB.NVDR.NV00._LSR
> > > > > due
> > > > > to
> > > > > previous error (AE_SUPPORT) (20220331/psparse-531)
> > > > > 
> > > > > 
> > > > > So I have to move toBuffer() out of Package{} and name LSA to
> > > > > hold
> > > > > the
> > > > > buffer. If you have better idea, pls. let me know.  
> > > > 
> > > > Would something like following work?
> > > > 
> > > > LocalX =  Buffer (Zero){}
> > > > LocalY = Package (0x01) { LocalX }  
> > > 
> > > 
> > > No, Package{} doesn't accept LocalX as elements.
> > > 
> > > PackageTerm :=
> > > Package (
> > > NumElements // Nothing | ByteConstExpr | TermArg => Integer
> > > ) {PackageList} => Package
> > > 
> > > PackageList :=
> > > Nothing | <PackageElement PackageListTail>
> > > 
> > > PackageElement :=
> > > DataObject | NameString  
>
Michael S. Tsirkin Oct. 20, 2022, 12:01 p.m. UTC | #7
On Thu, Oct 20, 2022 at 01:48:49PM +0200, Igor Mammedov wrote:
> On Thu, 20 Oct 2022 08:48:36 +0800
> Robert Hoo <robert.hu@linux.intel.com> wrote:
> 
> > Ping...
> 
> sorry, series got lost among the rest.
> I've just acked 4/5, but this patch doesn't apply anymore with
>  error: corrupt patch at line 172


updating binaries can be done when applying, this is why
we have the process of splitting these patches out.

If the diff looks good to you - then just ack this, and I will
check it's the diff when applying.


> > On Fri, 2022-10-07 at 21:27 +0800, Robert Hoo wrote:
> > > Ping...
> > > On Tue, 2022-09-27 at 08:30 +0800, Robert Hoo wrote:  
> > > > On Mon, 2022-09-26 at 15:22 +0200, Igor Mammedov wrote:  
> > > > > > > 0800200c9a66"), One, 0x05, Local0, One)
> > > > > > > +                    CreateDWordField (Local3, Zero, STTS)
> > > > > > > +                    CreateField (Local3, 0x20, (LEN <<
> > > > > > > 0x03),
> > > > > > > LDAT)
> > > > > > > +                    Name (LSA, Buffer (Zero){})
> > > > > > > +                    ToBuffer (LDAT, LSA) /*
> > > > > > > \_SB_.NVDR.NV00._LSR.LSA_ */
> > > > > > > +                    Local1 = Package (0x02)
> > > > > > > +                        {
> > > > > > > +                            STTS,
> > > > > > > +                            LSA
> > > > > > > +                        }    
> > > > > > 
> > > > > > Hi Igor,
> > > > > > 
> > > > > > Here is a little different from original proposal 
> > > > > > https://lore.kernel.org/qemu-devel/80b09055416c790922c7c3db60d2ba865792d1b0.camel@linux.intel.com/
> > > > > > 
> > > > > >    Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> > > > > > 
> > > > > > Because in my test, Linux guest complains:
> > > > > > 
> > > > > > [    3.884656] ACPI Error: AE_SUPPORT, Expressions within
> > > > > > package
> > > > > > elements are not supported (20220331/dspkginit-172)
> > > > > > [    3.887104] ACPI Error: Aborting method \_SB.NVDR.NV00._LSR
> > > > > > due
> > > > > > to
> > > > > > previous error (AE_SUPPORT) (20220331/psparse-531)
> > > > > > 
> > > > > > 
> > > > > > So I have to move toBuffer() out of Package{} and name LSA to
> > > > > > hold
> > > > > > the
> > > > > > buffer. If you have better idea, pls. let me know.  
> > > > > 
> > > > > Would something like following work?
> > > > > 
> > > > > LocalX =  Buffer (Zero){}
> > > > > LocalY = Package (0x01) { LocalX }  
> > > > 
> > > > 
> > > > No, Package{} doesn't accept LocalX as elements.
> > > > 
> > > > PackageTerm :=
> > > > Package (
> > > > NumElements // Nothing | ByteConstExpr | TermArg => Integer
> > > > ) {PackageList} => Package
> > > > 
> > > > PackageList :=
> > > > Nothing | <PackageElement PackageListTail>
> > > > 
> > > > PackageElement :=
> > > > DataObject | NameString  
> >
Igor Mammedov Oct. 20, 2022, 1:11 p.m. UTC | #8
On Thu, 20 Oct 2022 08:01:48 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Oct 20, 2022 at 01:48:49PM +0200, Igor Mammedov wrote:
> > On Thu, 20 Oct 2022 08:48:36 +0800
> > Robert Hoo <robert.hu@linux.intel.com> wrote:
> >   
> > > Ping...  
> > 
> > sorry, series got lost among the rest.
> > I've just acked 4/5, but this patch doesn't apply anymore with
> >  error: corrupt patch at line 172  
> 
> 
> updating binaries can be done when applying, this is why
> we have the process of splitting these patches out.
> 
> If the diff looks good to you - then just ack this, and I will
> check it's the diff when applying.


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

> 
> 
> > > On Fri, 2022-10-07 at 21:27 +0800, Robert Hoo wrote:  
> > > > Ping...
> > > > On Tue, 2022-09-27 at 08:30 +0800, Robert Hoo wrote:    
> > > > > On Mon, 2022-09-26 at 15:22 +0200, Igor Mammedov wrote:    
> > > > > > > > 0800200c9a66"), One, 0x05, Local0, One)
> > > > > > > > +                    CreateDWordField (Local3, Zero, STTS)
> > > > > > > > +                    CreateField (Local3, 0x20, (LEN <<
> > > > > > > > 0x03),
> > > > > > > > LDAT)
> > > > > > > > +                    Name (LSA, Buffer (Zero){})
> > > > > > > > +                    ToBuffer (LDAT, LSA) /*
> > > > > > > > \_SB_.NVDR.NV00._LSR.LSA_ */
> > > > > > > > +                    Local1 = Package (0x02)
> > > > > > > > +                        {
> > > > > > > > +                            STTS,
> > > > > > > > +                            LSA
> > > > > > > > +                        }      
> > > > > > > 
> > > > > > > Hi Igor,
> > > > > > > 
> > > > > > > Here is a little different from original proposal 
> > > > > > > https://lore.kernel.org/qemu-devel/80b09055416c790922c7c3db60d2ba865792d1b0.camel@linux.intel.com/
> > > > > > > 
> > > > > > >    Local1 = Package (0x2) {STTS, toBuffer(LDAT)}
> > > > > > > 
> > > > > > > Because in my test, Linux guest complains:
> > > > > > > 
> > > > > > > [    3.884656] ACPI Error: AE_SUPPORT, Expressions within
> > > > > > > package
> > > > > > > elements are not supported (20220331/dspkginit-172)
> > > > > > > [    3.887104] ACPI Error: Aborting method \_SB.NVDR.NV00._LSR
> > > > > > > due
> > > > > > > to
> > > > > > > previous error (AE_SUPPORT) (20220331/psparse-531)
> > > > > > > 
> > > > > > > 
> > > > > > > So I have to move toBuffer() out of Package{} and name LSA to
> > > > > > > hold
> > > > > > > the
> > > > > > > buffer. If you have better idea, pls. let me know.    
> > > > > > 
> > > > > > Would something like following work?
> > > > > > 
> > > > > > LocalX =  Buffer (Zero){}
> > > > > > LocalY = Package (0x01) { LocalX }    
> > > > > 
> > > > > 
> > > > > No, Package{} doesn't accept LocalX as elements.
> > > > > 
> > > > > PackageTerm :=
> > > > > Package (
> > > > > NumElements // Nothing | ByteConstExpr | TermArg => Integer
> > > > > ) {PackageList} => Package
> > > > > 
> > > > > PackageList :=
> > > > > Nothing | <PackageElement PackageListTail>
> > > > > 
> > > > > PackageElement :=
> > > > > DataObject | NameString    
> > >   
>