Reviewed-by: Guo Dong <guo.d...@intel.com>
> -----Original Message----- > From: Patrick Rudolph <patrick.rudo...@9elements.com> > Sent: Monday, June 21, 2021 1:10 AM > To: devel@edk2.groups.io > Cc: Ma, Maurice <maurice...@intel.com>; Dong, Guo > <guo.d...@intel.com>; You, Benjamin <benjamin....@intel.com> > Subject: [PATCH v3] UefiPayloadPkg/UefiPayloadEntry: Improve bootloader > memrange parsing > > Currently several DXE crash due to invalid memory resource settings. > The PciHostBridgeDxe which expects the MMCONF and PCI Aperature > to be EfiMemoryMappedIO, but currently those regions are (partly) > mapped as EfiReservedMemoryType. > > coreboot and slimbootloader provide an e820 compatible memory map, > which doesn't work well with EDK2 as the e820 spec is missing MMIO regions. > In e820 'reserved' could either mean "DRAM used by boot firmware" or > "MMIO > in use and not detectable by OS". > > Guess Top of lower usable DRAM (TOLUD) by walking the bootloader > provided > memory ranges. Memory types of RAM, ACPI and ACPI NVS below 4 GiB are > used > to increment TOLUD and reserved memory ranges touching TOLUD at the > base > are also assumed to be reserved DRAM, which increment TOLUD. > > Then mark everything reserved below TOLUD as EfiReservedMemoryType > and > everything reserved above TOLUD as EfiMemoryMappedIO. > > This fixes assertions seen in PciHostBridgeDxe. > > Signed-off-by: Patrick Rudolph <patrick.rudo...@9elements.com> > --- > .../UefiPayloadEntry/UefiPayloadEntry.c | 190 +++++++++++++++++- > .../UefiPayloadEntry/UefiPayloadEntry.h | 10 + > 2 files changed, 197 insertions(+), 3 deletions(-) > > diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > index 805f5448d9..04c58f776c 100644 > --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c > @@ -7,10 +7,159 @@ > > > #include "UefiPayloadEntry.h" > > > > +STATIC UINT32 mTopOfLowerUsableDram = 0; > > + > > /** > > Callback function to build resource descriptor HOB > > > > This function build a HOB based on the memory map entry info. > > + It creates only EFI_RESOURCE_MEMORY_MAPPED_IO and > EFI_RESOURCE_MEMORY_RESERVED > > + resources. > > + > > + @param MemoryMapEntry Memory map entry info got from > bootloader. > > + @param Params A pointer to ACPI_BOARD_INFO. > > + > > + @retval EFI_SUCCESS Successfully build a HOB. > > + @retval EFI_INVALID_PARAMETER Invalid parameter provided. > > +**/ > > +EFI_STATUS > > +MemInfoCallbackMmio ( > > + IN MEMROY_MAP_ENTRY *MemoryMapEntry, > > + IN VOID *Params > > + ) > > +{ > > + EFI_PHYSICAL_ADDRESS Base; > > + EFI_RESOURCE_TYPE Type; > > + UINT64 Size; > > + EFI_RESOURCE_ATTRIBUTE_TYPE Attribue; > > + ACPI_BOARD_INFO *AcpiBoardInfo; > > + > > + AcpiBoardInfo = (ACPI_BOARD_INFO *)Params; > > + if (AcpiBoardInfo == NULL) { > > + return EFI_INVALID_PARAMETER; > > + } > > + > > + // > > + // Skip types already handled in MemInfoCallback > > + // > > + if (MemoryMapEntry->Type == E820_RAM || MemoryMapEntry->Type == > E820_ACPI) { > > + return EFI_SUCCESS; > > + } > > + > > + if (MemoryMapEntry->Base == AcpiBoardInfo->PcieBaseAddress) { > > + // > > + // MMCONF is always MMIO > > + // > > + Type = EFI_RESOURCE_MEMORY_MAPPED_IO; > > + } else if (MemoryMapEntry->Base < mTopOfLowerUsableDram) { > > + // > > + // It's in DRAM and thus must be reserved > > + // > > + Type = EFI_RESOURCE_MEMORY_RESERVED; > > + } else if ((MemoryMapEntry->Base < 0x100000000ULL) && > (MemoryMapEntry->Base >= mTopOfLowerUsableDram)) { > > + // > > + // It's not in DRAM, must be MMIO > > + // > > + Type = EFI_RESOURCE_MEMORY_MAPPED_IO; > > + } else { > > + Type = EFI_RESOURCE_MEMORY_RESERVED; > > + } > > + > > + Base = MemoryMapEntry->Base; > > + Size = MemoryMapEntry->Size; > > + > > + Attribue = EFI_RESOURCE_ATTRIBUTE_PRESENT | > > + EFI_RESOURCE_ATTRIBUTE_INITIALIZED | > > + EFI_RESOURCE_ATTRIBUTE_TESTED | > > + EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE | > > + EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE | > > + EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE | > > + EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE; > > + > > + BuildResourceDescriptorHob (Type, Attribue, > (EFI_PHYSICAL_ADDRESS)Base, Size); > > + DEBUG ((DEBUG_INFO , "buildhob: base = 0x%lx, size = 0x%lx, type = > 0x%x\n", Base, Size, Type)); > > + > > + if (MemoryMapEntry->Type == E820_UNUSABLE || > > + MemoryMapEntry->Type == E820_DISABLED) { > > + BuildMemoryAllocationHob (Base, Size, EfiUnusableMemory); > > + } else if (MemoryMapEntry->Type == E820_PMEM) { > > + BuildMemoryAllocationHob (Base, Size, EfiPersistentMemory); > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > + > > +/** > > + Callback function to find TOLUD (Top of Lower Usable DRAM) > > + > > + Estimate where TOLUD (Top of Lower Usable DRAM) resides. The exact > position > > + would require platform specific code. > > + > > + @param MemoryMapEntry Memory map entry info got from > bootloader. > > + @param Params Not used for now. > > + > > + @retval EFI_SUCCESS Successfully updated > mTopOfLowerUsableDram. > > +**/ > > +EFI_STATUS > > +FindToludCallback ( > > + IN MEMROY_MAP_ENTRY *MemoryMapEntry, > > + IN VOID *Params > > + ) > > +{ > > + // > > + // This code assumes that the memory map on this x86 machine below > 4GiB is continous > > + // until TOLUD. In addition it assumes that the bootloader provided > memory tables have > > + // no "holes" and thus the first memory range not covered by e820 marks > the end of > > + // usable DRAM. In addition it's assumed that every reserved memory > region touching > > + // usable RAM is also covering DRAM, everything else that is marked > reserved thus must be > > + // MMIO not detectable by bootloader/OS > > + // > > + > > + // > > + // Skip memory types not RAM or reserved > > + // > > + if ((MemoryMapEntry->Type == E820_UNUSABLE) || (MemoryMapEntry- > >Type == E820_DISABLED) || > > + (MemoryMapEntry->Type == E820_PMEM)) { > > + return EFI_SUCCESS; > > + } > > + > > + // > > + // Skip resources above 4GiB > > + // > > + if ((MemoryMapEntry->Base + MemoryMapEntry->Size) > > 0x100000000ULL) { > > + return EFI_SUCCESS; > > + } > > + > > + if ((MemoryMapEntry->Type == E820_RAM) || (MemoryMapEntry->Type > == E820_ACPI) || > > + (MemoryMapEntry->Type == E820_NVS)) { > > + // > > + // It's usable DRAM. Update TOLUD. > > + // > > + if (mTopOfLowerUsableDram < (MemoryMapEntry->Base + > MemoryMapEntry->Size)) { > > + mTopOfLowerUsableDram = (UINT32)(MemoryMapEntry->Base + > MemoryMapEntry->Size); > > + } > > + } else { > > + // > > + // It might be 'reserved DRAM' or 'MMIO'. > > + // > > + // If it touches usable DRAM at Base assume it's DRAM as well, > > + // as it could be bootloader installed tables, TSEG, GTT, ... > > + // > > + if (mTopOfLowerUsableDram == MemoryMapEntry->Base) { > > + mTopOfLowerUsableDram = (UINT32)(MemoryMapEntry->Base + > MemoryMapEntry->Size); > > + } > > + } > > + > > + return EFI_SUCCESS; > > +} > > + > > + > > +/** > > + Callback function to build resource descriptor HOB > > + > > + This function build a HOB based on the memory map entry info. > > + Only add EFI_RESOURCE_SYSTEM_MEMORY. > > > > @param MemoryMapEntry Memory map entry info got from > bootloader. > > @param Params Not used for now. > > @@ -28,7 +177,16 @@ MemInfoCallback ( > UINT64 Size; > > EFI_RESOURCE_ATTRIBUTE_TYPE Attribue; > > > > - Type = (MemoryMapEntry->Type == 1) ? > EFI_RESOURCE_SYSTEM_MEMORY : EFI_RESOURCE_MEMORY_RESERVED; > > + // > > + // Skip everything not known to be usable DRAM. > > + // It will be added later. > > + // > > + if ((MemoryMapEntry->Type != E820_RAM) && (MemoryMapEntry- > >Type != E820_ACPI) && > > + (MemoryMapEntry->Type != E820_NVS)) { > > + return RETURN_SUCCESS; > > + } > > + > > + Type = EFI_RESOURCE_SYSTEM_MEMORY; > > Base = MemoryMapEntry->Base; > > Size = MemoryMapEntry->Size; > > > > @@ -40,7 +198,7 @@ MemInfoCallback ( > EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE | > > EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE; > > > > - if (Base >= BASE_4GB ) { > > + if (Base >= BASE_4GB) { > > // Remove tested attribute to avoid DXE core to dispatch driver to memory > above 4GB > > Attribue &= ~EFI_RESOURCE_ATTRIBUTE_TESTED; > > } > > @@ -48,6 +206,12 @@ MemInfoCallback ( > BuildResourceDescriptorHob (Type, Attribue, > (EFI_PHYSICAL_ADDRESS)Base, Size); > > DEBUG ((DEBUG_INFO , "buildhob: base = 0x%lx, size = 0x%lx, type = > 0x%x\n", Base, Size, Type)); > > > > + if (MemoryMapEntry->Type == E820_ACPI) { > > + BuildMemoryAllocationHob (Base, Size, EfiACPIReclaimMemory); > > + } else if (MemoryMapEntry->Type == E820_NVS) { > > + BuildMemoryAllocationHob (Base, Size, EfiACPIMemoryNVS); > > + } > > + > > return RETURN_SUCCESS; > > } > > > > @@ -236,8 +400,19 @@ BuildHobFromBl ( > EFI_PEI_GRAPHICS_DEVICE_INFO_HOB *NewGfxDeviceInfo; > > > > // > > - // Parse memory info and build memory HOBs > > + // First find TOLUD > > + // > > + DEBUG ((DEBUG_INFO , "Guessing Top of Lower Usable DRAM:\n")); > > + Status = ParseMemoryInfo (FindToludCallback, NULL); > > + if (EFI_ERROR(Status)) { > > + return Status; > > + } > > + DEBUG ((DEBUG_INFO , "Assuming TOLUD = 0x%x\n", > mTopOfLowerUsableDram)); > > + > > + // > > + // Parse memory info and build memory HOBs for Usable RAM > > // > > + DEBUG ((DEBUG_INFO , "Building ResourceDescriptorHobs for usable > memory:\n")); > > Status = ParseMemoryInfo (MemInfoCallback, NULL); > > if (EFI_ERROR(Status)) { > > return Status; > > @@ -289,6 +464,15 @@ BuildHobFromBl ( > DEBUG ((DEBUG_INFO, "Create acpi board info guid hob\n")); > > } > > > > + // > > + // Parse memory info and build memory HOBs for reserved DRAM and > MMIO > > + // > > + DEBUG ((DEBUG_INFO , "Building ResourceDescriptorHobs for reserved > memory:\n")); > > + Status = ParseMemoryInfo (MemInfoCallbackMmio, &AcpiBoardInfo); > > + if (EFI_ERROR(Status)) { > > + return Status; > > + } > > + > > // > > // Parse platform specific information. > > // > > diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h > b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h > index 2c84d6ed53..4fd50e47cd 100644 > --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h > +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h > @@ -38,6 +38,16 @@ > #define GET_OCCUPIED_SIZE(ActualSize, Alignment) \ > > ((ActualSize) + (((Alignment) - ((ActualSize) & ((Alignment) - 1))) & > ((Alignment) - 1))) > > > > + > > +#define E820_RAM 1 > > +#define E820_RESERVED 2 > > +#define E820_ACPI 3 > > +#define E820_NVS 4 > > +#define E820_UNUSABLE 5 > > +#define E820_DISABLED 6 > > +#define E820_PMEM 7 > > +#define E820_UNDEFINED 8 > > + > > /** > > Auto-generated function that calls the library constructors for all of the > module's > > dependent libraries. > > -- > 2.30.2 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76856): https://edk2.groups.io/g/devel/message/76856 Mute This Topic: https://groups.io/mt/83683813/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-