UefiPayloadPkg: assert error in PeiPcdLib


King Sumo
 

Hi,

Using coreboot 4.14 + UefiPayloadPkg the following error is shown:

ASSERT_EFI_ERROR (Status = Not Found)
ASSERT [PeiCore]
/work/edk2/edk2-orig/MdePkg/Library/PeiPcdLib/PeiPcdLib.c(43): !EFI_ERROR
(Status)


By doing a git bisect I found the following commit (doing a git revert
fixes the issue):
3900a63e3a UefiPayloadPkg/Pci: Use the PCIE Base Addr stored in
AcpiBoardInfo HOB

Basically this change gets the PCIe base address from the AcpiBoardInfo HOB.
I'm not sure if this AcpiBoardInfo HOB is really available for coreboot,
but the crash looks funny. It probably occurs while
setting PcdPciExpressBaseAddress or PcdPciExpressBaseSize (not sure, more
debug needed) but they are both defined...

Anyone run into the same problem?

Thanks,
Sumo


King Sumo
 

Hi,

If I revert the patch I can see the log output from PayloadEntry(), e.g.:
GET_BOOTLOADER_PARAMETER() = 0x17F766000

But with the patch nothing else is shown in the log besides:
ASSERT_EFI_ERROR (Status = Not Found)
ASSERT [PeiCore]
/work/edk2/edk2-orig/MdePkg/Library/PeiPcdLib/PeiPcdLib.c(43): !EFI_ERROR

So looks like it's crashing before the PayloadEntry() call. In fact, according to PeiPcdLib.c:43 it fails to locate gPcdPpiGuid:
Status = PeiServicesLocatePpi (&gPcdPpiGuid, 0, NULL, (VOID **)&PcdPpi);
Maybe it's failling inside ./UefiPayloadPkg/UefiPayloadEntry/Ia32/SecEntry.nasm... pretty odd.

Any advices?

Thanks,
Sumo


Andrew Fish
 

On Aug 20, 2021, at 2:18 PM, King Sumo <kingsumos@...> wrote:

Hi,

If I revert the patch I can see the log output from PayloadEntry(), e.g.:
GET_BOOTLOADER_PARAMETER() = 0x17F766000

But with the patch nothing else is shown in the log besides:
ASSERT_EFI_ERROR (Status = Not Found)
ASSERT [PeiCore]
/work/edk2/edk2-orig/MdePkg/Library/PeiPcdLib/PeiPcdLib.c(43): !EFI_ERROR

So looks like it's crashing before the PayloadEntry() call. In fact, according to PeiPcdLib.c:43 it fails to locate gPcdPpiGuid:
Status = PeiServicesLocatePpi (&gPcdPpiGuid, 0, NULL, (VOID **)&PcdPpi);
Maybe it's failling inside ./UefiPayloadPkg/UefiPayloadEntry/Ia32/SecEntry.nasm... pretty odd.

Any advices?
There is “build magic”(TM) around PCDs. There are different flavors of PCDs. PCDs can be build constants or looked up from a central database. The ASSERT is from the code trying to look something up. What flavor a PCD has is controlled by the platform build, so DSC file.

In yoru hash…
For: UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc

- gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|$(PCIE_BASE)

+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0

It looks like a PCD may have moved from a fixed at build (compile type constant) to something that needs to get looked up in the PCD database.

That PPI is usually produced by this PIEM:
MdeModulePkg/Universal/PCD/Pei/Pei.inf

Your ASSERT() is basically the above PEIM is missing.

If you move gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress back to being a compile time constant (location in the DSC file matters) that might get you past your ASSERT as a work around?

Thanks,

Andrew Fish

Thanks,
Sumo





King Sumo
 

On Fri, Aug 20, 2021 at 04:52 PM, Andrew Fish wrote:


If you move gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress back to being a
compile time constant (location in the DSC file matters) that might get you
past your ASSERT as a work around?
Yes, it works. Moved from PcdsDynamicDefault to PcdsFixedAtBuild and added hardcoded values.
I think Ray Ni <ray.ni@...> must be contacted to provide a fix...
The build is done using:
build -a IA32 -a X64 -p UefiPayloadPkg/UefiPayloadPkg.dsc -b DEBUG -t GCC5 -D BOOTLOADER=COREBOOT
(IA32 is also required for coreboot)

Perhaps the issue is related to the IA32 target? Calling PcdSet64S() is allowed?
./UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c:
if (GuidHob != NULL) {
AcpiBoardInfo = (ACPI_BOARD_INFO *)GET_GUID_HOB_DATA (GuidHob);
Status = PcdSet64S (PcdPciExpressBaseAddress, AcpiBoardInfo->PcieBaseAddress);
ASSERT_EFI_ERROR (Status);
Status = PcdSet64S (PcdPciExpressBaseSize, AcpiBoardInfo->PcieBaseSize);
ASSERT_EFI_ERROR (Status);
}

Thanks,
Sumo


King Sumo