* Possible [BUG] PeiCore: HOBs after cache-as-RAM teardown
@ 2021-07-07 5:55 Benjamin Doron
2021-07-08 21:12 ` [edk2-devel] " Michael Kubacki
0 siblings, 1 reply; 3+ messages in thread
From: Benjamin Doron @ 2021-07-07 5:55 UTC (permalink / raw)
To: devel
[-- Attachment #1: Type: text/plain, Size: 1403 bytes --]
Hi all,
I'm working on root-causing an issue where I'm unable to retrieve any debug logs after cache-as-RAM teardown on my MinPlatform board port for GSoC 2021. I'm using KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash.
My best guess at the moment is that it's related to HOBs, which this SerialPortLib uses. If I reinitialise the library stack, it largely works, but a HOB/context structure entry "CurrentWriteOffset" also looks like it's zeroed.
Also, GetFeatureImplemented() in MinPlatformPkg/Test/TestPointCheckLib/PeiTestPointCheckLib.c asserts with "Status = Not Found". Internally, the test point architecture uses HOBs. I don't think this should happen, so, if not for this, I would think that perhaps a pointer in the serial port library needed to be updated.
I can see that the heap is copied over to permanent memory by the PEI core dispatcher ( https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c#L881 ). However, I can't see that the HOB list pointer is updated, so it looks like it ( https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Hob/Hob.c#L44 ) still has the value that's copied over from the old PEI core's data ( https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c#L348 ).
Does this look like a bug, or might there be something else that I'm missing?
Best regards,
Benjamin
[-- Attachment #2: Type: text/html, Size: 1850 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] Possible [BUG] PeiCore: HOBs after cache-as-RAM teardown
2021-07-07 5:55 Possible [BUG] PeiCore: HOBs after cache-as-RAM teardown Benjamin Doron
@ 2021-07-08 21:12 ` Michael Kubacki
2021-07-13 7:25 ` Benjamin Doron
0 siblings, 1 reply; 3+ messages in thread
From: Michael Kubacki @ 2021-07-08 21:12 UTC (permalink / raw)
To: devel, benjamin.doron00
Hi Benjamin,
Based on what you've described, I'll try to provide my best guess as to
what is happening without actually debugging it. Ultimately, I will need
to submit a patch to update PeiSerialPortLibSpiFlash.
Are you're aware, gPchSpiPpiGuid is installed in
Silicon/Intel/KabylakeSiliconPkg/Pch/Library/PeiSpiLib/PeiSpiLib.c:
PeiSpiInstance = (PEI_SPI_INSTANCE *) AllocateZeroPool (sizeof
(PEI_SPI_INSTANCE));
if (NULL == PeiSpiInstance) {
return EFI_OUT_OF_RESOURCES;
}
SpiInstance = &(PeiSpiInstance->SpiInstance);
SpiProtocolConstructor (SpiInstance);
PeiSpiInstance->PpiDescriptor.Flags = EFI_PEI_PPI_DESCRIPTOR_PPI |
EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
PeiSpiInstance->PpiDescriptor.Guid = &gPchSpiPpiGuid;
PeiSpiInstance->PpiDescriptor.Ppi = &(SpiInstance->SpiProtocol);
Status = PeiServicesInstallPpi (&PeiSpiInstance->PpiDescriptor);
Note that this PPI descriptor and the PPI interface structure are
allocated via a pool allocation on the T-RAM heap. The allocation
contains the descriptor followed by the interface structure. There's
several pointers set within these structures.
In the descriptor, the GUID pointer is set to the GUID value within the
image (not the heap) and the PPI interface pointer is set to the
interface structure in the same pool allocation in the heap. This
interface structure contains function pointers for SPI flash operations
as defined in the PCH_SPI_PROTOCOL structure.
The function pointers are set in SpiProtocolConstructor() .
Silicon/Intel/KabylakeSiliconPkg/Pch/LibraryPrivate/BasePchSpiCommonLib/SpiCommon.c
Those function addresses are to image addresses at their XIP address in
T-RAM.
Starting from the heap, I suspect:
1. After PeiInstallPeiMemory () is called, PeiCheckAndSwitchStack() will
eventually get called by the dispatcher and see the stack switch signal
is now TRUE.
2. The heap and stack migration will occur. New heap offsets will be
calculated. Note that pointers to structures in the heap will be updated
after PeiCore() reentry.
3. PeiCore() will be reentered using the new permanent memory stack.
4. In this entry, ShadowedPeiCore is NULL. So the T-RAM heap and stack
are still around, the permanent memory heap and stack are known and
setup, but the PeiCore image is still executing at its T-RAM address.
5. The private pointer to the HOB list in the permanent memory heap (in
addition to other lists in the heap like PPI lists) are updated.
6. Eventually, ConvertPpiPointers() will convert pointers in the PPI
list to their permanent memory locations.
- This includes the PPI descriptor, PPI GUID, and PPI interface
pointers.
- These should include pointers into T-RAM memory allocations, heap,
and stack.
- Note: PPI structures initialized as global variables are not
handled. That is what PcdMigrateTemporaryRamFirmwareVolumes helps
address. Not particularly relevant in this specific scenario.
- Further note: For most platforms that execute from flash
mapped as MMIO, these address should still resolve as code fetches over
the SPI bus.
- Anyway, for gPchSpiPpiGuid, I suspect:
1. The PPI list pointer now points to the PPI descriptor in
permanent memory (updated)
2. The PPI descriptor pointer now points to the descriptor in
permanent memory (migrated/updated)
3. The PPI GUID pointer still points to the GUID at the XIP
address (not migrated/update attempted)
4. The PPI interface pointer now points to the interface structure
in the permanent memory heap (migrated/updated)
- The function pointers within the struct still point to the
functions at their XIP addresses (not migrated/not updated)
5. The HOB gSpiFlashDebugHobGuid, has been copied to the permanent
memory heap (with other heap contents) and the HOB heap pointer was updated.
- [other actions will obviously happen but I don't think will
significantly affect the outcome for this situation]
Although, a PPI notify was registered in PeiSerialPortLibSpiFlash on
gPchSpiPpiGuid. The PPI was not actually explicitly reinstalled (that I
can find in the code) so the HOB retains the T-RAM pointers which work
until NEM is disabled (because the PPI descriptor was in the T-RAM heap
not backed by MMIO so it can't be fetched again).
Even if the PPI was explicitly reinstalled, with this code printing
debug messages, the gap is too far between when a message will be
printed and the dispatcher would invoke notifications.
For something in the debug stack especially, it was a poor
implementation choice on my part to cache the PPI pointer in the HOB.
Additionally, this is a library so multiple HOB instances could be
produced which is undesirable. In conclusion, I will send a patch to
update PeiSerialPortLibSpiFlash.
Regarding, the test point checks, are you sure the HOB is actually
created? Were you able to successfully get the HOB at any point after it
should have been created? For example, it looks like there's several
preliminary checks in TestPointLibSetTable() that could prevent
publishing of the HOB.
Regards,
Michael
On 7/7/2021 1:55 AM, Benjamin Doron wrote:
> Hi all,
> I'm working on root-causing an issue where I'm unable to retrieve any
> debug logs after cache-as-RAM teardown on my MinPlatform board port for
> GSoC 2021. I'm using KabylakeOpenBoardPkg/PeiSerialPortLibSpiFlash.
>
> My best guess at the moment is that it's related to HOBs, which this
> SerialPortLib uses. If I reinitialise the library stack, it largely
> works, but a HOB/context structure entry "CurrentWriteOffset" also looks
> like it's zeroed.
>
> Also, GetFeatureImplemented() in
> MinPlatformPkg/Test/TestPointCheckLib/PeiTestPointCheckLib.c asserts
> with "Status = Not Found". Internally, the test point architecture uses
> HOBs. I don't think this should happen, so, if not for this, I would
> think that perhaps a pointer in the serial port library needed to be
> updated.
>
> I can see that the heap is copied over to permanent memory by the PEI
> core dispatcher
> (https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c#L881
> <https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c#L881>).
> However, I can't see that the HOB list pointer is updated, so it looks
> like it
> (https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Hob/Hob.c#L44
> <https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/Hob/Hob.c#L44>)
> still has the value that's copied over from the old PEI core's data
> (https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c#L348
> <https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c#L348>).
>
> Does this look like a bug, or might there be something else that I'm
> missing?
>
> Best regards,
> Benjamin
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] Possible [BUG] PeiCore: HOBs after cache-as-RAM teardown
2021-07-08 21:12 ` [edk2-devel] " Michael Kubacki
@ 2021-07-13 7:25 ` Benjamin Doron
0 siblings, 0 replies; 3+ messages in thread
From: Benjamin Doron @ 2021-07-13 7:25 UTC (permalink / raw)
To: Michael Kubacki, devel
[-- Attachment #1: Type: text/plain, Size: 852 bytes --]
Hi Michael,
I had missed the fact that the HobList pointer is updated in the entry to PEI core when ShadowedPeiCore is NULL. It had seemed odd to me that nobody else experienced issues, so in hindsight, that wouldn't have been the problem. I'm still not sure why CurrentWriteOffset is zeroed, which might imply that the only SpiFlashDebug HOB available is the one just built ( https://github.com/tianocore/edk2-platforms/blob/master/Platform/Intel/KabylakeOpenBoardPkg/Library/PeiSerialPortLibSpiFlash/PeiSerialPortLibSpiFlash.c#L300 ) when I call SerialPortInitialize() again. I may have to debug this further.
Regarding the test points, I guess I just assumed it would work, and without debug logs, I can't see where it would it fail. However, I'll have to assume that they're unrelated to this pointers-in-HOB issue.
Best regards,
Benjamin
[-- Attachment #2: Type: text/html, Size: 1165 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-13 7:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-07 5:55 Possible [BUG] PeiCore: HOBs after cache-as-RAM teardown Benjamin Doron
2021-07-08 21:12 ` [edk2-devel] " Michael Kubacki
2021-07-13 7:25 ` Benjamin Doron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox