From: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: "peter.maydell@linaro.org" <peter.maydell@linaro.org>,
"drjones@redhat.com" <drjones@redhat.com>,
"xiaoguangrong.eric@gmail.com" <xiaoguangrong.eric@gmail.com>,
Auger Eric <eric.auger@redhat.com>,
"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
Linuxarm <linuxarm@huawei.com>,
"shannon.zhaosl@gmail.com" <shannon.zhaosl@gmail.com>,
"qemu-arm@nongnu.org" <qemu-arm@nongnu.org>,
"xuwei \(O\)" <xuwei5@huawei.com>,
"lersek@redhat.com" <lersek@redhat.com>
Subject: RE: [PATCH 0/5] ARM virt: Add NVDIMM support
Date: Fri, 13 Dec 2019 12:52:15 +0000 [thread overview]
Message-ID: <effeee8f654c4bd985e24dafaf99e5b8@huawei.com> (raw)
In-Reply-To: <20191211085727.1ab9564e@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 5206 bytes --]
Hi Igor,
> -----Original Message-----
> From: Igor Mammedov [mailto:imammedo@redhat.com]
> Sent: 11 December 2019 07:57
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: xiaoguangrong.eric@gmail.com; peter.maydell@linaro.org;
> drjones@redhat.com; shannon.zhaosl@gmail.com; qemu-devel@nongnu.org;
> Linuxarm <linuxarm@huawei.com>; Auger Eric <eric.auger@redhat.com>;
> qemu-arm@nongnu.org; xuwei (O) <xuwei5@huawei.com>;
> lersek@redhat.com
> Subject: Re: [PATCH 0/5] ARM virt: Add NVDIMM support
[...]
> > I couldn't figure out yet, why this extra 4 bytes are added by aml code on
> ARM64
> > when the nvdimm_dsm_func_read_fit() returns NvdimmFuncReadFITOut
> without
> > any FIT data. ie, when the FIT buffer len (read_len) is zero.
> >
> > But the below will fix this issue,
> >
> > diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> > index f91eea3802..cddf95f4c1 100644
> > --- a/hw/acpi/nvdimm.c
> > +++ b/hw/acpi/nvdimm.c
> > @@ -588,7 +588,7 @@ static void
> nvdimm_dsm_func_read_fit(NVDIMMState *state, NvdimmDsmIn *in,
> > nvdimm_debug("Read FIT: offset %#x FIT size %#x Dirty %s.\n",
> > read_fit->offset, fit->len, fit_buf->dirty ? "Yes" : "No");
> >
> > - if (read_fit->offset > fit->len) {
> > + if (read_fit->offset >= fit->len) {
> > func_ret_status = NVDIMM_DSM_RET_STATUS_INVALID;
> > goto exit;
> > }
> >
> >
> > This will return error code to aml in the second iteration when there is no
> further
> > FIT data to report. But, I am not sure why this check was omitted in the first
> place.
> >
> > Please let me know if this is acceptable and then probably I can look into a v2
> of this
> > series.
> Sorry, I don't have capacity to debug this right now,
No problem.
> but I'd prefer if 'why' question was answered first.
Right.
> Anyways, if something is unclear in how concrete AML code is build/works,
> feel free to ask and I'll try to explain and guide you.
Thanks for your help. I did spend some more time debugging this further.
I tried to introduce a totally new Buffer field object with different
sizes and printing the size after creation.
--- SSDT.dsl 2019-12-12 15:28:21.976986949 +0000
+++ SSDT-arm64-dbg.dsl 2019-12-13 12:17:11.026806186 +0000
@@ -18,7 +18,7 @@
* Compiler ID "BXPC"
* Compiler Version 0x00000001 (1)
*/
-DefinitionBlock ("", "SSDT", 1, "BOCHS ", "NVDIMM", 0x00000001)
+DefinitionBlock ("", "SSDT", 1, "BOCHS ", "NVDIMM", 0x00000002)
{
Scope (\_SB)
{
@@ -48,6 +48,11 @@
RLEN, 32,
ODAT, 32736
}
+
+ Field (NRAM, DWordAcc, NoLock, Preserve)
+ {
+ NBUF, 32768
+ }
If ((Arg4 == Zero))
{
@@ -87,6 +92,12 @@
Local3 = DerefOf (Local2)
FARG = Local3
}
+
+ Local2 = 0x2
+ printf("AML:NVDIMM Creating TBUF with bytes %o", Local2)
+ CreateField (NBUF, Zero, (Local2 << 3), TBUF)
+ Concatenate (Buffer (Zero){}, TBUF, Local3)
+ printf("AML:NVDIMM Size of TBUF(Local3) %o", SizeOf(Local3))
NTFI = Local6
Local1 = (RLEN - 0x04)
And run it by changing Local2 with different values, It looks on ARM64,
For cases where, Local2 <8, the created buffer size is always 8 bytes
"AML:NVDIMM Creating TBUF with bytes 0000000000000002"
"AML:NVDIMM Size of TBUF(Local3) 0000000000000008"
...
"AML:NVDIMM Creating TBUF with bytes 0000000000000005"
"AML:NVDIMM Size of TBUF(Local3) 0000000000000008"
And once Local2 >=8, it gets the correct size,
"AML:NVDIMM Creating TBUF with bytes 0000000000000009"
"AML:NVDIMM Size of TBUF(Local3) 0000000000000009"
But on x86, the behavior is like,
For cases where, Local2 <4, the created buffer size is always 4 bytes
"AML:NVDIMM Creating TBUF with bytes 00000002"
"AML:NVDIMM Size of TBUF(Local3) 00000004"
....
"AML:NVDIMM Creating TBUF with bytes 00000003"
"AML:NVDIMM Size of TBUF(Local3) 00000004"
And once Local2 >= 4, it is ok
"AML:NVDIMM Creating TBUF with bytes 00000005"
"AML:NVDIMM Size of TBUF(Local3) 00000005"
...
"AML:NVDIMM Creating TBUF with bytes 00000009"
"AML:NVDIMM Size of TBUF(Local3) 00000009"
This is the reason why it works on x86 and not on ARM64. Because, if you
remember on second iteration of the FIT buffer, the requested buffer size is 4 .
I tried changing the AccessType of the below NBUF field from DWordAcc to
ByteAcc/BufferAcc, but no luck.
+ Field (NRAM, DWordAcc, NoLock, Preserve)
+ {
+ NBUF, 32768
+ }
Not sure what we need to change for ARM64 to create buffer object of size 4
here. Please let me know if you have any pointers to debug this further.
(I am attaching both x86 and ARM64 SSDT dsl used for reference)
Thanks,
Shameer
> > Thanks,
> > Shameer
> >
> >
> >
[-- Attachment #2: SSDT-arm64-dbg.dsl --]
[-- Type: application/octet-stream, Size: 6942 bytes --]
/*
* Intel ACPI Component Architecture
* AML/ASL+ Disassembler version 20180105 (64-bit version)
* Copyright (c) 2000 - 2018 Intel Corporation
*
* Disassembling to symbolic ASL+ operators
*
* Disassembly of SSDT, Thu Dec 12 15:28:21 2019
*
* Original Table Header:
* Signature "SSDT"
* Length 0x000002EF (751)
* Revision 0x01
* Checksum 0xA4
* OEM ID "BOCHS "
* OEM Table ID "NVDIMM"
* OEM Revision 0x00000001 (1)
* Compiler ID "BXPC"
* Compiler Version 0x00000001 (1)
*/
DefinitionBlock ("", "SSDT", 1, "BOCHS ", "NVDIMM", 0x00000002)
{
Scope (\_SB)
{
Device (NVDR)
{
Name (_HID, "ACPI0012" /* NVDIMM Root Device */) // _HID: Hardware ID
Method (NCAL, 5, Serialized)
{
Local6 = MEMA /* \MEMA */
OperationRegion (NPIO, SystemMemory, 0x09090000, 0x04)
OperationRegion (NRAM, SystemMemory, Local6, 0x1000)
Field (NPIO, DWordAcc, NoLock, Preserve)
{
NTFI, 32
}
Field (NRAM, DWordAcc, NoLock, Preserve)
{
HDLE, 32,
REVS, 32,
FUNC, 32,
FARG, 32672
}
Field (NRAM, DWordAcc, NoLock, Preserve)
{
RLEN, 32,
ODAT, 32736
}
Field (NRAM, DWordAcc, NoLock, Preserve)
{
NBUF, 32768
}
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 ((Arg2 == Zero))
{
Return (Buffer (One)
{
0x00 // .
})
}
Return (Buffer (One)
{
0x01 // .
})
}
HDLE = Arg4
REVS = Arg1
FUNC = Arg2
If (((ObjectType (Arg3) == 0x04) & (SizeOf (Arg3) == One)))
{
Local2 = Arg3 [Zero]
Local3 = DerefOf (Local2)
FARG = Local3
}
Local2 = 0xD
printf("AML:NVDIMM Creating TBUF with bytes %o", Local2)
CreateField (NBUF, Zero, (Local2 << 3), TBUF)
Concatenate (Buffer (Zero){}, TBUF, Local3)
printf("AML:NVDIMM Size of TBUF(Local3) %o", SizeOf(Local3))
NTFI = Local6
Local1 = (RLEN - 0x04)
Local1 = (Local1 << 0x03)
CreateField (ODAT, Zero, Local1, OBUF)
Concatenate (Buffer (Zero){}, OBUF, Local7)
Return (Local7)
}
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, Zero))
}
Name (RSTA, Zero)
Method (RFIT, 1, Serialized)
{
Name (OFST, Zero)
OFST = Arg0
Local0 = NCAL (ToUUID ("648b9cf2-cda1-4312-8ad9-49c4af32bd62"), One, One, Package (0x01)
{
OFST
}, 0x00010000)
CreateDWordField (Local0, Zero, STAU)
RSTA = STAU /* \_SB_.NVDR.RFIT.STAU */
If ((Zero != STAU))
{
Return (Buffer (Zero){})
}
Local1 = SizeOf (Local0)
Local1 -= 0x04
If ((Local1 == Zero))
{
Return (Buffer (Zero){})
}
CreateField (Local0, 0x20, (Local1 << 0x03), BUFF)
Return (BUFF) /* \_SB_.NVDR.RFIT.BUFF */
}
Method (_FIT, 0, Serialized) // _FIT: Firmware Interface Table
{
Local2 = Buffer (Zero){}
Local3 = Zero
While (One)
{
Local0 = RFIT (Local3)
Local1 = SizeOf (Local0)
If ((RSTA == 0x0100))
{
Local2 = Buffer (Zero){}
Local3 = Zero
}
Else
{
If ((Local1 == Zero))
{
Return (Local2)
}
Local3 += Local1
Concatenate (Local2, Local0, Local2)
}
}
}
Device (NV00)
{
Name (_ADR, One) // _ADR: Address
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, One))
}
}
Device (NV01)
{
Name (_ADR, 0x02) // _ADR: Address
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, 0x02))
}
}
Device (NV02)
{
Name (_ADR, 0x03) // _ADR: Address
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, 0x03))
}
}
Device (NV03)
{
Name (_ADR, 0x04) // _ADR: Address
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, 0x04))
}
}
Device (NV04)
{
Name (_ADR, 0x05) // _ADR: Address
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, 0x05))
}
}
}
}
Name (MEMA, 0xFFFF0000)
}
[-- Attachment #3: SSDT-x86-dbg.dsl --]
[-- Type: application/octet-stream, Size: 6934 bytes --]
/*
* Intel ACPI Component Architecture
* AML/ASL+ Disassembler version 20180105 (64-bit version)
* Copyright (c) 2000 - 2018 Intel Corporation
*
* Disassembling to symbolic ASL+ operators
*
* Disassembly of SSDT, Mon Nov 18 15:53:14 2019
*
* Original Table Header:
* Signature "SSDT"
* Length 0x000002ED (749)
* Revision 0x01
* Checksum 0x3D
* OEM ID "BOCHS "
* OEM Table ID "NVDIMM"
* OEM Revision 0x00000001 (1)
* Compiler ID "BXPC"
* Compiler Version 0x00000001 (1)
*/
DefinitionBlock ("", "SSDT", 1, "BOCHS ", "NVDIMM", 0x00000002)
{
Scope (\_SB)
{
Device (NVDR)
{
Name (_HID, "ACPI0012" /* NVDIMM Root Device */) // _HID: Hardware ID
Method (NCAL, 5, Serialized)
{
Local6 = MEMA /* \MEMA */
OperationRegion (NPIO, SystemIO, 0x0A18, 0x04)
OperationRegion (NRAM, SystemMemory, Local6, 0x1000)
Field (NPIO, DWordAcc, NoLock, Preserve)
{
NTFI, 32
}
Field (NRAM, DWordAcc, NoLock, Preserve)
{
HDLE, 32,
REVS, 32,
FUNC, 32,
FARG, 32672
}
Field (NRAM, DWordAcc, NoLock, Preserve)
{
RLEN, 32,
ODAT, 32736
}
Field (NRAM, DWordAcc, NoLock, Preserve)
{
NBUF, 32768
}
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 ((Arg2 == Zero))
{
Return (Buffer (One)
{
0x00 // .
})
}
Return (Buffer (One)
{
0x01 // .
})
}
HDLE = Arg4
REVS = Arg1
FUNC = Arg2
If (((ObjectType (Arg3) == 0x04) & (SizeOf (Arg3) == One)))
{
Local2 = Arg3 [Zero]
Local3 = DerefOf (Local2)
FARG = Local3
}
Local2 = 0x9
printf("AML:NVDIMM Creating TBUF with bytes %o", Local2)
CreateField (NBUF, Zero, (Local2 << 3), TBUF)
Concatenate (Buffer (Zero){}, TBUF, Local3)
printf("AML:NVDIMM Size of TBUF(Local3) %o", SizeOf(Local3))
NTFI = Local6
Local1 = (RLEN - 0x04)
Local1 = (Local1 << 0x03)
CreateField (ODAT, Zero, Local1, OBUF)
Concatenate (Buffer (Zero){}, OBUF, Local7)
Return (Local7)
}
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, Zero))
}
Name (RSTA, Zero)
Method (RFIT, 1, Serialized)
{
Name (OFST, Zero)
OFST = Arg0
Local0 = NCAL (ToUUID ("648b9cf2-cda1-4312-8ad9-49c4af32bd62"), One, One, Package (0x01)
{
OFST
}, 0x00010000)
CreateDWordField (Local0, Zero, STAU)
RSTA = STAU /* \_SB_.NVDR.RFIT.STAU */
If ((Zero != STAU))
{
Return (Buffer (Zero){})
}
Local1 = SizeOf (Local0)
Local1 -= 0x04
If ((Local1 == Zero))
{
Return (Buffer (Zero){})
}
CreateField (Local0, 0x20, (Local1 << 0x03), BUFF)
Return (BUFF) /* \_SB_.NVDR.RFIT.BUFF */
}
Method (_FIT, 0, Serialized) // _FIT: Firmware Interface Table
{
Local2 = Buffer (Zero){}
Local3 = Zero
While (One)
{
Local0 = RFIT (Local3)
Local1 = SizeOf (Local0)
If ((RSTA == 0x0100))
{
Local2 = Buffer (Zero){}
Local3 = Zero
}
Else
{
If ((Local1 == Zero))
{
Return (Local2)
}
Local3 += Local1
Concatenate (Local2, Local0, Local2)
}
}
}
Device (NV00)
{
Name (_ADR, One) // _ADR: Address
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, One))
}
}
Device (NV01)
{
Name (_ADR, 0x02) // _ADR: Address
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, 0x02))
}
}
Device (NV02)
{
Name (_ADR, 0x03) // _ADR: Address
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, 0x03))
}
}
Device (NV03)
{
Name (_ADR, 0x04) // _ADR: Address
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, 0x04))
}
}
Device (NV04)
{
Name (_ADR, 0x05) // _ADR: Address
Method (_DSM, 4, NotSerialized) // _DSM: Device-Specific Method
{
Return (NCAL (Arg0, Arg1, Arg2, Arg3, 0x05))
}
}
}
}
Name (MEMA, 0xBFBFD000)
}
next prev parent reply other threads:[~2019-12-13 21:22 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-04 15:52 [PATCH 0/5] ARM virt: Add NVDIMM support Shameer Kolothum
2019-10-04 15:52 ` [PATCH 1/5] hw/arm: Align ACPI blob len to PAGE size Shameer Kolothum
2019-11-08 16:17 ` Igor Mammedov
2019-11-11 12:47 ` Shameerali Kolothum Thodi
2019-12-09 13:04 ` Shameerali Kolothum Thodi
2019-10-04 15:52 ` [PATCH 2/5] nvdimm: Use configurable ACPI IO base and size Shameer Kolothum
2019-11-11 14:24 ` Igor Mammedov
2019-10-04 15:53 ` [PATCH 3/5] hw/arm/virt: Add nvdimm hot-plug infrastructure Shameer Kolothum
2019-11-11 14:38 ` Igor Mammedov
2019-10-04 15:53 ` [PATCH 4/5] hw/arm/boot: Expose the pmem nodes in the DT Shameer Kolothum
2019-11-11 14:46 ` Igor Mammedov
2019-10-04 15:53 ` [PATCH 5/5] hw/arm/virt: Add nvdimm hotplug support Shameer Kolothum
2019-11-12 13:01 ` Igor Mammedov
2019-10-18 16:39 ` [PATCH 0/5] ARM virt: Add NVDIMM support Auger Eric
2019-10-22 14:05 ` Shameerali Kolothum Thodi
2019-11-25 13:20 ` Shameerali Kolothum Thodi
2019-11-25 15:45 ` Igor Mammedov
2019-11-25 16:25 ` Shameerali Kolothum Thodi
2019-11-26 8:56 ` Igor Mammedov
2019-11-26 9:46 ` Andrew Jones
2019-11-28 12:36 ` Shameerali Kolothum Thodi
2019-12-09 17:39 ` Shameerali Kolothum Thodi
2019-12-11 7:57 ` Igor Mammedov
2019-12-13 12:52 ` Shameerali Kolothum Thodi [this message]
2020-01-06 17:06 ` Shameerali Kolothum Thodi
2020-01-09 17:13 ` Igor Mammedov
2020-01-13 13:11 ` Shameerali Kolothum Thodi
2019-11-12 14:39 ` Igor Mammedov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=effeee8f654c4bd985e24dafaf99e5b8@huawei.com \
--to=shameerali.kolothum.thodi@huawei.com \
--cc=drjones@redhat.com \
--cc=eric.auger@redhat.com \
--cc=imammedo@redhat.com \
--cc=lersek@redhat.com \
--cc=linuxarm@huawei.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=shannon.zhaosl@gmail.com \
--cc=xiaoguangrong.eric@gmail.com \
--cc=xuwei5@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).