Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods
On Wed, 2022-09-21 at 15:29 +0200, Igor Mammedov wrote:
> On Tue, 20 Sep 2022 20:28:31 +0800
> Robert Hoo wrote:
>
> > On Tue, 2022-09-20 at 11:13 +0200, Igor Mammedov wrote:
> > > On Fri, 16 Sep 2022 21:15:35 +0800
> > > Robert Hoo 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)
> > > > }
> >
> >
>
>
Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods
On Tue, 20 Sep 2022 20:28:31 +0800
Robert Hoo wrote:
> On Tue, 2022-09-20 at 11:13 +0200, Igor Mammedov wrote:
> > On Fri, 16 Sep 2022 21:15:35 +0800
> > Robert Hoo 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
On Tue, 2022-09-20 at 11:13 +0200, Igor Mammedov wrote:
> On Fri, 16 Sep 2022 21:15:35 +0800
> Robert Hoo 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
On Fri, 16 Sep 2022 21:15:35 +0800
Robert Hoo 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
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
On Fri, 16 Sep 2022 10:27:08 +0800
Robert Hoo 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 :=
> | |
> 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
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 :=
| |
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
On Fri, 09 Sep 2022 22:02:34 +0800
Robert Hoo wrote:
> On Fri, 2022-09-09 at 15:39 +0200, Igor Mammedov wrote:
> > On Thu, 1 Sep 2022 11:27:20 +0800
> > Robert Hoo 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
> > > Reviewed-by: Jingqi Liu
> >
> > 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)));
> > > +
Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods
On Fri, 2022-09-09 at 15:39 +0200, Igor Mammedov wrote:
> On Thu, 1 Sep 2022 11:27:20 +0800
> Robert Hoo 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
> > Reviewed-by: Jingqi Liu
>
> 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,
Re: [PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods
On Thu, 1 Sep 2022 11:27:20 +0800
Robert Hoo 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
> Reviewed-by: Jingqi Liu
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,
[PATCH v3 4/5] acpi/nvdimm: Implement ACPI NVDIMM Label Methods
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
Reviewed-by: Jingqi Liu
---
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,
