Hi Andrew,
After SmmReadyToLock, only SMM driver is allowed to write boot script. In DXE
SCRIPT_TABLE_PRIVATE_DATA instance, only the TableBase field will be used for
BootScriptExecute at S3 resume, other fields will be not used.
So the TableLength field in DXE SCRIPT_TABLE_PRIVATE_DATA instance could be set
to 0 after SmmReadyToLock to say it will be not synced, or even the whole DXE
SCRIPT_TABLE_PRIVATE_DATA instance could be emptied as the TableBase field has
been saved into lockbox and will be restored at S3 resume.
Thanks,
Star
From: Andrew Fish [mailto:af...@apple.com]
Sent: Wednesday, April 1, 2015 10:41 AM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] MdeModulePkg Maintainer: The PiDxeS3BootScriptLib does not
seem to set EFI_BOOT_SCRIPT_TABLE_HEADER.Version
On Mar 31, 2015, at 6:54 PM, Andrew Fish
<af...@apple.com<mailto:af...@apple.com>> wrote:
On Mar 31, 2015, at 6:25 PM, Yao, Jiewen
<jiewen....@intel.com<mailto:jiewen....@intel.com>> wrote:
Yes. It seems a bug.
We should initialize it.
Thanks!
The other thing that looked strange to me was: mS3BootScriptTablePtr and
mS3BootScriptTableSmmPtr of type SCRIPT_TABLE_PRIVATE_DATA.
//
// The boot script private data.
//
typedef struct {
UINT8 *TableBase;
UINT32 TableLength; // Record the actual memory length
UINT16 TableMemoryPageNumber; // Record the page number
Allocated for the table
BOOLEAN AtRuntime; // Record if current state is
after SmmReadyToLock
BOOLEAN InSmm; // Record if this library is in
SMM.
} SCRIPT_TABLE_PRIVATE_DATA;
Given mS3BootScriptTablePtr is a library global there are N copies of them in
the system. So TableLength and TableMemoryPageNumber can get out of sync with
reality. I don’t see the point in tracking the data in N places. Seems like all
the instances of SCRIPT_TABLE_PRIVATE_DATA.TableBase point to
EFI_BOOT_SCRIPT_TABLE_HEADER, so that is the only length that means anything.
I just noticed that mS3BootScriptTablePtr points to
PcdS3BootScriptTablePrivateDataPtr. So the delta is really only between the DXE
drivers and the SMM drivers. All the DXE Drivers share the same
mS3BootScriptTablePtr, and all the SMM drivers share the same
mS3BootScriptTablePtr. But both point to the same EFI_BOOT_SCRIPT_TABLE_HEADER.
DXE:
(lldb) p *mS3BootScriptTablePtr
(SCRIPT_TABLE_PRIVATE_DATA) $126 = {
TableBase = 0x000000007afbe000 "?"
TableLength = 0x000003d1
TableMemoryPageNumber = 0x0004
AtRuntime = true
InSmm = false
}
(lldb) p *(EFI_BOOT_SCRIPT_TABLE_HEADER *)0x000000007afbe000
(EFI_BOOT_SCRIPT_TABLE_HEADER) $127 = {
OpCode = 0x00aa
Length = 0x0d
Version = 0xafaf
TableLength = 0x000005c4
Reserved = ([0] = 0xafaf, [1] = 0xafaf)
}
SMM:
(lldb) p *(SCRIPT_TABLE_PRIVATE_DATA *)0x7b3efa90
(SCRIPT_TABLE_PRIVATE_DATA) $128 = {
TableBase = 0x000000007afbe000 "?"
TableLength = 0x000005c1
TableMemoryPageNumber = 0x0004
AtRuntime = true
InSmm = true
}
(lldb) p *(EFI_BOOT_SCRIPT_TABLE_HEADER *)0x000000007afbe000
(EFI_BOOT_SCRIPT_TABLE_HEADER) $129 = {
OpCode = 0x00aa
Length = 0x0d
Version = 0xafaf
TableLength = 0x000005c4
Reserved = ([0] = 0xafaf, [1] = 0xafaf)
}
I wonder if the reason it seem to work is some of the writes are the full set
and some writes are too short. So at the end of the day the lock box has the
correct data in it?
It still seems like it would be simpler, and safer to just use
EFI_BOOT_SCRIPT_TABLE_HEADER.TableLength?
Thanks,
Andrew Fish
It probably generally seems to work as a driver does writes in batches. But if
a driver did a batch of writes, and then did work in an event. Its cached
SCRIPT_TABLE_PRIVATE_DATA would be out of data with regards to TableLength and
TableMemoryPageNumber.
It looks like a bad value in mS3BootScriptTablePtr could end up truncating the
table. It could be that the later code has a larger value and it runs last? So
maybe it is a race, or we are just getting lucky? Or maybe there is some
syncing back from EFI_BOOT_SCRIPT_TABLE_HEADER that I missed in the code.
Thanks,
Andrew Fish
PS I’ve not seen any bad S3 data. I’m writing a debugger script to dump out the
tables, so I was poking around in memory to debug the script and noticed this
issue.
(lldb) p *mS3BootScriptTablePtr
(SCRIPT_TABLE_PRIVATE_DATA) $120 = {
TableBase = 0x000000007afbe000 "?"
TableLength = 0x000003d1
TableMemoryPageNumber = 0x0004
AtRuntime = true
InSmm = false
}
(lldb) p *(EFI_BOOT_SCRIPT_TABLE_HEADER *)0x000000007afbe000
(EFI_BOOT_SCRIPT_TABLE_HEADER) $121 = {
OpCode = 0x00aa
Length = 0x0d
Version = 0xafaf
TableLength = 0x000005c4
Reserved = ([0] = 0xafaf, [1] = 0xafaf)
}
The last instance of mS3BootScriptTablePtr is this one:
(lldb) p *(SCRIPT_TABLE_PRIVATE_DATA *)0x7b3efa90
(SCRIPT_TABLE_PRIVATE_DATA) $122 = {
TableBase = 0x000000007afbe000 "?"
TableLength = 0x000005c1
TableMemoryPageNumber = 0x0004
AtRuntime = true
InSmm = true
}
Thank you
Yao Jiewen
-----Original Message-----
From: Andrew Fish [mailto:af...@apple.com]
Sent: Wednesday, April 01, 2015 6:32 AM
To: edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
Subject: [edk2] MdeModulePkg Maintainer: The PiDxeS3BootScriptLib does not seem
to set EFI_BOOT_SCRIPT_TABLE_HEADER.Version
I don’t see any code in the PiDxeS3BootScriptLib that sets a value to
EFI_BOOT_SCRIPT_TABLE_HEADER.Version. The data is allocated via
gBS->AllocatePages, and on the system I looked at the Version was set to 0xafaf.
It seems like the driver should initialize this member to a known value?
Thanks,
Andrew Fish
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/edk2-devel
------------------------------------------------------------------------------
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the
conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
