Enhance the PCI Resource Allocation services to include a new state in
which only the base address of each root bridge is set. This new, partial
state is used in conjunction with GetProposedResources to retrieve the
Base Addresses of the resources before the allocation takes place, and
is reached only if pre-populated mappings are enabled.
The need for this information before allocation emerges from the handling
of pre-populated resources, that is, PCI BARs that have a fixed value
and should not be placed at a different address. For such a case, the
firmware already receives a fixed resource description of the root bridges
that agrees with those pre-populated resources.
For example, in Ovmf platform, the host can provide the specific resources
to be used for each root bridge in the guest and also indicate that there
are pre-populated resources that must be respected. In this case, those
resources need to be placed (offset-wise) before the rest of the resources.
The placement logic assigns a root bridge relative offset to each BAR,
therefore, the offsets of the pre-populated BARs need to be calculated
first and, for that, the root bridge' base address must be known
beforehand so that absolute addresses read from the devices can be
translated into a root bridge relative offset.
Cc: Alexander Graf <graf@amazon.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Nicolas Ojeda Leon <ncoleon@amazon.com>
---
Notes:
v4:
- Major restructuring of the concept to avoid modifications to the PCI
Resource Allocation protocol which comes from UEFI spec. Previous
rev3 patch was completely removed.
- Extended existing services of the protocol to integrate a new state
that allows the traditional GetProposedResources to retrieve, before
allocation phase, the base address of root bridges.
- Changes are guarded with PcdPciPreservePopulatedMappings token so that
existent platforms experience no change at all.
MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h | 1 +
.../Bus/Pci/PciBusDxe/PciEnumerator.c | 4 +-
.../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c | 93 ++++++++++++++++++-
.../Bus/Pci/PciHostBridgeDxe/PciHostBridge.h | 2 +
.../Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf | 4 +
.../Pci/PciHostBridgeDxe/PciHostResource.h | 1 +
MdeModulePkg/Include/Library/PciResourceLib.h | 20 ++++
7 files changed, 122 insertions(+), 3 deletions(-)
create mode 100644 MdeModulePkg/Include/Library/PciResourceLib.h
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 4b58c3ea9b..ecb3d3cb5d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -38,6 +38,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/UefiBootServicesTableLib.h>
#include <Library/DevicePathLib.h>
#include <Library/PcdLib.h>
+#include <Library/PciResourceLib.h>
#include <IndustryStandard/Pci.h>
#include <IndustryStandard/PeImage.h>
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c
index 3f8c6e6da7..fdf8984661 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c
@@ -1307,7 +1307,9 @@ GetResourceBase (
Ptr = (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *)Temp;
ResStatus = Ptr->AddrTranslationOffset;
- if (ResStatus == EFI_RESOURCE_SATISFIED) {
+ if ((ResStatus == EFI_RESOURCE_SATISFIED) ||
+ (ResStatus == PCI_RESOURCE_PARTIAL))
+ {
switch (Ptr->ResType) {
//
// Memory type aperture
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index b20bcd310a..cfe45a6e3f 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -884,8 +884,75 @@ NotifyPhase (
case EfiPciHostBridgeBeginResourceAllocation:
//
- // No specific action is required here, can perform any chipset specific programing
+ // In general, no specific action is required here, can perform any chipset specific programing
//
+ if (PcdGetBool (PcdPciPreservePopulatedMappings)) {
+ //
+ // If pre populated resources are present, resources are moved into
+ // ResBaseDiscovered state to enable the retrival of BaseAddresses.
+ // Set the base address of each resource with value in root bridges
+ // using same alignments but no allocation. This partial information is
+ // intended to be used by a preliminary call to GetProposedResoruces
+ // that retrieves only base addressed for resource placement purposes
+ //
+ for (Link = GetFirstNode (&HostBridge->RootBridges)
+ ; !IsNull (&HostBridge->RootBridges, Link)
+ ; Link = GetNextNode (&HostBridge->RootBridges, Link)
+ )
+ {
+ RootBridge = ROOT_BRIDGE_FROM_LINK (Link);
+
+ for (Index = TypeIo; Index < TypeBus; Index++) {
+ if (RootBridge->ResAllocNode[Index].Status == ResNone) {
+ Alignment = RootBridge->ResAllocNode[Index].Alignment;
+ BitsOfAlignment = LowBitSet64 (Alignment + 1);
+ BaseAddress = MAX_UINT64;
+
+ switch (Index) {
+ case TypeIo:
+ BitsOfAlignment = MIN (15, BitsOfAlignment);
+ BaseAddress = ALIGN_VALUE (RootBridge->Io.Base, Alignment + 1);
+ BaseAddress = ALIGN_VALUE (BaseAddress, LShiftU64 (1, BitsOfAlignment));
+ break;
+
+ case TypeMem32:
+ BitsOfAlignment = MIN (31, BitsOfAlignment);
+ BaseAddress = ALIGN_VALUE (RootBridge->Mem.Base, Alignment + 1);
+ BaseAddress = ALIGN_VALUE (BaseAddress, LShiftU64 (1, BitsOfAlignment));
+ break;
+
+ case TypePMem32:
+ BitsOfAlignment = MIN (31, BitsOfAlignment);
+ BaseAddress = ALIGN_VALUE (RootBridge->PMem.Base, Alignment + 1);
+ BaseAddress = ALIGN_VALUE (BaseAddress, LShiftU64 (1, BitsOfAlignment));
+ break;
+
+ case TypeMem64:
+ BitsOfAlignment = MIN (63, LowBitSet64 (Alignment + 1));
+ BaseAddress = ALIGN_VALUE (RootBridge->MemAbove4G.Base, Alignment + 1);
+ BaseAddress = ALIGN_VALUE (BaseAddress, LShiftU64 (1, BitsOfAlignment));
+ break;
+
+ case TypePMem64:
+ BitsOfAlignment = MIN (63, LowBitSet64 (Alignment + 1));
+ BaseAddress = ALIGN_VALUE (RootBridge->PMemAbove4G.Base, Alignment + 1);
+ BaseAddress = ALIGN_VALUE (BaseAddress, LShiftU64 (1, BitsOfAlignment));
+ break;
+
+ default:
+ ASSERT (FALSE);
+ break;
+ }
+
+ if (BaseAddress != MAX_UINT64) {
+ RootBridge->ResAllocNode[Index].Base = BaseAddress;
+ RootBridge->ResAllocNode[Index].Status = ResBaseDiscovered;
+ }
+ }
+ }
+ }
+ }
+
break;
case EfiPciHostBridgeAllocateResources:
@@ -919,7 +986,9 @@ NotifyPhase (
DEBUG ((DEBUG_INFO, " RootBridge: %s\n", RootBridge->DevicePathStr));
for (Index1 = TypeIo; Index1 < TypeBus; Index1++) {
- if (RootBridge->ResAllocNode[Index1].Status == ResNone) {
+ if ((RootBridge->ResAllocNode[Index1].Status == ResNone) ||
+ (RootBridge->ResAllocNode[Index1].Status == ResBaseDiscovered))
+ {
ResNodeHandled[Index1] = TRUE;
} else {
//
@@ -1571,6 +1640,22 @@ SubmitResources (
RootBridge->ResAllocNode[Type].Status = ResSubmitted;
}
+ if (PcdGetBool (PcdPciPreservePopulatedMappings)) {
+ //
+ // If pre-populated resources are present, causing the resources
+ // to go into ResBaseDiscovered before EfiPciHostBridgeAllocateResources
+ // clear nodes that are still in that state, which have a Base Address
+ // but no resources underneath them. Mark them as empty nodes since no
+ // resources were submitted for them, length required is zero.
+ //
+ for (Type = TypeIo; Type < TypeBus; Type++) {
+ if (RootBridge->ResAllocNode[Type].Status == ResBaseDiscovered) {
+ RootBridge->ResAllocNode[Type].Status = ResNone;
+ RootBridge->ResAllocNode[Type].Base = 0;
+ }
+ }
+ }
+
RootBridge->ResourceSubmitted = TRUE;
return EFI_SUCCESS;
}
@@ -1652,6 +1737,10 @@ GetProposedResources (
Descriptor->AddrTranslationOffset = (ResStatus == ResAllocated) ? EFI_RESOURCE_SATISFIED : PCI_RESOURCE_LESS;
Descriptor->AddrLen = RootBridge->ResAllocNode[Index].Length;
+ if (ResStatus == ResBaseDiscovered) {
+ Descriptor->AddrTranslationOffset = PCI_RESOURCE_PARTIAL;
+ }
+
switch (Index) {
case TypeIo:
Descriptor->ResType = ACPI_ADDRESS_SPACE_TYPE_IO;
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
index e7a30fd909..940adc5965 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
@@ -15,6 +15,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Library/UefiDriverEntryPoint.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/PciHostBridgeLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PciResourceLib.h>
#include <Protocol/PciHostBridgeResourceAllocation.h>
#include <Protocol/IoMmu.h>
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
index 9c24cacc30..42d1cc6289 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
@@ -39,6 +39,7 @@
UefiLib
PciHostBridgeLib
TimerLib
+ PcdLib
[Protocols]
gEfiCpuIo2ProtocolGuid ## CONSUMES
@@ -50,3 +51,6 @@
[Depex]
gEfiCpuIo2ProtocolGuid AND
gEfiCpuArchProtocolGuid
+
+[Pcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPciPreservePopulatedMappings## CONSUMES
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h
index 772f4b513f..12725d3797 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostResource.h
@@ -28,6 +28,7 @@ typedef enum {
ResNone,
ResSubmitted,
ResAllocated,
+ ResBaseDiscovered,
ResStatusMax
} RES_STATUS;
diff --git a/MdeModulePkg/Include/Library/PciResourceLib.h b/MdeModulePkg/Include/Library/PciResourceLib.h
new file mode 100644
index 0000000000..e16394c352
--- /dev/null
+++ b/MdeModulePkg/Include/Library/PciResourceLib.h
@@ -0,0 +1,20 @@
+/** @file
+ Pci resource library consumed by PciBusDxe and PciHostBridgeDxe drivers to
+ share non-UEFI spec artefacts used in both drivers
+
+ Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+ **/
+#ifndef __PCI_RESOURCE_LIB_H__
+#define __PCI_RESOURCE_LIB_H__
+
+#define PCI_RESOURCE_PARTIAL 0xFFFFFFFFFFFFFFFDULL
+
+#endif
--
2.17.1
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86384): https://edk2.groups.io/g/devel/message/86384
Mute This Topic: https://groups.io/mt/88893097/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi,
> - if (ResStatus == EFI_RESOURCE_SATISFIED) {
> + if ((ResStatus == EFI_RESOURCE_SATISFIED) ||
> + (ResStatus == PCI_RESOURCE_PARTIAL))
> + {
> +++ b/MdeModulePkg/Include/Library/PciResourceLib.h
> @@ -0,0 +1,20 @@
> +/** @file
> + Pci resource library consumed by PciBusDxe and PciHostBridgeDxe drivers to
> + share non-UEFI spec artefacts used in both drivers
> +
> + Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved.
> +
> + This program and the accompanying materials
> + are licensed and made available under the terms and conditions of the BSD License
> + which accompanies this distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> + **/
> +#ifndef __PCI_RESOURCE_LIB_H__
> +#define __PCI_RESOURCE_LIB_H__
> +
> +#define PCI_RESOURCE_PARTIAL 0xFFFFFFFFFFFFFFFDULL
> +
> +#endif
Hmm, so you are hiding the #define here to avoid updating
Protocol/PciHostBridgeResourceAllocation.h ...
I suspect if this can't be implemented in the pci enumerator alone
there is just no way around actually extending uefi protocols. But it's
also not fully clear to me why you need this new "partial" state.
Wouldn't you know either nothing or both base + size for a resource?
Also: Maybe split this series into two? One for the host-provided
bridges, and another for the pre-populated bars? So the former can
already be merged while the later is still sorted?
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86388): https://edk2.groups.io/g/devel/message/86388
Mute This Topic: https://groups.io/mt/88893097/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Hi Gerd, > Hmm, so you are hiding the #define here to avoid updating Protocol/PciHostBridgeResourceAllocation.h ... > > I suspect if this can't be implemented in the pci enumerator alone there is just no way around actually extending uefi protocols. But it's also not fully clear to me why you need this new "partial" state. > Wouldn't you know either nothing or both base + size for a resource? > Yes you are right, the current status is we have nothing and after the resources are allocated we have base and size. However, the allocation happens after CreateResourceMap is called, this function is the one that iterates over all resources of all devices under a root bridges and places them in a root bridge relative offset to then calculate the length of the resources required for that specific root bridge. For the pre-populated BARs I retrieve the Base Address of the Root Bridge before placing the resources at the root bridge relative offset so that I can translate the BAR address (set by the host) into an offset. That's why I created this patch to retrieve only the base address before the root bridge resource map is created, submitted and allocated. > Also: Maybe split this series into two? One for the host-provided bridges, and another for the pre-populated bars? So the former can already be merged while the later is still sorted? Yes I will do that. > take care, > Gerd Thanks, Nicolas Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86396): https://edk2.groups.io/g/devel/message/86396 Mute This Topic: https://groups.io/mt/88893097/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Fri, Feb 04, 2022 at 09:17:48AM +0000, Ojeda Leon, Nicolas wrote: > Hi Gerd, > > > Hmm, so you are hiding the #define here to avoid updating Protocol/PciHostBridgeResourceAllocation.h ... > > > > I suspect if this can't be implemented in the pci enumerator alone there is just no way around actually extending uefi protocols. But it's also not fully clear to me why you need this new "partial" state. > > Wouldn't you know either nothing or both base + size for a resource? > > > > Yes you are right, the current status is we have nothing and after the > resources are allocated we have base and size. However, the allocation > happens after CreateResourceMap is called, this function is the one > that iterates over all resources of all devices under a root bridges > and places them in a root bridge relative offset to then calculate the > length of the resources required for that specific root bridge. For > the pre-populated BARs I retrieve the Base Address of the Root Bridge > before placing the resources at the root bridge relative offset so > that I can translate the BAR address (set by the host) into an offset. > That's why I created this patch to retrieve only the base address > before the root bridge resource map is created, submitted and > allocated. Hmm, nasty init order issue. No good idea offhand. Have you tried to exclude the pre-populated BARs from the host bridge windows communicated to the edk2 pci core? The pci enumerator should not create overlapping pci bar assignment then. But it might very well be that this only shifts the problem to another place ... take care, Gerd -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86645): https://edk2.groups.io/g/devel/message/86645 Mute This Topic: https://groups.io/mt/88893097/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
>> > Hmm, so you are hiding the #define here to avoid updating Protocol/PciHostBridgeResourceAllocation.h ... >> > >> > I suspect if this can't be implemented in the pci enumerator alone there is just no way around actually extending uefi protocols. But it's also not fully clear to me why you need this new "partial" state. >> > Wouldn't you know either nothing or both base + size for a resource? >> > >> >> Yes you are right, the current status is we have nothing and after the >> resources are allocated we have base and size. However, the allocation >> happens after CreateResourceMap is called, this function is the one >> that iterates over all resources of all devices under a root bridges >> and places them in a root bridge relative offset to then calculate the >> length of the resources required for that specific root bridge. For >> the pre-populated BARs I retrieve the Base Address of the Root Bridge >> before placing the resources at the root bridge relative offset so >> that I can translate the BAR address (set by the host) into an offset. >> That's why I created this patch to retrieve only the base address >> before the root bridge resource map is created, submitted and >> allocated. > >Hmm, nasty init order issue. No good idea offhand. > >Have you tried to exclude the pre-populated BARs from the host bridge windows communicated to the edk2 pci core? The pci enumerator should not create overlapping pci bar assignment then. But it might very well be that this only shifts the problem to another place ... > >take care, > Gerd Haven't tried that but we are working on a new approach that do not touches the protocol at all but instead profits from the retry loop already in place and, after an allocation round, verifies if the pre-populated BARs are in the correct place, otherwise tries again. Similar to what is currently done when the allocation fails due to not enough space. Also we are creating a routine that "adjusts" the resources by analyzing pre-populated BAR situation to decide which device to reject before trying again. Expect a new revision soon. Best regards, Nicolas Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86658): https://edk2.groups.io/g/devel/message/86658 Mute This Topic: https://groups.io/mt/88893097/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Hi Gerd, > Also: Maybe split this series into two? One for the host-provided bridges, and another for the pre-populated bars? So the former can already be merged while the later is still sorted? Is it already possible for someone to merge the first 5 patches or should I submit a new revision with the 5 patches alone? Best regards, Nicolas Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#86397): https://edk2.groups.io/g/devel/message/86397 Mute This Topic: https://groups.io/mt/88893097/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.