From: "Ma, Maurice" <maurice.ma@intel.com>
To: Marcello Sylvester Bauer <marcello.bauer@9elements.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Patrick Rudolph <patrick.rudolph@9elements.com>,
Christian Walter <christian.walter@9elements.com>,
"Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>,
"Zeng, Star" <star.zeng@intel.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>,
"Gao, Liming" <liming.gao@intel.com>
Subject: Re: [PATCH v2 2/2] MdePkg: Add support for variable size MMCONF space
Date: Mon, 20 Jul 2020 15:26:07 +0000 [thread overview]
Message-ID: <DM6PR11MB2793FB633FA04C376B58C757897B0@DM6PR11MB2793.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20200716114820.14211-3-marcello.bauer@9elements.com>
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.bauer@9elements.com>
> Sent: Thursday, July 16, 2020 4:48
> To: devel@edk2.groups.io
> Cc: Patrick Rudolph <patrick.rudolph@9elements.com>; Christian Walter
> <christian.walter@9elements.com>; Ma, Maurice <maurice.ma@intel.com>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> Liming <liming.gao@intel.com>
> Subject: [PATCH v2 2/2] MdePkg: Add support for variable size MMCONF space
>
> From: Patrick Rudolph <patrick.rudolph@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.rudolph@9elements.com>
> Signed-off-by: Marcello Sylvester Bauer <marcello.bauer@9elements.com>
> Cc: Patrick Rudolph <patrick.rudolph@9elements.com>
> Cc: Christian Walter <christian.walter@9elements.com>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@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
next prev parent reply other threads:[~2020-07-20 15:26 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-16 11:48 [PATCH v2 0/2] UefiPayloadPkg: Runtime MMCONF Marcello Sylvester Bauer
2020-07-16 11:48 ` [PATCH v2 1/2] UefiPayloadPkg: Store the real size of the MMCONF window Marcello Sylvester Bauer
2020-07-20 15:22 ` Ma, Maurice
2020-07-16 11:48 ` [PATCH v2 2/2] MdePkg: Add support for variable size MMCONF space Marcello Sylvester Bauer
2020-07-20 15:26 ` Ma, Maurice [this message]
2020-07-21 0:49 ` Liming Gao
2020-07-22 12:55 ` [edk2-devel] " Marcello Sylvester Bauer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR11MB2793FB633FA04C376B58C757897B0@DM6PR11MB2793.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox