Hi Marcello, Since this patch mixes changes for both UefiPayloadPkg and MdePkg. I think you might want to split this patch into two so that it can be reviewed by the proper package maintainers.
Thanks Maurice > -----Original Message----- > From: Marcello Sylvester Bauer <marcello.ba...@9elements.com> > Sent: Thursday, July 16, 2020 4:48 > To: devel@edk2.groups.io > Cc: Patrick Rudolph <patrick.rudo...@9elements.com>; Christian Walter > <christian.wal...@9elements.com>; Ma, Maurice <maurice...@intel.com>; > Desimone, Nathaniel L <nathaniel.l.desim...@intel.com>; Zeng, Star > <star.z...@intel.com>; Kinney, Michael D <michael.d.kin...@intel.com>; Gao, > Liming <liming....@intel.com> > Subject: [PATCH v2 2/2] MdePkg: Add support for variable size MMCONF space > > From: Patrick Rudolph <patrick.rudo...@9elements.com> > > On embedded AMD platforms the MMCONF window is usually only 64MiB. > > Add support for arbitrary sized MMCONF by introducing a new PCD. > The default size is still 256MiB, but will be overwritten by UefiPayloadPkg > with > the real MMCONF size. > > Fixes crash on platforms not exposing 256 buses. > > Tested on: > * AMD Stoney Ridge > > Signed-off-by: Patrick Rudolph <patrick.rudo...@9elements.com> > Signed-off-by: Marcello Sylvester Bauer <marcello.ba...@9elements.com> > Cc: Patrick Rudolph <patrick.rudo...@9elements.com> > Cc: Christian Walter <christian.wal...@9elements.com> > Cc: Maurice Ma <maurice...@intel.com> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > Cc: Star Zeng <star.z...@intel.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Liming Gao <liming....@intel.com> > --- > MdePkg/MdePkg.dec | 4 + > UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc | 1 + > MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf | 6 +- > UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 1 + > MdePkg/Include/Library/PciExpressLib.h | 5 +- > MdePkg/Library/BasePciExpressLib/PciExpressLib.c | 118 > +++++++++++++++++++- > UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | 4 +- > 7 files changed, 131 insertions(+), 8 deletions(-) > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > 73f6c2407357..02e736a01126 100644 > --- a/MdePkg/MdePkg.dec > +++ b/MdePkg/MdePkg.dec > @@ -2274,6 +2274,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, > PcdsDynamic, PcdsDynamicEx] > # @Prompt PCI Express Base Address. > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xE0000000|UINT64| > 0x0000000a + ## This value is used to set the size of PCI express hierarchy. > The > default is 256 MB.+ # @Prompt PCI Express Base Size.+ > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize|0x0FFFFFFF|UINT64|0x00 > 00000f+ ## Default current ISO 639-2 language: English & French. # > @Prompt Default Value of LangCodes Variable. > gEfiMdePkgTokenSpaceGuid.PcdUefiVariableDefaultLangCodes|"engfraengfra" > |VOID*|0x0000001cdiff --git a/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc > b/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc > index a768a8702c66..162cbf47a83f 100644 > --- a/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc > +++ b/UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc > @@ -363,6 +363,7 @@ [PcdsDynamicDefault] > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutRow|31 > gEfiMdeModulePkgTokenSpaceGuid.PcdConOutColumn|100 > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0+ > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize|0 > ############################################################# > ################### #diff --git > a/MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf > b/MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf > index a7edb74cde71..12734b022ac7 100644 > --- a/MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf > +++ b/MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf > @@ -1,7 +1,7 @@ > ## @file-# Instance of PCI Express Library using the 256 MB PCI Express MMIO > window.+# Instance of PCI Express Library using the variable size PCI Express > MMIO window. #-# PCI Express Library that uses the 256 MB PCI Express MMIO > window to perform+# PCI Express Library that uses the variable size PCI > Express > MMIO window to perform # PCI Configuration cycles. Layers on top of an I/O > Library instance. # # Copyright (c) 2007 - 2018, Intel Corporation. All > rights > reserved.<BR>@@ -38,4 +38,4 @@ [LibraryClasses] > [Pcd] gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress ## > CONSUMES-+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize ## > CONSUMESdiff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf > b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf > index 1371d5eb7952..cebc81135565 100644 > --- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf > +++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf > @@ -54,6 +54,7 @@ [Pcd] > gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution > gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress+ > gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize [Depex] TRUEdiff --git > a/MdePkg/Include/Library/PciExpressLib.h > b/MdePkg/Include/Library/PciExpressLib.h > index 826fdcf7db6c..d78193a0a352 100644 > --- a/MdePkg/Include/Library/PciExpressLib.h > +++ b/MdePkg/Include/Library/PciExpressLib.h > @@ -2,8 +2,9 @@ > Provides services to access PCI Configuration Space using the MMIO PCI > Express window. This library is identical to the PCI Library, except the > access > method for performing PCI- configuration cycles must be through the 256 MB > PCI Express MMIO window whose base address- is defined by > PcdPciExpressBaseAddress.+ configuration cycles must be through the PCI > Express MMIO window whose base address+ is defined by > PcdPciExpressBaseAddress and size defined by PcdPciExpressBaseSize.+ > Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR> SPDX- > License-Identifier: BSD-2-Clause-Patentdiff --git > a/MdePkg/Library/BasePciExpressLib/PciExpressLib.c > b/MdePkg/Library/BasePciExpressLib/PciExpressLib.c > index 99a166c3609b..4d099f2c575e 100644 > --- a/MdePkg/Library/BasePciExpressLib/PciExpressLib.c > +++ b/MdePkg/Library/BasePciExpressLib/PciExpressLib.c > @@ -22,7 +22,8 @@ > /** Assert the validity of a PCI address. A valid PCI address should > contain 1's- > only in the low 28 bits.+ only in the low 28 bits. PcdPciExpressBaseSize > limits the > size to the real+ number of PCI busses in this segment. @param A The > address > to validate. @@ -58,7 +59,6 @@ PciExpressRegisterForRuntimeAccess ( > IN UINTN Address ) {- ASSERT_INVALID_PCI_ADDRESS (Address); return > RETURN_UNSUPPORTED; } @@ -79,6 +79,24 @@ GetPciExpressBaseAddress ( > return (VOID*)(UINTN) PcdGet64 (PcdPciExpressBaseAddress); } +/**+ Gets > the size of PCI Express.++ This internal functions retrieves PCI Express > Base Size > via a PCD entry+ PcdPciExpressBaseSize.++ @return The base address of PCI > Express.++**/+STATIC+UINTN+PcdPciExpressBaseSize (+ VOID+ )+{+ return > (UINTN) PcdGet64 (PcdPciExpressBaseSize);+}+ /** Reads an 8-bit PCI > configuration register. @@ -101,6 +119,9 @@ PciExpressRead8 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT8) ~0;+ } return MmioRead8 > ((UINTN) GetPciExpressBaseAddress () + Address); } @@ -128,6 +149,9 @@ > PciExpressWrite8 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT8) ~0;+ } return MmioWrite8 > ((UINTN) GetPciExpressBaseAddress () + Address, Value); } @@ -159,6 +183,9 > @@ PciExpressOr8 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT8) ~0;+ } return MmioOr8 > ((UINTN) > GetPciExpressBaseAddress () + Address, OrData); } @@ -190,6 +217,9 @@ > PciExpressAnd8 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT8) ~0;+ } return MmioAnd8 > ((UINTN) > GetPciExpressBaseAddress () + Address, AndData); } @@ -224,6 +254,9 @@ > PciExpressAndThenOr8 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT8) ~0;+ } return MmioAndThenOr8 > ( (UINTN) GetPciExpressBaseAddress () + Address, > AndData,@@ - > 261,6 +294,9 @@ PciExpressBitFieldRead8 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT8) ~0;+ } return > MmioBitFieldRead8 > ( (UINTN) GetPciExpressBaseAddress () + Address, > StartBit,@@ - > 302,6 +338,9 @@ PciExpressBitFieldWrite8 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT8) ~0;+ } return > MmioBitFieldWrite8 > ( (UINTN) GetPciExpressBaseAddress () + Address, > StartBit,@@ - > 347,6 +386,9 @@ PciExpressBitFieldOr8 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT8) ~0;+ } return MmioBitFieldOr8 > ( (UINTN) GetPciExpressBaseAddress () + Address, > StartBit,@@ - > 392,6 +434,9 @@ PciExpressBitFieldAnd8 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT8) ~0;+ } return > MmioBitFieldAnd8 > ( (UINTN) GetPciExpressBaseAddress () + Address, > StartBit,@@ - > 442,6 +487,9 @@ PciExpressBitFieldAndThenOr8 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT8) ~0;+ } return > MmioBitFieldAndThenOr8 ( (UINTN) GetPciExpressBaseAddress () + > Address, StartBit,@@ -474,6 +522,9 @@ PciExpressRead16 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return MmioRead16 > ((UINTN) GetPciExpressBaseAddress () + Address); } @@ -502,6 +553,9 @@ > PciExpressWrite16 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return MmioWrite16 > ((UINTN) GetPciExpressBaseAddress () + Address, Value); } @@ -534,6 +588,9 > @@ PciExpressOr16 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return MmioOr16 > ((UINTN) GetPciExpressBaseAddress () + Address, OrData); } @@ -566,6 +623,9 > @@ PciExpressAnd16 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return MmioAnd16 > ((UINTN) GetPciExpressBaseAddress () + Address, AndData); } @@ -601,6 > +661,9 @@ PciExpressAndThenOr16 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return > MmioAndThenOr16 ( (UINTN) GetPciExpressBaseAddress () + Address, > AndData,@@ -639,6 +702,9 @@ PciExpressBitFieldRead16 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return > MmioBitFieldRead16 ( (UINTN) GetPciExpressBaseAddress () + Address, > StartBit,@@ -681,6 +747,9 @@ PciExpressBitFieldWrite16 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return > MmioBitFieldWrite16 ( (UINTN) GetPciExpressBaseAddress () + > Address, > StartBit,@@ -727,6 +796,9 @@ PciExpressBitFieldOr16 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return > MmioBitFieldOr16 > ( (UINTN) GetPciExpressBaseAddress () + Address, > StartBit,@@ - > 773,6 +845,9 @@ PciExpressBitFieldAnd16 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return > MmioBitFieldAnd16 ( (UINTN) GetPciExpressBaseAddress () + Address, > StartBit,@@ -824,6 +899,9 @@ PciExpressBitFieldAndThenOr16 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return > MmioBitFieldAndThenOr16 ( (UINTN) GetPciExpressBaseAddress () + > Address, StartBit,@@ -856,6 +934,9 @@ PciExpressRead32 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return MmioRead32 > ((UINTN) GetPciExpressBaseAddress () + Address); } @@ -884,6 +965,9 @@ > PciExpressWrite32 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return MmioWrite32 > ((UINTN) GetPciExpressBaseAddress () + Address, Value); } @@ -916,6 +1000,9 > @@ PciExpressOr32 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return MmioOr32 > ((UINTN) GetPciExpressBaseAddress () + Address, OrData); } @@ -948,6 > +1035,9 @@ PciExpressAnd32 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return MmioAnd32 > ((UINTN) GetPciExpressBaseAddress () + Address, AndData); } @@ -983,6 > +1073,9 @@ PciExpressAndThenOr32 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return > MmioAndThenOr32 ( (UINTN) GetPciExpressBaseAddress () + Address, > AndData,@@ -1021,6 +1114,9 @@ PciExpressBitFieldRead32 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return > MmioBitFieldRead32 ( (UINTN) GetPciExpressBaseAddress () + Address, > StartBit,@@ -1063,6 +1159,9 @@ PciExpressBitFieldWrite32 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT16) ~0;+ } return > MmioBitFieldWrite32 ( (UINTN) GetPciExpressBaseAddress () + > Address, > StartBit,@@ -1109,6 +1208,9 @@ PciExpressBitFieldOr32 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT32) ~0;+ } return > MmioBitFieldOr32 > ( (UINTN) GetPciExpressBaseAddress () + Address, > StartBit,@@ - > 1155,6 +1257,9 @@ PciExpressBitFieldAnd32 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT32) ~0;+ } return > MmioBitFieldAnd32 ( (UINTN) GetPciExpressBaseAddress () + Address, > StartBit,@@ -1206,6 +1311,9 @@ PciExpressBitFieldAndThenOr32 ( > ) { ASSERT_INVALID_PCI_ADDRESS (Address);+ if (Address >= > PcdPciExpressBaseSize()) {+ return (UINT32) ~0;+ } return > MmioBitFieldAndThenOr32 ( (UINTN) GetPciExpressBaseAddress () + > Address, StartBit,@@ -1249,6 +1357,9 @@ PciExpressReadBuffer ( > UINTN ReturnValue; ASSERT_INVALID_PCI_ADDRESS (StartAddress);+ if > (StartAddress >= PcdPciExpressBaseSize()) {+ return (UINTN) ~0;+ } > ASSERT > (((StartAddress & 0xFFF) + Size) <= 0x1000); if (Size == 0) {@@ -1349,6 > +1460,9 @@ PciExpressWriteBuffer ( > UINTN ReturnValue; > ASSERT_INVALID_PCI_ADDRESS > (StartAddress);+ if (StartAddress >= PcdPciExpressBaseSize()) {+ return > (UINTN) ~0;+ } ASSERT (((StartAddress & 0xFFF) + Size) <= 0x1000); if > (Size > == 0) {diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c > b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c > index a3974dcc02f8..a746d0581ee3 100644 > --- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c > +++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c > @@ -155,13 +155,15 @@ BlDxeEntryPoint ( > } //- // Set PcdPciExpressBaseAddress by HOB info+ // Set > PcdPciExpressBaseAddress and PcdPciExpressBaseSize by HOB info // > GuidHob = GetFirstGuidHob (&gUefiAcpiBoardInfoGuid); 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); } return EFI_SUCCESS;-- > 2.27.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62849): https://edk2.groups.io/g/devel/message/62849 Mute This Topic: https://groups.io/mt/75539546/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-