public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg
@ 2023-03-17  6:50 Abdul Lateef Attar
  2023-03-17  6:50 ` [PATCH v2 RESEND 1/4] Platform/AMD: Adds BoardPkg and PlatformPkg Abdul Lateef Attar
                   ` (4 more replies)
  0 siblings, 5 replies; 28+ messages in thread
From: Abdul Lateef Attar @ 2023-03-17  6:50 UTC (permalink / raw)
  To: devel
  Cc: Abdul Lateef Attar, Ard Biesheuvel, Leif Lindholm, Abner Chang,
	Michael D Kinney

Resending with correct email

Adds AMD/BoardPkg to support MinPlatformPkg framework.
Adds AMD/PlatformPkg, which provide supporting modules and libraries for AMD based platform.

PR: https://github.com/tianocore/edk2-platforms/pull/69

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>

Abdul Lateef Attar (4):
  Platform/AMD: Adds BoardPkg and PlatformPkg
  Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation
  Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library
  Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers

 Platform/AMD/BoardPkg/BoardPkg.dec            |  18 +
 Platform/AMD/PlatformPkg/PlatformPkg.dec      |  31 ++
 Platform/AMD/BoardPkg/BoardPkg.dsc            |  30 ++
 Platform/AMD/PlatformPkg/PlatformPkg.dsc      |  43 +++
 .../SetCacheMtrrLib/SetCacheMtrrLib.inf       |  37 ++
 .../PlatformPkg/PciHotPlug/PciHotPlugInit.inf |  41 +++
 .../Library/SetCacheMtrrLib/SetCacheMtrrLib.c | 132 +++++++
 .../PlatformPkg/PciHotPlug/PciHotPlugInit.c   | 340 ++++++++++++++++++
 Maintainers.txt                               |   6 +
 9 files changed, 678 insertions(+)
 create mode 100644 Platform/AMD/BoardPkg/BoardPkg.dec
 create mode 100644 Platform/AMD/PlatformPkg/PlatformPkg.dec
 create mode 100644 Platform/AMD/BoardPkg/BoardPkg.dsc
 create mode 100644 Platform/AMD/PlatformPkg/PlatformPkg.dsc
 create mode 100644 Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
 create mode 100644 Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
 create mode 100644 Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
 create mode 100644 Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c

