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]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to