-- 
2.25.1


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 RESEND 1/4] Platform/AMD: Adds BoardPkg and PlatformPkg
  2023-03-17  6:50 [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg Abdul Lateef Attar
@ 2023-03-17  6:50 ` Abdul Lateef Attar
  2023-03-17 15:41   ` Leif Lindholm
  2023-03-17  6:50 ` [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation Abdul Lateef Attar
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 28+ messages in thread
From: Abdul Lateef Attar @ 2023-03-17  6:50 UTC (permalink / raw)
  To: devel
  Cc: Abdul Lateef Attar, Ard Biesheuvel, Leif Lindholm, Abner Chang,
	Michael D Kinney

Adds initial DEC and DSC file for BoardPkg and PlatformPkg packages,
which supports AMD processor family based boards and platforms.

Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Abner Chang <abner.chang@amd.com>
---
 Platform/AMD/BoardPkg/BoardPkg.dec       | 18 ++++++++++++++++++
 Platform/AMD/PlatformPkg/PlatformPkg.dec | 15 +++++++++++++++
 Platform/AMD/BoardPkg/BoardPkg.dsc       | 20 ++++++++++++++++++++
 Platform/AMD/PlatformPkg/PlatformPkg.dsc | 20 ++++++++++++++++++++
 4 files changed, 73 insertions(+)
 create mode 100644 Platform/AMD/BoardPkg/BoardPkg.dec
 create mode 100644 Platform/AMD/PlatformPkg/PlatformPkg.dec
 create mode 100644 Platform/AMD/BoardPkg/BoardPkg.dsc
 create mode 100644 Platform/AMD/PlatformPkg/PlatformPkg.dsc

diff --git a/Platform/AMD/BoardPkg/BoardPkg.dec b/Platform/AMD/BoardPkg/BoardPkg.dec
new file mode 100644
index 000000000000..3ceff33f4fc1
--- /dev/null
+++ b/Platform/AMD/BoardPkg/BoardPkg.dec
@@ -0,0 +1,18 @@
+## @file BoardPkg.dec
+#  Declaration file for AMD's BoardPkg.
+#
+#  This package supports AMD processor family based board as per the MinPlatform
+#  Arch specification.
+#
+#  Copyright (c) 2023, Advanced Micro Devices, Inc. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#  @par Specification Reference:
+#   -https://tianocore-docs.github.io/edk2-MinimumPlatformSpecification/draft/ 0.7
+##
+
+[Defines]
+  DEC_SPECIFICATION              = 1.27
+  PACKAGE_NAME                   = BoardPkg
+  PACKAGE_GUID                   = 44F9D761-9ECB-43DD-A5AC-177E5048701B
+  PACKAGE_VERSION                = 0.1
diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dec b/Platform/AMD/PlatformPkg/PlatformPkg.dec
new file mode 100644
index 000000000000..6155860979cb
--- /dev/null
+++ b/Platform/AMD/PlatformPkg/PlatformPkg.dec
@@ -0,0 +1,15 @@
+## @file PlatformPkg.dec
+#  Declaration file for AMD's PlatformPkg.
+#
+#  This package supports AMD processory family based platform.
+#
+#  Copyright (c) 2023, Advanced Micro Devices, Inc. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  DEC_SPECIFICATION              = 1.27
+  PACKAGE_NAME                   = PlatformPkg
+  PACKAGE_GUID                   = 38FBA311-E2AA-4620-9A90-9A23753D1878
+  PACKAGE_VERSION                = 0.1
diff --git a/Platform/AMD/BoardPkg/BoardPkg.dsc b/Platform/AMD/BoardPkg/BoardPkg.dsc
new file mode 100644
index 000000000000..cb4065b86c60
--- /dev/null
+++ b/Platform/AMD/BoardPkg/BoardPkg.dsc
@@ -0,0 +1,20 @@
+## @file
+#  BoardPkg.dsc
+#
+#  Description file for AMD BoardPkg
+#
+#  Copyright (c) 2023, Advanced Micro Devices, Inc. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  DSC_SPECIFICATION           = 1.30
+  PLATFORM_GUID               = 88F8A9AE-2FA0-4D58-A6F9-05F635C05F4E
+  PLATFORM_NAME               = BoardPkg
+  PLATFORM_VERSION            = 0.1
+  OUTPUT_DIRECTORY            = Build/$(PLATFORM_NAME)
+  BUILD_TARGETS               = DEBUG | RELEASE | NOOPT
+  SUPPORTED_ARCHITECTURES     = IA32 | X64
+
+[Packages]
+  BoardPkg/BoardPkg.dec
diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dsc b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
new file mode 100644
index 000000000000..704566b9ea73
--- /dev/null
+++ b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
@@ -0,0 +1,20 @@
+## @file
+#  PlatformPkg.dsc
+#
+#  Description file for AMD PlatformPkg
+#
+#  Copyright (c) 2023, Advanced Micro Devices, Inc. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  DSC_SPECIFICATION           = 1.30
+  PLATFORM_GUID               = 2F7C29F2-7F35-4B49-B97D-F0E61BD42FC0
+  PLATFORM_NAME               = PlatformPkg
+  PLATFORM_VERSION            = 0.1
+  OUTPUT_DIRECTORY            = Build/$(PLATFORM_NAME)
+  BUILD_TARGETS               = DEBUG | RELEASE | NOOPT
+  SUPPORTED_ARCHITECTURES     = IA32 | X64
+
+[Packages]
+  PlatformPkg/PlatformPkg.dec
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation
  2023-03-17  6:50 [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg Abdul Lateef Attar
  2023-03-17  6:50 ` [PATCH v2 RESEND 1/4] Platform/AMD: Adds BoardPkg and PlatformPkg Abdul Lateef Attar
@ 2023-03-17  6:50 ` Abdul Lateef Attar
  2023-03-17  6:52   ` Chang, Abner
  2023-03-17 15:55   ` [edk2-devel] " Leif Lindholm
  2023-03-17  6:50 ` [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library Abdul Lateef Attar
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 28+ messages in thread
From: Abdul Lateef Attar @ 2023-03-17  6:50 UTC (permalink / raw)
  To: devel
  Cc: Abdul Lateef Attar, Ard Biesheuvel, Leif Lindholm, Abner Chang,
	Michael D Kinney

Adds PCI hotplug init protocol implementation.

Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
---
 Platform/AMD/PlatformPkg/PlatformPkg.dec      |  16 +
 Platform/AMD/PlatformPkg/PlatformPkg.dsc      |  23 ++
 .../PlatformPkg/PciHotPlug/PciHotPlugInit.inf |  41 +++
 .../PlatformPkg/PciHotPlug/PciHotPlugInit.c   | 340 ++++++++++++++++++
 4 files changed, 420 insertions(+)
 create mode 100644 Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
 create mode 100644 Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c

diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dec b/Platform/AMD/PlatformPkg/PlatformPkg.dec
index 6155860979cb..1bc38d6025c3 100644
--- a/Platform/AMD/PlatformPkg/PlatformPkg.dec
+++ b/Platform/AMD/PlatformPkg/PlatformPkg.dec
@@ -13,3 +13,19 @@ [Defines]
   PACKAGE_NAME                   = PlatformPkg
   PACKAGE_GUID                   = 38FBA311-E2AA-4620-9A90-9A23753D1878
   PACKAGE_VERSION                = 0.1
+
+[Guids]
+  gPlatformPkgTokenSpaceGuid     = { 0x95ECA58D, 0x09B6, 0x4420, { 0xB4, 0xE7, 0x01, 0x7F, 0x6A, 0x5B, 0x26, 0x0F }}
+
+[PcdsDynamic, PcdsDynamicEx]
+  #
+  # PCI HotPlug Resource Padding
+  #
+  # IO Resource padding in bytes, default 4KB
+  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadIO|0x00001000|UINT64|0x10000000
+  # PreFetch Memory padding in bytes, default 2MB
+  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem|0x00200000|UINT64|0x10000001
+  # Non-PreFetch Memory padding in bytes, default 1MB
+  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadMem|0x00100000|UINT64|0x10000002
+  # PCI bus padding, number of bus to reserve, default 2 bus
+  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadBus|2|UINT8|0x10000003
diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dsc b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
index 704566b9ea73..9a693070ab3f 100644
--- a/Platform/AMD/PlatformPkg/PlatformPkg.dsc
+++ b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
@@ -16,5 +16,28 @@ [Defines]
   BUILD_TARGETS               = DEBUG | RELEASE | NOOPT
   SUPPORTED_ARCHITECTURES     = IA32 | X64
 
+
 [Packages]
   PlatformPkg/PlatformPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses.Common]
+  UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
+  UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
+  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
+  MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
+  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
+  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
+  RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf
+  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
+  DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
+  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+  PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
+  PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
+  PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
+
+[Components.X64]
+  PlatformPkg/PciHotPlug/PciHotPlugInit.inf
diff --git a/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
new file mode 100644
index 000000000000..0079c4acf14e
--- /dev/null
+++ b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
@@ -0,0 +1,41 @@
+## @file
+# This driver implements EFI_PCI_HOT_PLUG_INIT_PROTOCOL.
+# Adds resource padding information, for PCIe hotplug purposes.
+#
+# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+  INF_VERSION                    = 1.29
+  BASE_NAME                      = PciHotPlugInit
+  FILE_GUID                      = 8B67D95F-78B7-484F-8F16-5F22AB388B0C
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 0.1
+  ENTRY_POINT                    = PciHotPlugInitialize
+
+[Sources]
+  PciHotPlugInit.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  PlatformPkg/PlatformPkg.dec
+
+[LibraryClasses]
+  UefiDriverEntryPoint
+  UefiBootServicesTableLib
+  DebugLib
+  MemoryAllocationLib
+
+[Protocols]
+  gEfiPciHotPlugInitProtocolGuid
+
+[Pcd]
+  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadIO
+  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem
+  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadMem
+  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadBus
+
+[Depex]
+  TRUE
diff --git a/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
new file mode 100644
index 000000000000..b977406bbcae
--- /dev/null
+++ b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
@@ -0,0 +1,340 @@
+/** @file
+  This file declares EFI PCI Hot Plug Init Protocol.
+
+  This protocol provides the necessary functionality to initialize the Hot Plug
+  Controllers (HPCs) and the buses that they control. This protocol also provides
+  information regarding resource padding.
+
+  @par Note:
+    This source has the reference of OVMF PciHotPluginit.c and Intel platform PciHotPlug.c.
+
+    This protocol is required only on platforms that support one or more PCI Hot
+    Plug* slots or CardBus sockets.
+
+  The EFI_PCI_HOT_PLUG_INIT_PROTOCOL provides a mechanism for the PCI bus enumerator
+  to properly initialize the HPCs and CardBus sockets that require initialization.
+  The HPC initialization takes place before the PCI enumeration process is complete.
+  There cannot be more than one instance of this protocol in a system. This protocol
+  is installed on its own separate handle.
+
+  Because the system may include multiple HPCs, one instance of this protocol
+  should represent all of them. The protocol functions use the device path of
+  the HPC to identify the HPC. When the PCI bus enumerator finds a root HPC, it
+  will call EFI_PCI_HOT_PLUG_INIT_PROTOCOL.InitializeRootHpc(). If InitializeRootHpc()
+  is unable to initialize a root HPC, the PCI enumerator will ignore that root HPC
+  and continue the enumeration process. If the HPC is not initialized, the devices
+  that it controls may not be initialized, and no resource padding will be provided.
+
+  From the standpoint of the PCI bus enumerator, HPCs are divided into the following
+  two classes:
+
+    - Root HPC:
+        These HPCs must be initialized by calling InitializeRootHpc() during the
+        enumeration process. These HPCs will also require resource padding. The
+        platform code must have a priori knowledge of these devices and must know
+        how to initialize them. There may not be any way to access their PCI
+        configuration space before the PCI enumerator programs all the upstream
+        bridges and thus enables the path to these devices. The PCI bus enumerator
+        is responsible for determining the PCI bus address of the HPC before it
+        calls InitializeRootHpc().
+    - Nonroot HPC:
+        These HPCs will not need explicit initialization during enumeration process.
+        These HPCs will require resource padding. The platform code does not have
+        to have a priori knowledge of these devices.
+
+  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (C) 2016, Red Hat, Inc.<BR>
+  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Revision Reference:
+  This Protocol is defined in UEFI Platform Initialization Specification 1.2
+  Volume 5: Standards
+
+**/
+
+#include <Uefi/UefiBaseType.h>
+#include <Uefi/UefiSpec.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Protocol/PciHotPlugInit.h>
+
+//
+// The protocol interface this driver produces.
+//
+STATIC EFI_PCI_HOT_PLUG_INIT_PROTOCOL  mPciHotPlugInit;
+
+/**
+  Returns a list of root Hot Plug Controllers (HPCs) that require initialization
+  during the boot process.
+
+  This procedure returns a list of root HPCs. The PCI bus driver must initialize
+  these controllers during the boot process. The PCI bus driver may or may not be
+  able to detect these HPCs. If the platform includes a PCI-to-CardBus bridge, it
+  can be included in this list if it requires initialization.  The HpcList must be
+  self consistent. An HPC cannot control any of its parent buses. Only one HPC can
+  control a PCI bus. Because this list includes only root HPCs, no HPC in the list
+  can be a child of another HPC. This policy must be enforced by the
+  EFI_PCI_HOT_PLUG_INIT_PROTOCOL.   The PCI bus driver may not check for such
+  invalid conditions.  The callee allocates the buffer HpcList
+
+  @param[in]  This       Pointer to the EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
+  @param[out] HpcCount   The number of root HPCs that were returned.
+  @param[out] HpcList    The list of root HPCs. HpcCount defines the number of
+                         elements in this list.
+
+  @retval EFI_SUCCESS             HpcList was returned.
+  @retval EFI_OUT_OF_RESOURCES    HpcList was not returned due to insufficient
+                                  resources.
+  @retval EFI_INVALID_PARAMETER   HpcCount is NULL or HpcList is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+GetRootHpcList (
+  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL  *This,
+  OUT UINTN                           *HpcCount,
+  OUT EFI_HPC_LOCATION                **HpcList
+  )
+{
+  if ((HpcCount == NULL) || (HpcList == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // Platform BIOS not doing any extra/special HPC initialization
+  // Hence returning the HpcCount as zero and HpcList as NULL
+  //
+  *HpcCount = 0;
+  *HpcList  = NULL;
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Initializes one root Hot Plug Controller (HPC). This process may causes
+  initialization of its subordinate buses.
+
+  This function initializes the specified HPC. At the end of initialization,
+  the hot-plug slots or sockets (controlled by this HPC) are powered and are
+  connected to the bus. All the necessary registers in the HPC are set up. For
+  a Standard (PCI) Hot Plug Controller (SHPC), the registers that must be set
+  up are defined in the PCI Standard Hot Plug Controller and Subsystem
+  Specification.
+
+  @param[in]  This            Pointer to the EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
+  @param[in]  HpcDevicePath   The device path to the HPC that is being initialized.
+  @param[in]  HpcPciAddress   The address of the HPC function on the PCI bus.
+  @param[in]  Event           The event that should be signaled when the HPC
+                              initialization is complete.  Set to NULL if the
+                              caller wants to wait until the entire initialization
+                              process is complete.
+  @param[out] HpcState        The state of the HPC hardware. The state is
+                              EFI_HPC_STATE_INITIALIZED or EFI_HPC_STATE_ENABLED.
+
+  @retval EFI_SUCCESS             If Event is NULL, the specific HPC was successfully
+                                  initialized. If Event is not NULL, Event will be
+                                  signaled at a later time when initialization is complete.
+  @retval EFI_UNSUPPORTED         This instance of EFI_PCI_HOT_PLUG_INIT_PROTOCOL
+                                  does not support the specified HPC.
+  @retval EFI_OUT_OF_RESOURCES    Initialization failed due to insufficient
+                                  resources.
+  @retval EFI_INVALID_PARAMETER   HpcState is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+InitializeRootHpc (
+  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL *This,
+  IN  EFI_DEVICE_PATH_PROTOCOL *HpcDevicePath,
+  IN  UINT64 HpcPciAddress,
+  IN  EFI_EVENT Event, OPTIONAL
+  OUT EFI_HPC_STATE                   *HpcState
+  )
+{
+  if (HpcState == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // This Platform doesnt have any non-enumerated HPC.
+  // Hence no extra initialization required from Platform BIOS.
+  //
+  return EFI_UNSUPPORTED;
+}
+
+/**
+  Returns the resource padding that is required by the PCI bus that is controlled
+  by the specified Hot Plug Controller (HPC).
+
+  This function returns the resource padding that is required by the PCI bus that
+  is controlled by the specified HPC. This member function is called for all the
+  root HPCs and nonroot HPCs that are detected by the PCI bus enumerator. This
+  function will be called before PCI resource allocation is completed. This function
+  must be called after all the root HPCs, with the possible exception of a
+  PCI-to-CardBus bridge, have completed initialization.
+
+  @param[in]  This            Pointer to the EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
+  @param[in]  HpcDevicePath   The device path to the HPC.
+  @param[in]  HpcPciAddress   The address of the HPC function on the PCI bus.
+  @param[out] HpcState        The state of the HPC hardware.
+  @param[out] Padding         The amount of resource padding that is required by the
+                              PCI bus under the control of the specified HPC.
+  @param[out] Attributes      Describes how padding is accounted for. The padding
+                              is returned in the form of ACPI 2.0 resource descriptors.
+
+  @retval EFI_SUCCESS             The resource padding was successfully returned.
+  @retval EFI_UNSUPPORTED         This instance of the EFI_PCI_HOT_PLUG_INIT_PROTOCOL
+                                  does not support the specified HPC.
+  @retval EFI_NOT_READY           This function was called before HPC initialization
+                                  is complete.
+  @retval EFI_INVALID_PARAMETER   HpcState or Padding or Attributes is NULL.
+  @retval EFI_OUT_OF_RESOURCES    ACPI 2.0 resource descriptors for Padding
+                                  cannot be allocated due to insufficient resources.
+
+**/
+EFI_STATUS
+EFIAPI
+GetResourcePadding (
+  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL  *This,
+  IN  EFI_DEVICE_PATH_PROTOCOL        *HpcDevicePath,
+  IN  UINT64                          HpcPciAddress,
+  OUT EFI_HPC_STATE                   *HpcState,
+  OUT VOID                            **Padding,
+  OUT EFI_HPC_PADDING_ATTRIBUTES      *Attributes
+  )
+{
+  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  *PaddingResource;
+
+  //
+  // Need total 5 resources
+  // 1 - IO resource
+  // 2 - Mem resource
+  // 3 - PMem resource
+  // 4 - Bus resource
+  // 5 - end tag resource
+  PaddingResource = AllocateZeroPool (4 * sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + sizeof (EFI_ACPI_END_TAG_DESCRIPTOR));
+  if (PaddingResource == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  *Padding = (VOID *)PaddingResource;
+
+  //
+  // Padding for bus
+  //
+  *Attributes = EfiPaddingPciBus;
+
+  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
+  PaddingResource->Len  = (UINT16)(
+                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
+                                   OFFSET_OF (
+                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
+                                     ResType
+                                     )
+                                   );
+  PaddingResource->ResType      = ACPI_ADDRESS_SPACE_TYPE_BUS;
+  PaddingResource->GenFlag      = 0x0;
+  PaddingResource->SpecificFlag = 0;
+  PaddingResource->AddrRangeMin = 0;
+  PaddingResource->AddrRangeMax = 0;
+  PaddingResource->AddrLen      = PcdGet8 (PcdPciHotPlugResourcePadBus);
+
+  //
+  // Padding for non-prefetchable memory
+  //
+  PaddingResource++;
+  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
+  PaddingResource->Len  = (UINT16)(
+                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
+                                   OFFSET_OF (
+                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
+                                     ResType
+                                     )
+                                   );
+  PaddingResource->ResType              = ACPI_ADDRESS_SPACE_TYPE_MEM;
+  PaddingResource->GenFlag              = 0x0;
+  PaddingResource->AddrSpaceGranularity = 32;
+  PaddingResource->SpecificFlag         = EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_NON_CACHEABLE;
+  PaddingResource->AddrRangeMin         = 0;
+  PaddingResource->AddrLen              = PcdGet64 (PcdPciHotPlugResourcePadMem);
+  PaddingResource->AddrRangeMax         = PaddingResource->AddrLen - 1;
+
+  //
+  // Padding for prefetchable memory
+  //
+  PaddingResource++;
+  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
+  PaddingResource->Len  = (UINT16)(
+                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
+                                   OFFSET_OF (
+                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
+                                     ResType
+                                     )
+                                   );
+  PaddingResource->ResType              = ACPI_ADDRESS_SPACE_TYPE_MEM;
+  PaddingResource->GenFlag              = 0x0;
+  PaddingResource->AddrSpaceGranularity = 32;
+  PaddingResource->SpecificFlag         = EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE;
+  PaddingResource->AddrLen              = PcdGet64 (PcdPciHotPlugResourcePadPMem);
+  PaddingResource->AddrRangeMax         = PaddingResource->AddrLen - 1;
+
+  //
+  // Padding for I/O
+  //
+  PaddingResource++;
+  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
+  PaddingResource->Len  = (UINT16)(
+                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
+                                   OFFSET_OF (
+                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
+                                     ResType
+                                     )
+                                   );
+  PaddingResource->ResType      = ACPI_ADDRESS_SPACE_TYPE_IO;
+  PaddingResource->GenFlag      = 0x0;
+  PaddingResource->SpecificFlag = 0;
+  PaddingResource->AddrRangeMin = 0;
+  PaddingResource->AddrLen      = PcdGet64 (PcdPciHotPlugResourcePadIO);
+  PaddingResource->AddrRangeMax = PaddingResource->AddrLen - 1;
+
+  //
+  // Terminate the entries.
+  //
+  PaddingResource++;
+  ((EFI_ACPI_END_TAG_DESCRIPTOR *)PaddingResource)->Desc     = ACPI_END_TAG_DESCRIPTOR;
+  ((EFI_ACPI_END_TAG_DESCRIPTOR *)PaddingResource)->Checksum = 0x0;
+
+  *HpcState = EFI_HPC_STATE_INITIALIZED | EFI_HPC_STATE_ENABLED;
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Entry point for this driver.
+
+  @param[in] ImageHandle  Image handle of this driver.
+  @param[in] SystemTable  Pointer to SystemTable.
+
+  @retval EFI_SUCESS       Driver has loaded successfully.
+  @return                  Error codes from lower level functions.
+
+**/
+EFI_STATUS
+EFIAPI
+PciHotPlugInitialize (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  mPciHotPlugInit.GetRootHpcList     = GetRootHpcList;
+  mPciHotPlugInit.InitializeRootHpc  = InitializeRootHpc;
+  mPciHotPlugInit.GetResourcePadding = GetResourcePadding;
+  return gBS->InstallMultipleProtocolInterfaces (
+                &ImageHandle,
+                &gEfiPciHotPlugInitProtocolGuid,
+                &mPciHotPlugInit,
+                NULL
+                );
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library
  2023-03-17  6:50 [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg Abdul Lateef Attar
  2023-03-17  6:50 ` [PATCH v2 RESEND 1/4] Platform/AMD: Adds BoardPkg and PlatformPkg Abdul Lateef Attar
  2023-03-17  6:50 ` [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation Abdul Lateef Attar
@ 2023-03-17  6:50 ` Abdul Lateef Attar
  2023-03-17  6:52   ` Chang, Abner
  2023-03-17 15:59   ` Leif Lindholm
  2023-03-17  6:50 ` [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers Abdul Lateef Attar
  2023-03-17  6:54 ` [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg Chang, Abner
  4 siblings, 2 replies; 28+ messages in thread
From: Abdul Lateef Attar @ 2023-03-17  6:50 UTC (permalink / raw)
  To: devel
  Cc: Abdul Lateef Attar, Ard Biesheuvel, Leif Lindholm, Abner Chang,
	Michael D Kinney

Adds SetCacheMtrrLib library for AMD processor based boards.
This library sets MTRR value or various memory ranges.

Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
---
 Platform/AMD/BoardPkg/BoardPkg.dsc            |  10 ++
 .../SetCacheMtrrLib/SetCacheMtrrLib.inf       |  37 +++++
 .../Library/SetCacheMtrrLib/SetCacheMtrrLib.c | 132 ++++++++++++++++++
 3 files changed, 179 insertions(+)
 create mode 100644 Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
 create mode 100644 Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c

diff --git a/Platform/AMD/BoardPkg/BoardPkg.dsc b/Platform/AMD/BoardPkg/BoardPkg.dsc
index cb4065b86c60..aa0ee8287cd8 100644
--- a/Platform/AMD/BoardPkg/BoardPkg.dsc
+++ b/Platform/AMD/BoardPkg/BoardPkg.dsc
@@ -18,3 +18,13 @@ [Defines]
 
 [Packages]
   BoardPkg/BoardPkg.dec
+  MinPlatformPkg/MinPlatformPkg.dec
+  MdePkg/MdePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses.common.PEIM]
+  SetCacheMtrrLib|BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
+
+[Components.IA32]
+  BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
+
diff --git a/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
new file mode 100644
index 000000000000..c66661d3f8dc
--- /dev/null
+++ b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
@@ -0,0 +1,37 @@
+## @file
+# Component information file for Platform SetCacheMtrr Library.
+# This library implementation is for AMD processor based platforms.
+#
+# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 1.29
+  BASE_NAME                      = PeiSetCacheMtrrLib
+  FILE_GUID                      = 1E8468E0-5EB4-4088-9B52-BFDC6E4DAE87
+  MODULE_TYPE                    = PEIM
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = SetCacheMtrrLib
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  MtrrLib
+
+[Packages]
+  MinPlatformPkg/MinPlatformPkg.dec
+  MdePkg/MdePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+
+[Sources]
+  SetCacheMtrrLib.c
+
+[Guids]
+
+[Pcd]
+  gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress
+  gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize
+
diff --git a/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
new file mode 100644
index 000000000000..18404405d9fa
--- /dev/null
+++ b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
@@ -0,0 +1,132 @@
+/** @file
+
+SetCacheMtrr library functions.
+This library implementation is for AMD processor based platforms.
+
+Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Uefi.h>
+#include <PiPei.h>
+#include <Library/DebugLib.h>
+#include <Library/MtrrLib.h>
+
+/**
+  This function sets the cache MTRR values for PEI phase.
+**/
+VOID
+EFIAPI
+SetCacheMtrr (
+  VOID
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = MtrrSetMemoryAttribute (
+             0,
+             0xA0000,
+             CacheWriteBack
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Error(%r) in setting CacheWriteBack for 0-0x9FFFF\n",
+      Status
+      ));
+  }
+
+  Status = MtrrSetMemoryAttribute (
+             0xA0000,
+             0x20000,
+             CacheUncacheable
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Error(%r) in setting CacheUncacheable for 0xA0000-0xBFFFF\n",
+      Status
+      ));
+  }
+
+  Status = MtrrSetMemoryAttribute (
+             0xC0000,
+             0x40000,
+             CacheWriteProtected
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Error(%r) in setting CacheWriteProtected for 0xC0000-0xFFFFF\n",
+      Status
+      ));
+  }
+
+  Status = MtrrSetMemoryAttribute (
+             0x100000,
+             0xAFF00000,
+             CacheWriteBack
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Error(%r) in setting CacheWriteBack for 0x100000-0xAFFFFFFF\n",
+      Status
+      ));
+  }
+
+  Status = MtrrSetMemoryAttribute (
+             PcdGet32 (PcdFlashAreaBaseAddress),
+             PcdGet32 (PcdFlashAreaSize),
+             CacheWriteProtected
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Error(%r) in setting CacheWriteProtected for 0x%X-0x%X\n",
+      Status,
+      PcdGet32 (PcdFlashAreaBaseAddress),
+      PcdGet32 (PcdFlashAreaBaseAddress) + PcdGet32 (PcdFlashAreaSize)
+      ));
+  }
+
+  MtrrDebugPrintAllMtrrs ();
+  return;
+}
+
+/**
+  Update MTRR setting in EndOfPei phase.
+  This function will set the MTRR value as CacheUncacheable
+  for Flash address.
+
+  @retval  EFI_SUCCESS  The function completes successfully.
+  @retval  Others       Some error occurs.
+**/
+EFI_STATUS
+EFIAPI
+SetCacheMtrrAfterEndOfPei (
+  VOID
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = MtrrSetMemoryAttribute (
+             PcdGet32 (PcdFlashAreaBaseAddress),
+             PcdGet32 (PcdFlashAreaSize),
+             CacheUncacheable
+             );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "Error(%r) in setting CacheUncacheable for 0x%X-0x%X\n",
+      Status,
+      PcdGet32 (PcdFlashAreaBaseAddress),
+      PcdGet32 (PcdFlashAreaBaseAddress) + PcdGet32 (PcdFlashAreaSize)
+      ));
+  }
+
+  MtrrDebugPrintAllMtrrs ();
+  return EFI_SUCCESS;
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-17  6:50 [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg Abdul Lateef Attar
                   ` (2 preceding siblings ...)
  2023-03-17  6:50 ` [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library Abdul Lateef Attar
@ 2023-03-17  6:50 ` Abdul Lateef Attar
  2023-03-17  6:53   ` Chang, Abner
  2023-03-17 16:09   ` Leif Lindholm
  2023-03-17  6:54 ` [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg Chang, Abner
  4 siblings, 2 replies; 28+ messages in thread
From: Abdul Lateef Attar @ 2023-03-17  6:50 UTC (permalink / raw)
  To: devel
  Cc: Abdul Lateef Attar, Abdul Lateef Attar, Ard Biesheuvel,
	Leif Lindholm, Abner Chang, Michael D Kinney

From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>

Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.

Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Leif Lindholm <quic_llindhol@quicinc.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
---
 Maintainers.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 747191366070..bb8ab643e090 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
 M: Ard Biesheuvel <ardb+tianocore@kernel.org>
 M: Leif Lindholm <quic_llindhol@quicinc.com>
 
+AMD Platform
+F: Platform/AMD/BoardPkg
+F: Platform/AMD/PlatformPkg
+M: Abner Chang <abner.chang@amd.com>
+M: Abdul Lateef Attar <abdattar@amd.com>
+
 Ampere Computing
 F: Platform/Ampere
 F: Silicon/Ampere
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation
  2023-03-17  6:50 ` [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation Abdul Lateef Attar
@ 2023-03-17  6:52   ` Chang, Abner
  2023-03-17 15:55   ` [edk2-devel] " Leif Lindholm
  1 sibling, 0 replies; 28+ messages in thread
From: Chang, Abner @ 2023-03-17  6:52 UTC (permalink / raw)
  To: Attar, AbdulLateef (Abdul Lateef), devel@edk2.groups.io
  Cc: Attar, AbdulLateef (Abdul Lateef), Ard Biesheuvel, Leif Lindholm,
	Michael D Kinney

[AMD Official Use Only - General]

Reviewed-by: Abner Chang <abner.chang@amd.com>

> -----Original Message-----
> From: Abdul Lateef Attar <abdattar@amd.com>
> Sent: Friday, March 17, 2023 2:50 PM
> To: devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm
> <quic_llindhol@quicinc.com>; Chang, Abner <Abner.Chang@amd.com>;
> Michael D Kinney <michael.d.kinney@intel.com>
> Subject: [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds
> PciHotPlug init protocol implementation
> 
> Adds PCI hotplug init protocol implementation.
> 
> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  Platform/AMD/PlatformPkg/PlatformPkg.dec      |  16 +
>  Platform/AMD/PlatformPkg/PlatformPkg.dsc      |  23 ++
>  .../PlatformPkg/PciHotPlug/PciHotPlugInit.inf |  41 +++
>  .../PlatformPkg/PciHotPlug/PciHotPlugInit.c   | 340 ++++++++++++++++++
>  4 files changed, 420 insertions(+)
>  create mode 100644
> Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
>  create mode 100644
> Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> 
> diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dec
> b/Platform/AMD/PlatformPkg/PlatformPkg.dec
> index 6155860979cb..1bc38d6025c3 100644
> --- a/Platform/AMD/PlatformPkg/PlatformPkg.dec
> +++ b/Platform/AMD/PlatformPkg/PlatformPkg.dec
> @@ -13,3 +13,19 @@ [Defines]
>    PACKAGE_NAME                   = PlatformPkg
>    PACKAGE_GUID                   = 38FBA311-E2AA-4620-9A90-9A23753D1878
>    PACKAGE_VERSION                = 0.1
> +
> +[Guids]
> +  gPlatformPkgTokenSpaceGuid     = { 0x95ECA58D, 0x09B6, 0x4420, { 0xB4,
> 0xE7, 0x01, 0x7F, 0x6A, 0x5B, 0x26, 0x0F }}
> +
> +[PcdsDynamic, PcdsDynamicEx]
> +  #
> +  # PCI HotPlug Resource Padding
> +  #
> +  # IO Resource padding in bytes, default 4KB
> +
> +gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadIO|0x00001000|
> UINT64
> +|0x10000000
> +  # PreFetch Memory padding in bytes, default 2MB
> +
> +gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem|0x002000
> 00|UINT
> +64|0x10000001
> +  # Non-PreFetch Memory padding in bytes, default 1MB
> +
> +gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadMem|0x0010000
> 0|UINT6
> +4|0x10000002
> +  # PCI bus padding, number of bus to reserve, default 2 bus
> +
> +gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadBus|2|UINT8|0x
> 100000
> +03
> diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> index 704566b9ea73..9a693070ab3f 100644
> --- a/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> +++ b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> @@ -16,5 +16,28 @@ [Defines]
>    BUILD_TARGETS               = DEBUG | RELEASE | NOOPT
>    SUPPORTED_ARCHITECTURES     = IA32 | X64
> 
> +
>  [Packages]
>    PlatformPkg/PlatformPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses.Common]
> +
> +UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntr
> +yPoint.inf
> +
> +UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiB
> o
> +otServicesTableLib.inf
> +
> +DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPor
> t.i
> +nf
> +
> +MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMem
> oryAl
> +locationLib.inf
> +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> +  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> +
> +RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLi
> +bNull.inf
> +
> +SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPo
> r
> +tLib16550.inf
> +  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> +
> +DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/Ba
> se
> +DebugPrintErrorLevelLib.inf
> +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> +
> +PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePl
> atfo
> +rmHookLibNull.inf
> +  PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
> +  PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
> +
> +[Components.X64]
> +  PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> diff --git a/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> new file mode 100644
> index 000000000000..0079c4acf14e
> --- /dev/null
> +++ b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> @@ -0,0 +1,41 @@
> +## @file
> +# This driver implements EFI_PCI_HOT_PLUG_INIT_PROTOCOL.
> +# Adds resource padding information, for PCIe hotplug purposes.
> +#
> +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved #
> +SPDX-License-Identifier: BSD-2-Clause-Patent ##
> +
> +[Defines]
> +  INF_VERSION                    = 1.29
> +  BASE_NAME                      = PciHotPlugInit
> +  FILE_GUID                      = 8B67D95F-78B7-484F-8F16-5F22AB388B0C
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 0.1
> +  ENTRY_POINT                    = PciHotPlugInitialize
> +
> +[Sources]
> +  PciHotPlugInit.c
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  PlatformPkg/PlatformPkg.dec
> +
> +[LibraryClasses]
> +  UefiDriverEntryPoint
> +  UefiBootServicesTableLib
> +  DebugLib
> +  MemoryAllocationLib
> +
> +[Protocols]
> +  gEfiPciHotPlugInitProtocolGuid
> +
> +[Pcd]
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadIO
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadMem
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadBus
> +
> +[Depex]
> +  TRUE
> diff --git a/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> new file mode 100644
> index 000000000000..b977406bbcae
> --- /dev/null
> +++ b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> @@ -0,0 +1,340 @@
> +/** @file
> +  This file declares EFI PCI Hot Plug Init Protocol.
> +
> +  This protocol provides the necessary functionality to initialize the
> + Hot Plug  Controllers (HPCs) and the buses that they control. This
> + protocol also provides  information regarding resource padding.
> +
> +  @par Note:
> +    This source has the reference of OVMF PciHotPluginit.c and Intel platform
> PciHotPlug.c.
> +
> +    This protocol is required only on platforms that support one or more PCI
> Hot
> +    Plug* slots or CardBus sockets.
> +
> +  The EFI_PCI_HOT_PLUG_INIT_PROTOCOL provides a mechanism for the
> PCI
> + bus enumerator  to properly initialize the HPCs and CardBus sockets that
> require initialization.
> +  The HPC initialization takes place before the PCI enumeration process is
> complete.
> +  There cannot be more than one instance of this protocol in a system.
> + This protocol  is installed on its own separate handle.
> +
> +  Because the system may include multiple HPCs, one instance of this
> + protocol  should represent all of them. The protocol functions use the
> + device path of  the HPC to identify the HPC. When the PCI bus
> + enumerator finds a root HPC, it  will call
> + EFI_PCI_HOT_PLUG_INIT_PROTOCOL.InitializeRootHpc(). If
> + InitializeRootHpc()  is unable to initialize a root HPC, the PCI
> + enumerator will ignore that root HPC  and continue the enumeration
> process. If the HPC is not initialized, the devices  that it controls may not be
> initialized, and no resource padding will be provided.
> +
> +  From the standpoint of the PCI bus enumerator, HPCs are divided into
> + the following  two classes:
> +
> +    - Root HPC:
> +        These HPCs must be initialized by calling InitializeRootHpc() during the
> +        enumeration process. These HPCs will also require resource padding.
> The
> +        platform code must have a priori knowledge of these devices and must
> know
> +        how to initialize them. There may not be any way to access their PCI
> +        configuration space before the PCI enumerator programs all the
> upstream
> +        bridges and thus enables the path to these devices. The PCI bus
> enumerator
> +        is responsible for determining the PCI bus address of the HPC before it
> +        calls InitializeRootHpc().
> +    - Nonroot HPC:
> +        These HPCs will not need explicit initialization during enumeration
> process.
> +        These HPCs will require resource padding. The platform code does not
> have
> +        to have a priori knowledge of these devices.
> +
> +  Copyright (c) 2007 - 2018, Intel Corporation. All rights
> + reserved.<BR>  Copyright (C) 2016, Red Hat, Inc.<BR>  Copyright (C)
> + 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Revision Reference:
> +  This Protocol is defined in UEFI Platform Initialization
> + Specification 1.2  Volume 5: Standards
> +
> +**/
> +
> +#include <Uefi/UefiBaseType.h>
> +#include <Uefi/UefiSpec.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h> #include
> +<Protocol/PciHotPlugInit.h>
> +
> +//
> +// The protocol interface this driver produces.
> +//
> +STATIC EFI_PCI_HOT_PLUG_INIT_PROTOCOL  mPciHotPlugInit;
> +
> +/**
> +  Returns a list of root Hot Plug Controllers (HPCs) that require
> +initialization
> +  during the boot process.
> +
> +  This procedure returns a list of root HPCs. The PCI bus driver must
> + initialize  these controllers during the boot process. The PCI bus
> + driver may or may not be  able to detect these HPCs. If the platform
> + includes a PCI-to-CardBus bridge, it  can be included in this list if
> + it requires initialization.  The HpcList must be  self consistent. An
> + HPC cannot control any of its parent buses. Only one HPC can  control
> + a PCI bus. Because this list includes only root HPCs, no HPC in the list  can be
> a child of another HPC. This policy must be enforced by the
> +  EFI_PCI_HOT_PLUG_INIT_PROTOCOL.   The PCI bus driver may not check
> for such
> +  invalid conditions.  The callee allocates the buffer HpcList
> +
> +  @param[in]  This       Pointer to the EFI_PCI_HOT_PLUG_INIT_PROTOCOL
> instance.
> +  @param[out] HpcCount   The number of root HPCs that were returned.
> +  @param[out] HpcList    The list of root HPCs. HpcCount defines the number
> of
> +                         elements in this list.
> +
> +  @retval EFI_SUCCESS             HpcList was returned.
> +  @retval EFI_OUT_OF_RESOURCES    HpcList was not returned due to
> insufficient
> +                                  resources.
> +  @retval EFI_INVALID_PARAMETER   HpcCount is NULL or HpcList is NULL.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetRootHpcList (
> +  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL  *This,
> +  OUT UINTN                           *HpcCount,
> +  OUT EFI_HPC_LOCATION                **HpcList
> +  )
> +{
> +  if ((HpcCount == NULL) || (HpcList == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Platform BIOS not doing any extra/special HPC initialization  //
> + Hence returning the HpcCount as zero and HpcList as NULL  //
> + *HpcCount = 0;  *HpcList  = NULL;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Initializes one root Hot Plug Controller (HPC). This process may
> +causes
> +  initialization of its subordinate buses.
> +
> +  This function initializes the specified HPC. At the end of
> + initialization,  the hot-plug slots or sockets (controlled by this
> + HPC) are powered and are  connected to the bus. All the necessary
> + registers in the HPC are set up. For  a Standard (PCI) Hot Plug
> + Controller (SHPC), the registers that must be set  up are defined in
> + the PCI Standard Hot Plug Controller and Subsystem  Specification.
> +
> +  @param[in]  This            Pointer to the
> EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
> +  @param[in]  HpcDevicePath   The device path to the HPC that is being
> initialized.
> +  @param[in]  HpcPciAddress   The address of the HPC function on the PCI
> bus.
> +  @param[in]  Event           The event that should be signaled when the HPC
> +                              initialization is complete.  Set to NULL if the
> +                              caller wants to wait until the entire initialization
> +                              process is complete.
> +  @param[out] HpcState        The state of the HPC hardware. The state is
> +                              EFI_HPC_STATE_INITIALIZED or EFI_HPC_STATE_ENABLED.
> +
> +  @retval EFI_SUCCESS             If Event is NULL, the specific HPC was
> successfully
> +                                  initialized. If Event is not NULL, Event will be
> +                                  signaled at a later time when initialization is complete.
> +  @retval EFI_UNSUPPORTED         This instance of
> EFI_PCI_HOT_PLUG_INIT_PROTOCOL
> +                                  does not support the specified HPC.
> +  @retval EFI_OUT_OF_RESOURCES    Initialization failed due to insufficient
> +                                  resources.
> +  @retval EFI_INVALID_PARAMETER   HpcState is NULL.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +InitializeRootHpc (
> +  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL *This,
> +  IN  EFI_DEVICE_PATH_PROTOCOL *HpcDevicePath,
> +  IN  UINT64 HpcPciAddress,
> +  IN  EFI_EVENT Event, OPTIONAL
> +  OUT EFI_HPC_STATE                   *HpcState
> +  )
> +{
> +  if (HpcState == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // This Platform doesnt have any non-enumerated HPC.
> +  // Hence no extra initialization required from Platform BIOS.
> +  //
> +  return EFI_UNSUPPORTED;
> +}
> +
> +/**
> +  Returns the resource padding that is required by the PCI bus that is
> +controlled
> +  by the specified Hot Plug Controller (HPC).
> +
> +  This function returns the resource padding that is required by the
> + PCI bus that  is controlled by the specified HPC. This member function
> + is called for all the  root HPCs and nonroot HPCs that are detected by
> + the PCI bus enumerator. This  function will be called before PCI
> + resource allocation is completed. This function  must be called after
> + all the root HPCs, with the possible exception of a  PCI-to-CardBus bridge,
> have completed initialization.
> +
> +  @param[in]  This            Pointer to the
> EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
> +  @param[in]  HpcDevicePath   The device path to the HPC.
> +  @param[in]  HpcPciAddress   The address of the HPC function on the PCI
> bus.
> +  @param[out] HpcState        The state of the HPC hardware.
> +  @param[out] Padding         The amount of resource padding that is
> required by the
> +                              PCI bus under the control of the specified HPC.
> +  @param[out] Attributes      Describes how padding is accounted for. The
> padding
> +                              is returned in the form of ACPI 2.0 resource descriptors.
> +
> +  @retval EFI_SUCCESS             The resource padding was successfully
> returned.
> +  @retval EFI_UNSUPPORTED         This instance of the
> EFI_PCI_HOT_PLUG_INIT_PROTOCOL
> +                                  does not support the specified HPC.
> +  @retval EFI_NOT_READY           This function was called before HPC
> initialization
> +                                  is complete.
> +  @retval EFI_INVALID_PARAMETER   HpcState or Padding or Attributes is
> NULL.
> +  @retval EFI_OUT_OF_RESOURCES    ACPI 2.0 resource descriptors for
> Padding
> +                                  cannot be allocated due to insufficient resources.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetResourcePadding (
> +  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL  *This,
> +  IN  EFI_DEVICE_PATH_PROTOCOL        *HpcDevicePath,
> +  IN  UINT64                          HpcPciAddress,
> +  OUT EFI_HPC_STATE                   *HpcState,
> +  OUT VOID                            **Padding,
> +  OUT EFI_HPC_PADDING_ATTRIBUTES      *Attributes
> +  )
> +{
> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  *PaddingResource;
> +
> +  //
> +  // Need total 5 resources
> +  // 1 - IO resource
> +  // 2 - Mem resource
> +  // 3 - PMem resource
> +  // 4 - Bus resource
> +  // 5 - end tag resource
> +  PaddingResource = AllocateZeroPool (4 * sizeof
> + (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + sizeof
> (EFI_ACPI_END_TAG_DESCRIPTOR));  if (PaddingResource == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  *Padding = (VOID *)PaddingResource;
> +
> +  //
> +  // Padding for bus
> +  //
> +  *Attributes = EfiPaddingPciBus;
> +
> +  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> + PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType      = ACPI_ADDRESS_SPACE_TYPE_BUS;
> +  PaddingResource->GenFlag      = 0x0;
> +  PaddingResource->SpecificFlag = 0;
> +  PaddingResource->AddrRangeMin = 0;
> +  PaddingResource->AddrRangeMax = 0;
> +  PaddingResource->AddrLen      = PcdGet8 (PcdPciHotPlugResourcePadBus);
> +
> +  //
> +  // Padding for non-prefetchable memory  //  PaddingResource++;
> + PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> + PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType              = ACPI_ADDRESS_SPACE_TYPE_MEM;
> +  PaddingResource->GenFlag              = 0x0;
> +  PaddingResource->AddrSpaceGranularity = 32;
> +  PaddingResource->SpecificFlag         =
> EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_NON_CACHEABLE;
> +  PaddingResource->AddrRangeMin         = 0;
> +  PaddingResource->AddrLen              = PcdGet64
> (PcdPciHotPlugResourcePadMem);
> +  PaddingResource->AddrRangeMax         = PaddingResource->AddrLen - 1;
> +
> +  //
> +  // Padding for prefetchable memory
> +  //
> +  PaddingResource++;
> +  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> + PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType              = ACPI_ADDRESS_SPACE_TYPE_MEM;
> +  PaddingResource->GenFlag              = 0x0;
> +  PaddingResource->AddrSpaceGranularity = 32;
> +  PaddingResource->SpecificFlag         =
> EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABL
> E;
> +  PaddingResource->AddrLen              = PcdGet64
> (PcdPciHotPlugResourcePadPMem);
> +  PaddingResource->AddrRangeMax         = PaddingResource->AddrLen - 1;
> +
> +  //
> +  // Padding for I/O
> +  //
> +  PaddingResource++;
> +  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> + PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType      = ACPI_ADDRESS_SPACE_TYPE_IO;
> +  PaddingResource->GenFlag      = 0x0;
> +  PaddingResource->SpecificFlag = 0;
> +  PaddingResource->AddrRangeMin = 0;
> +  PaddingResource->AddrLen      = PcdGet64 (PcdPciHotPlugResourcePadIO);
> +  PaddingResource->AddrRangeMax = PaddingResource->AddrLen - 1;
> +
> +  //
> +  // Terminate the entries.
> +  //
> +  PaddingResource++;
> +  ((EFI_ACPI_END_TAG_DESCRIPTOR *)PaddingResource)->Desc     =
> ACPI_END_TAG_DESCRIPTOR;
> +  ((EFI_ACPI_END_TAG_DESCRIPTOR *)PaddingResource)->Checksum = 0x0;
> +
> +  *HpcState = EFI_HPC_STATE_INITIALIZED | EFI_HPC_STATE_ENABLED;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Entry point for this driver.
> +
> +  @param[in] ImageHandle  Image handle of this driver.
> +  @param[in] SystemTable  Pointer to SystemTable.
> +
> +  @retval EFI_SUCESS       Driver has loaded successfully.
> +  @return                  Error codes from lower level functions.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +PciHotPlugInitialize (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  mPciHotPlugInit.GetRootHpcList     = GetRootHpcList;
> +  mPciHotPlugInit.InitializeRootHpc  = InitializeRootHpc;
> +  mPciHotPlugInit.GetResourcePadding = GetResourcePadding;
> +  return gBS->InstallMultipleProtocolInterfaces (
> +                &ImageHandle,
> +                &gEfiPciHotPlugInitProtocolGuid,
> +                &mPciHotPlugInit,
> +                NULL
> +                );
> +}
> --
> 2.25.1

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library
  2023-03-17  6:50 ` [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library Abdul Lateef Attar
@ 2023-03-17  6:52   ` Chang, Abner
  2023-03-17 15:59   ` Leif Lindholm
  1 sibling, 0 replies; 28+ messages in thread
From: Chang, Abner @ 2023-03-17  6:52 UTC (permalink / raw)
  To: Attar, AbdulLateef (Abdul Lateef), devel@edk2.groups.io
  Cc: Attar, AbdulLateef (Abdul Lateef), Ard Biesheuvel, Leif Lindholm,
	Michael D Kinney

[AMD Official Use Only - General]

Reviewed-by: Abner Chang <abner.chang@amd.com>

> -----Original Message-----
> From: Abdul Lateef Attar <abdattar@amd.com>
> Sent: Friday, March 17, 2023 2:50 PM
> To: devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm
> <quic_llindhol@quicinc.com>; Chang, Abner <Abner.Chang@amd.com>;
> Michael D Kinney <michael.d.kinney@intel.com>
> Subject: [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds
> SetCacheMtrrLib library
> 
> Adds SetCacheMtrrLib library for AMD processor based boards.
> This library sets MTRR value or various memory ranges.
> 
> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  Platform/AMD/BoardPkg/BoardPkg.dsc            |  10 ++
>  .../SetCacheMtrrLib/SetCacheMtrrLib.inf       |  37 +++++
>  .../Library/SetCacheMtrrLib/SetCacheMtrrLib.c | 132 ++++++++++++++++++
>  3 files changed, 179 insertions(+)
>  create mode 100644
> Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
>  create mode 100644
> Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
> 
> diff --git a/Platform/AMD/BoardPkg/BoardPkg.dsc
> b/Platform/AMD/BoardPkg/BoardPkg.dsc
> index cb4065b86c60..aa0ee8287cd8 100644
> --- a/Platform/AMD/BoardPkg/BoardPkg.dsc
> +++ b/Platform/AMD/BoardPkg/BoardPkg.dsc
> @@ -18,3 +18,13 @@ [Defines]
> 
>  [Packages]
>    BoardPkg/BoardPkg.dec
> +  MinPlatformPkg/MinPlatformPkg.dec
> +  MdePkg/MdePkg.dec
> +  UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses.common.PEIM]
> +  SetCacheMtrrLib|BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> +
> +[Components.IA32]
> +  BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> +
> diff --git
> a/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> new file mode 100644
> index 000000000000..c66661d3f8dc
> --- /dev/null
> +++
> b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> @@ -0,0 +1,37 @@
> +## @file
> +# Component information file for Platform SetCacheMtrr Library.
> +# This library implementation is for AMD processor based platforms.
> +#
> +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> +reserved.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> +
> +[Defines]
> +  INF_VERSION                    = 1.29
> +  BASE_NAME                      = PeiSetCacheMtrrLib
> +  FILE_GUID                      = 1E8468E0-5EB4-4088-9B52-BFDC6E4DAE87
> +  MODULE_TYPE                    = PEIM
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = SetCacheMtrrLib
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  MtrrLib
> +
> +[Packages]
> +  MinPlatformPkg/MinPlatformPkg.dec
> +  MdePkg/MdePkg.dec
> +  UefiCpuPkg/UefiCpuPkg.dec
> +
> +[Sources]
> +  SetCacheMtrrLib.c
> +
> +[Guids]
> +
> +[Pcd]
> +  gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress
> +  gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize
> +
> diff --git
> a/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
> b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
> new file mode 100644
> index 000000000000..18404405d9fa
> --- /dev/null
> +++ b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
> @@ -0,0 +1,132 @@
> +/** @file
> +
> +SetCacheMtrr library functions.
> +This library implementation is for AMD processor based platforms.
> +
> +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
> +
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <PiPei.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MtrrLib.h>
> +
> +/**
> +  This function sets the cache MTRR values for PEI phase.
> +**/
> +VOID
> +EFIAPI
> +SetCacheMtrr (
> +  VOID
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0,
> +             0xA0000,
> +             CacheWriteBack
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteBack for 0-0x9FFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0xA0000,
> +             0x20000,
> +             CacheUncacheable
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheUncacheable for 0xA0000-0xBFFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0xC0000,
> +             0x40000,
> +             CacheWriteProtected
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteProtected for 0xC0000-0xFFFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0x100000,
> +             0xAFF00000,
> +             CacheWriteBack
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteBack for 0x100000-0xAFFFFFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             PcdGet32 (PcdFlashAreaBaseAddress),
> +             PcdGet32 (PcdFlashAreaSize),
> +             CacheWriteProtected
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteProtected for 0x%X-0x%X\n",
> +      Status,
> +      PcdGet32 (PcdFlashAreaBaseAddress),
> +      PcdGet32 (PcdFlashAreaBaseAddress) + PcdGet32 (PcdFlashAreaSize)
> +      ));
> +  }
> +
> +  MtrrDebugPrintAllMtrrs ();
> +  return;
> +}
> +
> +/**
> +  Update MTRR setting in EndOfPei phase.
> +  This function will set the MTRR value as CacheUncacheable
> +  for Flash address.
> +
> +  @retval  EFI_SUCCESS  The function completes successfully.
> +  @retval  Others       Some error occurs.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SetCacheMtrrAfterEndOfPei (
> +  VOID
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = MtrrSetMemoryAttribute (
> +             PcdGet32 (PcdFlashAreaBaseAddress),
> +             PcdGet32 (PcdFlashAreaSize),
> +             CacheUncacheable
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheUncacheable for 0x%X-0x%X\n",
> +      Status,
> +      PcdGet32 (PcdFlashAreaBaseAddress),
> +      PcdGet32 (PcdFlashAreaBaseAddress) + PcdGet32 (PcdFlashAreaSize)
> +      ));
> +  }
> +
> +  MtrrDebugPrintAllMtrrs ();
> +  return EFI_SUCCESS;
> +}
> --
> 2.25.1

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-17  6:50 ` [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers Abdul Lateef Attar
@ 2023-03-17  6:53   ` Chang, Abner
  2023-03-17 16:09   ` Leif Lindholm
  1 sibling, 0 replies; 28+ messages in thread
From: Chang, Abner @ 2023-03-17  6:53 UTC (permalink / raw)
  To: Attar, AbdulLateef (Abdul Lateef), devel@edk2.groups.io
  Cc: Attar, AbdulLateef (Abdul Lateef),
	Attar, AbdulLateef (Abdul Lateef), Ard Biesheuvel, Leif Lindholm,
	Michael D Kinney

[AMD Official Use Only - General]

Acked-by: Abner Chang <abner.chang@amd.com>

> -----Original Message-----
> From: Abdul Lateef Attar <abdattar@amd.com>
> Sent: Friday, March 17, 2023 2:50 PM
> To: devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Attar,
> AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Leif Lindholm <quic_llindhol@quicinc.com>;
> Chang, Abner <Abner.Chang@amd.com>; Michael D Kinney
> <michael.d.kinney@intel.com>
> Subject: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and
> AMD/PlatformPkg maintainers
> 
> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> 
> Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> 
> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  Maintainers.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt index
> 747191366070..bb8ab643e090 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
>  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
>  M: Leif Lindholm <quic_llindhol@quicinc.com>
> 
> +AMD Platform
> +F: Platform/AMD/BoardPkg
> +F: Platform/AMD/PlatformPkg
> +M: Abner Chang <abner.chang@amd.com>
> +M: Abdul Lateef Attar <abdattar@amd.com>
> +
>  Ampere Computing
>  F: Platform/Ampere
>  F: Silicon/Ampere
> --
> 2.25.1

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg
  2023-03-17  6:50 [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg Abdul Lateef Attar
                   ` (3 preceding siblings ...)
  2023-03-17  6:50 ` [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers Abdul Lateef Attar
@ 2023-03-17  6:54 ` Chang, Abner
  4 siblings, 0 replies; 28+ messages in thread
From: Chang, Abner @ 2023-03-17  6:54 UTC (permalink / raw)
  To: Attar, AbdulLateef (Abdul Lateef), devel@edk2.groups.io
  Cc: Attar, AbdulLateef (Abdul Lateef), Ard Biesheuvel, Leif Lindholm,
	Michael D Kinney

[AMD Official Use Only - General]

Hi Abdul,
I had gave reviewed-by to all patches. Let's wait for the reviewed-by given by maintainers for the updates of Maintainers.txt.
Thanks
Abner

> -----Original Message-----
> From: Abdul Lateef Attar <abdattar@amd.com>
> Sent: Friday, March 17, 2023 2:50 PM
> To: devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm
> <quic_llindhol@quicinc.com>; Chang, Abner <Abner.Chang@amd.com>;
> Michael D Kinney <michael.d.kinney@intel.com>
> Subject: [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg
> 
> Resending with correct email
> 
> Adds AMD/BoardPkg to support MinPlatformPkg framework.
> Adds AMD/PlatformPkg, which provide supporting modules and libraries for
> AMD based platform.
> 
> PR: https://github.com/tianocore/edk2-platforms/pull/69
> 
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> 
> Abdul Lateef Attar (4):
>   Platform/AMD: Adds BoardPkg and PlatformPkg
>   Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation
>   Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library
>   Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
> 
>  Platform/AMD/BoardPkg/BoardPkg.dec            |  18 +
>  Platform/AMD/PlatformPkg/PlatformPkg.dec      |  31 ++
>  Platform/AMD/BoardPkg/BoardPkg.dsc            |  30 ++
>  Platform/AMD/PlatformPkg/PlatformPkg.dsc      |  43 +++
>  .../SetCacheMtrrLib/SetCacheMtrrLib.inf       |  37 ++
>  .../PlatformPkg/PciHotPlug/PciHotPlugInit.inf |  41
> +++  .../Library/SetCacheMtrrLib/SetCacheMtrrLib.c | 132 +++++++
>  .../PlatformPkg/PciHotPlug/PciHotPlugInit.c   | 340 ++++++++++++++++++
>  Maintainers.txt                               |   6 +
>  9 files changed, 678 insertions(+)
>  create mode 100644 Platform/AMD/BoardPkg/BoardPkg.dec
>  create mode 100644 Platform/AMD/PlatformPkg/PlatformPkg.dec
>  create mode 100644 Platform/AMD/BoardPkg/BoardPkg.dsc
>  create mode 100644 Platform/AMD/PlatformPkg/PlatformPkg.dsc
>  create mode 100644
> Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
>  create mode 100644
> Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
>  create mode 100644
> Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
>  create mode 100644
> Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> 
> --
> 2.25.1

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 1/4] Platform/AMD: Adds BoardPkg and PlatformPkg
  2023-03-17  6:50 ` [PATCH v2 RESEND 1/4] Platform/AMD: Adds BoardPkg and PlatformPkg Abdul Lateef Attar
@ 2023-03-17 15:41   ` Leif Lindholm
  0 siblings, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2023-03-17 15:41 UTC (permalink / raw)
  To: Abdul Lateef Attar; +Cc: devel, Ard Biesheuvel, Abner Chang, Michael D Kinney

On Fri, Mar 17, 2023 at 12:20:04 +0530, Abdul Lateef Attar wrote:
> Adds initial DEC and DSC file for BoardPkg and PlatformPkg packages,
> which supports AMD processor family based boards and platforms.
> 
> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Reviewed-by: Abner Chang <abner.chang@amd.com>

Reviewed-by: Leif Lindholm <quic_llindhol@quicinc.com>

> ---
>  Platform/AMD/BoardPkg/BoardPkg.dec       | 18 ++++++++++++++++++
>  Platform/AMD/PlatformPkg/PlatformPkg.dec | 15 +++++++++++++++
>  Platform/AMD/BoardPkg/BoardPkg.dsc       | 20 ++++++++++++++++++++
>  Platform/AMD/PlatformPkg/PlatformPkg.dsc | 20 ++++++++++++++++++++
>  4 files changed, 73 insertions(+)
>  create mode 100644 Platform/AMD/BoardPkg/BoardPkg.dec
>  create mode 100644 Platform/AMD/PlatformPkg/PlatformPkg.dec
>  create mode 100644 Platform/AMD/BoardPkg/BoardPkg.dsc
>  create mode 100644 Platform/AMD/PlatformPkg/PlatformPkg.dsc
> 
> diff --git a/Platform/AMD/BoardPkg/BoardPkg.dec b/Platform/AMD/BoardPkg/BoardPkg.dec
> new file mode 100644
> index 000000000000..3ceff33f4fc1
> --- /dev/null
> +++ b/Platform/AMD/BoardPkg/BoardPkg.dec
> @@ -0,0 +1,18 @@
> +## @file BoardPkg.dec
> +#  Declaration file for AMD's BoardPkg.
> +#
> +#  This package supports AMD processor family based board as per the MinPlatform
> +#  Arch specification.
> +#
> +#  Copyright (c) 2023, Advanced Micro Devices, Inc. All rights reserved.
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#  @par Specification Reference:
> +#   -https://tianocore-docs.github.io/edk2-MinimumPlatformSpecification/draft/ 0.7
> +##
> +
> +[Defines]
> +  DEC_SPECIFICATION              = 1.27
> +  PACKAGE_NAME                   = BoardPkg
> +  PACKAGE_GUID                   = 44F9D761-9ECB-43DD-A5AC-177E5048701B
> +  PACKAGE_VERSION                = 0.1
> diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dec b/Platform/AMD/PlatformPkg/PlatformPkg.dec
> new file mode 100644
> index 000000000000..6155860979cb
> --- /dev/null
> +++ b/Platform/AMD/PlatformPkg/PlatformPkg.dec
> @@ -0,0 +1,15 @@
> +## @file PlatformPkg.dec
> +#  Declaration file for AMD's PlatformPkg.
> +#
> +#  This package supports AMD processory family based platform.
> +#
> +#  Copyright (c) 2023, Advanced Micro Devices, Inc. All rights reserved.
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  DEC_SPECIFICATION              = 1.27
> +  PACKAGE_NAME                   = PlatformPkg
> +  PACKAGE_GUID                   = 38FBA311-E2AA-4620-9A90-9A23753D1878
> +  PACKAGE_VERSION                = 0.1
> diff --git a/Platform/AMD/BoardPkg/BoardPkg.dsc b/Platform/AMD/BoardPkg/BoardPkg.dsc
> new file mode 100644
> index 000000000000..cb4065b86c60
> --- /dev/null
> +++ b/Platform/AMD/BoardPkg/BoardPkg.dsc
> @@ -0,0 +1,20 @@
> +## @file
> +#  BoardPkg.dsc
> +#
> +#  Description file for AMD BoardPkg
> +#
> +#  Copyright (c) 2023, Advanced Micro Devices, Inc. All rights reserved.
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  DSC_SPECIFICATION           = 1.30
> +  PLATFORM_GUID               = 88F8A9AE-2FA0-4D58-A6F9-05F635C05F4E
> +  PLATFORM_NAME               = BoardPkg
> +  PLATFORM_VERSION            = 0.1
> +  OUTPUT_DIRECTORY            = Build/$(PLATFORM_NAME)
> +  BUILD_TARGETS               = DEBUG | RELEASE | NOOPT
> +  SUPPORTED_ARCHITECTURES     = IA32 | X64
> +
> +[Packages]
> +  BoardPkg/BoardPkg.dec
> diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dsc b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> new file mode 100644
> index 000000000000..704566b9ea73
> --- /dev/null
> +++ b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> @@ -0,0 +1,20 @@
> +## @file
> +#  PlatformPkg.dsc
> +#
> +#  Description file for AMD PlatformPkg
> +#
> +#  Copyright (c) 2023, Advanced Micro Devices, Inc. All rights reserved.
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  DSC_SPECIFICATION           = 1.30
> +  PLATFORM_GUID               = 2F7C29F2-7F35-4B49-B97D-F0E61BD42FC0
> +  PLATFORM_NAME               = PlatformPkg
> +  PLATFORM_VERSION            = 0.1
> +  OUTPUT_DIRECTORY            = Build/$(PLATFORM_NAME)
> +  BUILD_TARGETS               = DEBUG | RELEASE | NOOPT
> +  SUPPORTED_ARCHITECTURES     = IA32 | X64
> +
> +[Packages]
> +  PlatformPkg/PlatformPkg.dec
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [edk2-devel] [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation
  2023-03-17  6:50 ` [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation Abdul Lateef Attar
  2023-03-17  6:52   ` Chang, Abner
@ 2023-03-17 15:55   ` Leif Lindholm
  2023-03-20 10:01     ` Attar, AbdulLateef (Abdul Lateef)
  1 sibling, 1 reply; 28+ messages in thread
From: Leif Lindholm @ 2023-03-17 15:55 UTC (permalink / raw)
  To: devel, abdattar; +Cc: Ard Biesheuvel, Abner Chang, Michael D Kinney

On Fri, Mar 17, 2023 at 12:20:05 +0530, Abdul Lateef Attar via groups.io wrote:
> Adds PCI hotplug init protocol implementation.

What are some notable aspects of this initial implementation?
What groups of platforms is it intended to cover?

> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  Platform/AMD/PlatformPkg/PlatformPkg.dec      |  16 +
>  Platform/AMD/PlatformPkg/PlatformPkg.dsc      |  23 ++
>  .../PlatformPkg/PciHotPlug/PciHotPlugInit.inf |  41 +++
>  .../PlatformPkg/PciHotPlug/PciHotPlugInit.c   | 340 ++++++++++++++++++
>  4 files changed, 420 insertions(+)
>  create mode 100644 Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
>  create mode 100644 Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> 
> diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dec b/Platform/AMD/PlatformPkg/PlatformPkg.dec
> index 6155860979cb..1bc38d6025c3 100644
> --- a/Platform/AMD/PlatformPkg/PlatformPkg.dec
> +++ b/Platform/AMD/PlatformPkg/PlatformPkg.dec
> @@ -13,3 +13,19 @@ [Defines]
>    PACKAGE_NAME                   = PlatformPkg
>    PACKAGE_GUID                   = 38FBA311-E2AA-4620-9A90-9A23753D1878
>    PACKAGE_VERSION                = 0.1
> +
> +[Guids]
> +  gPlatformPkgTokenSpaceGuid     = { 0x95ECA58D, 0x09B6, 0x4420, { 0xB4, 0xE7, 0x01, 0x7F, 0x6A, 0x5B, 0x26, 0x0F }}
> +
> +[PcdsDynamic, PcdsDynamicEx]
> +  #
> +  # PCI HotPlug Resource Padding
> +  #
> +  # IO Resource padding in bytes, default 4KB
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadIO|0x00001000|UINT64|0x10000000
> +  # PreFetch Memory padding in bytes, default 2MB
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem|0x00200000|UINT64|0x10000001
> +  # Non-PreFetch Memory padding in bytes, default 1MB
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadMem|0x00100000|UINT64|0x10000002
> +  # PCI bus padding, number of bus to reserve, default 2 bus
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadBus|2|UINT8|0x10000003
> diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dsc b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> index 704566b9ea73..9a693070ab3f 100644
> --- a/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> +++ b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> @@ -16,5 +16,28 @@ [Defines]
>    BUILD_TARGETS               = DEBUG | RELEASE | NOOPT
>    SUPPORTED_ARCHITECTURES     = IA32 | X64
>  
> +

Please don't add spurious whitespaces.

>  [Packages]
>    PlatformPkg/PlatformPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec

I won't be maintaining this code, but I generally advocate sorting
these types of enumerations alphabetically in order to speed up reading.

(That's not really possible for the block below.)

> +
> +[LibraryClasses.Common]
> +  UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
> +  UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
> +  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> +  MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> +  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> +  RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf
> +  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> +  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> +  DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> +  PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
> +  PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
> +  PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
> +
> +[Components.X64]
> +  PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> diff --git a/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> new file mode 100644
> index 000000000000..0079c4acf14e
> --- /dev/null
> +++ b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> @@ -0,0 +1,41 @@
> +## @file
> +# This driver implements EFI_PCI_HOT_PLUG_INIT_PROTOCOL.
> +# Adds resource padding information, for PCIe hotplug purposes.
> +#
> +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 1.29
> +  BASE_NAME                      = PciHotPlugInit
> +  FILE_GUID                      = 8B67D95F-78B7-484F-8F16-5F22AB388B0C
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 0.1
> +  ENTRY_POINT                    = PciHotPlugInitialize
> +
> +[Sources]
> +  PciHotPlugInit.c
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  PlatformPkg/PlatformPkg.dec

But these would benefit from sorting

> +
> +[LibraryClasses]
> +  UefiDriverEntryPoint
> +  UefiBootServicesTableLib
> +  DebugLib
> +  MemoryAllocationLib

And these

> +
> +[Protocols]
> +  gEfiPciHotPlugInitProtocolGuid
> +
> +[Pcd]
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadIO
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadMem
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadBus
> +
> +[Depex]
> +  TRUE
> diff --git a/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> new file mode 100644
> index 000000000000..b977406bbcae
> --- /dev/null
> +++ b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> @@ -0,0 +1,340 @@
> +/** @file
> +  This file declares EFI PCI Hot Plug Init Protocol.
> +
> +  This protocol provides the necessary functionality to initialize the Hot Plug
> +  Controllers (HPCs) and the buses that they control. This protocol also provides
> +  information regarding resource padding.
> +
> +  @par Note:
> +    This source has the reference of OVMF PciHotPluginit.c and Intel platform PciHotPlug.c.
> +
> +    This protocol is required only on platforms that support one or more PCI Hot
> +    Plug* slots or CardBus sockets.
> +
> +  The EFI_PCI_HOT_PLUG_INIT_PROTOCOL provides a mechanism for the PCI bus enumerator
> +  to properly initialize the HPCs and CardBus sockets that require initialization.
> +  The HPC initialization takes place before the PCI enumeration process is complete.
> +  There cannot be more than one instance of this protocol in a system. This protocol
> +  is installed on its own separate handle.
> +
> +  Because the system may include multiple HPCs, one instance of this protocol
> +  should represent all of them. The protocol functions use the device path of
> +  the HPC to identify the HPC. When the PCI bus enumerator finds a root HPC, it
> +  will call EFI_PCI_HOT_PLUG_INIT_PROTOCOL.InitializeRootHpc(). If InitializeRootHpc()
> +  is unable to initialize a root HPC, the PCI enumerator will ignore that root HPC
> +  and continue the enumeration process. If the HPC is not initialized, the devices
> +  that it controls may not be initialized, and no resource padding will be provided.
> +
> +  From the standpoint of the PCI bus enumerator, HPCs are divided into the following
> +  two classes:
> +
> +    - Root HPC:
> +        These HPCs must be initialized by calling InitializeRootHpc() during the
> +        enumeration process. These HPCs will also require resource padding. The
> +        platform code must have a priori knowledge of these devices and must know
> +        how to initialize them. There may not be any way to access their PCI
> +        configuration space before the PCI enumerator programs all the upstream
> +        bridges and thus enables the path to these devices. The PCI bus enumerator
> +        is responsible for determining the PCI bus address of the HPC before it
> +        calls InitializeRootHpc().
> +    - Nonroot HPC:
> +        These HPCs will not need explicit initialization during enumeration process.
> +        These HPCs will require resource padding. The platform code does not have
> +        to have a priori knowledge of these devices.
> +
> +  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
> +  Copyright (C) 2016, Red Hat, Inc.<BR>
> +  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Revision Reference:
> +  This Protocol is defined in UEFI Platform Initialization Specification 1.2

I know you say above that this is based on existing code, but 1.2 is
ancient. Latest version is 1.7 - is there anything missing from being
compliant with that version?

> +  Volume 5: Standards
> +
> +**/
> +
> +#include <Uefi/UefiBaseType.h>
> +#include <Uefi/UefiSpec.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>

I (personally) also like to sort include files within each
Uefi/Library/Protocol group alphabetically.

/
    Leif

> +#include <Protocol/PciHotPlugInit.h>
> +
> +//
> +// The protocol interface this driver produces.
> +//
> +STATIC EFI_PCI_HOT_PLUG_INIT_PROTOCOL  mPciHotPlugInit;
> +
> +/**
> +  Returns a list of root Hot Plug Controllers (HPCs) that require initialization
> +  during the boot process.
> +
> +  This procedure returns a list of root HPCs. The PCI bus driver must initialize
> +  these controllers during the boot process. The PCI bus driver may or may not be
> +  able to detect these HPCs. If the platform includes a PCI-to-CardBus bridge, it
> +  can be included in this list if it requires initialization.  The HpcList must be
> +  self consistent. An HPC cannot control any of its parent buses. Only one HPC can
> +  control a PCI bus. Because this list includes only root HPCs, no HPC in the list
> +  can be a child of another HPC. This policy must be enforced by the
> +  EFI_PCI_HOT_PLUG_INIT_PROTOCOL.   The PCI bus driver may not check for such
> +  invalid conditions.  The callee allocates the buffer HpcList
> +
> +  @param[in]  This       Pointer to the EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
> +  @param[out] HpcCount   The number of root HPCs that were returned.
> +  @param[out] HpcList    The list of root HPCs. HpcCount defines the number of
> +                         elements in this list.
> +
> +  @retval EFI_SUCCESS             HpcList was returned.
> +  @retval EFI_OUT_OF_RESOURCES    HpcList was not returned due to insufficient
> +                                  resources.
> +  @retval EFI_INVALID_PARAMETER   HpcCount is NULL or HpcList is NULL.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetRootHpcList (
> +  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL  *This,
> +  OUT UINTN                           *HpcCount,
> +  OUT EFI_HPC_LOCATION                **HpcList
> +  )
> +{
> +  if ((HpcCount == NULL) || (HpcList == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Platform BIOS not doing any extra/special HPC initialization
> +  // Hence returning the HpcCount as zero and HpcList as NULL
> +  //
> +  *HpcCount = 0;
> +  *HpcList  = NULL;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Initializes one root Hot Plug Controller (HPC). This process may causes
> +  initialization of its subordinate buses.
> +
> +  This function initializes the specified HPC. At the end of initialization,
> +  the hot-plug slots or sockets (controlled by this HPC) are powered and are
> +  connected to the bus. All the necessary registers in the HPC are set up. For
> +  a Standard (PCI) Hot Plug Controller (SHPC), the registers that must be set
> +  up are defined in the PCI Standard Hot Plug Controller and Subsystem
> +  Specification.
> +
> +  @param[in]  This            Pointer to the EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
> +  @param[in]  HpcDevicePath   The device path to the HPC that is being initialized.
> +  @param[in]  HpcPciAddress   The address of the HPC function on the PCI bus.
> +  @param[in]  Event           The event that should be signaled when the HPC
> +                              initialization is complete.  Set to NULL if the
> +                              caller wants to wait until the entire initialization
> +                              process is complete.
> +  @param[out] HpcState        The state of the HPC hardware. The state is
> +                              EFI_HPC_STATE_INITIALIZED or EFI_HPC_STATE_ENABLED.
> +
> +  @retval EFI_SUCCESS             If Event is NULL, the specific HPC was successfully
> +                                  initialized. If Event is not NULL, Event will be
> +                                  signaled at a later time when initialization is complete.
> +  @retval EFI_UNSUPPORTED         This instance of EFI_PCI_HOT_PLUG_INIT_PROTOCOL
> +                                  does not support the specified HPC.
> +  @retval EFI_OUT_OF_RESOURCES    Initialization failed due to insufficient
> +                                  resources.
> +  @retval EFI_INVALID_PARAMETER   HpcState is NULL.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +InitializeRootHpc (
> +  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL *This,
> +  IN  EFI_DEVICE_PATH_PROTOCOL *HpcDevicePath,
> +  IN  UINT64 HpcPciAddress,
> +  IN  EFI_EVENT Event, OPTIONAL
> +  OUT EFI_HPC_STATE                   *HpcState
> +  )
> +{
> +  if (HpcState == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // This Platform doesnt have any non-enumerated HPC.
> +  // Hence no extra initialization required from Platform BIOS.
> +  //
> +  return EFI_UNSUPPORTED;
> +}
> +
> +/**
> +  Returns the resource padding that is required by the PCI bus that is controlled
> +  by the specified Hot Plug Controller (HPC).
> +
> +  This function returns the resource padding that is required by the PCI bus that
> +  is controlled by the specified HPC. This member function is called for all the
> +  root HPCs and nonroot HPCs that are detected by the PCI bus enumerator. This
> +  function will be called before PCI resource allocation is completed. This function
> +  must be called after all the root HPCs, with the possible exception of a
> +  PCI-to-CardBus bridge, have completed initialization.
> +
> +  @param[in]  This            Pointer to the EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
> +  @param[in]  HpcDevicePath   The device path to the HPC.
> +  @param[in]  HpcPciAddress   The address of the HPC function on the PCI bus.
> +  @param[out] HpcState        The state of the HPC hardware.
> +  @param[out] Padding         The amount of resource padding that is required by the
> +                              PCI bus under the control of the specified HPC.
> +  @param[out] Attributes      Describes how padding is accounted for. The padding
> +                              is returned in the form of ACPI 2.0 resource descriptors.
> +
> +  @retval EFI_SUCCESS             The resource padding was successfully returned.
> +  @retval EFI_UNSUPPORTED         This instance of the EFI_PCI_HOT_PLUG_INIT_PROTOCOL
> +                                  does not support the specified HPC.
> +  @retval EFI_NOT_READY           This function was called before HPC initialization
> +                                  is complete.
> +  @retval EFI_INVALID_PARAMETER   HpcState or Padding or Attributes is NULL.
> +  @retval EFI_OUT_OF_RESOURCES    ACPI 2.0 resource descriptors for Padding
> +                                  cannot be allocated due to insufficient resources.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetResourcePadding (
> +  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL  *This,
> +  IN  EFI_DEVICE_PATH_PROTOCOL        *HpcDevicePath,
> +  IN  UINT64                          HpcPciAddress,
> +  OUT EFI_HPC_STATE                   *HpcState,
> +  OUT VOID                            **Padding,
> +  OUT EFI_HPC_PADDING_ATTRIBUTES      *Attributes
> +  )
> +{
> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  *PaddingResource;
> +
> +  //
> +  // Need total 5 resources
> +  // 1 - IO resource
> +  // 2 - Mem resource
> +  // 3 - PMem resource
> +  // 4 - Bus resource
> +  // 5 - end tag resource
> +  PaddingResource = AllocateZeroPool (4 * sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + sizeof (EFI_ACPI_END_TAG_DESCRIPTOR));
> +  if (PaddingResource == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  *Padding = (VOID *)PaddingResource;
> +
> +  //
> +  // Padding for bus
> +  //
> +  *Attributes = EfiPaddingPciBus;
> +
> +  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> +  PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType      = ACPI_ADDRESS_SPACE_TYPE_BUS;
> +  PaddingResource->GenFlag      = 0x0;
> +  PaddingResource->SpecificFlag = 0;
> +  PaddingResource->AddrRangeMin = 0;
> +  PaddingResource->AddrRangeMax = 0;
> +  PaddingResource->AddrLen      = PcdGet8 (PcdPciHotPlugResourcePadBus);
> +
> +  //
> +  // Padding for non-prefetchable memory
> +  //
> +  PaddingResource++;
> +  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> +  PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType              = ACPI_ADDRESS_SPACE_TYPE_MEM;
> +  PaddingResource->GenFlag              = 0x0;
> +  PaddingResource->AddrSpaceGranularity = 32;
> +  PaddingResource->SpecificFlag         = EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_NON_CACHEABLE;
> +  PaddingResource->AddrRangeMin         = 0;
> +  PaddingResource->AddrLen              = PcdGet64 (PcdPciHotPlugResourcePadMem);
> +  PaddingResource->AddrRangeMax         = PaddingResource->AddrLen - 1;
> +
> +  //
> +  // Padding for prefetchable memory
> +  //
> +  PaddingResource++;
> +  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> +  PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType              = ACPI_ADDRESS_SPACE_TYPE_MEM;
> +  PaddingResource->GenFlag              = 0x0;
> +  PaddingResource->AddrSpaceGranularity = 32;
> +  PaddingResource->SpecificFlag         = EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE;
> +  PaddingResource->AddrLen              = PcdGet64 (PcdPciHotPlugResourcePadPMem);
> +  PaddingResource->AddrRangeMax         = PaddingResource->AddrLen - 1;
> +
> +  //
> +  // Padding for I/O
> +  //
> +  PaddingResource++;
> +  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> +  PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType      = ACPI_ADDRESS_SPACE_TYPE_IO;
> +  PaddingResource->GenFlag      = 0x0;
> +  PaddingResource->SpecificFlag = 0;
> +  PaddingResource->AddrRangeMin = 0;
> +  PaddingResource->AddrLen      = PcdGet64 (PcdPciHotPlugResourcePadIO);
> +  PaddingResource->AddrRangeMax = PaddingResource->AddrLen - 1;
> +
> +  //
> +  // Terminate the entries.
> +  //
> +  PaddingResource++;
> +  ((EFI_ACPI_END_TAG_DESCRIPTOR *)PaddingResource)->Desc     = ACPI_END_TAG_DESCRIPTOR;
> +  ((EFI_ACPI_END_TAG_DESCRIPTOR *)PaddingResource)->Checksum = 0x0;
> +
> +  *HpcState = EFI_HPC_STATE_INITIALIZED | EFI_HPC_STATE_ENABLED;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Entry point for this driver.
> +
> +  @param[in] ImageHandle  Image handle of this driver.
> +  @param[in] SystemTable  Pointer to SystemTable.
> +
> +  @retval EFI_SUCESS       Driver has loaded successfully.
> +  @return                  Error codes from lower level functions.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +PciHotPlugInitialize (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  mPciHotPlugInit.GetRootHpcList     = GetRootHpcList;
> +  mPciHotPlugInit.InitializeRootHpc  = InitializeRootHpc;
> +  mPciHotPlugInit.GetResourcePadding = GetResourcePadding;
> +  return gBS->InstallMultipleProtocolInterfaces (
> +                &ImageHandle,
> +                &gEfiPciHotPlugInitProtocolGuid,
> +                &mPciHotPlugInit,
> +                NULL
> +                );
> +}
> -- 
> 2.25.1
> 
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library
  2023-03-17  6:50 ` [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library Abdul Lateef Attar
  2023-03-17  6:52   ` Chang, Abner
@ 2023-03-17 15:59   ` Leif Lindholm
  2023-03-20 10:02     ` Attar, AbdulLateef (Abdul Lateef)
  1 sibling, 1 reply; 28+ messages in thread
From: Leif Lindholm @ 2023-03-17 15:59 UTC (permalink / raw)
  To: Abdul Lateef Attar; +Cc: devel, Ard Biesheuvel, Abner Chang, Michael D Kinney

Typo in subject: BoarkPkg

On Fri, Mar 17, 2023 at 12:20:06 +0530, Abdul Lateef Attar wrote:
> Adds SetCacheMtrrLib library for AMD processor based boards.
> This library sets MTRR value or various memory ranges.
> 
> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  Platform/AMD/BoardPkg/BoardPkg.dsc            |  10 ++
>  .../SetCacheMtrrLib/SetCacheMtrrLib.inf       |  37 +++++
>  .../Library/SetCacheMtrrLib/SetCacheMtrrLib.c | 132 ++++++++++++++++++
>  3 files changed, 179 insertions(+)
>  create mode 100644 Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
>  create mode 100644 Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
> 
> diff --git a/Platform/AMD/BoardPkg/BoardPkg.dsc b/Platform/AMD/BoardPkg/BoardPkg.dsc
> index cb4065b86c60..aa0ee8287cd8 100644
> --- a/Platform/AMD/BoardPkg/BoardPkg.dsc
> +++ b/Platform/AMD/BoardPkg/BoardPkg.dsc
> @@ -18,3 +18,13 @@ [Defines]
>  
>  [Packages]
>    BoardPkg/BoardPkg.dec
> +  MinPlatformPkg/MinPlatformPkg.dec
> +  MdePkg/MdePkg.dec

Sort?

> +  UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses.common.PEIM]
> +  SetCacheMtrrLib|BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> +
> +[Components.IA32]
> +  BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> +

Please drop blank line at end of file.

> diff --git a/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> new file mode 100644
> index 000000000000..c66661d3f8dc
> --- /dev/null
> +++ b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> @@ -0,0 +1,37 @@
> +## @file
> +# Component information file for Platform SetCacheMtrr Library.
> +# This library implementation is for AMD processor based platforms.
> +#
> +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 1.29
> +  BASE_NAME                      = PeiSetCacheMtrrLib
> +  FILE_GUID                      = 1E8468E0-5EB4-4088-9B52-BFDC6E4DAE87
> +  MODULE_TYPE                    = PEIM
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = SetCacheMtrrLib
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  MtrrLib
> +
> +[Packages]
> +  MinPlatformPkg/MinPlatformPkg.dec
> +  MdePkg/MdePkg.dec

Sort?

> +  UefiCpuPkg/UefiCpuPkg.dec
> +
> +[Sources]
> +  SetCacheMtrrLib.c
> +
> +[Guids]
> +
> +[Pcd]
> +  gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress
> +  gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize
> +

Please drop blank line at end of file.

/
    Leif

> diff --git a/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
> new file mode 100644
> index 000000000000..18404405d9fa
> --- /dev/null
> +++ b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
> @@ -0,0 +1,132 @@
> +/** @file
> +
> +SetCacheMtrr library functions.
> +This library implementation is for AMD processor based platforms.
> +
> +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
> +
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <PiPei.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MtrrLib.h>
> +
> +/**
> +  This function sets the cache MTRR values for PEI phase.
> +**/
> +VOID
> +EFIAPI
> +SetCacheMtrr (
> +  VOID
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0,
> +             0xA0000,
> +             CacheWriteBack
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteBack for 0-0x9FFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0xA0000,
> +             0x20000,
> +             CacheUncacheable
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheUncacheable for 0xA0000-0xBFFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0xC0000,
> +             0x40000,
> +             CacheWriteProtected
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteProtected for 0xC0000-0xFFFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0x100000,
> +             0xAFF00000,
> +             CacheWriteBack
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteBack for 0x100000-0xAFFFFFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             PcdGet32 (PcdFlashAreaBaseAddress),
> +             PcdGet32 (PcdFlashAreaSize),
> +             CacheWriteProtected
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteProtected for 0x%X-0x%X\n",
> +      Status,
> +      PcdGet32 (PcdFlashAreaBaseAddress),
> +      PcdGet32 (PcdFlashAreaBaseAddress) + PcdGet32 (PcdFlashAreaSize)
> +      ));
> +  }
> +
> +  MtrrDebugPrintAllMtrrs ();
> +  return;
> +}
> +
> +/**
> +  Update MTRR setting in EndOfPei phase.
> +  This function will set the MTRR value as CacheUncacheable
> +  for Flash address.
> +
> +  @retval  EFI_SUCCESS  The function completes successfully.
> +  @retval  Others       Some error occurs.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SetCacheMtrrAfterEndOfPei (
> +  VOID
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = MtrrSetMemoryAttribute (
> +             PcdGet32 (PcdFlashAreaBaseAddress),
> +             PcdGet32 (PcdFlashAreaSize),
> +             CacheUncacheable
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheUncacheable for 0x%X-0x%X\n",
> +      Status,
> +      PcdGet32 (PcdFlashAreaBaseAddress),
> +      PcdGet32 (PcdFlashAreaBaseAddress) + PcdGet32 (PcdFlashAreaSize)
> +      ));
> +  }
> +
> +  MtrrDebugPrintAllMtrrs ();
> +  return EFI_SUCCESS;
> +}
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-17  6:50 ` [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers Abdul Lateef Attar
  2023-03-17  6:53   ` Chang, Abner
@ 2023-03-17 16:09   ` Leif Lindholm
  2023-03-18  9:03     ` Chang, Abner
                       ` (2 more replies)
  1 sibling, 3 replies; 28+ messages in thread
From: Leif Lindholm @ 2023-03-17 16:09 UTC (permalink / raw)
  To: Abdul Lateef Attar
  Cc: devel, Abdul Lateef Attar, Ard Biesheuvel, Abner Chang,
	Michael D Kinney

On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> 
> Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> 
> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  Maintainers.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 747191366070..bb8ab643e090 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
>  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
>  M: Leif Lindholm <quic_llindhol@quicinc.com>
>  
> +AMD Platform

Annoyingly, I'm now going to ask the question I have been avoiding up
until now. What does "AMD Platform" mean?
I mean, you've placed this straight after the entry for the Seattle
platforms.
I think the name, and package names, are too generic.

/
    Leif

> +F: Platform/AMD/BoardPkg
> +F: Platform/AMD/PlatformPkg
> +M: Abner Chang <abner.chang@amd.com>
> +M: Abdul Lateef Attar <abdattar@amd.com>
> +
>  Ampere Computing
>  F: Platform/Ampere
>  F: Silicon/Ampere
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-17 16:09   ` Leif Lindholm
@ 2023-03-18  9:03     ` Chang, Abner
  2023-03-20 12:54       ` [edk2-devel] " Leif Lindholm
  2023-03-18  9:16     ` Chang, Abner
  2023-03-20 10:11     ` Attar, AbdulLateef (Abdul Lateef)
  2 siblings, 1 reply; 28+ messages in thread
From: Chang, Abner @ 2023-03-18  9:03 UTC (permalink / raw)
  To: Leif Lindholm, Attar, AbdulLateef (Abdul Lateef)
  Cc: devel@edk2.groups.io, Attar, AbdulLateef (Abdul Lateef),
	Ard Biesheuvel, Michael D Kinney

[AMD Official Use Only - General]

Hi Leif,
Thanks to your review.
AMD\PlatformPkg is mainly for the modules that are generic to AMD's server, client or other platforms, while AMD\BoardPkg that contains the modules that support MinPlatformrPkg framework. AMD\BoardPkg may also provide the modules those are common to all AMD boards and leverage by the board specific packages under AMD\BoardPkg. Perhaps AMD\Board would a better naming that contains both common board modules and the specific board packages. I don't know the context of OverdriveBoard , however we would like to see OverdriveBoard is relocated to under AMD\Board(Pkg) as well. With this we can have a well organized AMD directory under edk2-platform that provides AMD opensource edk2 solution to industry.

Thanks
Abner

> -----Original Message-----
> From: Leif Lindholm <quic_llindhol@quicinc.com>
> Sent: Saturday, March 18, 2023 12:09 AM
> To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Chang, Abner <Abner.Chang@amd.com>;
> Michael D Kinney <michael.d.kinney@intel.com>
> Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg
> and AMD/PlatformPkg maintainers
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> > From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> >
> > Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> >
> > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > ---
> >  Maintainers.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Maintainers.txt b/Maintainers.txt index
> > 747191366070..bb8ab643e090 100644
> > --- a/Maintainers.txt
> > +++ b/Maintainers.txt
> > @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
> >  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >  M: Leif Lindholm <quic_llindhol@quicinc.com>
> >
> > +AMD Platform
> 
> Annoyingly, I'm now going to ask the question I have been avoiding up until
> now. What does "AMD Platform" mean?
> I mean, you've placed this straight after the entry for the Seattle platforms.
> I think the name, and package names, are too generic.
> 
> /
>     Leif
> 
> > +F: Platform/AMD/BoardPkg
> > +F: Platform/AMD/PlatformPkg
> > +M: Abner Chang <abner.chang@amd.com>
> > +M: Abdul Lateef Attar <abdattar@amd.com>
> > +
> >  Ampere Computing
> >  F: Platform/Ampere
> >  F: Silicon/Ampere
> > --
> > 2.25.1
> >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-17 16:09   ` Leif Lindholm
  2023-03-18  9:03     ` Chang, Abner
@ 2023-03-18  9:16     ` Chang, Abner
  2023-03-18  9:42       ` Ard Biesheuvel
  2023-03-20 13:05       ` [edk2-devel] " Leif Lindholm
  2023-03-20 10:11     ` Attar, AbdulLateef (Abdul Lateef)
  2 siblings, 2 replies; 28+ messages in thread
From: Chang, Abner @ 2023-03-18  9:16 UTC (permalink / raw)
  To: Leif Lindholm, Attar, AbdulLateef (Abdul Lateef)
  Cc: devel@edk2.groups.io, Attar, AbdulLateef (Abdul Lateef),
	Ard Biesheuvel, Michael D Kinney

[AMD Official Use Only - General]

BTW Leif,
I don't see any modules under OverdriveBoard, is this package still in use? Do you still remember where is FDF and DSC come from (as there is AMD copyright 2014-2016) back to the moment when you was introduced this package?

Thanks
Abner

> -----Original Message-----
> From: Leif Lindholm <quic_llindhol@quicinc.com>
> Sent: Saturday, March 18, 2023 12:09 AM
> To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Chang, Abner <Abner.Chang@amd.com>;
> Michael D Kinney <michael.d.kinney@intel.com>
> Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg
> and AMD/PlatformPkg maintainers
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> > From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> >
> > Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> >
> > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > Cc: Abner Chang <abner.chang@amd.com>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > ---
> >  Maintainers.txt | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/Maintainers.txt b/Maintainers.txt index
> > 747191366070..bb8ab643e090 100644
> > --- a/Maintainers.txt
> > +++ b/Maintainers.txt
> > @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
> >  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >  M: Leif Lindholm <quic_llindhol@quicinc.com>
> >
> > +AMD Platform
> 
> Annoyingly, I'm now going to ask the question I have been avoiding up until
> now. What does "AMD Platform" mean?
> I mean, you've placed this straight after the entry for the Seattle platforms.
> I think the name, and package names, are too generic.
> 
> /
>     Leif
> 
> > +F: Platform/AMD/BoardPkg
> > +F: Platform/AMD/PlatformPkg
> > +M: Abner Chang <abner.chang@amd.com>
> > +M: Abdul Lateef Attar <abdattar@amd.com>
> > +
> >  Ampere Computing
> >  F: Platform/Ampere
> >  F: Silicon/Ampere
> > --
> > 2.25.1
> >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-18  9:16     ` Chang, Abner
@ 2023-03-18  9:42       ` Ard Biesheuvel
  2023-03-18 15:34         ` Chang, Abner
  2023-03-20 13:05       ` [edk2-devel] " Leif Lindholm
  1 sibling, 1 reply; 28+ messages in thread
From: Ard Biesheuvel @ 2023-03-18  9:42 UTC (permalink / raw)
  To: Chang, Abner
  Cc: Leif Lindholm, Attar, AbdulLateef (Abdul Lateef),
	devel@edk2.groups.io, Ard Biesheuvel, Michael D Kinney

On Sat, 18 Mar 2023 at 10:16, Chang, Abner <Abner.Chang@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> BTW Leif,
> I don't see any modules under OverdriveBoard, is this package still in use? Do you still remember where is FDF and DSC come from (as there is AMD copyright 2014-2016) back to the moment when you was introduced this package?
>

Hello Abner,

I can help with this, but I'm not sure I understand the question.

Check with Leo Duran - we worked with him on the Huskyboard at the
time, and he provided us with access to the SeattleFDK, some of which
became part of the OverdriveBoard/CelloBoard platform implementation.

The package is still in use, at least by me :-)

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-18  9:42       ` Ard Biesheuvel
@ 2023-03-18 15:34         ` Chang, Abner
  0 siblings, 0 replies; 28+ messages in thread
From: Chang, Abner @ 2023-03-18 15:34 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Leif Lindholm, Attar, AbdulLateef (Abdul Lateef),
	devel@edk2.groups.io, Ard Biesheuvel, Michael D Kinney

[AMD Official Use Only - General]

Hi Ard,
Thanks for letting us know this history. I know Leo and I will reach out to him if necessary.
We are going to relocate OverdriveBoard to under AMD/BoardPkg, hope it won't bring you too much problems. 😊
I can see some people had contributions to OverdriveBoard, however I don’t know some of them and I don’t have their email address neither.
I will send the notification to devel mailing list about this relocation.

Thanks
Abner

> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Saturday, March 18, 2023 5:43 PM
> To: Chang, Abner <Abner.Chang@amd.com>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>; Attar, AbdulLateef (Abdul
> Lateef) <AbdulLateef.Attar@amd.com>; devel@edk2.groups.io; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> <michael.d.kinney@intel.com>
> Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg
> and AMD/PlatformPkg maintainers
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> On Sat, 18 Mar 2023 at 10:16, Chang, Abner <Abner.Chang@amd.com> wrote:
> >
> > [AMD Official Use Only - General]
> >
> > BTW Leif,
> > I don't see any modules under OverdriveBoard, is this package still in use?
> Do you still remember where is FDF and DSC come from (as there is AMD
> copyright 2014-2016) back to the moment when you was introduced this
> package?
> >
> 
> Hello Abner,
> 
> I can help with this, but I'm not sure I understand the question.
> 
> Check with Leo Duran - we worked with him on the Huskyboard at the time,
> and he provided us with access to the SeattleFDK, some of which became
> part of the OverdriveBoard/CelloBoard platform implementation.
> 
> The package is still in use, at least by me :-)

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [edk2-devel] [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation
  2023-03-17 15:55   ` [edk2-devel] " Leif Lindholm
@ 2023-03-20 10:01     ` Attar, AbdulLateef (Abdul Lateef)
  0 siblings, 0 replies; 28+ messages in thread
From: Attar, AbdulLateef (Abdul Lateef) @ 2023-03-20 10:01 UTC (permalink / raw)
  To: Leif Lindholm, devel@edk2.groups.io
  Cc: Ard Biesheuvel, Chang, Abner, Michael D Kinney

[AMD Official Use Only - General]

Hi Leif,
        Please see inline my reply [Abdul]
I'll address the remaining review comments and submit V3 version.
Thanks
AbduL
-----Original Message-----
From: Leif Lindholm <quic_llindhol@quicinc.com>
Sent: 17 March 2023 21:25
To: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Chang, Abner <Abner.Chang@amd.com>; Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Fri, Mar 17, 2023 at 12:20:05 +0530, Abdul Lateef Attar via groups.io wrote:
> Adds PCI hotplug init protocol implementation.

What are some notable aspects of this initial implementation?
[Abdul]         AMD/BoardPkg and AMD/PlatformPkg will contains module/drivers to support MinPlatformPkg framework.
In initial implementation we will be adding modules which need different implementation from Intel based platforms
to adapt MinPlatformPkg, e.g. SetCacheMtrrLib library etc., as we progress more we will add more modules.

What groups of platforms is it intended to cover?
[Abdul] Its generic for all AMD processor based platforms based on latest(as of 2023) MinPlatformPkg framework.

> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  Platform/AMD/PlatformPkg/PlatformPkg.dec      |  16 +
>  Platform/AMD/PlatformPkg/PlatformPkg.dsc      |  23 ++
>  .../PlatformPkg/PciHotPlug/PciHotPlugInit.inf |  41 +++
>  .../PlatformPkg/PciHotPlug/PciHotPlugInit.c   | 340 ++++++++++++++++++
>  4 files changed, 420 insertions(+)
>  create mode 100644
> Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
>  create mode 100644
> Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
>
> diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dec
> b/Platform/AMD/PlatformPkg/PlatformPkg.dec
> index 6155860979cb..1bc38d6025c3 100644
> --- a/Platform/AMD/PlatformPkg/PlatformPkg.dec
> +++ b/Platform/AMD/PlatformPkg/PlatformPkg.dec
> @@ -13,3 +13,19 @@ [Defines]
>    PACKAGE_NAME                   = PlatformPkg
>    PACKAGE_GUID                   = 38FBA311-E2AA-4620-9A90-9A23753D1878
>    PACKAGE_VERSION                = 0.1
> +
> +[Guids]
> +  gPlatformPkgTokenSpaceGuid     = { 0x95ECA58D, 0x09B6, 0x4420, { 0xB4, 0xE7, 0x01, 0x7F, 0x6A, 0x5B, 0x26, 0x0F }}
> +
> +[PcdsDynamic, PcdsDynamicEx]
> +  #
> +  # PCI HotPlug Resource Padding
> +  #
> +  # IO Resource padding in bytes, default 4KB
> +
> +gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadIO|0x00001000|UINT
> +64|0x10000000
> +  # PreFetch Memory padding in bytes, default 2MB
> +
> +gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem|0x00200000|UI
> +NT64|0x10000001
> +  # Non-PreFetch Memory padding in bytes, default 1MB
> +
> +gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadMem|0x00100000|UIN
> +T64|0x10000002
> +  # PCI bus padding, number of bus to reserve, default 2 bus
> +
> +gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadBus|2|UINT8|0x1000
> +0003
> diff --git a/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> index 704566b9ea73..9a693070ab3f 100644
> --- a/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> +++ b/Platform/AMD/PlatformPkg/PlatformPkg.dsc
> @@ -16,5 +16,28 @@ [Defines]
>    BUILD_TARGETS               = DEBUG | RELEASE | NOOPT
>    SUPPORTED_ARCHITECTURES     = IA32 | X64
>
> +

Please don't add spurious whitespaces.

>  [Packages]
>    PlatformPkg/PlatformPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec

I won't be maintaining this code, but I generally advocate sorting these types of enumerations alphabetically in order to speed up reading.

(That's not really possible for the block below.)

> +
> +[LibraryClasses.Common]
> +
> +UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEn
> +tryPoint.inf
> +
> +UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/Uefi
> +BootServicesTableLib.inf
> +
> +DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort
> +.inf
> +
> +MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemory
> +AllocationLib.inf
> +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> +  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> +
> +RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilter
> +LibNull.inf
> +
> +SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialP
> +ortLib16550.inf
> +  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
> +
> +DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/Ba
> +seDebugPrintErrorLevelLib.inf
> +  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
> +
> +PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlat
> +formHookLibNull.inf
> +  PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
> +
> +PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
> +
> +[Components.X64]
> +  PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> diff --git a/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> new file mode 100644
> index 000000000000..0079c4acf14e
> --- /dev/null
> +++ b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.inf
> @@ -0,0 +1,41 @@
> +## @file
> +# This driver implements EFI_PCI_HOT_PLUG_INIT_PROTOCOL.
> +# Adds resource padding information, for PCIe hotplug purposes.
> +#
> +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved
> +# SPDX-License-Identifier: BSD-2-Clause-Patent ##
> +
> +[Defines]
> +  INF_VERSION                    = 1.29
> +  BASE_NAME                      = PciHotPlugInit
> +  FILE_GUID                      = 8B67D95F-78B7-484F-8F16-5F22AB388B0C
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 0.1
> +  ENTRY_POINT                    = PciHotPlugInitialize
> +
> +[Sources]
> +  PciHotPlugInit.c
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  PlatformPkg/PlatformPkg.dec

But these would benefit from sorting

> +
> +[LibraryClasses]
> +  UefiDriverEntryPoint
> +  UefiBootServicesTableLib
> +  DebugLib
> +  MemoryAllocationLib

And these

> +
> +[Protocols]
> +  gEfiPciHotPlugInitProtocolGuid
> +
> +[Pcd]
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadIO
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadPMem
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadMem
> +  gPlatformPkgTokenSpaceGuid.PcdPciHotPlugResourcePadBus
> +
> +[Depex]
> +  TRUE
> diff --git a/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> new file mode 100644
> index 000000000000..b977406bbcae
> --- /dev/null
> +++ b/Platform/AMD/PlatformPkg/PciHotPlug/PciHotPlugInit.c
> @@ -0,0 +1,340 @@
> +/** @file
> +  This file declares EFI PCI Hot Plug Init Protocol.
> +
> +  This protocol provides the necessary functionality to initialize
> + the Hot Plug  Controllers (HPCs) and the buses that they control.
> + This protocol also provides  information regarding resource padding.
> +
> +  @par Note:
> +    This source has the reference of OVMF PciHotPluginit.c and Intel platform PciHotPlug.c.
> +
> +    This protocol is required only on platforms that support one or more PCI Hot
> +    Plug* slots or CardBus sockets.
> +
> +  The EFI_PCI_HOT_PLUG_INIT_PROTOCOL provides a mechanism for the PCI
> + bus enumerator  to properly initialize the HPCs and CardBus sockets that require initialization.
> +  The HPC initialization takes place before the PCI enumeration process is complete.
> +  There cannot be more than one instance of this protocol in a
> + system. This protocol  is installed on its own separate handle.
> +
> +  Because the system may include multiple HPCs, one instance of this
> + protocol  should represent all of them. The protocol functions use
> + the device path of  the HPC to identify the HPC. When the PCI bus
> + enumerator finds a root HPC, it  will call
> + EFI_PCI_HOT_PLUG_INIT_PROTOCOL.InitializeRootHpc(). If
> + InitializeRootHpc()  is unable to initialize a root HPC, the PCI
> + enumerator will ignore that root HPC  and continue the enumeration process. If the HPC is not initialized, the devices  that it controls may not be initialized, and no resource padding will be provided.
> +
> +  From the standpoint of the PCI bus enumerator, HPCs are divided
> + into the following  two classes:
> +
> +    - Root HPC:
> +        These HPCs must be initialized by calling InitializeRootHpc() during the
> +        enumeration process. These HPCs will also require resource padding. The
> +        platform code must have a priori knowledge of these devices and must know
> +        how to initialize them. There may not be any way to access their PCI
> +        configuration space before the PCI enumerator programs all the upstream
> +        bridges and thus enables the path to these devices. The PCI bus enumerator
> +        is responsible for determining the PCI bus address of the HPC before it
> +        calls InitializeRootHpc().
> +    - Nonroot HPC:
> +        These HPCs will not need explicit initialization during enumeration process.
> +        These HPCs will require resource padding. The platform code does not have
> +        to have a priori knowledge of these devices.
> +
> +  Copyright (c) 2007 - 2018, Intel Corporation. All rights
> + reserved.<BR>  Copyright (C) 2016, Red Hat, Inc.<BR>  Copyright (C)
> + 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +  @par Revision Reference:
> +  This Protocol is defined in UEFI Platform Initialization
> + Specification 1.2

I know you say above that this is based on existing code, but 1.2 is ancient. Latest version is 1.7 - is there anything missing from being compliant with that version?

> +  Volume 5: Standards
> +
> +**/
> +
> +#include <Uefi/UefiBaseType.h>
> +#include <Uefi/UefiSpec.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>

I (personally) also like to sort include files within each Uefi/Library/Protocol group alphabetically.

/
    Leif

> +#include <Protocol/PciHotPlugInit.h>
> +
> +//
> +// The protocol interface this driver produces.
> +//
> +STATIC EFI_PCI_HOT_PLUG_INIT_PROTOCOL  mPciHotPlugInit;
> +
> +/**
> +  Returns a list of root Hot Plug Controllers (HPCs) that require
> +initialization
> +  during the boot process.
> +
> +  This procedure returns a list of root HPCs. The PCI bus driver must
> + initialize  these controllers during the boot process. The PCI bus
> + driver may or may not be  able to detect these HPCs. If the platform
> + includes a PCI-to-CardBus bridge, it  can be included in this list
> + if it requires initialization.  The HpcList must be  self
> + consistent. An HPC cannot control any of its parent buses. Only one
> + HPC can  control a PCI bus. Because this list includes only root HPCs, no HPC in the list  can be a child of another HPC. This policy must be enforced by the
> +  EFI_PCI_HOT_PLUG_INIT_PROTOCOL.   The PCI bus driver may not check for such
> +  invalid conditions.  The callee allocates the buffer HpcList
> +
> +  @param[in]  This       Pointer to the EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
> +  @param[out] HpcCount   The number of root HPCs that were returned.
> +  @param[out] HpcList    The list of root HPCs. HpcCount defines the number of
> +                         elements in this list.
> +
> +  @retval EFI_SUCCESS             HpcList was returned.
> +  @retval EFI_OUT_OF_RESOURCES    HpcList was not returned due to insufficient
> +                                  resources.
> +  @retval EFI_INVALID_PARAMETER   HpcCount is NULL or HpcList is NULL.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetRootHpcList (
> +  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL  *This,
> +  OUT UINTN                           *HpcCount,
> +  OUT EFI_HPC_LOCATION                **HpcList
> +  )
> +{
> +  if ((HpcCount == NULL) || (HpcList == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // Platform BIOS not doing any extra/special HPC initialization  //
> + Hence returning the HpcCount as zero and HpcList as NULL  //
> + *HpcCount = 0;  *HpcList  = NULL;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Initializes one root Hot Plug Controller (HPC). This process may
> +causes
> +  initialization of its subordinate buses.
> +
> +  This function initializes the specified HPC. At the end of
> + initialization,  the hot-plug slots or sockets (controlled by this
> + HPC) are powered and are  connected to the bus. All the necessary
> + registers in the HPC are set up. For  a Standard (PCI) Hot Plug
> + Controller (SHPC), the registers that must be set  up are defined in
> + the PCI Standard Hot Plug Controller and Subsystem  Specification.
> +
> +  @param[in]  This            Pointer to the EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
> +  @param[in]  HpcDevicePath   The device path to the HPC that is being initialized.
> +  @param[in]  HpcPciAddress   The address of the HPC function on the PCI bus.
> +  @param[in]  Event           The event that should be signaled when the HPC
> +                              initialization is complete.  Set to NULL if the
> +                              caller wants to wait until the entire initialization
> +                              process is complete.
> +  @param[out] HpcState        The state of the HPC hardware. The state is
> +                              EFI_HPC_STATE_INITIALIZED or EFI_HPC_STATE_ENABLED.
> +
> +  @retval EFI_SUCCESS             If Event is NULL, the specific HPC was successfully
> +                                  initialized. If Event is not NULL, Event will be
> +                                  signaled at a later time when initialization is complete.
> +  @retval EFI_UNSUPPORTED         This instance of EFI_PCI_HOT_PLUG_INIT_PROTOCOL
> +                                  does not support the specified HPC.
> +  @retval EFI_OUT_OF_RESOURCES    Initialization failed due to insufficient
> +                                  resources.
> +  @retval EFI_INVALID_PARAMETER   HpcState is NULL.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +InitializeRootHpc (
> +  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL *This,
> +  IN  EFI_DEVICE_PATH_PROTOCOL *HpcDevicePath,
> +  IN  UINT64 HpcPciAddress,
> +  IN  EFI_EVENT Event, OPTIONAL
> +  OUT EFI_HPC_STATE                   *HpcState
> +  )
> +{
> +  if (HpcState == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  //
> +  // This Platform doesnt have any non-enumerated HPC.
> +  // Hence no extra initialization required from Platform BIOS.
> +  //
> +  return EFI_UNSUPPORTED;
> +}
> +
> +/**
> +  Returns the resource padding that is required by the PCI bus that
> +is controlled
> +  by the specified Hot Plug Controller (HPC).
> +
> +  This function returns the resource padding that is required by the
> + PCI bus that  is controlled by the specified HPC. This member
> + function is called for all the  root HPCs and nonroot HPCs that are
> + detected by the PCI bus enumerator. This  function will be called
> + before PCI resource allocation is completed. This function  must be
> + called after all the root HPCs, with the possible exception of a  PCI-to-CardBus bridge, have completed initialization.
> +
> +  @param[in]  This            Pointer to the EFI_PCI_HOT_PLUG_INIT_PROTOCOL instance.
> +  @param[in]  HpcDevicePath   The device path to the HPC.
> +  @param[in]  HpcPciAddress   The address of the HPC function on the PCI bus.
> +  @param[out] HpcState        The state of the HPC hardware.
> +  @param[out] Padding         The amount of resource padding that is required by the
> +                              PCI bus under the control of the specified HPC.
> +  @param[out] Attributes      Describes how padding is accounted for. The padding
> +                              is returned in the form of ACPI 2.0 resource descriptors.
> +
> +  @retval EFI_SUCCESS             The resource padding was successfully returned.
> +  @retval EFI_UNSUPPORTED         This instance of the EFI_PCI_HOT_PLUG_INIT_PROTOCOL
> +                                  does not support the specified HPC.
> +  @retval EFI_NOT_READY           This function was called before HPC initialization
> +                                  is complete.
> +  @retval EFI_INVALID_PARAMETER   HpcState or Padding or Attributes is NULL.
> +  @retval EFI_OUT_OF_RESOURCES    ACPI 2.0 resource descriptors for Padding
> +                                  cannot be allocated due to insufficient resources.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +GetResourcePadding (
> +  IN  EFI_PCI_HOT_PLUG_INIT_PROTOCOL  *This,
> +  IN  EFI_DEVICE_PATH_PROTOCOL        *HpcDevicePath,
> +  IN  UINT64                          HpcPciAddress,
> +  OUT EFI_HPC_STATE                   *HpcState,
> +  OUT VOID                            **Padding,
> +  OUT EFI_HPC_PADDING_ATTRIBUTES      *Attributes
> +  )
> +{
> +  EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR  *PaddingResource;
> +
> +  //
> +  // Need total 5 resources
> +  // 1 - IO resource
> +  // 2 - Mem resource
> +  // 3 - PMem resource
> +  // 4 - Bus resource
> +  // 5 - end tag resource
> +  PaddingResource = AllocateZeroPool (4 * sizeof
> + (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) + sizeof (EFI_ACPI_END_TAG_DESCRIPTOR));  if (PaddingResource == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  *Padding = (VOID *)PaddingResource;
> +
> +  //
> +  // Padding for bus
> +  //
> +  *Attributes = EfiPaddingPciBus;
> +
> +  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> + PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType      = ACPI_ADDRESS_SPACE_TYPE_BUS;
> +  PaddingResource->GenFlag      = 0x0;
> +  PaddingResource->SpecificFlag = 0;
> +  PaddingResource->AddrRangeMin = 0;
> +  PaddingResource->AddrRangeMax = 0;
> +  PaddingResource->AddrLen      = PcdGet8 (PcdPciHotPlugResourcePadBus);
> +
> +  //
> +  // Padding for non-prefetchable memory  //  PaddingResource++;
> + PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> + PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType              = ACPI_ADDRESS_SPACE_TYPE_MEM;
> +  PaddingResource->GenFlag              = 0x0;
> +  PaddingResource->AddrSpaceGranularity = 32;
> +  PaddingResource->SpecificFlag         = EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_NON_CACHEABLE;
> +  PaddingResource->AddrRangeMin         = 0;
> +  PaddingResource->AddrLen              = PcdGet64 (PcdPciHotPlugResourcePadMem);
> +  PaddingResource->AddrRangeMax         = PaddingResource->AddrLen - 1;
> +
> +  //
> +  // Padding for prefetchable memory
> +  //
> +  PaddingResource++;
> +  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> + PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType              = ACPI_ADDRESS_SPACE_TYPE_MEM;
> +  PaddingResource->GenFlag              = 0x0;
> +  PaddingResource->AddrSpaceGranularity = 32;
> +  PaddingResource->SpecificFlag         = EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE;
> +  PaddingResource->AddrLen              = PcdGet64 (PcdPciHotPlugResourcePadPMem);
> +  PaddingResource->AddrRangeMax         = PaddingResource->AddrLen - 1;
> +
> +  //
> +  // Padding for I/O
> +  //
> +  PaddingResource++;
> +  PaddingResource->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
> + PaddingResource->Len  = (UINT16)(
> +                                   sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
> +                                   OFFSET_OF (
> +                                     EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
> +                                     ResType
> +                                     )
> +                                   );
> +  PaddingResource->ResType      = ACPI_ADDRESS_SPACE_TYPE_IO;
> +  PaddingResource->GenFlag      = 0x0;
> +  PaddingResource->SpecificFlag = 0;
> +  PaddingResource->AddrRangeMin = 0;
> +  PaddingResource->AddrLen      = PcdGet64 (PcdPciHotPlugResourcePadIO);
> +  PaddingResource->AddrRangeMax = PaddingResource->AddrLen - 1;
> +
> +  //
> +  // Terminate the entries.
> +  //
> +  PaddingResource++;
> +  ((EFI_ACPI_END_TAG_DESCRIPTOR *)PaddingResource)->Desc     = ACPI_END_TAG_DESCRIPTOR;
> +  ((EFI_ACPI_END_TAG_DESCRIPTOR *)PaddingResource)->Checksum = 0x0;
> +
> +  *HpcState = EFI_HPC_STATE_INITIALIZED | EFI_HPC_STATE_ENABLED;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Entry point for this driver.
> +
> +  @param[in] ImageHandle  Image handle of this driver.
> +  @param[in] SystemTable  Pointer to SystemTable.
> +
> +  @retval EFI_SUCESS       Driver has loaded successfully.
> +  @return                  Error codes from lower level functions.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +PciHotPlugInitialize (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  mPciHotPlugInit.GetRootHpcList     = GetRootHpcList;
> +  mPciHotPlugInit.InitializeRootHpc  = InitializeRootHpc;
> +  mPciHotPlugInit.GetResourcePadding = GetResourcePadding;
> +  return gBS->InstallMultipleProtocolInterfaces (
> +                &ImageHandle,
> +                &gEfiPciHotPlugInitProtocolGuid,
> +                &mPciHotPlugInit,
> +                NULL
> +                );
> +}
> --
> 2.25.1
>
>
>
> 
>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library
  2023-03-17 15:59   ` Leif Lindholm
@ 2023-03-20 10:02     ` Attar, AbdulLateef (Abdul Lateef)
  0 siblings, 0 replies; 28+ messages in thread
From: Attar, AbdulLateef (Abdul Lateef) @ 2023-03-20 10:02 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Chang, Abner,
	Michael D Kinney

[AMD Official Use Only - General]

Thanks for reviewing, will submit V3 patch.

-----Original Message-----
From: Leif Lindholm <quic_llindhol@quicinc.com>
Sent: 17 March 2023 21:30
To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Chang, Abner <Abner.Chang@amd.com>; Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Typo in subject: BoarkPkg

On Fri, Mar 17, 2023 at 12:20:06 +0530, Abdul Lateef Attar wrote:
> Adds SetCacheMtrrLib library for AMD processor based boards.
> This library sets MTRR value or various memory ranges.
>
> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  Platform/AMD/BoardPkg/BoardPkg.dsc            |  10 ++
>  .../SetCacheMtrrLib/SetCacheMtrrLib.inf       |  37 +++++
>  .../Library/SetCacheMtrrLib/SetCacheMtrrLib.c | 132
> ++++++++++++++++++
>  3 files changed, 179 insertions(+)
>  create mode 100644
> Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
>  create mode 100644
> Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
>
> diff --git a/Platform/AMD/BoardPkg/BoardPkg.dsc
> b/Platform/AMD/BoardPkg/BoardPkg.dsc
> index cb4065b86c60..aa0ee8287cd8 100644
> --- a/Platform/AMD/BoardPkg/BoardPkg.dsc
> +++ b/Platform/AMD/BoardPkg/BoardPkg.dsc
> @@ -18,3 +18,13 @@ [Defines]
>
>  [Packages]
>    BoardPkg/BoardPkg.dec
> +  MinPlatformPkg/MinPlatformPkg.dec
> +  MdePkg/MdePkg.dec

Sort?

> +  UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses.common.PEIM]
> +
> +SetCacheMtrrLib|BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> +
> +[Components.IA32]
> +  BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> +

Please drop blank line at end of file.

> diff --git
> a/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.inf
> new file mode 100644
> index 000000000000..c66661d3f8dc
> --- /dev/null
> +++ b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.in
> +++ f
> @@ -0,0 +1,37 @@
> +## @file
> +# Component information file for Platform SetCacheMtrr Library.
> +# This library implementation is for AMD processor based platforms.
> +#
> +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> +reserved.<BR> # # SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> +
> +[Defines]
> +  INF_VERSION                    = 1.29
> +  BASE_NAME                      = PeiSetCacheMtrrLib
> +  FILE_GUID                      = 1E8468E0-5EB4-4088-9B52-BFDC6E4DAE87
> +  MODULE_TYPE                    = PEIM
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = SetCacheMtrrLib
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  MtrrLib
> +
> +[Packages]
> +  MinPlatformPkg/MinPlatformPkg.dec
> +  MdePkg/MdePkg.dec

Sort?

> +  UefiCpuPkg/UefiCpuPkg.dec
> +
> +[Sources]
> +  SetCacheMtrrLib.c
> +
> +[Guids]
> +
> +[Pcd]
> +  gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaBaseAddress
> +  gMinPlatformPkgTokenSpaceGuid.PcdFlashAreaSize
> +

Please drop blank line at end of file.

/
    Leif

> diff --git
> a/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
> b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
> new file mode 100644
> index 000000000000..18404405d9fa
> --- /dev/null
> +++ b/Platform/AMD/BoardPkg/Library/SetCacheMtrrLib/SetCacheMtrrLib.c
> @@ -0,0 +1,132 @@
> +/** @file
> +
> +SetCacheMtrr library functions.
> +This library implementation is for AMD processor based platforms.
> +
> +Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
> +
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Uefi.h>
> +#include <PiPei.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MtrrLib.h>
> +
> +/**
> +  This function sets the cache MTRR values for PEI phase.
> +**/
> +VOID
> +EFIAPI
> +SetCacheMtrr (
> +  VOID
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0,
> +             0xA0000,
> +             CacheWriteBack
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteBack for 0-0x9FFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0xA0000,
> +             0x20000,
> +             CacheUncacheable
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheUncacheable for 0xA0000-0xBFFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0xC0000,
> +             0x40000,
> +             CacheWriteProtected
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteProtected for 0xC0000-0xFFFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             0x100000,
> +             0xAFF00000,
> +             CacheWriteBack
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteBack for 0x100000-0xAFFFFFFF\n",
> +      Status
> +      ));
> +  }
> +
> +  Status = MtrrSetMemoryAttribute (
> +             PcdGet32 (PcdFlashAreaBaseAddress),
> +             PcdGet32 (PcdFlashAreaSize),
> +             CacheWriteProtected
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheWriteProtected for 0x%X-0x%X\n",
> +      Status,
> +      PcdGet32 (PcdFlashAreaBaseAddress),
> +      PcdGet32 (PcdFlashAreaBaseAddress) + PcdGet32 (PcdFlashAreaSize)
> +      ));
> +  }
> +
> +  MtrrDebugPrintAllMtrrs ();
> +  return;
> +}
> +
> +/**
> +  Update MTRR setting in EndOfPei phase.
> +  This function will set the MTRR value as CacheUncacheable
> +  for Flash address.
> +
> +  @retval  EFI_SUCCESS  The function completes successfully.
> +  @retval  Others       Some error occurs.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SetCacheMtrrAfterEndOfPei (
> +  VOID
> +  )
> +{
> +  EFI_STATUS  Status;
> +
> +  Status = MtrrSetMemoryAttribute (
> +             PcdGet32 (PcdFlashAreaBaseAddress),
> +             PcdGet32 (PcdFlashAreaSize),
> +             CacheUncacheable
> +             );
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((
> +      DEBUG_ERROR,
> +      "Error(%r) in setting CacheUncacheable for 0x%X-0x%X\n",
> +      Status,
> +      PcdGet32 (PcdFlashAreaBaseAddress),
> +      PcdGet32 (PcdFlashAreaBaseAddress) + PcdGet32 (PcdFlashAreaSize)
> +      ));
> +  }
> +
> +  MtrrDebugPrintAllMtrrs ();
> +  return EFI_SUCCESS;
> +}
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-17 16:09   ` Leif Lindholm
  2023-03-18  9:03     ` Chang, Abner
  2023-03-18  9:16     ` Chang, Abner
@ 2023-03-20 10:11     ` Attar, AbdulLateef (Abdul Lateef)
  2 siblings, 0 replies; 28+ messages in thread
From: Attar, AbdulLateef (Abdul Lateef) @ 2023-03-20 10:11 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Chang, Abner,
	Michael D Kinney

[AMD Official Use Only - General]

Hi Leif,
How about adding a separate entry?
Currently its generic to adapt the MinPlatformPkg, later will add more specific platforms.

AMD board and platform
F: Platform/AMD/BoardPkg
F: Platform/AMD/PlatformPkg
M: maintainer1
M: maintainer2

AMD Seattle
F: Platform/AMD/OverdriveBoard/
F: Platform/LeMaker/CelloBoard/
F: Platform/SoftIron/
F: Silicon/AMD/Styx/
M: Ard Biesheuvel <ardb+tianocore@kernel.org>
M: Leif Lindholm <quic_llindhol@quicinc.com>

Thanks
AbduL

-----Original Message-----
From: Leif Lindholm <quic_llindhol@quicinc.com>
Sent: 17 March 2023 21:39
To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Chang, Abner <Abner.Chang@amd.com>; Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
>
> Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
>
> Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> ---
>  Maintainers.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/Maintainers.txt b/Maintainers.txt index
> 747191366070..bb8ab643e090 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
>  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
>  M: Leif Lindholm <quic_llindhol@quicinc.com>
>
> +AMD Platform

Annoyingly, I'm now going to ask the question I have been avoiding up until now. What does "AMD Platform" mean?
I mean, you've placed this straight after the entry for the Seattle platforms.
I think the name, and package names, are too generic.

/
    Leif

> +F: Platform/AMD/BoardPkg
> +F: Platform/AMD/PlatformPkg
> +M: Abner Chang <abner.chang@amd.com>
> +M: Abdul Lateef Attar <abdattar@amd.com>
> +
>  Ampere Computing
>  F: Platform/Ampere
>  F: Silicon/Ampere
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-18  9:03     ` Chang, Abner
@ 2023-03-20 12:54       ` Leif Lindholm
  0 siblings, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2023-03-20 12:54 UTC (permalink / raw)
  To: devel, abner.chang
  Cc: Attar, AbdulLateef (Abdul Lateef), Ard Biesheuvel,
	Michael D Kinney

Hi Abner,

On Sat, Mar 18, 2023 at 09:03:50 +0000, Chang, Abner via groups.io wrote:
> AMD\PlatformPkg is mainly for the modules that are generic to AMD's
> server, client or other platforms, while AMD\BoardPkg that contains
> the modules that support MinPlatformrPkg framework. AMD\BoardPkg may
> also provide the modules those are common to all AMD boards and
> leverage by the board specific packages under AMD\BoardPkg. Perhaps
> AMD\Board would a better naming that contains both common board
> modules and the specific board packages.

We don't really have a rigorous "here is what the contents of a vendor
directory should look like" ruleset.

My interpretation is as follows:
a Pkg has a .dec and a Pkg shouldn't live in a Pkg.

So, I'm not opposed to non-package subdirectories to AMD that
themselves contain packages.

But then, what's the difference between a board and a platform?

> I don't know the context of
> OverdriveBoard , however we would like to see OverdriveBoard is
> relocated to under AMD\Board(Pkg) as well. With this we can have a
> well organized AMD directory under edk2-platform that provides AMD
> opensource edk2 solution to industry.

That sounds ideal to me.
We just need to figure out a directory layout that will work, for
Overdrive/Styx, the platforms supported by this set, and future
platforms.

Regards,

Leif


> 
> Thanks
> Abner
> 
> > -----Original Message-----
> > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > Sent: Saturday, March 18, 2023 12:09 AM
> > To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> > Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> > <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> > <ardb+tianocore@kernel.org>; Chang, Abner <Abner.Chang@amd.com>;
> > Michael D Kinney <michael.d.kinney@intel.com>
> > Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg
> > and AMD/PlatformPkg maintainers
> > 
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> > 
> > 
> > On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> > > From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> > >
> > > Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> > >
> > > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > > Cc: Abner Chang <abner.chang@amd.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > ---
> > >  Maintainers.txt | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/Maintainers.txt b/Maintainers.txt index
> > > 747191366070..bb8ab643e090 100644
> > > --- a/Maintainers.txt
> > > +++ b/Maintainers.txt
> > > @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
> > >  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > >  M: Leif Lindholm <quic_llindhol@quicinc.com>
> > >
> > > +AMD Platform
> > 
> > Annoyingly, I'm now going to ask the question I have been avoiding up until
> > now. What does "AMD Platform" mean?
> > I mean, you've placed this straight after the entry for the Seattle platforms.
> > I think the name, and package names, are too generic.
> > 
> > /
> >     Leif
> > 
> > > +F: Platform/AMD/BoardPkg
> > > +F: Platform/AMD/PlatformPkg
> > > +M: Abner Chang <abner.chang@amd.com>
> > > +M: Abdul Lateef Attar <abdattar@amd.com>
> > > +
> > >  Ampere Computing
> > >  F: Platform/Ampere
> > >  F: Silicon/Ampere
> > > --
> > > 2.25.1
> > >
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-18  9:16     ` Chang, Abner
  2023-03-18  9:42       ` Ard Biesheuvel
@ 2023-03-20 13:05       ` Leif Lindholm
  2023-03-20 14:09         ` Attar, AbdulLateef (Abdul Lateef)
  1 sibling, 1 reply; 28+ messages in thread
From: Leif Lindholm @ 2023-03-20 13:05 UTC (permalink / raw)
  To: devel, abner.chang
  Cc: Attar, AbdulLateef (Abdul Lateef), Ard Biesheuvel,
	Michael D Kinney

Hi Abner,

On Sat, Mar 18, 2023 at 09:16:17 +0000, Chang, Abner via groups.io wrote:
> I don't see any modules under OverdriveBoard, is this package still

The code is split between Platform/AMD and Silicon/AMD/Styx, the
latter also being used by SoftIron/Overdrive1000 and LeMaker Cello
(although that one is pretty much defunct and should probably be
dropped).

> in use? Do you still remember where is FDF and DSC come from (as
> there is AMD copyright 2014-2016) back to the moment when you was
> introduced this package?

As the git history tells you:
---
commit f4d38e50c0f24eb78eb003a94f583025621c63db
Author: Leif Lindholm <leif.lindholm@linaro.org>
Date:   Thu Aug 3 12:24:22 2017 +0100

    Platform,Silicon: import AMD Styx SoC support and platforms

    Common files for AMD Overdrive, SoftIron Overdrive 1000
    and LeMaker Cello, as well as actual platform support.
    Imported from commit efd798c1eb of
    https://git.linaro.org/uefi/OpenPlatformPkg.git
---

and the initial commit of the platform in that repository is:
---
From: Leo Duran <leo.duran@amd.com>
Date: Thu, 20 Aug 2015 13:30:24 -0500

    Subject: Platforms/AMD: add support for AMD Overdrive and Lemaker Cello

    This adds support for the AMD Seattle based Overdrive and Husky
    platforms,
    and the Lemaker Cello which is derived from it.

    This code was tested with upstream EDK2 commit 758ea94651.

    The binaries in this branch are based on SeattleFDK 1.0.0.2
    (Linaro SeattleFDK commit 4b419f2ef2)
---

Regards,

Leif

> 
> Thanks
> Abner
> 
> > -----Original Message-----
> > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > Sent: Saturday, March 18, 2023 12:09 AM
> > To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> > Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> > <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> > <ardb+tianocore@kernel.org>; Chang, Abner <Abner.Chang@amd.com>;
> > Michael D Kinney <michael.d.kinney@intel.com>
> > Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg
> > and AMD/PlatformPkg maintainers
> > 
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> > 
> > 
> > On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> > > From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> > >
> > > Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> > >
> > > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > > Cc: Abner Chang <abner.chang@amd.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > ---
> > >  Maintainers.txt | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/Maintainers.txt b/Maintainers.txt index
> > > 747191366070..bb8ab643e090 100644
> > > --- a/Maintainers.txt
> > > +++ b/Maintainers.txt
> > > @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
> > >  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > >  M: Leif Lindholm <quic_llindhol@quicinc.com>
> > >
> > > +AMD Platform
> > 
> > Annoyingly, I'm now going to ask the question I have been avoiding up until
> > now. What does "AMD Platform" mean?
> > I mean, you've placed this straight after the entry for the Seattle platforms.
> > I think the name, and package names, are too generic.
> > 
> > /
> >     Leif
> > 
> > > +F: Platform/AMD/BoardPkg
> > > +F: Platform/AMD/PlatformPkg
> > > +M: Abner Chang <abner.chang@amd.com>
> > > +M: Abdul Lateef Attar <abdattar@amd.com>
> > > +
> > >  Ampere Computing
> > >  F: Platform/Ampere
> > >  F: Silicon/Ampere
> > > --
> > > 2.25.1
> > >
> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-20 13:05       ` [edk2-devel] " Leif Lindholm
@ 2023-03-20 14:09         ` Attar, AbdulLateef (Abdul Lateef)
  2023-03-20 18:02           ` Leif Lindholm
  0 siblings, 1 reply; 28+ messages in thread
From: Attar, AbdulLateef (Abdul Lateef) @ 2023-03-20 14:09 UTC (permalink / raw)
  To: Leif Lindholm, devel@edk2.groups.io, Chang, Abner
  Cc: Ard Biesheuvel, Michael D Kinney

[Public]

Hi Leif,

AMD/BoardPkg will contains the modules/drivers to support MinPlatformPkg framework.
   BoardPkg will be generic across all AMD boards which are based on MinPlatformPkg framework.
   It's like "edk2-platforms/Platform/Qemu/QemuOpenBoardPkg".
   We will gradually add modules and libraries to it.

AMD/PlatformPkg will contain the modules/drivers which are generic to future AMD platform.
   Currently we don't have complete platform, we can drop PlatformPkg for now till we have complete(reasonable modules) solution.

It's better to not touch existing AMD/OverdriveBoard, restructuring requires changes to .dsc and .fdf files, might break backward compatibility.

How about having just BoardPkg like below?

AMD board package
F: Platform/AMD/BoardPkg
M: Maintainer1
M: Maintainer2

Please let me know your thoughts.

Thanks
AbduL




-----Original Message-----
From: Leif Lindholm <quic_llindhol@quicinc.com>
Sent: 20 March 2023 18:35
To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Hi Abner,

On Sat, Mar 18, 2023 at 09:16:17 +0000, Chang, Abner via groups.io wrote:
> I don't see any modules under OverdriveBoard, is this package still

The code is split between Platform/AMD and Silicon/AMD/Styx, the latter also being used by SoftIron/Overdrive1000 and LeMaker Cello (although that one is pretty much defunct and should probably be dropped).

> in use? Do you still remember where is FDF and DSC come from (as there
> is AMD copyright 2014-2016) back to the moment when you was introduced
> this package?

As the git history tells you:
---
commit f4d38e50c0f24eb78eb003a94f583025621c63db
Author: Leif Lindholm <leif.lindholm@linaro.org>
Date:   Thu Aug 3 12:24:22 2017 +0100

    Platform,Silicon: import AMD Styx SoC support and platforms

    Common files for AMD Overdrive, SoftIron Overdrive 1000
    and LeMaker Cello, as well as actual platform support.
    Imported from commit efd798c1eb of
    https://git.linaro.org/uefi/OpenPlatformPkg.git
---

and the initial commit of the platform in that repository is:
---
From: Leo Duran <leo.duran@amd.com>
Date: Thu, 20 Aug 2015 13:30:24 -0500

    Subject: Platforms/AMD: add support for AMD Overdrive and Lemaker Cello

    This adds support for the AMD Seattle based Overdrive and Husky
    platforms,
    and the Lemaker Cello which is derived from it.

    This code was tested with upstream EDK2 commit 758ea94651.

    The binaries in this branch are based on SeattleFDK 1.0.0.2
    (Linaro SeattleFDK commit 4b419f2ef2)
---

Regards,

Leif

>
> Thanks
> Abner
>
> > -----Original Message-----
> > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > Sent: Saturday, March 18, 2023 12:09 AM
> > To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> > Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> > <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> > <ardb+tianocore@kernel.org>; Chang, Abner <Abner.Chang@amd.com>;
> > Michael D Kinney <michael.d.kinney@intel.com>
> > Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> > AMD/BoardPkg and AMD/PlatformPkg maintainers
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> > > From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> > >
> > > Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> > >
> > > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > > Cc: Abner Chang <abner.chang@amd.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > ---
> > >  Maintainers.txt | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > >
> > > diff --git a/Maintainers.txt b/Maintainers.txt index
> > > 747191366070..bb8ab643e090 100644
> > > --- a/Maintainers.txt
> > > +++ b/Maintainers.txt
> > > @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
> > >  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > >  M: Leif Lindholm <quic_llindhol@quicinc.com>
> > >
> > > +AMD Platform
> >
> > Annoyingly, I'm now going to ask the question I have been avoiding
> > up until now. What does "AMD Platform" mean?
> > I mean, you've placed this straight after the entry for the Seattle platforms.
> > I think the name, and package names, are too generic.
> >
> > /
> >     Leif
> >
> > > +F: Platform/AMD/BoardPkg
> > > +F: Platform/AMD/PlatformPkg
> > > +M: Abner Chang <abner.chang@amd.com>
> > > +M: Abdul Lateef Attar <abdattar@amd.com>
> > > +
> > >  Ampere Computing
> > >  F: Platform/Ampere
> > >  F: Silicon/Ampere
> > > --
> > > 2.25.1
> > >
>
>
> 
>
>

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-20 14:09         ` Attar, AbdulLateef (Abdul Lateef)
@ 2023-03-20 18:02           ` Leif Lindholm
  2023-03-21  3:00             ` Attar, AbdulLateef (Abdul Lateef)
  0 siblings, 1 reply; 28+ messages in thread
From: Leif Lindholm @ 2023-03-20 18:02 UTC (permalink / raw)
  To: Attar, AbdulLateef (Abdul Lateef)
  Cc: devel@edk2.groups.io, Chang, Abner, Ard Biesheuvel,
	Michael D Kinney

Hi Abdul,

On Mon, Mar 20, 2023 at 14:09:11 +0000, Attar, AbdulLateef (Abdul Lateef) wrote:
> AMD/BoardPkg will contains the modules/drivers to support MinPlatformPkg framework.
>    BoardPkg will be generic across all AMD boards which are based on MinPlatformPkg framework.
>    It's like "edk2-platforms/Platform/Qemu/QemuOpenBoardPkg".
>    We will gradually add modules and libraries to it.

So, if the purpose is exclusively to support MinPlatformPkg platforms,
I think that should be part of the name.
Like Platform/AMD/AmdMinPlatformPkg (for example).

> AMD/PlatformPkg will contain the modules/drivers which are generic to future AMD platform.
>    Currently we don't have complete platform, we can drop
>    PlatformPkg for now till we have complete(reasonable modules)
>    solution.

It will certainly be easier to reason about what the preferred
naming/layout should be once there is code to look at for examples.
It may be that parts of it would live more naturally under
Silicon/AMD, for example.

> It's better to not touch existing AMD/OverdriveBoard, restructuring
> requires changes to .dsc and .fdf files, might break backward
> compatibility.

No problem. Config files occasionally need to be revamped, but that's
simply a mechanical exercise.

> How about having just BoardPkg like below?

That, too, is a completely generic name that conveys no information
about *what* boards one can expect to find in there.

Regards,

Leif

> AMD board package
> F: Platform/AMD/BoardPkg
> M: Maintainer1
> M: Maintainer2
> 
> Please let me know your thoughts.
> 
> Thanks
> AbduL
> 
> 
> 
> 
> -----Original Message-----
> From: Leif Lindholm <quic_llindhol@quicinc.com>
> Sent: 20 March 2023 18:35
> To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> Hi Abner,
> 
> On Sat, Mar 18, 2023 at 09:16:17 +0000, Chang, Abner via groups.io wrote:
> > I don't see any modules under OverdriveBoard, is this package still
> 
> The code is split between Platform/AMD and Silicon/AMD/Styx, the latter also being used by SoftIron/Overdrive1000 and LeMaker Cello (although that one is pretty much defunct and should probably be dropped).
> 
> > in use? Do you still remember where is FDF and DSC come from (as there
> > is AMD copyright 2014-2016) back to the moment when you was introduced
> > this package?
> 
> As the git history tells you:
> ---
> commit f4d38e50c0f24eb78eb003a94f583025621c63db
> Author: Leif Lindholm <leif.lindholm@linaro.org>
> Date:   Thu Aug 3 12:24:22 2017 +0100
> 
>     Platform,Silicon: import AMD Styx SoC support and platforms
> 
>     Common files for AMD Overdrive, SoftIron Overdrive 1000
>     and LeMaker Cello, as well as actual platform support.
>     Imported from commit efd798c1eb of
>     https://git.linaro.org/uefi/OpenPlatformPkg.git
> ---
> 
> and the initial commit of the platform in that repository is:
> ---
> From: Leo Duran <leo.duran@amd.com>
> Date: Thu, 20 Aug 2015 13:30:24 -0500
> 
>     Subject: Platforms/AMD: add support for AMD Overdrive and Lemaker Cello
> 
>     This adds support for the AMD Seattle based Overdrive and Husky
>     platforms,
>     and the Lemaker Cello which is derived from it.
> 
>     This code was tested with upstream EDK2 commit 758ea94651.
> 
>     The binaries in this branch are based on SeattleFDK 1.0.0.2
>     (Linaro SeattleFDK commit 4b419f2ef2)
> ---
> 
> Regards,
> 
> Leif
> 
> >
> > Thanks
> > Abner
> >
> > > -----Original Message-----
> > > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > > Sent: Saturday, March 18, 2023 12:09 AM
> > > To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> > > Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> > > <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> > > <ardb+tianocore@kernel.org>; Chang, Abner <Abner.Chang@amd.com>;
> > > Michael D Kinney <michael.d.kinney@intel.com>
> > > Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> > > AMD/BoardPkg and AMD/PlatformPkg maintainers
> > >
> > > Caution: This message originated from an External Source. Use proper
> > > caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> > > > From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> > > >
> > > > Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> > > >
> > > > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > Cc: Abner Chang <abner.chang@amd.com>
> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > ---
> > > >  Maintainers.txt | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/Maintainers.txt b/Maintainers.txt index
> > > > 747191366070..bb8ab643e090 100644
> > > > --- a/Maintainers.txt
> > > > +++ b/Maintainers.txt
> > > > @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
> > > >  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > >  M: Leif Lindholm <quic_llindhol@quicinc.com>
> > > >
> > > > +AMD Platform
> > >
> > > Annoyingly, I'm now going to ask the question I have been avoiding
> > > up until now. What does "AMD Platform" mean?
> > > I mean, you've placed this straight after the entry for the Seattle platforms.
> > > I think the name, and package names, are too generic.
> > >
> > > /
> > >     Leif
> > >
> > > > +F: Platform/AMD/BoardPkg
> > > > +F: Platform/AMD/PlatformPkg
> > > > +M: Abner Chang <abner.chang@amd.com>
> > > > +M: Abdul Lateef Attar <abdattar@amd.com>
> > > > +
> > > >  Ampere Computing
> > > >  F: Platform/Ampere
> > > >  F: Silicon/Ampere
> > > > --
> > > > 2.25.1
> > > >
> >
> >
> > 
> >
> >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-20 18:02           ` Leif Lindholm
@ 2023-03-21  3:00             ` Attar, AbdulLateef (Abdul Lateef)
  2023-03-22  9:54               ` Chang, Abner
  0 siblings, 1 reply; 28+ messages in thread
From: Attar, AbdulLateef (Abdul Lateef) @ 2023-03-21  3:00 UTC (permalink / raw)
  To: Leif Lindholm
  Cc: devel@edk2.groups.io, Chang, Abner, Ard Biesheuvel,
	Michael D Kinney

[AMD Official Use Only - General]

Hi Leif,
        AMD/BoardPkg will implements modules specific to boards(one or more motherboards).
It will not contain any module specific to Platform or Silicon, which are part of MinPlatformPkg.

How about just Min prefix?
AMD/MinBoardPkg (I am avoiding the Amd prefix because its already in AMD folder).

AMD board package for MinPlatformPkg
 F: Platform/AMD/MinBoardPkg
 M: Maintainer1
 M: Maintainer2

Thanks
AbduL

-----Original Message-----
From: Leif Lindholm <quic_llindhol@quicinc.com>
Sent: 20 March 2023 23:33
To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Hi Abdul,

On Mon, Mar 20, 2023 at 14:09:11 +0000, Attar, AbdulLateef (Abdul Lateef) wrote:
> AMD/BoardPkg will contains the modules/drivers to support MinPlatformPkg framework.
>    BoardPkg will be generic across all AMD boards which are based on MinPlatformPkg framework.
>    It's like "edk2-platforms/Platform/Qemu/QemuOpenBoardPkg".
>    We will gradually add modules and libraries to it.

So, if the purpose is exclusively to support MinPlatformPkg platforms, I think that should be part of the name.
Like Platform/AMD/AmdMinPlatformPkg (for example).

> AMD/PlatformPkg will contain the modules/drivers which are generic to future AMD platform.
>    Currently we don't have complete platform, we can drop
>    PlatformPkg for now till we have complete(reasonable modules)
>    solution.

It will certainly be easier to reason about what the preferred naming/layout should be once there is code to look at for examples.
It may be that parts of it would live more naturally under Silicon/AMD, for example.

> It's better to not touch existing AMD/OverdriveBoard, restructuring
> requires changes to .dsc and .fdf files, might break backward
> compatibility.

No problem. Config files occasionally need to be revamped, but that's simply a mechanical exercise.

> How about having just BoardPkg like below?

That, too, is a completely generic name that conveys no information about *what* boards one can expect to find in there.

Regards,

Leif

> AMD board package
> F: Platform/AMD/BoardPkg
> M: Maintainer1
> M: Maintainer2
>
> Please let me know your thoughts.
>
> Thanks
> AbduL
>
>
>
>
> -----Original Message-----
> From: Leif Lindholm <quic_llindhol@quicinc.com>
> Sent: 20 March 2023 18:35
> To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> AMD/BoardPkg and AMD/PlatformPkg maintainers
>
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> Hi Abner,
>
> On Sat, Mar 18, 2023 at 09:16:17 +0000, Chang, Abner via groups.io wrote:
> > I don't see any modules under OverdriveBoard, is this package still
>
> The code is split between Platform/AMD and Silicon/AMD/Styx, the latter also being used by SoftIron/Overdrive1000 and LeMaker Cello (although that one is pretty much defunct and should probably be dropped).
>
> > in use? Do you still remember where is FDF and DSC come from (as
> > there is AMD copyright 2014-2016) back to the moment when you was
> > introduced this package?
>
> As the git history tells you:
> ---
> commit f4d38e50c0f24eb78eb003a94f583025621c63db
> Author: Leif Lindholm <leif.lindholm@linaro.org>
> Date:   Thu Aug 3 12:24:22 2017 +0100
>
>     Platform,Silicon: import AMD Styx SoC support and platforms
>
>     Common files for AMD Overdrive, SoftIron Overdrive 1000
>     and LeMaker Cello, as well as actual platform support.
>     Imported from commit efd798c1eb of
>     https://git.linaro.org/uefi/OpenPlatformPkg.git
> ---
>
> and the initial commit of the platform in that repository is:
> ---
> From: Leo Duran <leo.duran@amd.com>
> Date: Thu, 20 Aug 2015 13:30:24 -0500
>
>     Subject: Platforms/AMD: add support for AMD Overdrive and Lemaker
> Cello
>
>     This adds support for the AMD Seattle based Overdrive and Husky
>     platforms,
>     and the Lemaker Cello which is derived from it.
>
>     This code was tested with upstream EDK2 commit 758ea94651.
>
>     The binaries in this branch are based on SeattleFDK 1.0.0.2
>     (Linaro SeattleFDK commit 4b419f2ef2)
> ---
>
> Regards,
>
> Leif
>
> >
> > Thanks
> > Abner
> >
> > > -----Original Message-----
> > > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > > Sent: Saturday, March 18, 2023 12:09 AM
> > > To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> > > Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> > > <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> > > <ardb+tianocore@kernel.org>; Chang, Abner <Abner.Chang@amd.com>;
> > > Michael D Kinney <michael.d.kinney@intel.com>
> > > Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> > > AMD/BoardPkg and AMD/PlatformPkg maintainers
> > >
> > > Caution: This message originated from an External Source. Use
> > > proper caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> > > > From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> > > >
> > > > Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> > > >
> > > > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > Cc: Abner Chang <abner.chang@amd.com>
> > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > ---
> > > >  Maintainers.txt | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/Maintainers.txt b/Maintainers.txt index
> > > > 747191366070..bb8ab643e090 100644
> > > > --- a/Maintainers.txt
> > > > +++ b/Maintainers.txt
> > > > @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
> > > >  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > >  M: Leif Lindholm <quic_llindhol@quicinc.com>
> > > >
> > > > +AMD Platform
> > >
> > > Annoyingly, I'm now going to ask the question I have been avoiding
> > > up until now. What does "AMD Platform" mean?
> > > I mean, you've placed this straight after the entry for the Seattle platforms.
> > > I think the name, and package names, are too generic.
> > >
> > > /
> > >     Leif
> > >
> > > > +F: Platform/AMD/BoardPkg
> > > > +F: Platform/AMD/PlatformPkg
> > > > +M: Abner Chang <abner.chang@amd.com>
> > > > +M: Abdul Lateef Attar <abdattar@amd.com>
> > > > +
> > > >  Ampere Computing
> > > >  F: Platform/Ampere
> > > >  F: Silicon/Ampere
> > > > --
> > > > 2.25.1
> > > >
> >
> >
> > 
> >
> >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-21  3:00             ` Attar, AbdulLateef (Abdul Lateef)
@ 2023-03-22  9:54               ` Chang, Abner
  2023-03-22 11:59                 ` Attar, AbdulLateef (Abdul Lateef)
  0 siblings, 1 reply; 28+ messages in thread
From: Chang, Abner @ 2023-03-22  9:54 UTC (permalink / raw)
  To: Attar, AbdulLateef (Abdul Lateef), Leif Lindholm
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Michael D Kinney

[AMD Official Use Only - General]



> -----Original Message-----
> From: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> Sent: Tuesday, March 21, 2023 11:01 AM
> To: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> AMD/BoardPkg and AMD/PlatformPkg maintainers
> 
> [AMD Official Use Only - General]
> 
> Hi Leif,
>         AMD/BoardPkg will implements modules specific to boards(one or more
> motherboards).
> It will not contain any module specific to Platform or Silicon, which are part of
> MinPlatformPkg.
> 
> How about just Min prefix?
> AMD/MinBoardPkg (I am avoiding the Amd prefix because its already in AMD
> folder).
I am good with this naming. So we will have AMD boards that leverage MinPlatform under MinBoardPkg. Those boards which are not Minplatform based can just stay under Platforms/AMD (e.g., OverdriveBoard.), is my understanding correct Abdul?
Abner
> 
> AMD board package for MinPlatformPkg
>  F: Platform/AMD/MinBoardPkg
>  M: Maintainer1
>  M: Maintainer2
> 
> Thanks
> AbduL
> 
> -----Original Message-----
> From: Leif Lindholm <quic_llindhol@quicinc.com>
> Sent: 20 March 2023 23:33
> To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> Cc: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> AMD/BoardPkg and AMD/PlatformPkg maintainers
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> Hi Abdul,
> 
> On Mon, Mar 20, 2023 at 14:09:11 +0000, Attar, AbdulLateef (Abdul Lateef)
> wrote:
> > AMD/BoardPkg will contains the modules/drivers to support
> MinPlatformPkg framework.
> >    BoardPkg will be generic across all AMD boards which are based on
> MinPlatformPkg framework.
> >    It's like "edk2-platforms/Platform/Qemu/QemuOpenBoardPkg".
> >    We will gradually add modules and libraries to it.
> 
> So, if the purpose is exclusively to support MinPlatformPkg platforms, I think
> that should be part of the name.
> Like Platform/AMD/AmdMinPlatformPkg (for example).
> 
> > AMD/PlatformPkg will contain the modules/drivers which are generic to
> future AMD platform.
> >    Currently we don't have complete platform, we can drop
> >    PlatformPkg for now till we have complete(reasonable modules)
> >    solution.
> 
> It will certainly be easier to reason about what the preferred naming/layout
> should be once there is code to look at for examples.
> It may be that parts of it would live more naturally under Silicon/AMD, for
> example.
> 
> > It's better to not touch existing AMD/OverdriveBoard, restructuring
> > requires changes to .dsc and .fdf files, might break backward
> > compatibility.
> 
> No problem. Config files occasionally need to be revamped, but that's simply
> a mechanical exercise.
> 
> > How about having just BoardPkg like below?
> 
> That, too, is a completely generic name that conveys no information about
> *what* boards one can expect to find in there.
> 
> Regards,
> 
> Leif
> 
> > AMD board package
> > F: Platform/AMD/BoardPkg
> > M: Maintainer1
> > M: Maintainer2
> >
> > Please let me know your thoughts.
> >
> > Thanks
> > AbduL
> >
> >
> >
> >
> > -----Original Message-----
> > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > Sent: 20 March 2023 18:35
> > To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> > Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Ard
> > Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> > <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> > AMD/BoardPkg and AMD/PlatformPkg maintainers
> >
> > Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> >
> >
> > Hi Abner,
> >
> > On Sat, Mar 18, 2023 at 09:16:17 +0000, Chang, Abner via groups.io wrote:
> > > I don't see any modules under OverdriveBoard, is this package still
> >
> > The code is split between Platform/AMD and Silicon/AMD/Styx, the latter
> also being used by SoftIron/Overdrive1000 and LeMaker Cello (although that
> one is pretty much defunct and should probably be dropped).
> >
> > > in use? Do you still remember where is FDF and DSC come from (as
> > > there is AMD copyright 2014-2016) back to the moment when you was
> > > introduced this package?
> >
> > As the git history tells you:
> > ---
> > commit f4d38e50c0f24eb78eb003a94f583025621c63db
> > Author: Leif Lindholm <leif.lindholm@linaro.org>
> > Date:   Thu Aug 3 12:24:22 2017 +0100
> >
> >     Platform,Silicon: import AMD Styx SoC support and platforms
> >
> >     Common files for AMD Overdrive, SoftIron Overdrive 1000
> >     and LeMaker Cello, as well as actual platform support.
> >     Imported from commit efd798c1eb of
> >     https://git.linaro.org/uefi/OpenPlatformPkg.git
> > ---
> >
> > and the initial commit of the platform in that repository is:
> > ---
> > From: Leo Duran <leo.duran@amd.com>
> > Date: Thu, 20 Aug 2015 13:30:24 -0500
> >
> >     Subject: Platforms/AMD: add support for AMD Overdrive and Lemaker
> > Cello
> >
> >     This adds support for the AMD Seattle based Overdrive and Husky
> >     platforms,
> >     and the Lemaker Cello which is derived from it.
> >
> >     This code was tested with upstream EDK2 commit 758ea94651.
> >
> >     The binaries in this branch are based on SeattleFDK 1.0.0.2
> >     (Linaro SeattleFDK commit 4b419f2ef2)
> > ---
> >
> > Regards,
> >
> > Leif
> >
> > >
> > > Thanks
> > > Abner
> > >
> > > > -----Original Message-----
> > > > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > Sent: Saturday, March 18, 2023 12:09 AM
> > > > To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> > > > Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> > > > <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> > > > <ardb+tianocore@kernel.org>; Chang, Abner
> <Abner.Chang@amd.com>;
> > > > Michael D Kinney <michael.d.kinney@intel.com>
> > > > Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> > > > AMD/BoardPkg and AMD/PlatformPkg maintainers
> > > >
> > > > Caution: This message originated from an External Source. Use
> > > > proper caution when opening attachments, clicking links, or responding.
> > > >
> > > >
> > > > On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> > > > > From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> > > > >
> > > > > Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> > > > >
> > > > > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > > > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > > > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > > Cc: Abner Chang <abner.chang@amd.com>
> > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > > ---
> > > > >  Maintainers.txt | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/Maintainers.txt b/Maintainers.txt index
> > > > > 747191366070..bb8ab643e090 100644
> > > > > --- a/Maintainers.txt
> > > > > +++ b/Maintainers.txt
> > > > > @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
> > > > >  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > > >  M: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > >
> > > > > +AMD Platform
> > > >
> > > > Annoyingly, I'm now going to ask the question I have been avoiding
> > > > up until now. What does "AMD Platform" mean?
> > > > I mean, you've placed this straight after the entry for the Seattle
> platforms.
> > > > I think the name, and package names, are too generic.
> > > >
> > > > /
> > > >     Leif
> > > >
> > > > > +F: Platform/AMD/BoardPkg
> > > > > +F: Platform/AMD/PlatformPkg
> > > > > +M: Abner Chang <abner.chang@amd.com>
> > > > > +M: Abdul Lateef Attar <abdattar@amd.com>
> > > > > +
> > > > >  Ampere Computing
> > > > >  F: Platform/Ampere
> > > > >  F: Silicon/Ampere
> > > > > --
> > > > > 2.25.1
> > > > >
> > >
> > >
> > > 
> > >
> > >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-22  9:54               ` Chang, Abner
@ 2023-03-22 11:59                 ` Attar, AbdulLateef (Abdul Lateef)
  2023-03-22 12:29                   ` Leif Lindholm
  0 siblings, 1 reply; 28+ messages in thread
From: Attar, AbdulLateef (Abdul Lateef) @ 2023-03-22 11:59 UTC (permalink / raw)
  To: Chang, Abner, Leif Lindholm
  Cc: devel@edk2.groups.io, Ard Biesheuvel, Michael D Kinney

[Public]

Hi Abner,
        That's right, Platform/AMD/MinBoardPkg will contains all modules/library/drivers required for AMD boards which are based on MinPlatformPkg framework.
Thanks
AbduL

-----Original Message-----
From: Chang, Abner <Abner.Chang@amd.com>
Sent: 22 March 2023 15:25
To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Leif Lindholm <quic_llindhol@quicinc.com>
Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers

[AMD Official Use Only - General]



> -----Original Message-----
> From: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> Sent: Tuesday, March 21, 2023 11:01 AM
> To: Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> AMD/BoardPkg and AMD/PlatformPkg maintainers
>
> [AMD Official Use Only - General]
>
> Hi Leif,
>         AMD/BoardPkg will implements modules specific to boards(one or
> more motherboards).
> It will not contain any module specific to Platform or Silicon, which
> are part of MinPlatformPkg.
>
> How about just Min prefix?
> AMD/MinBoardPkg (I am avoiding the Amd prefix because its already in
> AMD folder).
I am good with this naming. So we will have AMD boards that leverage MinPlatform under MinBoardPkg. Those boards which are not Minplatform based can just stay under Platforms/AMD (e.g., OverdriveBoard.), is my understanding correct Abdul?
Abner
>
> AMD board package for MinPlatformPkg
>  F: Platform/AMD/MinBoardPkg
>  M: Maintainer1
>  M: Maintainer2
>
> Thanks
> AbduL
>
> -----Original Message-----
> From: Leif Lindholm <quic_llindhol@quicinc.com>
> Sent: 20 March 2023 23:33
> To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> Cc: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> <michael.d.kinney@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> AMD/BoardPkg and AMD/PlatformPkg maintainers
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Hi Abdul,
>
> On Mon, Mar 20, 2023 at 14:09:11 +0000, Attar, AbdulLateef (Abdul
> Lateef)
> wrote:
> > AMD/BoardPkg will contains the modules/drivers to support
> MinPlatformPkg framework.
> >    BoardPkg will be generic across all AMD boards which are based on
> MinPlatformPkg framework.
> >    It's like "edk2-platforms/Platform/Qemu/QemuOpenBoardPkg".
> >    We will gradually add modules and libraries to it.
>
> So, if the purpose is exclusively to support MinPlatformPkg platforms,
> I think that should be part of the name.
> Like Platform/AMD/AmdMinPlatformPkg (for example).
>
> > AMD/PlatformPkg will contain the modules/drivers which are generic
> > to
> future AMD platform.
> >    Currently we don't have complete platform, we can drop
> >    PlatformPkg for now till we have complete(reasonable modules)
> >    solution.
>
> It will certainly be easier to reason about what the preferred
> naming/layout should be once there is code to look at for examples.
> It may be that parts of it would live more naturally under
> Silicon/AMD, for example.
>
> > It's better to not touch existing AMD/OverdriveBoard, restructuring
> > requires changes to .dsc and .fdf files, might break backward
> > compatibility.
>
> No problem. Config files occasionally need to be revamped, but that's
> simply a mechanical exercise.
>
> > How about having just BoardPkg like below?
>
> That, too, is a completely generic name that conveys no information
> about
> *what* boards one can expect to find in there.
>
> Regards,
>
> Leif
>
> > AMD board package
> > F: Platform/AMD/BoardPkg
> > M: Maintainer1
> > M: Maintainer2
> >
> > Please let me know your thoughts.
> >
> > Thanks
> > AbduL
> >
> >
> >
> >
> > -----Original Message-----
> > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > Sent: 20 March 2023 18:35
> > To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> > Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>;
> > Ard Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> > <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt:
> > Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
> >
> > Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> >
> >
> > Hi Abner,
> >
> > On Sat, Mar 18, 2023 at 09:16:17 +0000, Chang, Abner via groups.io wrote:
> > > I don't see any modules under OverdriveBoard, is this package
> > > still
> >
> > The code is split between Platform/AMD and Silicon/AMD/Styx, the
> > latter
> also being used by SoftIron/Overdrive1000 and LeMaker Cello (although
> that one is pretty much defunct and should probably be dropped).
> >
> > > in use? Do you still remember where is FDF and DSC come from (as
> > > there is AMD copyright 2014-2016) back to the moment when you was
> > > introduced this package?
> >
> > As the git history tells you:
> > ---
> > commit f4d38e50c0f24eb78eb003a94f583025621c63db
> > Author: Leif Lindholm <leif.lindholm@linaro.org>
> > Date:   Thu Aug 3 12:24:22 2017 +0100
> >
> >     Platform,Silicon: import AMD Styx SoC support and platforms
> >
> >     Common files for AMD Overdrive, SoftIron Overdrive 1000
> >     and LeMaker Cello, as well as actual platform support.
> >     Imported from commit efd798c1eb of
> >     https://git.linaro.org/uefi/OpenPlatformPkg.git
> > ---
> >
> > and the initial commit of the platform in that repository is:
> > ---
> > From: Leo Duran <leo.duran@amd.com>
> > Date: Thu, 20 Aug 2015 13:30:24 -0500
> >
> >     Subject: Platforms/AMD: add support for AMD Overdrive and
> > Lemaker Cello
> >
> >     This adds support for the AMD Seattle based Overdrive and Husky
> >     platforms,
> >     and the Lemaker Cello which is derived from it.
> >
> >     This code was tested with upstream EDK2 commit 758ea94651.
> >
> >     The binaries in this branch are based on SeattleFDK 1.0.0.2
> >     (Linaro SeattleFDK commit 4b419f2ef2)
> > ---
> >
> > Regards,
> >
> > Leif
> >
> > >
> > > Thanks
> > > Abner
> > >
> > > > -----Original Message-----
> > > > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > Sent: Saturday, March 18, 2023 12:09 AM
> > > > To: Attar, AbdulLateef (Abdul Lateef)
> > > > <AbdulLateef.Attar@amd.com>
> > > > Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> > > > <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> > > > <ardb+tianocore@kernel.org>; Chang, Abner
> <Abner.Chang@amd.com>;
> > > > Michael D Kinney <michael.d.kinney@intel.com>
> > > > Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> > > > AMD/BoardPkg and AMD/PlatformPkg maintainers
> > > >
> > > > Caution: This message originated from an External Source. Use
> > > > proper caution when opening attachments, clicking links, or responding.
> > > >
> > > >
> > > > On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> > > > > From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> > > > >
> > > > > Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> > > > >
> > > > > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > > > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > > > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > > Cc: Abner Chang <abner.chang@amd.com>
> > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > > ---
> > > > >  Maintainers.txt | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > >
> > > > > diff --git a/Maintainers.txt b/Maintainers.txt index
> > > > > 747191366070..bb8ab643e090 100644
> > > > > --- a/Maintainers.txt
> > > > > +++ b/Maintainers.txt
> > > > > @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
> > > > >  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > > >  M: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > >
> > > > > +AMD Platform
> > > >
> > > > Annoyingly, I'm now going to ask the question I have been
> > > > avoiding up until now. What does "AMD Platform" mean?
> > > > I mean, you've placed this straight after the entry for the
> > > > Seattle
> platforms.
> > > > I think the name, and package names, are too generic.
> > > >
> > > > /
> > > >     Leif
> > > >
> > > > > +F: Platform/AMD/BoardPkg
> > > > > +F: Platform/AMD/PlatformPkg
> > > > > +M: Abner Chang <abner.chang@amd.com>
> > > > > +M: Abdul Lateef Attar <abdattar@amd.com>
> > > > > +
> > > > >  Ampere Computing
> > > > >  F: Platform/Ampere
> > > > >  F: Silicon/Ampere
> > > > > --
> > > > > 2.25.1
> > > > >
> > >
> > >
> > > 
> > >
> > >

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
  2023-03-22 11:59                 ` Attar, AbdulLateef (Abdul Lateef)
@ 2023-03-22 12:29                   ` Leif Lindholm
  0 siblings, 0 replies; 28+ messages in thread
From: Leif Lindholm @ 2023-03-22 12:29 UTC (permalink / raw)
  To: Attar, AbdulLateef (Abdul Lateef)
  Cc: Chang, Abner, devel@edk2.groups.io, Ard Biesheuvel,
	Michael D Kinney

On Wed, Mar 22, 2023 at 11:59:38 +0000, Attar, AbdulLateef (Abdul Lateef) wrote:
> [Public]
> 
> Hi Abner,
>         That's right, Platform/AMD/MinBoardPkg will contains all
>         modules/library/drivers required for AMD boards which are
>         based on MinPlatformPkg framework.

That works for me.

/
    Leif

> Thanks
> AbduL
> 
> -----Original Message-----
> From: Chang, Abner <Abner.Chang@amd.com>
> Sent: 22 March 2023 15:25
> To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Leif Lindholm <quic_llindhol@quicinc.com>
> Cc: devel@edk2.groups.io; Ard Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
> 
> [AMD Official Use Only - General]
> 
> 
> 
> > -----Original Message-----
> > From: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> > Sent: Tuesday, March 21, 2023 11:01 AM
> > To: Leif Lindholm <quic_llindhol@quicinc.com>
> > Cc: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Ard
> > Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> > <michael.d.kinney@intel.com>
> > Subject: RE: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> > AMD/BoardPkg and AMD/PlatformPkg maintainers
> >
> > [AMD Official Use Only - General]
> >
> > Hi Leif,
> >         AMD/BoardPkg will implements modules specific to boards(one or
> > more motherboards).
> > It will not contain any module specific to Platform or Silicon, which
> > are part of MinPlatformPkg.
> >
> > How about just Min prefix?
> > AMD/MinBoardPkg (I am avoiding the Amd prefix because its already in
> > AMD folder).
> I am good with this naming. So we will have AMD boards that leverage MinPlatform under MinBoardPkg. Those boards which are not Minplatform based can just stay under Platforms/AMD (e.g., OverdriveBoard.), is my understanding correct Abdul?
> Abner
> >
> > AMD board package for MinPlatformPkg
> >  F: Platform/AMD/MinBoardPkg
> >  M: Maintainer1
> >  M: Maintainer2
> >
> > Thanks
> > AbduL
> >
> > -----Original Message-----
> > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > Sent: 20 March 2023 23:33
> > To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
> > Cc: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>; Ard
> > Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> > <michael.d.kinney@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> > AMD/BoardPkg and AMD/PlatformPkg maintainers
> >
> > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> >
> >
> > Hi Abdul,
> >
> > On Mon, Mar 20, 2023 at 14:09:11 +0000, Attar, AbdulLateef (Abdul
> > Lateef)
> > wrote:
> > > AMD/BoardPkg will contains the modules/drivers to support
> > MinPlatformPkg framework.
> > >    BoardPkg will be generic across all AMD boards which are based on
> > MinPlatformPkg framework.
> > >    It's like "edk2-platforms/Platform/Qemu/QemuOpenBoardPkg".
> > >    We will gradually add modules and libraries to it.
> >
> > So, if the purpose is exclusively to support MinPlatformPkg platforms,
> > I think that should be part of the name.
> > Like Platform/AMD/AmdMinPlatformPkg (for example).
> >
> > > AMD/PlatformPkg will contain the modules/drivers which are generic
> > > to
> > future AMD platform.
> > >    Currently we don't have complete platform, we can drop
> > >    PlatformPkg for now till we have complete(reasonable modules)
> > >    solution.
> >
> > It will certainly be easier to reason about what the preferred
> > naming/layout should be once there is code to look at for examples.
> > It may be that parts of it would live more naturally under
> > Silicon/AMD, for example.
> >
> > > It's better to not touch existing AMD/OverdriveBoard, restructuring
> > > requires changes to .dsc and .fdf files, might break backward
> > > compatibility.
> >
> > No problem. Config files occasionally need to be revamped, but that's
> > simply a mechanical exercise.
> >
> > > How about having just BoardPkg like below?
> >
> > That, too, is a completely generic name that conveys no information
> > about
> > *what* boards one can expect to find in there.
> >
> > Regards,
> >
> > Leif
> >
> > > AMD board package
> > > F: Platform/AMD/BoardPkg
> > > M: Maintainer1
> > > M: Maintainer2
> > >
> > > Please let me know your thoughts.
> > >
> > > Thanks
> > > AbduL
> > >
> > >
> > >
> > >
> > > -----Original Message-----
> > > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > > Sent: 20 March 2023 18:35
> > > To: devel@edk2.groups.io; Chang, Abner <Abner.Chang@amd.com>
> > > Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>;
> > > Ard Biesheuvel <ardb+tianocore@kernel.org>; Michael D Kinney
> > > <michael.d.kinney@intel.com>
> > > Subject: Re: [edk2-devel] [PATCH v2 RESEND 4/4] Maintainers.txt:
> > > Adds AMD/BoardPkg and AMD/PlatformPkg maintainers
> > >
> > > Caution: This message originated from an External Source. Use proper
> > caution when opening attachments, clicking links, or responding.
> > >
> > >
> > > Hi Abner,
> > >
> > > On Sat, Mar 18, 2023 at 09:16:17 +0000, Chang, Abner via groups.io wrote:
> > > > I don't see any modules under OverdriveBoard, is this package
> > > > still
> > >
> > > The code is split between Platform/AMD and Silicon/AMD/Styx, the
> > > latter
> > also being used by SoftIron/Overdrive1000 and LeMaker Cello (although
> > that one is pretty much defunct and should probably be dropped).
> > >
> > > > in use? Do you still remember where is FDF and DSC come from (as
> > > > there is AMD copyright 2014-2016) back to the moment when you was
> > > > introduced this package?
> > >
> > > As the git history tells you:
> > > ---
> > > commit f4d38e50c0f24eb78eb003a94f583025621c63db
> > > Author: Leif Lindholm <leif.lindholm@linaro.org>
> > > Date:   Thu Aug 3 12:24:22 2017 +0100
> > >
> > >     Platform,Silicon: import AMD Styx SoC support and platforms
> > >
> > >     Common files for AMD Overdrive, SoftIron Overdrive 1000
> > >     and LeMaker Cello, as well as actual platform support.
> > >     Imported from commit efd798c1eb of
> > >     https://git.linaro.org/uefi/OpenPlatformPkg.git
> > > ---
> > >
> > > and the initial commit of the platform in that repository is:
> > > ---
> > > From: Leo Duran <leo.duran@amd.com>
> > > Date: Thu, 20 Aug 2015 13:30:24 -0500
> > >
> > >     Subject: Platforms/AMD: add support for AMD Overdrive and
> > > Lemaker Cello
> > >
> > >     This adds support for the AMD Seattle based Overdrive and Husky
> > >     platforms,
> > >     and the Lemaker Cello which is derived from it.
> > >
> > >     This code was tested with upstream EDK2 commit 758ea94651.
> > >
> > >     The binaries in this branch are based on SeattleFDK 1.0.0.2
> > >     (Linaro SeattleFDK commit 4b419f2ef2)
> > > ---
> > >
> > > Regards,
> > >
> > > Leif
> > >
> > > >
> > > > Thanks
> > > > Abner
> > > >
> > > > > -----Original Message-----
> > > > > From: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > > Sent: Saturday, March 18, 2023 12:09 AM
> > > > > To: Attar, AbdulLateef (Abdul Lateef)
> > > > > <AbdulLateef.Attar@amd.com>
> > > > > Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef)
> > > > > <AbdulLateef.Attar@amd.com>; Ard Biesheuvel
> > > > > <ardb+tianocore@kernel.org>; Chang, Abner
> > <Abner.Chang@amd.com>;
> > > > > Michael D Kinney <michael.d.kinney@intel.com>
> > > > > Subject: Re: [PATCH v2 RESEND 4/4] Maintainers.txt: Adds
> > > > > AMD/BoardPkg and AMD/PlatformPkg maintainers
> > > > >
> > > > > Caution: This message originated from an External Source. Use
> > > > > proper caution when opening attachments, clicking links, or responding.
> > > > >
> > > > >
> > > > > On Fri, Mar 17, 2023 at 12:20:07 +0530, Abdul Lateef Attar wrote:
> > > > > > From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>
> > > > > >
> > > > > > Adds maintainers for AMD/BoardPkg and AMD/PlatformPkg.
> > > > > >
> > > > > > Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
> > > > > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > > > > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > > > Cc: Abner Chang <abner.chang@amd.com>
> > > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > > > > ---
> > > > > >  Maintainers.txt | 6 ++++++
> > > > > >  1 file changed, 6 insertions(+)
> > > > > >
> > > > > > diff --git a/Maintainers.txt b/Maintainers.txt index
> > > > > > 747191366070..bb8ab643e090 100644
> > > > > > --- a/Maintainers.txt
> > > > > > +++ b/Maintainers.txt
> > > > > > @@ -91,6 +91,12 @@ F: Silicon/AMD/Styx/
> > > > > >  M: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > > > >  M: Leif Lindholm <quic_llindhol@quicinc.com>
> > > > > >
> > > > > > +AMD Platform
> > > > >
> > > > > Annoyingly, I'm now going to ask the question I have been
> > > > > avoiding up until now. What does "AMD Platform" mean?
> > > > > I mean, you've placed this straight after the entry for the
> > > > > Seattle
> > platforms.
> > > > > I think the name, and package names, are too generic.
> > > > >
> > > > > /
> > > > >     Leif
> > > > >
> > > > > > +F: Platform/AMD/BoardPkg
> > > > > > +F: Platform/AMD/PlatformPkg
> > > > > > +M: Abner Chang <abner.chang@amd.com>
> > > > > > +M: Abdul Lateef Attar <abdattar@amd.com>
> > > > > > +
> > > > > >  Ampere Computing
> > > > > >  F: Platform/Ampere
> > > > > >  F: Silicon/Ampere
> > > > > > --
> > > > > > 2.25.1
> > > > > >
> > > >
> > > >
> > > > 
> > > >
> > > >

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2023-03-22 12:29 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-17  6:50 [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg Abdul Lateef Attar
2023-03-17  6:50 ` [PATCH v2 RESEND 1/4] Platform/AMD: Adds BoardPkg and PlatformPkg Abdul Lateef Attar
2023-03-17 15:41   ` Leif Lindholm
2023-03-17  6:50 ` [PATCH v2 RESEND 2/4] Platform/AMD/PlatformPkg: Adds PciHotPlug init protocol implementation Abdul Lateef Attar
2023-03-17  6:52   ` Chang, Abner
2023-03-17 15:55   ` [edk2-devel] " Leif Lindholm
2023-03-20 10:01     ` Attar, AbdulLateef (Abdul Lateef)
2023-03-17  6:50 ` [PATCH v2 RESEND 3/4] Platform/AMD/BoarkPkg: Adds SetCacheMtrrLib library Abdul Lateef Attar
2023-03-17  6:52   ` Chang, Abner
2023-03-17 15:59   ` Leif Lindholm
2023-03-20 10:02     ` Attar, AbdulLateef (Abdul Lateef)
2023-03-17  6:50 ` [PATCH v2 RESEND 4/4] Maintainers.txt: Adds AMD/BoardPkg and AMD/PlatformPkg maintainers Abdul Lateef Attar
2023-03-17  6:53   ` Chang, Abner
2023-03-17 16:09   ` Leif Lindholm
2023-03-18  9:03     ` Chang, Abner
2023-03-20 12:54       ` [edk2-devel] " Leif Lindholm
2023-03-18  9:16     ` Chang, Abner
2023-03-18  9:42       ` Ard Biesheuvel
2023-03-18 15:34         ` Chang, Abner
2023-03-20 13:05       ` [edk2-devel] " Leif Lindholm
2023-03-20 14:09         ` Attar, AbdulLateef (Abdul Lateef)
2023-03-20 18:02           ` Leif Lindholm
2023-03-21  3:00             ` Attar, AbdulLateef (Abdul Lateef)
2023-03-22  9:54               ` Chang, Abner
2023-03-22 11:59                 ` Attar, AbdulLateef (Abdul Lateef)
2023-03-22 12:29                   ` Leif Lindholm
2023-03-20 10:11     ` Attar, AbdulLateef (Abdul Lateef)
2023-03-17  6:54 ` [PATCH v2 RESEND 0/4] Adds AMD/BoardPkg and AMD/PlatformPkg Chang, Abner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox