public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/8] Handling of multiple PCI host bridges specified
@ 2022-01-20 17:10 Ojeda Leon, Nicolas
  2022-01-20 17:10 ` [PATCH v2 1/8] OvmfPkg/Library: Create base HardwareInfoLib for PCI Host Bridges Ojeda Leon, Nicolas
                   ` (7 more replies)
  0 siblings, 8 replies; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-20 17:10 UTC (permalink / raw)
  To: devel; +Cc: atugup, Nicolas Ojeda Leon, Alexander Graf, Gerd Hoffmann

Increased control is provided in Ovmf platforms to define and configure
the specifications of multiple PCI host bridges in the hypervisor. The
host propagates this information to the guest, initially through fw-cfg
interface.

In some AWS EC2 platforms, we expose a PCI topology including several
root bridges portraying information about physical distribution that
enables the guest to optimize accesses. Current PCI driver for Ovmf
enables the explicit definition of multiple root bridges and contains
the logic to fix their resources based on a platform-specific PCD entries.
However, we need a way to control, from the hypervisor, how many and which
resources each PCI host bridge can use. For this reason, this patch series
introduces a mechanism to provide PCI host bridges information like bus
number range, attributes, allocation attributes, PIO aperture as well as
32 and 64- bit prefetchable and non-prefetchable MMIO ranges through a
fw-cfg item created by the hypervisor and consumed by the guest firmware.
In order to offer a generic and extensible way to disclose non-discoverable
hardware information from the host to the guest, a new library called
HardwareInfoLib is created in the OvmfPkg. In essence, this library offers
the functionality to parse a generic BLOB into a list as well as the methods
to iterate over such list, including filtering options. The library is
conceived in a generic way so that further hardware elements can also be
described using it. For such purpose the length of the BLOB is not
restricted but instead regarded as a sequence of header-info elements that
allow the parsing during runtime. Furthermore, specific functionality is
provided wrapping QemuFwCfgReadBytes to extract hardware descriptions, in
the aforementioned format, in a static way so that early in the Pei stage
the library can be used to identify address space requirements. The core of
the library offers enough flexibility to process as many elements, even from
different hardware types (heterogenous), as needed in a single run. This
library is extended for the particular use case already exposed, PCI host
bridges, and this same code offers an example of how to tailor it for
further hardware components.

Furthermore, in this kind of high-performance platforms, we exploit PCIe
features like Access Control Services to configure peer-to-peer channels
between devices. This allows us to create direct communication channels
that do not require packets to reach the Root Complex but instead can
follow a direct path from source to target. To enable Guest Virtual Machines
to profit from this performance improvement, we configure resources (BARs)
of peer-to-peer intended devices with Host Physical Addresses. In this
scenario, devices can be instructed, from the guest VM, to perform DMA
operations targeting a peer address space, and the PCIe fabric can take
care of directly routing them. Therefore, long and busy links towards the
Root Complex are avoided. When we configure resources this way, the guest
must respect the pre-populated BARs so that devices preserve the address
ranges configured in the apertures of physical PCIe ports that enable
routing at the hardware level. Similarly, revealing details about the
underlying PCI hierarchy empowers the guest to perform topology-aware
optimizations and benefit from an enhanced performance.

Nicolas Ojeda Leon (8):
  OvmfPkg/Library: Create base HardwareInfoLib for PCI Host Bridges
  Ovmf/HardwareInfoLib: Parse data directly from fw-cfg
  Ovmf/HardwareInfoLib: Add core to parse heterogenous data
  Ovmf/PlatformPei: Extend 64-bit MMIO range to fit resources
  OvmfPkg/PciHostBridgeUtilityLib: Initialize RootBridges apertures with
    spec
  MdeModulePkg, OvmfPkg: Add Pcd token for PCI pre-populated BARs
  MdeModulePkg/Pci MdePkg: Create service to retrieve PCI base addresses
  MdeModulePkg/PciBusDxe: Handling of pre-populated PCI BARs

 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h       |   1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |   1 +
 .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c  |   5 +-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c       |  21 +
 .../Bus/Pci/PciBusDxe/PciResourceSupport.c    | 277 +++++++++-
 .../Bus/Pci/PciBusDxe/PciResourceSupport.h    |  20 +
 .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c  |  81 +++
 .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.h  |  29 +
 MdeModulePkg/MdeModulePkg.dec                 |   6 +
 .../PciHostBridgeResourceAllocation.h         |  33 ++
 OvmfPkg/Include/Library/HardwareInfoLib.h     | 128 +++++
 .../Library/QemuFwCfgHardwareInfoLib.h        |  59 ++
 .../Library/HardwareInfoLib/HardwareInfoLib.c | 256 +++++++++
 .../HardwareInfoLib/HardwareInfoLib.inf       |  43 ++
 .../HardwareInfoPciHostBridgeLib.c            | 515 ++++++++++++++++++
 .../HardwareInfoPciHostBridgeLib.h            | 262 +++++++++
 .../HardwareInfoLib/HardwareInfoTypesLib.h    |  64 +++
 .../QemuFwCfgHardwareInfoLib.c                |  89 +++
 .../QemuFwCfgHardwareInfoLib.inf              |  42 ++
 .../PciHostBridgeUtilityLib.c                 | 362 +++++++++---
 .../PciHostBridgeUtilityLib.inf               |   5 +
 OvmfPkg/OvmfPkgX64.dsc                        |   3 +
 OvmfPkg/PlatformPei/MemDetect.c               | 110 ++++
 OvmfPkg/PlatformPei/PlatformPei.inf           |   2 +
 24 files changed, 2332 insertions(+), 82 deletions(-)
 create mode 100644 OvmfPkg/Include/Library/HardwareInfoLib.h
 create mode 100644 OvmfPkg/Include/Library/QemuFwCfgHardwareInfoLib.h
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.c
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.inf
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/HardwareInfoPciHostBridgeLib.c
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/HardwareInfoPciHostBridgeLib.h
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/HardwareInfoTypesLib.h
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.c
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.inf

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




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

* [PATCH v2 1/8] OvmfPkg/Library: Create base HardwareInfoLib for PCI Host Bridges
  2022-01-20 17:10 [PATCH v2 0/8] Handling of multiple PCI host bridges specified Ojeda Leon, Nicolas
@ 2022-01-20 17:10 ` Ojeda Leon, Nicolas
  2022-01-21 10:09   ` Gerd Hoffmann
  2022-01-20 17:10 ` [PATCH v2 2/8] Ovmf/HardwareInfoLib: Parse data directly from fw-cfg Ojeda Leon, Nicolas
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-20 17:10 UTC (permalink / raw)
  To: devel; +Cc: atugup, Nicolas Ojeda Leon, Alexander Graf, Gerd Hoffmann

Create the Hardware Info library base together with the specifics to
describe PCI Host Bridges.

The Hardware Info library is intended to be used for disclosing
non-discoverable hardware information from the host to the guest in
Ovmf platforms. Core functionality will provide the possibility to
parse information from a generic BLOB into runtime structures. The
library is conceived in a generic way so that further hardware
elements can also be described using it. For such purpose the length
of the BLOB is not restricted but instead regarded as a sequence of
header-info elements that allow the parsing during runtime. The first
type of hardware defined will be PCI host bridges, providing the
possibility to define multiple and specify the resources each of them
can use. This enables the guest firmware to configure PCI resources
properly. Having the size of each individual element favors the reuse
of a single interface to convey descriptions of an arbitrary number
of heterogenous hardware elements. Furthermore, flexible access
mechanisms coupled with the size will grant the possibility of
interpreting them in a single run.

Define the base types of the generic Hardware Info library to parse
heterogeneous data. Also provide the specific changes to support
PCI host bridges as the first hardware type supported by the
library.
Additionally, define the HOST_BRIDGE_INFO structure to describe PCI
host bridges along with the functionality to parse such information
into proper structures used by the PCI driver in a centralized manner
and taking care of versioning.

As an example and motivation, the library will be used to define
multiple PCI host bridges for complex platforms that require it.
The first means of transportation that will be used is going to be
fw-cfg, over which a stream of bytes will be transferred and later
parsed by the hardware info library. Accordingly, the PCI driver
will make use of these host bridges definitions to populate the
list of Root Bridges and proceed with the configuration and discovery
of underlying hardware components.

As mentioned before, the binary data to be parsed by the Hardware
Info library should be organized as a sequence of Header-element
pairs in which the header describes the type and size of the associated
element that comes right after it. As an illustration, to provide
inforation of 3 host bridges the data, conceptually, would look
like this:

Header PCI Host Bridge (type and size) # 1
PCI Host Bridge info # 1
Header PCI Host Bridge (type and size) # 2
PCI Host Bridge info # 2
Header PCI Host Bridge (type and size) # 3
PCI Host Bridge info # 3

Cc: Alexander Graf <graf@amazon.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Nicolas Ojeda Leon <ncoleon@amazon.com>
---
 .../HardwareInfoPciHostBridgeLib.c            | 515 ++++++++++++++++++
 .../HardwareInfoPciHostBridgeLib.h            | 262 +++++++++
 .../HardwareInfoLib/HardwareInfoTypesLib.h    |  64 +++
 3 files changed, 841 insertions(+)
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/HardwareInfoPciHostBridgeLib.c
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/HardwareInfoPciHostBridgeLib.h
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/HardwareInfoTypesLib.h

diff --git a/OvmfPkg/Library/HardwareInfoLib/HardwareInfoPciHostBridgeLib.c b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoPciHostBridgeLib.c
new file mode 100644
index 0000000000..ef72d4a95c
--- /dev/null
+++ b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoPciHostBridgeLib.c
@@ -0,0 +1,515 @@
+/**@file
+  Hardware info library with types and accessors to parse information about
+  PCI host bridges.
+
+  Copyright 2021 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.
+ **/
+
+#include <Library/DebugLib.h>
+
+#include "HardwareInfoPciHostBridgeLib.h"
+
+#define IS_RANGE_INVALID(Start, Size, MaxValue)  (Start >= MaxValue || Size == 0)
+
+/**
+  Calculate the last (inclusive) address of a range.
+
+  @param[in] Start  First address of the range
+  @param[in] Size   Size of the range
+  @return Last address of the range
+**/
+STATIC
+UINT64
+GetRangeEnd (
+  IN  UINT64  Start,
+  IN  UINT64  Size,
+  IN  UINT64  MaxValue
+  )
+{
+  if (IS_RANGE_INVALID (Start, Size, MaxValue)) {
+    return 0;
+  }
+
+  return Start + Size - 1;
+}
+
+/**
+  Internal helper to update LastAddress if the Limit address
+  of the Mem aperture is higher than the provided value.
+
+  @param[in]  Mem           Pointer to aperture whose limit is
+                            to be compared against accumulative
+                            last address.
+  @param[out] LastAddress   Pointer to accumulative last address
+                            to be updated if Limit is higher
+**/
+STATIC
+VOID
+UpdateLastAddressIfHigher (
+  IN  PCI_ROOT_BRIDGE_APERTURE  *Mem,
+  OUT UINT64                    *LastAddress
+  )
+{
+  if (Mem->Limit > *LastAddress) {
+    *LastAddress = Mem->Limit;
+  }
+}
+
+EFI_STATUS
+HardwareInfoPciHostBridgeLastMmioAddress (
+  IN CONST  HOST_BRIDGE_INFO  *HostBridge,
+  IN        UINTN             DataSize,
+  IN        BOOLEAN           HighMem,
+  OUT       UINT64            *LastMmioAddress
+  )
+{
+  EFI_STATUS                Status;
+  PCI_ROOT_BRIDGE_APERTURE  Mem;
+  PCI_ROOT_BRIDGE_APERTURE  MemAbove4G;
+  PCI_ROOT_BRIDGE_APERTURE  PMem;
+  PCI_ROOT_BRIDGE_APERTURE  PMemAbove4G;
+
+  if (LastMmioAddress == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // Set the output to the lowest possible value so that if some step fails
+  // the overall outcome reflects no information found
+  //
+  *LastMmioAddress = 0;
+
+  Status = HardwareInfoPciHostBridgeGetApertures (
+             HostBridge,
+             DataSize,
+             NULL,
+             &Mem,
+             &MemAbove4G,
+             &PMem,
+             &PMemAbove4G,
+             NULL
+             );
+
+  //
+  // Forward error to caller but ignore warnings given that, very likely,
+  // the host bridge will have a PIO aperture we are explicitly
+  // ignoring here since we care only about MMIO resources.
+  //
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  if (HighMem) {
+    UpdateLastAddressIfHigher (&MemAbove4G, LastMmioAddress);
+    UpdateLastAddressIfHigher (&PMemAbove4G, LastMmioAddress);
+  } else {
+    UpdateLastAddressIfHigher (&Mem, LastMmioAddress);
+    UpdateLastAddressIfHigher (&PMem, LastMmioAddress);
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Set the boundaries of an aperture to invalid values having
+  size zero and start MaxValue (yields Start > Limit which
+  depicts an invalid range)
+
+  @param[in]  MaxValue    Max value of the aperture's range (depends
+                          on the data type)
+  @param[out] Aperture    Aperture object to invalidate
+**/
+STATIC
+VOID
+InvalidateRootBridgeAperture (
+  IN  UINT64                    MaxValue,
+  OUT PCI_ROOT_BRIDGE_APERTURE  *Aperture
+  )
+{
+  if (Aperture == NULL) {
+    return;
+  }
+
+  Aperture->Base  = MaxValue;
+  Aperture->Limit = 0;
+}
+
+/**
+  Fill a PCI ROOT BRIDGE APERTURE with the proper values calculated
+  from the provided start and size.
+
+  @param[in]  Start       Start address of the aperture
+  @param[in]  Size        Size, in bytes, of the aperture
+  @param[in]  MaxValue    Max value a valid address could take and which
+                          represents an invalid start address.
+  @param[out] Aperture    Pointer to the aperture to be filled
+
+  @retval EFI_SUCCESS                 Aperture was filled successfully
+  @retval EFI_INVALID_PARAMETER       Range depicted by Start and Size is
+                                      valid but ignored because aperture
+                                      pointer is NULL
+  @retval EFI_WARN_BUFFER_TOO_SMALL   Aperture pointer is invalid but the
+                                      range also is so no harm.
+**/
+STATIC
+EFI_STATUS
+FillHostBridgeAperture (
+  IN  UINT64                    Start,
+  IN  UINT64                    Size,
+  IN  UINT64                    MaxValue,
+  OUT PCI_ROOT_BRIDGE_APERTURE  *Aperture
+  )
+{
+  UINT64  End;
+
+  End = GetRangeEnd (Start, Size, MaxValue);
+
+  if (Aperture == NULL) {
+    if (!IS_RANGE_INVALID (Start, Size, MaxValue)) {
+      //
+      // Report an error to the caller since the range specified in
+      // the host bridge's resources is non-empty but the provided
+      // aperture pointer is null, thus the valid range is ignored.
+      //
+      return EFI_INVALID_PARAMETER;
+    }
+
+    return EFI_WARN_BUFFER_TOO_SMALL;
+  }
+
+  if (IS_RANGE_INVALID (Start, Size, MaxValue)) {
+    //
+    // Fill Aperture with invalid range values to signal the
+    // absence of an address space (empty range)
+    //
+    InvalidateRootBridgeAperture (MaxValue, Aperture);
+  } else {
+    Aperture->Base  = Start;
+    Aperture->Limit = End;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Merge 2 ranges (normal and prefetchable) into a single aperture
+  comprehending the addresses encompassed by both of them. If both
+  ranges are not empty they must be contiguous for correctness.
+
+  @param[in]  Start       Range start address
+  @param[in]  Size        Range size in bytes
+  @param[in]  PStart      Prefetchable range start address
+  @param[in]  PSize       Prefetchable range size in bytes
+  @param[in]  MaxValue    Max value a valid address could take and which
+                          represents an invalid start address.
+  @param[out] Aperture    Pointer to the aperture to be filled
+
+  @retval EFI_SUCCESS                 Aperture was filled successfully
+  @retval EFI_INVALID_PARAMETER       Either range depicted by Start, Size
+                                      or PStart, PSize or both are valid
+                                      but ignored because aperture pointer
+                                      is NULL
+  @retval EFI_WARN_BUFFER_TOO_SMALL   Aperture pointer is invalid but both
+                                      ranges are too so no harm.
+**/
+STATIC
+EFI_STATUS
+MergeHostBridgeApertures (
+  IN  UINT64                    Start,
+  IN  UINT64                    Size,
+  IN  UINT64                    PStart,
+  IN  UINT64                    PSize,
+  IN  UINT64                    MaxValue,
+  OUT PCI_ROOT_BRIDGE_APERTURE  *Aperture
+  )
+{
+  UINT64  PEnd;
+
+  if (Aperture == NULL) {
+    if (!IS_RANGE_INVALID (Start, Size, MaxValue) ||
+        !IS_RANGE_INVALID (PStart, PSize, MaxValue))
+    {
+      //
+      // Report an error to the caller since the range specified in
+      // the host bridge's resources is non-empty but the provided
+      // aperture pointer is null, thus the valid range is ignored.
+      //
+      return EFI_INVALID_PARAMETER;
+    }
+
+    return EFI_WARN_BUFFER_TOO_SMALL;
+  }
+
+  //
+  // Start from an empty range (Limit < Base)
+  //
+  InvalidateRootBridgeAperture (MaxValue, Aperture);
+
+  if (!IS_RANGE_INVALID (Start, Size, MaxValue)) {
+    Aperture->Base  = Start;
+    Aperture->Limit = Start + Size - 1;
+  }
+
+  if (!IS_RANGE_INVALID (PStart, PSize, MaxValue)) {
+    PEnd = PStart + PSize - 1;
+
+    if (PStart < Aperture->Base) {
+      Aperture->Base = PStart;
+    }
+
+    if (PEnd > Aperture->Limit) {
+      Aperture->Limit = PEnd;
+    }
+  }
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+HardwareInfoPciHostBridgeGetBusNrRange (
+  IN CONST  HOST_BRIDGE_INFO  *HostBridge,
+  IN        UINTN             DataSize,
+  OUT       UINTN             *BusNrStart,
+  OUT       UINTN             *BusNrLast
+  )
+{
+  if ((HostBridge == NULL) || (DataSize == 0) ||
+      (BusNrStart == NULL) || (BusNrLast == NULL))
+  {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // For now only version 0 is supported
+  //
+  if (HostBridge->Version != 0) {
+    return EFI_INCOMPATIBLE_VERSION;
+  }
+
+  *BusNrStart = HostBridge->BusNrStart;
+  *BusNrLast  = HostBridge->BusNrLast;
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+HardwareInfoPciHostBridgeGetApertures (
+  IN CONST  HOST_BRIDGE_INFO          *HostBridge,
+  IN        UINTN                     DataSize,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *Io,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *Mem,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *MemAbove4G,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PMem,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PMemAbove4G,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PcieConfig
+  )
+{
+  EFI_STATUS  Status;
+  BOOLEAN     StickyError;
+
+  StickyError = FALSE;
+  if ((HostBridge == NULL) || (DataSize == 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // For now only version 0 is supported
+  //
+  if (HostBridge->Version != 0) {
+    return EFI_INCOMPATIBLE_VERSION;
+  }
+
+  Status = FillHostBridgeAperture (
+             HostBridge->IoStart,
+             HostBridge->IoSize,
+             MAX_UINT32,
+             Io
+             );
+  StickyError |= EFI_ERROR (Status);
+
+  Status = FillHostBridgeAperture (
+             HostBridge->PcieConfigStart,
+             HostBridge->PcieConfigSize,
+             MAX_UINT64,
+             PcieConfig
+             );
+  StickyError |= EFI_ERROR (Status);
+
+  if (HostBridge->Flags.Bits.CombineMemPMem) {
+    Status = MergeHostBridgeApertures (
+               HostBridge->MemStart,
+               HostBridge->MemSize,
+               HostBridge->PMemStart,
+               HostBridge->PMemSize,
+               MAX_UINT32,
+               Mem
+               );
+    StickyError |= EFI_ERROR (Status);
+
+    Status = MergeHostBridgeApertures (
+               HostBridge->MemAbove4GStart,
+               HostBridge->MemAbove4GSize,
+               HostBridge->PMemAbove4GStart,
+               HostBridge->PMemAbove4GSize,
+               MAX_UINT64,
+               MemAbove4G
+               );
+    StickyError |= EFI_ERROR (Status);
+
+    //
+    // Invalidate unused apertures
+    //
+    InvalidateRootBridgeAperture (MAX_UINT32, PMem);
+    InvalidateRootBridgeAperture (MAX_UINT64, PMemAbove4G);
+  } else {
+    Status = FillHostBridgeAperture (
+               HostBridge->MemStart,
+               HostBridge->MemSize,
+               MAX_UINT32,
+               Mem
+               );
+    StickyError |= EFI_ERROR (Status);
+
+    Status = FillHostBridgeAperture (
+               HostBridge->PMemStart,
+               HostBridge->PMemSize,
+               MAX_UINT32,
+               PMem
+               );
+    StickyError |= EFI_ERROR (Status);
+
+    Status = FillHostBridgeAperture (
+               HostBridge->MemAbove4GStart,
+               HostBridge->MemAbove4GSize,
+               MAX_UINT64,
+               MemAbove4G
+               );
+    StickyError |= EFI_ERROR (Status);
+
+    Status = FillHostBridgeAperture (
+               HostBridge->PMemAbove4GStart,
+               HostBridge->PMemAbove4GSize,
+               MAX_UINT64,
+               PMem
+               );
+    StickyError |= EFI_ERROR (Status);
+  }
+
+  if (StickyError) {
+    //
+    // If any function returned an error it is due to a valid range
+    // specified in the host bridge that was ignored due to a NULL
+    // pointer. Translate it to a warning to allow for calling with
+    // only a subset of the apertures.
+    //
+    return EFI_WARN_STALE_DATA;
+  }
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+HardwareInfoPciHostBridgeGetFlags (
+  IN CONST  HOST_BRIDGE_INFO  *HostBridge,
+  IN        UINTN             DataSize,
+  OUT       UINT64            *Attributes               OPTIONAL,
+  OUT       BOOLEAN           *DmaAbove4G               OPTIONAL,
+  OUT       BOOLEAN           *NoExtendedConfigSpace    OPTIONAL,
+  OUT       BOOLEAN           *CombineMemPMem           OPTIONAL
+  )
+{
+  if ((HostBridge == NULL) || (DataSize == 0)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // For now only version 0 is supported
+  //
+  if (HostBridge->Version != 0) {
+    return EFI_INCOMPATIBLE_VERSION;
+  }
+
+  if (Attributes) {
+    *Attributes = HostBridge->Attributes;
+  }
+
+  if (DmaAbove4G) {
+    *DmaAbove4G = !!HostBridge->Flags.Bits.DmaAbove4G;
+  }
+
+  if (NoExtendedConfigSpace) {
+    *NoExtendedConfigSpace = !!HostBridge->Flags.Bits.NoExtendedConfigSpace;
+  }
+
+  if (CombineMemPMem) {
+    *CombineMemPMem = !!HostBridge->Flags.Bits.CombineMemPMem;
+  }
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+HardwareInfoPciHostBridgeGet (
+  IN CONST  HOST_BRIDGE_INFO          *HostBridge,
+  IN        UINTN                     DataSize,
+  OUT       UINTN                     *BusNrStart,
+  OUT       UINTN                     *BusNrLast,
+  OUT       UINT64                    *Attributes               OPTIONAL,
+  OUT       BOOLEAN                   *DmaAbove4G               OPTIONAL,
+  OUT       BOOLEAN                   *NoExtendedConfigSpace    OPTIONAL,
+  OUT       BOOLEAN                   *CombineMemPMem           OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *Io                       OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *Mem                      OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *MemAbove4G               OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PMem                     OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PMemAbove4G              OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PcieConfig               OPTIONAL
+  )
+{
+  EFI_STATUS  Status;
+
+  Status = HardwareInfoPciHostBridgeGetBusNrRange (
+             HostBridge,
+             DataSize,
+             BusNrStart,
+             BusNrLast
+             );
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = HardwareInfoPciHostBridgeGetFlags (
+             HostBridge,
+             DataSize,
+             Attributes,
+             DmaAbove4G,
+             NoExtendedConfigSpace,
+             CombineMemPMem
+             );
+
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = HardwareInfoPciHostBridgeGetApertures (
+             HostBridge,
+             DataSize,
+             Io,
+             Mem,
+             MemAbove4G,
+             PMem,
+             PMemAbove4G,
+             PcieConfig
+             );
+
+  return Status;
+}
diff --git a/OvmfPkg/Library/HardwareInfoLib/HardwareInfoPciHostBridgeLib.h b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoPciHostBridgeLib.h
new file mode 100644
index 0000000000..397c8f8a5d
--- /dev/null
+++ b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoPciHostBridgeLib.h
@@ -0,0 +1,262 @@
+/**@file
+  Hardware info library with types and accessors to parse information about
+  PCI host bridges.
+
+  Copyright 2021 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 __HARDWARE_INFO_PCI_HOST_BRIDGE_LIB_H__
+#define __HARDWARE_INFO_PCI_HOST_BRIDGE_LIB_H__
+
+#include <Uefi/UefiBaseType.h>
+#include <Uefi/UefiSpec.h>
+#include <Library/PciHostBridgeLib.h>
+
+//
+// Host Bridge resources information
+//
+#pragma pack(1)
+typedef struct {
+  //
+  // Feature tracking, initially 0
+  //
+  UINT64    Version;
+
+  //
+  // Host bridge enabled attributes (EFI_PCI_ATTRIBUTE_*)
+  //
+  UINT64    Attributes;
+
+  union {
+    UINT64    Uint64;
+    struct {
+      UINT64    DmaAbove4G            : 1;
+      UINT64    NoExtendedConfigSpace : 1;
+      UINT64    CombineMemPMem        : 1;
+      UINT64    Reserved              : 61;
+    } Bits;
+  } Flags;
+
+  //
+  // Bus number range
+  //
+  UINT8     BusNrStart;
+  UINT8     BusNrLast;
+
+  UINT8     Padding[6];
+
+  //
+  // IO aperture
+  //
+  UINT64    IoStart;
+  UINT64    IoSize;
+
+  //
+  // 32-bit MMIO aperture
+  //
+  UINT64    MemStart;
+  UINT64    MemSize;
+
+  //
+  // 32-bit prefetchable MMIO aperture
+  //
+  UINT64    PMemStart;
+  UINT64    PMemSize;
+
+  //
+  // 64-bit MMIO aperture
+  //
+  UINT64    MemAbove4GStart;
+  UINT64    MemAbove4GSize;
+
+  //
+  // 64-bit prefetchable MMIO aperture
+  //
+  UINT64    PMemAbove4GStart;
+  UINT64    PMemAbove4GSize;
+
+  //
+  // MMIO accessible PCIe config space (ECAM)
+  //
+  UINT64    PcieConfigStart;
+  UINT64    PcieConfigSize;
+} HOST_BRIDGE_INFO;
+#pragma pack()
+
+/**
+  Extract the last MMIO address, either from high (64-bit) or low (32-bit)
+  memory used by the HostBridge's apertures.
+
+  @param[in]  HostBridge      Root bridge's resources specification
+  @param[in]  DataSize        Size in bytes of the actually filled
+                              data available in the HostBridge object
+  @param[in]  HighMem         64-bit (true) or 32-bit (false) MMIO
+                              address
+  @param[out] LastMmioAddress Pointer to last MMIO address
+
+  @retval EFI_SUCCESS               Operation succeeded
+  @retval EFI_INVALID_PARAMETER     One or more pointer parameters are
+                                    invalid
+  @retval EFI_INCOMPATIBLE_VERSION  HostBridge information belongs to
+                                    an unsupported version
+**/
+EFI_STATUS
+HardwareInfoPciHostBridgeLastMmioAddress (
+  IN CONST  HOST_BRIDGE_INFO  *HostBridge,
+  IN        UINTN             DataSize,
+  IN        BOOLEAN           HighMem,
+  OUT       UINT64            *LastMmioAddress
+  );
+
+/**
+  Interpret the HostBridge resources and extact the bus number
+  range.
+
+  @param[in]  HostBridge   Root bridge's resources specification
+  @param[in]  DataSize     Size in bytes of the actually filled
+                           data available in the HostBridge object
+  @param[out] BusNrStart   Pointer to the Bus Number range start
+  @param[out] BusNrLast    Pointer to the Bus Number range end
+
+  @retval EFI_SUCCESS               Retrieved the bus number range
+                                    without any issues.
+  @retval EFI_INVALID_PARAMETER     One of the parameters is invalid,
+                                    either NULL pointer or size 0
+  @retval EFI_INCOMPATIBLE_VERSION  HostBridge data of unsupported
+                                    version
+**/
+EFI_STATUS
+HardwareInfoPciHostBridgeGetBusNrRange (
+  IN CONST  HOST_BRIDGE_INFO  *HostBridge,
+  IN        UINTN             DataSize,
+  OUT       UINTN             *BusNrStart,
+  OUT       UINTN             *BusNrLast
+  );
+
+/**
+  Interpret the MMIO resources in HostBridge and set the apertures
+  in 32-bit space (Mem), 64-bit space (MemAbove4G), PIO (IO) and
+  ECAM (PcieConfig) accordingly.
+
+  The 2 types of apertures in each MMIO space (prefetchable and
+  non-prefetchable) may be merged into a single window, hence if both
+  types of apertures are defined while the CombineMemPMem flag is set,
+  the ranges must be contiguous.
+
+  @param[in]  HostBridge   Root bridge's resources specification
+  @param[in]  DataSize     Size in bytes of the actually filled
+                           data available in the HostBridge object
+  @param[out] Mem          Pointer to 32-bit MMIO aperture
+  @param[out] MemAbove4G   Pointer to 64-bit MMIO aperture
+  @param[out] PMem         Pointer to the 32-bit prefetchable MMIO aperture
+  @param[out] PMemAbove4G  Pointer to the 64-bit prefetchable MMIO aperture
+  @param[out] PcieConfig   Pointer to MMIO mapped PCIe config aperture (ECAM)
+
+  @retval EFI_INVALID_PARAMETER     HostBridge object is invalid
+  @retval EFI_INCOMPATIBLE_VERSION  HostBridge information belongs to
+                                    an unsupported version
+  @retval EFI_WARN_STALE_DATA       One or more valid aperture in the
+                                    HostBridge's resources were ignored
+                                    because corresponding aperture pointer
+                                    is NULL.
+  @retval EFI_SUCCESS               Operation executed cleanly, all valid
+                                    ranges were parsed into the corresponding
+                                    aperture object.
+**/
+EFI_STATUS
+HardwareInfoPciHostBridgeGetApertures (
+  IN  CONST HOST_BRIDGE_INFO          *HostBridge,
+  IN        UINTN                     DataSize,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *Io,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *Mem,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *MemAbove4G,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PMem,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PMemAbove4G,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PcieConfig
+  );
+
+/**
+  Retrieve all flags and attributes of a host bridge describing the
+  resources and capabilities.
+
+  @param[in]  HostBridge            Host bridge information object
+  @param[in]  DataSize              Size in bytes of the actually filled
+                                    data available in the HostBridge object
+  @param[out] Attributes            Pointer to the host bridge's attributes
+  @param[out] DmaAbove4G            Pointer to the DMA Above 4G flag
+  @param[out] NoExtendedConfigSpace Pointer to the Extended Config Space flag
+  @param[out] CombineMemPMem        Pointer to the Combine Mem and PMem flag
+
+  @retval EFI_INVALID_PARAMETER     HostBridge object is invalid
+  @retval EFI_INCOMPATIBLE_VERSION  HostBridge information belongs to
+                                    an unsupported version
+  @retval EFI_SUCCESS               Operation executed cleanly
+**/
+EFI_STATUS
+HardwareInfoPciHostBridgeGetFlags (
+  IN CONST  HOST_BRIDGE_INFO  *HostBridge,
+  IN        UINTN             DataSize,
+  OUT       UINT64            *Attributes               OPTIONAL,
+  OUT       BOOLEAN           *DmaAbove4G               OPTIONAL,
+  OUT       BOOLEAN           *NoExtendedConfigSpace    OPTIONAL,
+  OUT       BOOLEAN           *CombineMemPMem           OPTIONAL
+  );
+
+/**
+  Getter that parses information from a HOST_BRIDGE_INFO object
+  into smaller chunks of types handled by the PciHostBridgeLib.
+
+  @param[in]  HostBridge            Host bridge information object
+  @param[in]  DataSize              Size in bytes of the actually filled
+                                    data available in the HostBridge object
+  @param[out] BusNrStart            Pointer to the Bus Number range start
+  @param[out] BusNrLast             Pointer to the Bus Number range end
+  @param[out] Attributes            Pointer to the host bridge's attributes
+  @param[out] DmaAbove4G            Pointer to the DMA Above 4G flag
+  @param[out] NoExtendedConfigSpace Pointer to the Extended Config Space flag
+  @param[out] CombineMemPMem        Pointer to the Combine Mem and PMem flag
+  @param[out] Io                    Pointer to the PIO aperture object
+  @param[out] Mem                   Pointer to the 32-bit MMIO aperture object
+  @param[out] MemAbove4G            Pointer to the 64-bit MMIO aperture object
+  @param[out] PMem                  Pointer to the 32-bit prefetchable MMIO
+                                    aperture object
+  @param[out] PMemAbove4G           Pointer to the 64-bit prefetchable MMIO
+                                    aperture object
+  @param[out] PcieConfig            MMIO mapped PCIe config aperture (ECAM)
+
+  @retval EFI_SUCCESS               Whole operation succeeded
+  @retval EFI_INVALID_PARAMETER     HostBridge object and/or non-optional
+                                    output parameters are invalid
+  @retval EFI_INCOMPATIBLE_VERSION  HostBridge information provided belongs to
+                                    and unsupported version
+  @retval EFI_WARN_STALE_DATA       One or more apertures having valid ranges
+                                    in the HostBridge info were ignored because
+                                    the correspnding aperture pointer is NULL
+**/
+EFI_STATUS
+HardwareInfoPciHostBridgeGet (
+  IN CONST  HOST_BRIDGE_INFO          *HostBridge,
+  IN        UINTN                     DataSize,
+  OUT       UINTN                     *BusNrStart,
+  OUT       UINTN                     *BusNrLast,
+  OUT       UINT64                    *Attributes               OPTIONAL,
+  OUT       BOOLEAN                   *DmaAbove4G               OPTIONAL,
+  OUT       BOOLEAN                   *NoExtendedConfigSpace    OPTIONAL,
+  OUT       BOOLEAN                   *CombineMemPMem           OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *Io                       OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *Mem                      OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *MemAbove4G               OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PMem                     OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PMemAbove4G              OPTIONAL,
+  OUT       PCI_ROOT_BRIDGE_APERTURE  *PcieConfig               OPTIONAL
+  );
+
+#endif // __HARDWARE_INFO_PCI_HOST_BRIDGE_LIB_H__
diff --git a/OvmfPkg/Library/HardwareInfoLib/HardwareInfoTypesLib.h b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoTypesLib.h
new file mode 100644
index 0000000000..84edd1c6ac
--- /dev/null
+++ b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoTypesLib.h
@@ -0,0 +1,64 @@
+/**@file
+  Hardware info types' definitions.
+
+  General hardware info types to parse the binary data
+
+  Copyright 2021 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 __HARDWARE_INFO_TYPES_LIB_H__
+#define __HARDWARE_INFO_TYPES_LIB_H__
+
+// Specific hardware types:
+#include "HardwareInfoPciHostBridgeLib.h"
+
+//
+// Hardware info types enumeration listing the supported
+// types which have an associated type definition
+//
+typedef enum {
+  HardwareInfoTypeUndefined  = 0,
+  HardwareInfoTypeHostBridge = 1,
+} HARDWARE_INFO_TYPE;
+
+//
+// Header format preceding and describing an associated hardware
+// info element
+//
+#pragma pack(1)
+typedef struct {
+  union {
+    UINT64                Uint64;
+    HARDWARE_INFO_TYPE    Value;
+  } Type;
+  UINT64    Size;
+} HARDWARE_INFO_HEADER;
+#pragma pack()
+
+//
+// Generic data structure to access any supported hardware type
+// resource definition
+//
+#pragma pack(1)
+typedef struct {
+  LIST_ENTRY              Link;
+  HARDWARE_INFO_HEADER    Header;
+  union {
+    UINT8               *Raw;
+    HOST_BRIDGE_INFO    *PciHostBridge;
+  } Data;
+} HARDWARE_INFO;
+#pragma pack()
+
+#define HARDWARE_INFO_FROM_LINK(a) \
+  BASE_CR (a, HARDWARE_INFO, Link)
+
+#endif // __HARDWARE_INFO_TYPES_LIB_H__
-- 
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




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

* [PATCH v2 2/8] Ovmf/HardwareInfoLib: Parse data directly from fw-cfg
  2022-01-20 17:10 [PATCH v2 0/8] Handling of multiple PCI host bridges specified Ojeda Leon, Nicolas
  2022-01-20 17:10 ` [PATCH v2 1/8] OvmfPkg/Library: Create base HardwareInfoLib for PCI Host Bridges Ojeda Leon, Nicolas
@ 2022-01-20 17:10 ` Ojeda Leon, Nicolas
  2022-01-20 17:10 ` [PATCH v2 3/8] Ovmf/HardwareInfoLib: Add core to parse heterogenous data Ojeda Leon, Nicolas
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-20 17:10 UTC (permalink / raw)
  To: devel; +Cc: atugup, Nicolas Ojeda Leon, Alexander Graf, Gerd Hoffmann

Create QemuFwCfgHardwareInfoLib which extends the base HardwareInfoLib
and provide fw-cfg wrappers to read hardware info elements out of a
fw-cfg file.

The Hardware Info library is intended to describe non-discoverable
hardware information and share that from the host to the guest in Ovmf
platforms. The QEMU fw-cfg extension for this library provides a first
variation to parse hardware info by reading it directly from a fw-cfg
file. This library offers a wrapper function to the plain
QmeuFwCfgReadBytes which, specifically, parses header-data pairs out
of the binary values in the file. For this purpose, the approach is
incremental, reading the file block by block and outputting the values
only for a specific known hardware type (e.g. PCI host bridges). One
element is returned in each call until the end of the file is reached.

Considering fw-cfg as the first means to transport hardware info from
the host to the guest, this wrapping library offers the possibility
to statically, and in steps, read a specific type of hardware info
elements out of the file. This method reads one hardware element of a
specific type at a time, without the need to pre-allocate memory and
read the whole file or dynamically allocate memory for each new
element found.

As a usage example, the static approach followed by this library
enables early UEFI stages to use and read hardware information
supplied by the host. For instance, in early times of the PEI stage,
hardware information can be parsed out from a fw-cfg file prescinding
from memory services, that may not yet be available, and avoiding
dynamic memory allocations.

Cc: Alexander Graf <graf@amazon.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Nicolas Ojeda Leon <ncoleon@amazon.com>
---
 .../Library/QemuFwCfgHardwareInfoLib.h        | 59 ++++++++++++
 .../QemuFwCfgHardwareInfoLib.c                | 89 +++++++++++++++++++
 .../QemuFwCfgHardwareInfoLib.inf              | 42 +++++++++
 OvmfPkg/OvmfPkgX64.dsc                        |  1 +
 4 files changed, 191 insertions(+)
 create mode 100644 OvmfPkg/Include/Library/QemuFwCfgHardwareInfoLib.h
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.c
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.inf

diff --git a/OvmfPkg/Include/Library/QemuFwCfgHardwareInfoLib.h b/OvmfPkg/Include/Library/QemuFwCfgHardwareInfoLib.h
new file mode 100644
index 0000000000..f6c351a4e4
--- /dev/null
+++ b/OvmfPkg/Include/Library/QemuFwCfgHardwareInfoLib.h
@@ -0,0 +1,59 @@
+/*/@file
+  Qemu fw-cfg wrappers for hardware info parsing.
+  Provides an alternative to parse hardware information from a fw-cfg
+  file without relying on dynamic memory allocations.
+
+  Copyright 2021 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 __QEMU_FW_CFG_HARDWARE_INFO_LIB_H__
+#define __QEMU_FW_CFG_HARDWARE_INFO_LIB_H__
+
+#include "../Library/HardwareInfoLib/HardwareInfoTypesLib.h"
+
+/**
+  Read, if available, the next Type element in the FwCfg file.
+  The FwCfg item must already be selected, this is a wrapper around
+  QemuFwCfgReadBytes and the Data pointer should be set to an existent
+  memory location with TypeSize bytes allocated for the date to be
+  properly written. If a Type element is found in the file which has a
+  size (in the header) greater than TypeSize, it is skipped.
+
+  @param[in]    Type             Hardware Info Type to search for
+  @param[in]    TypeSize         Size (in bytes) of the structure intended to
+                                 be used to dereference the data
+  @param[in]    TotalFileSize    Total size (in bytes) of the FwCfg file from
+                                 which the data is read.
+  @param[out]   Data             Pointer to a memory allocated instance into
+                                 which the data is written to.
+  @param[out]   DataSize         Size in bytes of the actually filled
+                                 data available in the Data object after a
+                                 successful operation
+  @param[inout] ReadIndex        Index of the next byte to be read. Incremented
+                                 accordingly after a read operation to reflect
+                                 up to date status
+
+  @retval EFI_SUCCESS             Next element found and read into Data
+  @retval EFI_INVALID_PARAMETER   Operation failed
+  @retval EFI_END_OF_FILE         End of the file reached, no more elements
+                                  to read.
+**/
+EFI_STATUS
+QemuFwCfgReadNextHardwareInfoByType (
+  IN      HARDWARE_INFO_TYPE  Type,
+  IN      UINTN               TypeSize,
+  IN      UINTN               TotalFileSize,
+  OUT     VOID                *Data,
+  OUT     UINTN               *DataSize         OPTIONAL,
+  IN OUT  UINTN               *ReadIndex
+  );
+
+#endif // __QEMU_FW_CFG_HARDWARE_INFO_LIB_H__
diff --git a/OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.c b/OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.c
new file mode 100644
index 0000000000..059da546d1
--- /dev/null
+++ b/OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.c
@@ -0,0 +1,89 @@
+/*/@file
+  Qemu fw-cfg wrappers for hardware info parsing.
+  Provides an alternative to parse hardware information from a fw-cfg
+  file without relying on dynamic memory allocations.
+
+  Copyright 2021 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.
+**/
+
+#include <Library/DebugLib.h>
+#include <Library/QemuFwCfgLib.h>
+
+#include <Library/QemuFwCfgHardwareInfoLib.h>
+
+/**
+  Update an optional pointer value if possible
+
+  @param[out] DataSize     Pointer to variable to be updated
+  @param[in]  Value        Value to set the pointed variable to.
+**/
+STATIC
+VOID
+UpdateDataSize (
+  OUT UINTN  *DataSize,
+  IN  UINTN  Value
+  )
+{
+  if (DataSize == NULL) {
+    return;
+  }
+
+  *DataSize = Value;
+}
+
+EFI_STATUS
+QemuFwCfgReadNextHardwareInfoByType (
+  IN      HARDWARE_INFO_TYPE  Type,
+  IN      UINTN               TypeSize,
+  IN      UINTN               TotalFileSize,
+  OUT     VOID                *Data,
+  OUT     UINTN               *DataSize         OPTIONAL,
+  IN OUT  UINTN               *ReadIndex
+  )
+{
+  HARDWARE_INFO_HEADER  Header;
+
+  if ((Data == NULL) ||
+      (ReadIndex == NULL) ||
+      (TypeSize == 0) ||
+      (Type == HardwareInfoTypeUndefined) ||
+      (TotalFileSize == 0))
+  {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  UpdateDataSize (DataSize, 0);
+
+  while (*ReadIndex < TotalFileSize) {
+    QemuFwCfgReadBytes (sizeof (Header), &Header);
+    *ReadIndex += sizeof (Header);
+
+    if ((Header.Type.Value == Type) && (Header.Size <= TypeSize)) {
+      QemuFwCfgReadBytes (Header.Size, Data);
+
+      *ReadIndex += Header.Size;
+      UpdateDataSize (DataSize, Header.Size);
+
+      return EFI_SUCCESS;
+    }
+
+    //
+    // Skip the bytes corresponding to the next element as it is
+    // not of the expected type and/or size. The TotalFileSize
+    // and individual elements sizes should match so the size
+    // check is skipped.
+    //
+    QemuFwCfgSkipBytes (Header.Size);
+    *ReadIndex += Header.Size;
+  }
+
+  return EFI_END_OF_FILE;
+}
diff --git a/OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.inf b/OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.inf
new file mode 100644
index 0000000000..0a2eca3c00
--- /dev/null
+++ b/OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.inf
@@ -0,0 +1,42 @@
+## @file
+#  Hardware information library wrappers over Qemu fw-cfg functionality to parse,
+#  in a static manner, non-discoverable hardware information.
+#
+#  Copyright 2021 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.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = QemuFwCfgHardwareInfoLib
+  FILE_GUID                      = 3D5011B3-9CBB-4C0B-88E8-1D758283C659
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = QemuFwCfgHardwareInfoLib
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+#  VALID_ARCHITECTURES           = X64
+#
+
+[Sources]
+  QemuFwCfgHardwareInfoLib.c
+  HardwareInfoPciHostBridgeLib.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  DebugLib
+  QemuFwCfgLib
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 4589adff38..fb82fd02eb 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -175,6 +175,7 @@
   SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
   SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
+  QemuFwCfgHardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.inf
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
   QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
   VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
-- 
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




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

* [PATCH v2 3/8] Ovmf/HardwareInfoLib: Add core to parse heterogenous data
  2022-01-20 17:10 [PATCH v2 0/8] Handling of multiple PCI host bridges specified Ojeda Leon, Nicolas
  2022-01-20 17:10 ` [PATCH v2 1/8] OvmfPkg/Library: Create base HardwareInfoLib for PCI Host Bridges Ojeda Leon, Nicolas
  2022-01-20 17:10 ` [PATCH v2 2/8] Ovmf/HardwareInfoLib: Parse data directly from fw-cfg Ojeda Leon, Nicolas
@ 2022-01-20 17:10 ` Ojeda Leon, Nicolas
  2022-01-21 10:09   ` Gerd Hoffmann
  2022-01-20 17:10 ` [PATCH v2 4/8] Ovmf/PlatformPei: Extend 64-bit MMIO range to fit resources Ojeda Leon, Nicolas
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-20 17:10 UTC (permalink / raw)
  To: devel; +Cc: atugup, Nicolas Ojeda Leon, Alexander Graf, Gerd Hoffmann

Following the Hardware Info library, this commit extends the base lib
and the static parsing methods to the generic, and thus the core of
the library, approach, capable of parsing heterogeneous hardware
information. The list-like API grants callers a flexible and common
pattern to retrieve the data. Moreover, the initial source is a BLOB
which generalizes the host-to-guest transmission mechanism.

The Hardware Info library main objective is to provide a way to
describe non-discoverable hardware so that the host can share the
available resources with the guest in Ovmf platforms. This change
features and embraces the main idea behind the library by providing
an API that parses a BLOB into a linked list to retrieve hardware
data from any source. Additionally, list-like APIs are provided so
that the hardware info list can be traversed conveniently.
Similarly, the capability is provided to filter results by specific
hardware types. However, heterogeneous elements can be added to the
list, increasing the flexibility. This way, a single source, for
example a fw-cfg file, can be used to describe several instances of
multiple types of hardware.

This part of the Hardware Info library makes use of dynamic memory
and is intended for stages in which memory services are available.
A motivation example is the PciHostBridgeLib. This library, part
of the PCI driver populates the list of PCI root bridges during DXE
stage for future steps to discover the resources under them. The
hardware info library can be used to obtain the detailed description
of available host bridges, for instance in the form of a fw-cfg file,
and parse that information into a dynmaic list that allows, first to
verify consistency of the data, and second discover the resources
availabe for each root bridge.

Cc: Alexander Graf <graf@amazon.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Nicolas Ojeda Leon <ncoleon@amazon.com>
---
 OvmfPkg/Include/Library/HardwareInfoLib.h     | 128 +++++++++
 .../Library/HardwareInfoLib/HardwareInfoLib.c | 256 ++++++++++++++++++
 .../HardwareInfoLib/HardwareInfoLib.inf       |  43 +++
 OvmfPkg/OvmfPkgX64.dsc                        |   1 +
 4 files changed, 428 insertions(+)
 create mode 100644 OvmfPkg/Include/Library/HardwareInfoLib.h
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.c
 create mode 100644 OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.inf

diff --git a/OvmfPkg/Include/Library/HardwareInfoLib.h b/OvmfPkg/Include/Library/HardwareInfoLib.h
new file mode 100644
index 0000000000..dc7f209b09
--- /dev/null
+++ b/OvmfPkg/Include/Library/HardwareInfoLib.h
@@ -0,0 +1,128 @@
+/*/@file
+  Hardware info parsing functions.
+  Binary data is expected as a consecutive series of header - object pairs.
+  Complete library providing static Qemu fw-cfg wrappers as well as list-like
+  interface to dynamically manipulate hardware info objects and parsing from
+  a generic blob.
+
+  Copyright 2021 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 __HARDWARE_INFO_LIB_H__
+#define __HARDWARE_INFO_LIB_H__
+
+#include "QemuFwCfgHardwareInfoLib.h"
+
+/**
+  Parse binary data containing resource information of multiple hardware
+  elements into a list of interpreted resources.
+  The translation is done on a copy-parse base so the blob can be freed
+  afterwards.
+
+  @param[in]  Blob           Binary data to be parsed
+  @param[in]  BlobSize       Size (in bytes) of the binary data
+  @param[in]  TypeFilter     Optional type to filter entries. Set to
+                             undefined to disable filtering and retrieve all
+  @param[out] ListHead       Head of the list to populate hardware information
+
+  @retval EFI_SUCCESS            Succeed.
+  @retval EFI_INVALID_PARAMETER  Provided Blob inforation is invalid
+  @retval EFI_OUT_OF_RESOURCES   Out of memory, list populated as far as
+                                 possible
+**/
+EFI_STATUS
+CreateHardwareInfoList (
+  IN  UINT8               *Blob,
+  IN  UINTN               BlobSize,
+  IN  HARDWARE_INFO_TYPE  TypeFilter,
+  OUT LIST_ENTRY          *ListHead
+  );
+
+/**
+  Free the dynamically allocated list of HADWARE_INFO items populated
+  during parsing of Blob
+
+  @param ListHead         Head of the list to be destroyed
+**/
+VOID
+FreeHardwareInfoList (
+  IN OUT  LIST_ENTRY  *ListHead
+  );
+
+/**
+  Retrieve the number of hardware components of a specific type
+  in the list.
+
+  @param[in]  ListHead       Head of the hardware info list
+  @param[in]  Type           Type of hardware elements to count
+  @param[in]  TypeSize       Size (in bytes) of the structure intended to
+                             be used to dereference the data
+  @return Count of elements of Type found
+**/
+UINTN
+GetHardwareInfoCountByType (
+  IN  LIST_ENTRY          *ListHead,
+  IN  HARDWARE_INFO_TYPE  Type,
+  IN  UINTN               TypeSize
+  );
+
+/**
+  Get the First Hardware Info entry in the list of the specified type
+
+  @param[in]  ListHead     Head of the hardware info list
+  @param[in]  Type         Hardware Info Type to search for
+  @param[in]  TypeSize     Size (in bytes) of the structure intended to
+                           be used to dereference the data
+  @return Link of first entry of specified type or list head if not found
+**/
+LIST_ENTRY *
+GetFirstHardwareInfoByType (
+  IN  LIST_ENTRY          *ListHead,
+  IN  HARDWARE_INFO_TYPE  Type,
+  IN  UINTN               TypeSize
+  );
+
+/**
+  Get the Next Hardware Info entry in the list with the specified
+  type, which follows the provided Node.
+
+  @param[in]  ListHead       Head of the hardware info list
+  @param[in]  Node           Current, already processed, node's link
+  @param[in]  Type           Hardware Info Type to search for
+  @param[in]  TypeSize       Size (in bytes) of the structure intended to
+                             be used to dereference the data
+  @return Link of next entry, after Node, of the specified type.
+          List head otherwise
+**/
+LIST_ENTRY *
+GetNextHardwareInfoByType (
+  IN  LIST_ENTRY          *ListHead,
+  IN  LIST_ENTRY          *Node,
+  IN  HARDWARE_INFO_TYPE  Type,
+  IN  UINTN               TypeSize
+  );
+
+/**
+  Assess if Node stands at the end of the doubly linked list
+
+  @param[in]  ListHead      Head of the hardware info list
+  @param[in]  Node          Current Node link
+
+  @retval TRUE  Node is at the end of the list
+  @retval FALSE Node is not at the end of the list
+**/
+BOOLEAN
+EndOfHardwareInfoList (
+  IN  LIST_ENTRY  *ListHead,
+  IN  LIST_ENTRY  *Node
+  );
+
+#endif // __HARDWARE_INFO_LIB_H__
diff --git a/OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.c b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.c
new file mode 100644
index 0000000000..5136c55031
--- /dev/null
+++ b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.c
@@ -0,0 +1,256 @@
+/*/@file
+  Hardware info parsing functions.
+  Binary data is expected as a consecutive series of header - object pairs.
+  Complete library providing static Qemu fw-cfg wrappers as well as list-like
+  interface to dynamically manipulate hardware info objects and parsing from
+  a generic blob.
+
+  Copyright 2021 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.
+**/
+
+#include <string.h>
+#include <Uefi/UefiBaseType.h>
+#include <Uefi/UefiSpec.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/BaseLib.h>
+#include <Library/UefiLib.h>
+
+#include <Library/HardwareInfoLib.h>
+
+EFI_STATUS
+CreateHardwareInfoList (
+  IN  UINT8               *Blob,
+  IN  UINTN               BlobSize,
+  IN  HARDWARE_INFO_TYPE  TypeFilter,
+  OUT LIST_ENTRY          *ListHead
+  )
+{
+  UINT8          *Index;
+  HARDWARE_INFO  *HwComponent;
+
+  if ((Blob == NULL) || (BlobSize <= 0) ||
+      (ListHead == NULL))
+  {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Index = Blob;
+  while (Index < (Blob + BlobSize)) {
+    HwComponent = AllocateZeroPool (sizeof (HARDWARE_INFO));
+
+    if (HwComponent == NULL) {
+      goto FailedAllocate;
+    }
+
+    HwComponent->Header.Type.Uint64 = *((UINT64 *)Index);
+    Index                          += sizeof (HwComponent->Header.Type);
+    HwComponent->Header.Size        = *((UINT64 *)(Index));
+    Index                          += sizeof (HwComponent->Header.Size);
+
+    //
+    // Check if optional TypeFilter is set, skip if the current
+    // object is of a different type and release the partially
+    // allocated object
+    //
+    if ((TypeFilter != HardwareInfoTypeUndefined) &&
+        (HwComponent->Header.Type.Value != TypeFilter))
+    {
+      FreePool (HwComponent);
+      Index += HwComponent->Header.Size;
+      continue;
+    }
+
+    HwComponent->Data.Raw = AllocateZeroPool (HwComponent->Header.Size);
+    if (HwComponent->Data.Raw == NULL) {
+      goto FreeResources;
+    }
+
+    CopyMem (HwComponent->Data.Raw, Index, HwComponent->Header.Size);
+    Index += HwComponent->Header.Size;
+
+    InsertTailList (ListHead, &HwComponent->Link);
+  }
+
+  return EFI_SUCCESS;
+
+FreeResources:
+  //
+  // Clean the resources allocated in the incomplete cycle
+  //
+  FreePool (HwComponent);
+
+FailedAllocate:
+  DEBUG ((
+    EFI_D_ERROR,
+    "%a: Failed to allocate memory for hardware info\n",
+    __FUNCTION__
+    ));
+
+  return EFI_OUT_OF_RESOURCES;
+}
+
+VOID
+FreeHardwareInfoList (
+  IN OUT  LIST_ENTRY  *ListHead
+  )
+{
+  LIST_ENTRY     *CurrentLink;
+  HARDWARE_INFO  *HwComponent;
+
+  if (IsListEmpty (ListHead)) {
+    return;
+  }
+
+  CurrentLink = ListHead->ForwardLink;
+  while (CurrentLink != NULL && CurrentLink != ListHead) {
+    HwComponent = HARDWARE_INFO_FROM_LINK (CurrentLink);
+
+    //
+    // Remove item from list before invalidating the pointers
+    //
+    CurrentLink = RemoveEntryList (CurrentLink);
+
+    FreePool (HwComponent->Data.Raw);
+    FreePool (HwComponent);
+  }
+}
+
+/**
+  Validates if the specified Node has a valid data size and is of
+  specified type.
+  The data size can be less or equal to the provided type size to be
+  regarded as valid and thus accessible with the typed pointer.
+
+  For future compatibility the size is allowed to be smaller so that
+  different versions interpret fields differently and, particularly,
+  have smaller data structures. However, it cannot be larger than the
+  type size to avoid accessing memory out of bounds.
+
+  @param[in]  Node      Hardware Info node to be validated
+  @param[in]  TypeSize  Size (in bytes) of the data type intended to be
+                        used to dereference the data.
+  @retval TRUE  Node is valid and can be accessed
+  @retval FALSE Node is not valid
+/*/
+STATIC
+BOOLEAN
+IsHardwareInfoNodeValidByType (
+  IN  LIST_ENTRY          *ListHead,
+  IN  LIST_ENTRY          *Link,
+  IN  HARDWARE_INFO_TYPE  Type,
+  IN  UINTN               TypeSize
+  )
+{
+  HARDWARE_INFO  *HwComponent;
+
+  if (IsNull (ListHead, Link)) {
+    return FALSE;
+  }
+
+  HwComponent = HARDWARE_INFO_FROM_LINK (Link);
+
+  //
+  // Verify if the node type is the specified one and the size of
+  // the data allocated to the node is greater than the size of
+  // the type intended to dereference it in order to avoid access
+  // to memory out of bondaries.
+  //
+  if ((HwComponent->Header.Type.Value == Type) &&
+      (HwComponent->Header.Size >= TypeSize))
+  {
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+UINTN
+GetHardwareInfoCountByType (
+  IN  LIST_ENTRY          *ListHead,
+  IN  HARDWARE_INFO_TYPE  Type,
+  IN  UINTN               TypeSize
+  )
+{
+  UINTN       Count;
+  LIST_ENTRY  *Link;
+
+  Count = 0;
+  for (Link = GetFirstHardwareInfoByType (ListHead, Type, TypeSize);
+       !IsNull (ListHead, Link);
+       Link = GetNextHardwareInfoByType (ListHead, Link, Type, TypeSize))
+  {
+    if (IsHardwareInfoNodeValidByType (ListHead, Link, Type, TypeSize)) {
+      Count++;
+    }
+  }
+
+  return Count;
+}
+
+LIST_ENTRY *
+GetFirstHardwareInfoByType (
+  IN  LIST_ENTRY          *ListHead,
+  IN  HARDWARE_INFO_TYPE  Type,
+  IN  UINTN               TypeSize
+  )
+{
+  LIST_ENTRY  *Link;
+
+  if (IsListEmpty (ListHead)) {
+    return ListHead;
+  }
+
+  Link = GetFirstNode (ListHead);
+
+  if (IsHardwareInfoNodeValidByType (ListHead, Link, Type, TypeSize)) {
+    return Link;
+  }
+
+  return GetNextHardwareInfoByType (ListHead, Link, Type, TypeSize);
+}
+
+LIST_ENTRY *
+GetNextHardwareInfoByType (
+  IN  LIST_ENTRY          *ListHead,
+  IN  LIST_ENTRY          *Node,
+  IN  HARDWARE_INFO_TYPE  Type,
+  IN  UINTN               TypeSize
+  )
+{
+  LIST_ENTRY  *Link;
+
+  Link = GetNextNode (ListHead, Node);
+
+  while (!IsNull (ListHead, Link)) {
+    if (IsHardwareInfoNodeValidByType (ListHead, Link, Type, TypeSize)) {
+      //
+      // Found a node of specified type and with valid size. Break and
+      // return the found node.
+      //
+      break;
+    }
+
+    Link = GetNextNode (ListHead, Link);
+  }
+
+  return Link;
+}
+
+BOOLEAN
+EndOfHardwareInfoList (
+  IN  LIST_ENTRY  *ListHead,
+  IN  LIST_ENTRY  *Node
+  )
+{
+  return IsNull (ListHead, Node);
+}
diff --git a/OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.inf b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.inf
new file mode 100644
index 0000000000..c099b072a3
--- /dev/null
+++ b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.inf
@@ -0,0 +1,43 @@
+## @file
+#  Hardware information library to describe non-discoverable hardware resources
+#
+#  Copyright 2021 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.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = HardwareInfoLib
+  FILE_GUID                      = F60B206A-5C56-11EC-AEAC-67CB080BCFF2
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = HardwareInfoLib
+
+#
+# The following information is for reference only and not required by the build
+# tools.
+#
+#  VALID_ARCHITECTURES           = X64
+#
+
+[Sources]
+  HardwareInfoLib.c
+  HardwareInfoPciHostBridgeLib.c
+  QemuFwCfgHardwareInfoLib.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+
+[LibraryClasses]
+  BaseMemoryLib
+  DebugLib
+  MemoryAllocationLib
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index fb82fd02eb..062e16482d 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -175,6 +175,7 @@
   SecurityManagementLib|MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
   SerializeVariablesLib|OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.inf
+  HardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.inf
   QemuFwCfgHardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.inf
   QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxeLib.inf
   QemuFwCfgSimpleParserLib|OvmfPkg/Library/QemuFwCfgSimpleParserLib/QemuFwCfgSimpleParserLib.inf
-- 
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




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

* [PATCH v2 4/8] Ovmf/PlatformPei: Extend 64-bit MMIO range to fit resources
  2022-01-20 17:10 [PATCH v2 0/8] Handling of multiple PCI host bridges specified Ojeda Leon, Nicolas
                   ` (2 preceding siblings ...)
  2022-01-20 17:10 ` [PATCH v2 3/8] Ovmf/HardwareInfoLib: Add core to parse heterogenous data Ojeda Leon, Nicolas
@ 2022-01-20 17:10 ` Ojeda Leon, Nicolas
  2022-01-21  9:57   ` Gerd Hoffmann
  2022-01-20 17:10 ` [PATCH v2 5/8] OvmfPkg/PciHostBridgeUtilityLib: Initialize RootBridges apertures with spec Ojeda Leon, Nicolas
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-20 17:10 UTC (permalink / raw)
  To: devel; +Cc: atugup, Nicolas Ojeda Leon, Alexander Graf, Gerd Hoffmann

Read the "hardware-info" item from fw-cfg to extract specifications
of PCI host bridges and analyze the 64-bit apertures of them to
find out if 64-bit PCI size needs to be increased in order to
allocate enough address space to fit all the host bridges in a
valid region.

Using the static Hardware Info Lib API, read the fw-cfg file of
hardware information to extract, one by one, all the host
bridges. Find the last 64-bit MMIO address of each host bridge,
using the HardwareInfoPciHostBridgeLib API, and compare it to an
accumulate value to discover the highest address used, which
corresponds to the highest value that must be covered by the
64-bit PCI MMIO window.

Given that platforms with multiple host bridges may provide the PCI
apertures' addresses, the PCD token used to initialize the available
address space (PcdPciMmio64Size) must include those addresses for
the allocation process to be able to place resources in the
intended regions. Since the Pcd entry defining the 64-bit PCI MMIO
size is dynamic and initialized with a platform-dependent value,
it is updated to a higher value if that is required for all
high memory PCI host bridge apertures to fit in the valid address
space.

As an example, if the PcdPciMmio64Size is initialized to 0x800000000
and the runtime calculated base address produces the value 0x800000000,
the last 64-bit MMIO address a PCI host bridge may use would be
0xFFFFFFFFF. If one of the host-specified bridges defines a high
memory window 0x1000000000 - 0x17FFFFFFFF (inclusive) it would be
outside of the valid range defined by the Base address + PcdPciMmio64Size.
Therefore, parsing the HardwwareInfo resource allows to notice the higher
requirement and increase the size to 0x1000000000 so that the MMIO
address space solicited by the host can be met and the range effectively
extended to 0x17FFFFFFFF.

Cc: Alexander Graf <graf@amazon.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Nicolas Ojeda Leon <ncoleon@amazon.com>
---
 OvmfPkg/PlatformPei/MemDetect.c     | 110 ++++++++++++++++++++++++++++
 OvmfPkg/PlatformPei/PlatformPei.inf |   1 +
 2 files changed, 111 insertions(+)

diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
index 1bcb5a08bc..b0bdd435cb 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -36,6 +36,7 @@ Module Name:
 #include <Library/MtrrLib.h>
 #include <Library/QemuFwCfgLib.h>
 #include <Library/QemuFwCfgSimpleParserLib.h>
+#include <Library/QemuFwCfgHardwareInfoLib.h>
 
 #include "Platform.h"
 #include "Cmos.h"
@@ -369,6 +370,110 @@ GetSystemMemorySizeAbove4gb (
   return LShiftU64 (Size, 16);
 }
 
+/**
+  Iterate over the PCI host bridges resources information optionally provided
+  in fw-cfg.
+
+  Find the highest address used by the PCI bridges in 64-bit MMIO space to
+  calculate and modify the PCI aperture size accordingly (PciMmio64Size)
+
+  @param[in] PciMmio64Base    Base address (start) of the 64-bit PCI MMIO
+                              address space.
+
+  @param[inout] PciMmio64Size Size of the PCI 64-bit MMIO aperture provided
+                              as input and modified (output) if the resources
+                              indicated by fw_cfg require a larger address
+                              space.
+
+  @retval EFI_SUCCESS         The fw_cfg host-bridges-info was found and
+                              processed.
+
+  @retval EFI_PROTOCOL_ERROR  The host bridges information file was found,
+                              but its size wasn't a whole multiple of
+                              sizeof(HOST_BRIDGE_INFO). No entry was processed.
+
+  @retval EFI_NOT_FOUND       fw-cfg file with host bridges information was not
+                              found. Does not constitute an errro since the file
+                              is optional and used in special cases.
+
+  @retval EFI_UNSUPPORTED     fw-cfg is unavailable
+
+**/
+STATIC
+EFI_STATUS
+ScanPci64BitApertureSize (
+  IN UINT64      PciMmio64Base,
+  IN OUT UINT64  *PciMmio64Size
+  )
+{
+  EFI_STATUS            Status;
+  HOST_BRIDGE_INFO      HostBridge;
+  FIRMWARE_CONFIG_ITEM  FwCfgItem;
+  UINTN                 FwCfgSize;
+  UINTN                 FwCfgReadIndex;
+  UINTN                 ReadDataSize;
+  UINT64                PciEnd;
+  UINT64                PcdPciEnd;
+  UINT64                Above4GMmioEnd;
+
+  PciEnd         = 0;
+  Above4GMmioEnd = 0;
+  PcdPciEnd      = PciMmio64Base + *PciMmio64Size - 1;
+
+  Status = QemuFwCfgFindFile ("etc/hardware-info", &FwCfgItem, &FwCfgSize);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  QemuFwCfgSelectItem (FwCfgItem);
+
+  FwCfgReadIndex = 0;
+  while (FwCfgReadIndex < FwCfgSize) {
+    Status = QemuFwCfgReadNextHardwareInfoByType (
+               HardwareInfoTypeHostBridge,
+               sizeof (HostBridge),
+               FwCfgSize,
+               &HostBridge,
+               &ReadDataSize,
+               &FwCfgReadIndex
+               );
+
+    if (Status != EFI_SUCCESS) {
+      //
+      // No more data available to read in the file, break
+      // loop and finish process
+      //
+      break;
+    }
+
+    Status = HardwareInfoPciHostBridgeLastMmioAddress (
+               &HostBridge,
+               ReadDataSize,
+               TRUE,
+               &Above4GMmioEnd
+               );
+
+    if (Status != EFI_SUCCESS) {
+      //
+      // Error parsing MMIO apertures and extracting last MMIO
+      // address, bail out and propagate error
+      //
+      return Status;
+    }
+
+    if (Above4GMmioEnd > PciEnd) {
+      PciEnd = Above4GMmioEnd;
+    }
+  }
+
+  if (PciEnd > PcdPciEnd) {
+    *PciMmio64Size = PciEnd - PciMmio64Base + 1;
+    *PciMmio64Size = ALIGN_VALUE (*PciMmio64Size, (UINT64)SIZE_1GB);
+  }
+
+  return EFI_SUCCESS;
+}
+
 /**
   Return the highest address that DXE could possibly use, plus one.
 **/
@@ -515,6 +620,11 @@ GetFirstNonAddress (
   //
   Pci64Base = ALIGN_VALUE (Pci64Base, GetPowerOfTwo64 (Pci64Size));
 
+  //
+  // Extend Pci64Size if fw_cfg Host bridges specification requires it
+  //
+  ScanPci64BitApertureSize (Pci64Base, &Pci64Size);
+
   if (mBootMode != BOOT_ON_S3_RESUME) {
     //
     // The core PciHostBridgeDxe driver will automatically add this range to
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 8ef404168c..413b92b855 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -64,6 +64,7 @@
   MemEncryptSevLib
   PcdLib
   VmgExitLib
+  QemuFwCfgHardwareInfoLib
 
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
-- 
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




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

* [PATCH v2 5/8] OvmfPkg/PciHostBridgeUtilityLib: Initialize RootBridges apertures with spec
  2022-01-20 17:10 [PATCH v2 0/8] Handling of multiple PCI host bridges specified Ojeda Leon, Nicolas
                   ` (3 preceding siblings ...)
  2022-01-20 17:10 ` [PATCH v2 4/8] Ovmf/PlatformPei: Extend 64-bit MMIO range to fit resources Ojeda Leon, Nicolas
@ 2022-01-20 17:10 ` Ojeda Leon, Nicolas
  2022-01-20 17:10 ` [PATCH v2 6/8] MdeModulePkg, OvmfPkg: Add Pcd token for PCI pre-populated BARs Ojeda Leon, Nicolas
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-20 17:10 UTC (permalink / raw)
  To: devel; +Cc: atugup, Nicolas Ojeda Leon, Alexander Graf, Gerd Hoffmann

Consume the host-provided specification of PCI host bridges if
available. Using the Hardware Info library, populate a list of
hardware descriptors based on the content of the "hardware-info"
fw-cfg file, if provided. In the affirmative case, use the
resources and attributes specified by the hypervisor for each
Host Bridge to create the RootBridge elements.

In Ovmf platforms, the host can provide the specification of
non-discoverable hardware resources like PCI host bridges. If the
proper fw-cfg file is found, parse the contents provided by the
host into a linked list by using the Hardware Info library. Then,
using the list of PCI host bridges' descriptions, populate the
PCI_ROOT_BRIDGES array with the resources and attributes specified
by the host. If the file is not provided or no Host Bridge is found
in it, fold back to the legacy method based on pre-defined
apertures and rules.

In some use cases, the host requires additional control over the
hardware resources' configurations in the guest for performance and
discoverability reasons. For instance, to disclose information about
the PCI hierarchy to the guest so that this can profit from
optimized accesses. In this case, the host can decide to describe
multiple PCI Host Bridges and provide a specific set of resources
(e.g. MMIO apertures) so that the guest uses the values provided.
Using the provided values may entitle the guest to added performance,
for example by using specific MMIO mappings that can enable peer-to-peer
communication across the PCI hierarchy or by allocating memory closer
to a device for faster DMA transactions.

Cc: Alexander Graf <graf@amazon.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Nicolas Ojeda Leon <ncoleon@amazon.com>
---
 .../PciHostBridgeUtilityLib.c                 | 345 ++++++++++++++----
 .../PciHostBridgeUtilityLib.inf               |   1 +
 2 files changed, 268 insertions(+), 78 deletions(-)

diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
index 92e1ea812f..b0e3b5e3cf 100644
--- a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
+++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
@@ -12,13 +12,16 @@
 
 #include <IndustryStandard/Acpi10.h>
 #include <IndustryStandard/Pci.h>
+#include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/DevicePathLib.h>
+#include <Library/HardwareInfoLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PciHostBridgeUtilityLib.h>
 #include <Library/PciLib.h>
 #include <Library/QemuFwCfgLib.h>
+#include <Protocol/PciHostBridgeResourceAllocation.h>
 
 #pragma pack(1)
 typedef struct {
@@ -234,14 +237,29 @@ PciHostBridgeUtilityGetRootBridges (
   IN  PCI_ROOT_BRIDGE_APERTURE  *PMemAbove4G
   )
 {
-  EFI_STATUS            Status;
-  FIRMWARE_CONFIG_ITEM  FwCfgItem;
-  UINTN                 FwCfgSize;
-  UINT64                ExtraRootBridges;
-  PCI_ROOT_BRIDGE       *Bridges;
-  UINTN                 Initialized;
-  UINTN                 LastRootBridgeNumber;
-  UINTN                 RootBridgeNumber;
+  EFI_STATUS                Status;
+  FIRMWARE_CONFIG_ITEM      FwCfgItem;
+  UINTN                     FwCfgSize;
+  UINT64                    ExtraRootBridges;
+  PCI_ROOT_BRIDGE           *Bridges;
+  UINTN                     Initialized;
+  UINTN                     LastRootBridgeNumber;
+  UINTN                     RootBridgeNumber;
+  UINTN                     PciHostBridgeCount;
+  UINT8                     *HardwareInfoBlob;
+  LIST_ENTRY                HwInfoList;
+  LIST_ENTRY                *HwLink;
+  HARDWARE_INFO             *HwInfo;
+  UINT64                    HwInfoAttributes;
+  UINT64                    HwInfoAllocationAttributes;
+  BOOLEAN                   HwInfoDmaAbove4G;
+  BOOLEAN                   HwInfoNoExtendedConfigSpace;
+  BOOLEAN                   HwInfoCombineMemPMem;
+  PCI_ROOT_BRIDGE_APERTURE  HwInfoIo;
+  PCI_ROOT_BRIDGE_APERTURE  HwInfoMem;
+  PCI_ROOT_BRIDGE_APERTURE  HwInfoMemAbove4G;
+  PCI_ROOT_BRIDGE_APERTURE  HwInfoPMem;
+  PCI_ROOT_BRIDGE_APERTURE  HwInfoPMemAbove4G;
 
   *Count = 0;
 
@@ -294,103 +312,266 @@ PciHostBridgeUtilityGetRootBridges (
       ));
   }
 
+  //
+  // Initialize the Hardware Info list head to start with an empty but valid
+  // list head.
+  //
+  InitializeListHead (&HwInfoList);
+  HardwareInfoBlob   =  NULL;
+  Initialized        = 0;
+  Bridges            = NULL;
+  PciHostBridgeCount = 0;
+
+  //
+  // Hypervisor can provide the specifications (resources) for one or more
+  // PCI host bridges. Such information comes through fw-cfg as part of
+  // the hardware-info file.
+  //
+  Status = QemuFwCfgFindFile ("etc/hardware-info", &FwCfgItem, &FwCfgSize);
+
+  if (!EFI_ERROR (Status)) {
+    HardwareInfoBlob = AllocatePool (FwCfgSize);
+
+    if (HardwareInfoBlob == NULL) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: Failed to allocate memory for hardware resources info\n",
+        __FUNCTION__
+        ));
+      return NULL;
+    }
+
+    QemuFwCfgSelectItem (FwCfgItem);
+    QemuFwCfgReadBytes (FwCfgSize, HardwareInfoBlob);
+
+    //
+    // Create the list of hardware info devices filtering for PCI host
+    // bridges
+    //
+    Status = CreateHardwareInfoList (
+               HardwareInfoBlob,
+               FwCfgSize,
+               HardwareInfoTypeHostBridge,
+               &HwInfoList
+               );
+
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: Failed to create hardware info list to retrieve host "
+        "bridges information\n",
+        __FUNCTION__
+        ));
+
+      goto FreeBridges;
+    }
+
+    PciHostBridgeCount = GetHardwareInfoCountByType (
+                           &HwInfoList,
+                           HardwareInfoTypeHostBridge,
+                           sizeof (HOST_BRIDGE_INFO)
+                           );
+
+    if ((1 + ExtraRootBridges) != PciHostBridgeCount) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: Invalid number of host bridges in hardware info list. "
+        "Expected %ld, got %ld\n",
+        __FUNCTION__,
+        (1 + ExtraRootBridges),
+        PciHostBridgeCount
+        ));
+      goto FreeBridges;
+    }
+
+    DEBUG ((
+      DEBUG_INFO,
+      "%a: Resources for %Lu root buses found in fw-cfg\n",
+      __FUNCTION__,
+      PciHostBridgeCount
+      ));
+  }
+
   //
   // Allocate the "main" root bridge, and any extra root bridges.
   //
   Bridges = AllocatePool ((1 + (UINTN)ExtraRootBridges) * sizeof *Bridges);
   if (Bridges == NULL) {
     DEBUG ((DEBUG_ERROR, "%a: %r\n", __FUNCTION__, EFI_OUT_OF_RESOURCES));
-    return NULL;
+    goto FreeBridges;
   }
 
-  Initialized = 0;
+  if (!IsListEmpty (&HwInfoList)) {
+    //
+    // If Host Bridges' specification was obtained from fw-cfg, the list
+    // contains information to populate all root bridges in the system
+    // including resources and attributes.
+    //
+    HwLink = GetFirstHardwareInfoByType (
+               &HwInfoList,
+               HardwareInfoTypeHostBridge,
+               sizeof (HOST_BRIDGE_INFO)
+               );
+
+    while (!EndOfHardwareInfoList (&HwInfoList, HwLink)) {
+      HwInfo = HARDWARE_INFO_FROM_LINK (HwLink);
+
+      HardwareInfoPciHostBridgeGet (
+        HwInfo->Data.PciHostBridge,
+        HwInfo->Header.Size,
+        &RootBridgeNumber,
+        &LastRootBridgeNumber,
+        &HwInfoAttributes,
+        &HwInfoDmaAbove4G,
+        &HwInfoNoExtendedConfigSpace,
+        &HwInfoCombineMemPMem,
+        &HwInfoIo,
+        &HwInfoMem,
+        &HwInfoMemAbove4G,
+        &HwInfoPMem,
+        &HwInfoPMemAbove4G,
+        NULL
+        );
 
-  //
-  // The "main" root bus is always there.
-  //
-  LastRootBridgeNumber = BusMin;
+      HwInfoAllocationAttributes = 0;
+      if (HwInfoCombineMemPMem) {
+        HwInfoAllocationAttributes |= EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM;
+      }
 
-  //
-  // Scan all other root buses. If function 0 of any device on a bus returns a
-  // VendorId register value different from all-bits-one, then that bus is
-  // alive.
-  //
-  for (RootBridgeNumber = BusMin + 1;
-       RootBridgeNumber <= BusMax && Initialized < ExtraRootBridges;
-       ++RootBridgeNumber)
-  {
-    UINTN  Device;
-
-    for (Device = 0; Device <= PCI_MAX_DEVICE; ++Device) {
-      if (PciRead16 (
-            PCI_LIB_ADDRESS (
-              RootBridgeNumber,
-              Device,
-              0,
-              PCI_VENDOR_ID_OFFSET
-              )
-            ) != MAX_UINT16)
+      if ((HwInfoMemAbove4G.Limit > HwInfoMemAbove4G.Base) ||
+          (HwInfoPMemAbove4G.Limit > HwInfoPMemAbove4G.Base))
       {
-        break;
+        HwInfoAllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
       }
-    }
 
-    if (Device <= PCI_MAX_DEVICE) {
-      //
-      // Found the next root bus. We can now install the *previous* one,
-      // because now we know how big a bus number range *that* one has, for any
-      // subordinate buses that might exist behind PCI bridges hanging off it.
-      //
       Status = PciHostBridgeUtilityInitRootBridge (
-                 Attributes,
-                 Attributes,
-                 AllocationAttributes,
-                 DmaAbove4G,
-                 NoExtendedConfigSpace,
-                 (UINT8)LastRootBridgeNumber,
-                 (UINT8)(RootBridgeNumber - 1),
-                 Io,
-                 Mem,
-                 MemAbove4G,
-                 PMem,
-                 PMemAbove4G,
+                 HwInfoAttributes,
+                 HwInfoAttributes,
+                 HwInfoAllocationAttributes,
+                 HwInfoDmaAbove4G,
+                 HwInfoNoExtendedConfigSpace,
+                 RootBridgeNumber,
+                 LastRootBridgeNumber,
+                 &HwInfoIo,
+                 &HwInfoMem,
+                 &HwInfoMemAbove4G,
+                 &HwInfoPMem,
+                 &HwInfoPMemAbove4G,
                  &Bridges[Initialized]
                  );
+
       if (EFI_ERROR (Status)) {
         goto FreeBridges;
       }
 
       ++Initialized;
-      LastRootBridgeNumber = RootBridgeNumber;
+
+      HwLink = GetNextHardwareInfoByType (
+                 &HwInfoList,
+                 HwLink,
+                 HardwareInfoTypeHostBridge,
+                 sizeof (HOST_BRIDGE_INFO)
+                 );
     }
+  } else {
+    Initialized = 0;
+
+    //
+    // The "main" root bus is always there.
+    //
+    LastRootBridgeNumber = BusMin;
+
+    //
+    // Scan all other root buses. If function 0 of any device on a bus returns a
+    // VendorId register value different from all-bits-one, then that bus is
+    // alive.
+    //
+    for (RootBridgeNumber = BusMin + 1;
+         RootBridgeNumber <= BusMax && Initialized < ExtraRootBridges;
+         ++RootBridgeNumber)
+    {
+      UINTN  Device;
+
+      for (Device = 0; Device <= PCI_MAX_DEVICE; ++Device) {
+        if (PciRead16 (
+              PCI_LIB_ADDRESS (
+                RootBridgeNumber,
+                Device,
+                0,
+                PCI_VENDOR_ID_OFFSET
+                )
+              ) != MAX_UINT16)
+        {
+          break;
+        }
+      }
+
+      if (Device <= PCI_MAX_DEVICE) {
+        //
+        // Found the next root bus. We can now install the *previous* one,
+        // because now we know how big a bus number range *that* one has, for any
+        // subordinate buses that might exist behind PCI bridges hanging off it.
+        //
+        Status = PciHostBridgeUtilityInitRootBridge (
+                   Attributes,
+                   Attributes,
+                   AllocationAttributes,
+                   DmaAbove4G,
+                   NoExtendedConfigSpace,
+                   (UINT8)LastRootBridgeNumber,
+                   (UINT8)(RootBridgeNumber - 1),
+                   Io,
+                   Mem,
+                   MemAbove4G,
+                   PMem,
+                   PMemAbove4G,
+                   &Bridges[Initialized]
+                   );
+        if (EFI_ERROR (Status)) {
+          goto FreeBridges;
+        }
+
+        ++Initialized;
+        LastRootBridgeNumber = RootBridgeNumber;
+      }
+    }
+
+    //
+    // Install the last root bus (which might be the only, ie. main, root bus, if
+    // we've found no extra root buses).
+    //
+    Status = PciHostBridgeUtilityInitRootBridge (
+               Attributes,
+               Attributes,
+               AllocationAttributes,
+               DmaAbove4G,
+               NoExtendedConfigSpace,
+               (UINT8)LastRootBridgeNumber,
+               (UINT8)BusMax,
+               Io,
+               Mem,
+               MemAbove4G,
+               PMem,
+               PMemAbove4G,
+               &Bridges[Initialized]
+               );
+    if (EFI_ERROR (Status)) {
+      goto FreeBridges;
+    }
+
+    ++Initialized;
   }
 
+  *Count = Initialized;
+
+  // If resources were allocated for host bridges info, release them
   //
-  // Install the last root bus (which might be the only, ie. main, root bus, if
-  // we've found no extra root buses).
-  //
-  Status = PciHostBridgeUtilityInitRootBridge (
-             Attributes,
-             Attributes,
-             AllocationAttributes,
-             DmaAbove4G,
-             NoExtendedConfigSpace,
-             (UINT8)LastRootBridgeNumber,
-             (UINT8)BusMax,
-             Io,
-             Mem,
-             MemAbove4G,
-             PMem,
-             PMemAbove4G,
-             &Bridges[Initialized]
-             );
-  if (EFI_ERROR (Status)) {
-    goto FreeBridges;
+  if (HardwareInfoBlob) {
+    FreePool (HardwareInfoBlob);
   }
 
-  ++Initialized;
+  FreeHardwareInfoList (&HwInfoList);
 
-  *Count = Initialized;
   return Bridges;
 
 FreeBridges:
@@ -399,7 +580,15 @@ FreeBridges:
     PciHostBridgeUtilityUninitRootBridge (&Bridges[Initialized]);
   }
 
-  FreePool (Bridges);
+  if (Bridges) {
+    FreePool (Bridges);
+  }
+
+  if (HardwareInfoBlob) {
+    FreePool (HardwareInfoBlob);
+  }
+
+  FreeHardwareInfoList (&HwInfoList);
   return NULL;
 }
 
diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
index 83a734c172..5b32eaf5d2 100644
--- a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
+++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
@@ -38,6 +38,7 @@
   BaseMemoryLib
   DebugLib
   DevicePathLib
+  HardwareInfoLib
   MemoryAllocationLib
   PciLib
   QemuFwCfgLib
-- 
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




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

* [PATCH v2 6/8] MdeModulePkg, OvmfPkg: Add Pcd token for PCI pre-populated BARs
  2022-01-20 17:10 [PATCH v2 0/8] Handling of multiple PCI host bridges specified Ojeda Leon, Nicolas
                   ` (4 preceding siblings ...)
  2022-01-20 17:10 ` [PATCH v2 5/8] OvmfPkg/PciHostBridgeUtilityLib: Initialize RootBridges apertures with spec Ojeda Leon, Nicolas
@ 2022-01-20 17:10 ` Ojeda Leon, Nicolas
  2022-01-20 17:10 ` [PATCH v2 7/8] MdeModulePkg/Pci MdePkg: Create service to retrieve PCI base addresses Ojeda Leon, Nicolas
  2022-01-20 17:10 ` [PATCH v2 8/8] MdeModulePkg/PciBusDxe: Handling of pre-populated PCI BARs Ojeda Leon, Nicolas
  7 siblings, 0 replies; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-20 17:10 UTC (permalink / raw)
  To: devel; +Cc: atugup, Nicolas Ojeda Leon, Alexander Graf, Gerd Hoffmann

Create a new PCD boolean token in MdeModulePkg for global use.
We use this token to indicate if the configuration, parsed from
fw-cfg, requires pre-populated BARs to be preserved.

During creation of root bridges configurations, the flag is set
according to the "pre-populated-bars" item in fw-cfg. The Pcd token
is created as a dynamic item so it can be modified at runtime and
it is consumed in both Pei and Dxe PCI modules.

In virtualized environments, the hypervisor provides a layer of
isolation between the VMs and the hardware. This isolation may
include address translations and shadowing of configurations that
prevent guests directly modifying hardware registers. The hypervisor
then takes care of emulating the hardware accesses requested by
the guest. In some cases, the host may want the guest to use a
specific value for some or all PCI BARs so that transactions in
GPA level (particularly DMA) can make use of the controlled set
of addresses. The host then indicates the guest to preserve
those BARs that are pre-populated, that is, BARs that already
report a value even before guest's firmware BAR placement.

The token provides a globally accessible configuration flag to
determine, during PCI BAR allocation, if pre-populated BARs must
be respected. The pre-allocated PCI BARs are used in platforms in
which MMIO resources are configured with the same host physical
addresses in the guest so that DMA transactions can happen between
PCI devices without packets going through the IOMMU. Performance is
improved due to PCI packets travelling shorter distances and
avoiding links reaching the Root Complex, which can get busy during
I/O intensive periods.

Cc: Alexander Graf <graf@amazon.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Nicolas Ojeda Leon <ncoleon@amazon.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf    |  1 +
 MdeModulePkg/MdeModulePkg.dec                   |  6 ++++++
 .../PciHostBridgeUtilityLib.c                   | 17 +++++++++++++++++
 .../PciHostBridgeUtilityLib.inf                 |  4 ++++
 OvmfPkg/OvmfPkgX64.dsc                          |  1 +
 OvmfPkg/PlatformPei/PlatformPei.inf             |  1 +
 6 files changed, 30 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index e317169d9c..046876bb3b 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -107,6 +107,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdMrIovSupport                ## CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration    ## SOMETIMES_CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdPcieResizableBarSupport     ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciPreservePopulatedMappings## CONSUMES
 
 [UserExtensions.TianoCore."ExtraFiles"]
   PciBusDxeExtra.uni
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 463e889e9a..078877ba7f 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1902,6 +1902,12 @@
   # @Prompt Disable full PCI enumeration.
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|FALSE|BOOLEAN|0x10000048
 
+  ## The flag to control preservation of pre-populated PCI BARs
+  #   TRUE  - Respect pre-populated PCI BARs
+  #   FALSE - No pre-populated BARs, place all BARs
+  # @Prompt Enable preservsation of pre-populated PCI BARs
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciPreservePopulatedMappings|FALSE|BOOLEAN|0x10000050
+
   ## Disk I/O - Number of Data Buffer block.
   # Define the size in block of the pre-allocated buffer. It provide better
   # performance for large Disk I/O requests.
diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
index b0e3b5e3cf..5c86f67f76 100644
--- a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
+++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.c
@@ -18,6 +18,7 @@
 #include <Library/DevicePathLib.h>
 #include <Library/HardwareInfoLib.h>
 #include <Library/MemoryAllocationLib.h>
+#include <Library/PcdLib.h>
 #include <Library/PciHostBridgeUtilityLib.h>
 #include <Library/PciLib.h>
 #include <Library/QemuFwCfgLib.h>
@@ -260,6 +261,7 @@ PciHostBridgeUtilityGetRootBridges (
   PCI_ROOT_BRIDGE_APERTURE  HwInfoMemAbove4G;
   PCI_ROOT_BRIDGE_APERTURE  HwInfoPMem;
   PCI_ROOT_BRIDGE_APERTURE  HwInfoPMemAbove4G;
+  UINT64                    PrePopulatedBars;
 
   *Count = 0;
 
@@ -312,6 +314,21 @@ PciHostBridgeUtilityGetRootBridges (
       ));
   }
 
+  //
+  // Find file for pre-populated bars and set Pcd token if enabled
+  //
+  Status           = QemuFwCfgFindFile ("etc/pre-populated-bars", &FwCfgItem, &FwCfgSize);
+  PrePopulatedBars = 0;
+  if (!EFI_ERROR (Status) && (FwCfgSize == sizeof (PrePopulatedBars))) {
+    QemuFwCfgSelectItem (FwCfgItem);
+    QemuFwCfgReadBytes (FwCfgSize, &PrePopulatedBars);
+
+    if (PrePopulatedBars) {
+      PcdSetBoolS (PcdPciPreservePopulatedMappings, TRUE);
+      DEBUG ((DEBUG_INFO, "%a: Pre-populated BARs present\n", __FUNCTION__));
+    }
+  }
+
   //
   // Initialize the Hardware Info list head to start with an empty but valid
   // list head.
diff --git a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
index 5b32eaf5d2..e08ff64244 100644
--- a/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
+++ b/OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
@@ -40,5 +40,9 @@
   DevicePathLib
   HardwareInfoLib
   MemoryAllocationLib
+  PcdLib
   PciLib
   QemuFwCfgLib
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciPreservePopulatedMappings
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 062e16482d..628e70f906 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -594,6 +594,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0
 !endif
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciPreservePopulatedMappings|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable|FALSE
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
index 413b92b855..37d4e1092f 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -96,6 +96,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciPreservePopulatedMappings
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
   gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack
-- 
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




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

* [PATCH v2 7/8] MdeModulePkg/Pci MdePkg: Create service to retrieve PCI base addresses
  2022-01-20 17:10 [PATCH v2 0/8] Handling of multiple PCI host bridges specified Ojeda Leon, Nicolas
                   ` (5 preceding siblings ...)
  2022-01-20 17:10 ` [PATCH v2 6/8] MdeModulePkg, OvmfPkg: Add Pcd token for PCI pre-populated BARs Ojeda Leon, Nicolas
@ 2022-01-20 17:10 ` Ojeda Leon, Nicolas
  2022-01-20 17:10 ` [PATCH v2 8/8] MdeModulePkg/PciBusDxe: Handling of pre-populated PCI BARs Ojeda Leon, Nicolas
  7 siblings, 0 replies; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-20 17:10 UTC (permalink / raw)
  To: devel; +Cc: atugup, Nicolas Ojeda Leon, Alexander Graf, Gerd Hoffmann

Extend the PCI host bridge resource allocation protocol to include one
more service that retrieves the base addresses of all resources of a
given root bridge.
The service is defined to provide, on runtime, the possibility to fetch
the base addresses of a root bridge, replicating the address alignment
used when placing the host bridge's resources in the Gcd memory map.

The intention of this service, initially, is to allow the PCI allocation
process to get the base addresses before allocating the individual BARs
grouped under a root bridge. This enables the placing logic to be
enhanced to account and calculate offsets for pre-populated BARs (PCI
devices' BARs that are already configured and need to be respected).

Cc: Alexander Graf <graf@amazon.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Nicolas Ojeda Leon <ncoleon@amazon.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c       | 10 +++
 .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c  | 81 +++++++++++++++++++
 .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.h  | 29 +++++++
 .../PciHostBridgeResourceAllocation.h         | 33 ++++++++
 4 files changed, 153 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
index 63d149b3b8..2ffbc08256 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
@@ -582,6 +582,16 @@ PciHostBridgeResourceAllocator (
                        PciResUsageTypical
                        );
 
+      Status = PciResAlloc->GetResourcesBases (
+                              PciResAlloc,
+                              RootBridgeDev->Handle,
+                              &IoBase,
+                              &Mem32Base,
+                              &PMem32Base,
+                              &Mem64Base,
+                              &PMem64Base
+                              );
+
       //
       // Get the max ROM size that the root bridge can process
       // Insert to resource map so that there will be dedicate MEM32 resource range for Option ROM.
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index b20bcd310a..ddd31f78d6 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -591,6 +591,7 @@ InitializePciHostBridge (
     HostBridge->ResAlloc.SubmitResources      = SubmitResources;
     HostBridge->ResAlloc.GetProposedResources = GetProposedResources;
     HostBridge->ResAlloc.PreprocessController = PreprocessController;
+    HostBridge->ResAlloc.GetResourcesBases    = GetResourcesBases;
 
     Status = gBS->InstallMultipleProtocolInterfaces (
                     &HostBridge->Handle,
@@ -1734,3 +1735,83 @@ PreprocessController (
 
   return EFI_INVALID_PARAMETER;
 }
+
+/**
+
+  Retrieve the aligned base addresses for all resources of a root bridge.
+
+  @param This              The EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL instance.
+  @param RootBridgeHandle  RootBridgeHandle returned by GetNextRootBridge to locate the
+                           root bridge of interest among the list of root bridges.
+  @param IoBase            Returns the PIO aperture base address.
+  @param Mem32Base         Returns the 32-bit aperture base address.
+  @param PMem32Base        Returns the 32-bit prefetchable aperture base address.
+  @param Mem64Base         Returns the 64-bit aperture base address.
+  @param PMem64Base        Returns the 64-bit prefetchable aperture base address.
+
+  @retval EFI_SUCCESS      Succeed.
+  @retval EFI_NOT_FOUND    Root bridge was not found.
+
+**/
+EFI_STATUS
+EFIAPI
+GetResourcesBases (
+  IN EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL  *This,
+  IN EFI_HANDLE                                        RootBridgeHandle,
+  OUT UINT64                                           *IoBase,
+  OUT UINT64                                           *Mem32Base,
+  OUT UINT64                                           *PMem32Base,
+  OUT UINT64                                           *Mem64Base,
+  OUT UINT64                                           *PMem64Base
+  )
+{
+  PCI_HOST_BRIDGE_INSTANCE  *HostBridge;
+  PCI_ROOT_BRIDGE_INSTANCE  *RootBridge;
+  LIST_ENTRY                *Link;
+  UINT64                    Alignment;
+  UINTN                     BitsOfAlignment;
+
+  HostBridge = PCI_HOST_BRIDGE_FROM_THIS (This);
+
+  for (Link = GetFirstNode (&HostBridge->RootBridges)
+       ; !IsNull (&HostBridge->RootBridges, Link)
+       ; Link = GetNextNode (&HostBridge->RootBridges, Link)
+       )
+  {
+    RootBridge = ROOT_BRIDGE_FROM_LINK (Link);
+
+    if (RootBridgeHandle == RootBridge->Handle) {
+      //
+      // Have to make sure Alignment is handled since we are doing direct address allocation
+      //
+      Alignment       = RootBridge->ResAllocNode[TypeIo].Alignment;
+      BitsOfAlignment = MIN (15, LowBitSet64 (Alignment + 1));
+      *IoBase         = ALIGN_VALUE (RootBridge->Io.Base, Alignment + 1);
+      *IoBase         = ALIGN_VALUE (*IoBase, LShiftU64 (1, BitsOfAlignment));
+
+      Alignment       = RootBridge->ResAllocNode[TypeMem32].Alignment;
+      BitsOfAlignment = MIN (31, LowBitSet64 (Alignment + 1));
+      *Mem32Base      = ALIGN_VALUE (RootBridge->Mem.Base, Alignment + 1);
+      *Mem32Base      = ALIGN_VALUE (*Mem32Base, LShiftU64 (1, BitsOfAlignment));
+
+      Alignment       = RootBridge->ResAllocNode[TypePMem32].Alignment;
+      BitsOfAlignment = MIN (31, LowBitSet64 (Alignment + 1));
+      *PMem32Base     = ALIGN_VALUE (RootBridge->PMem.Base, Alignment + 1);
+      *PMem32Base     = ALIGN_VALUE (*PMem32Base, LShiftU64 (1, BitsOfAlignment));
+
+      Alignment       = RootBridge->ResAllocNode[TypeMem64].Alignment;
+      BitsOfAlignment = MIN (63, LowBitSet64 (Alignment + 1));
+      *Mem64Base      = ALIGN_VALUE (RootBridge->MemAbove4G.Base, Alignment + 1);
+      *Mem64Base      = ALIGN_VALUE (*Mem64Base, LShiftU64 (1, BitsOfAlignment));
+
+      Alignment       = RootBridge->ResAllocNode[TypePMem64].Alignment;
+      BitsOfAlignment = MIN (63, LowBitSet64 (Alignment + 1));
+      *PMem64Base     = ALIGN_VALUE (RootBridge->PMemAbove4G.Base, Alignment + 1);
+      *PMem64Base     = ALIGN_VALUE (*PMem64Base, LShiftU64 (1, BitsOfAlignment));
+
+      return EFI_SUCCESS;
+    }
+  }
+
+  return EFI_NOT_FOUND;
+}
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
index e7a30fd909..07ba496602 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.h
@@ -240,6 +240,35 @@ PreprocessController (
   IN EFI_PCI_CONTROLLER_RESOURCE_ALLOCATION_PHASE      Phase
   );
 
+/**
+
+  Retrieve the aligned base addresses for all resources of a root bridge.
+
+  @param This              The EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL instance.
+  @param RootBridgeHandle  RootBridgeHandle returned by GetNextRootBridge to locate the
+                           root bridge of interest among the list of root bridges.
+  @param IoBase            Returns the PIO aperture base address.
+  @param Mem32Base         Returns the 32-bit aperture base address.
+  @param PMem32Base        Returns the 32-bit prefetchable aperture base address.
+  @param Mem64Base         Returns the 64-bit aperture base address.
+  @param PMem64Base        Returns the 64-bit prefetchable aperture base address.
+
+  @retval EFI_SUCCESS      Succeed.
+  @retval EFI_NOT_FOUND    Root bridge was not found.
+
+**/
+EFI_STATUS
+EFIAPI
+GetResourcesBases (
+  IN EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL  *This,
+  IN EFI_HANDLE                                        RootBridgeHandle,
+  OUT UINT64                                           *IoBase,
+  OUT UINT64                                           *Mem32Base,
+  OUT UINT64                                           *PMem32Base,
+  OUT UINT64                                           *Mem64Base,
+  OUT UINT64                                           *PMem64Base
+  );
+
 /**
   This routine constructs the resource descriptors for all root bridges and call PciHostBridgeResourceConflict().
 
diff --git a/MdePkg/Include/Protocol/PciHostBridgeResourceAllocation.h b/MdePkg/Include/Protocol/PciHostBridgeResourceAllocation.h
index 5ef7c000d6..ab91acb174 100644
--- a/MdePkg/Include/Protocol/PciHostBridgeResourceAllocation.h
+++ b/MdePkg/Include/Protocol/PciHostBridgeResourceAllocation.h
@@ -367,6 +367,33 @@ EFI_STATUS
   IN EFI_PCI_CONTROLLER_RESOURCE_ALLOCATION_PHASE      Phase
   );
 
+/**
+  Retrieves the base addresses of ost bridge resources.
+
+  @param This              The pointer to the EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL instance.
+  @param RootBridgeHandle  The PCI root bridge handle.
+  @param IoBase            The pointer to PIO aperture base address.
+  @param Mem32Base         The pointer to 32-bit aperture base address.
+  @param PMem32Base        The pointer to 32-bit prefetchable aperture base address.
+  @param Mem64Base         The pointer to 64-bit aperture base address.
+  @param PMem64Base        The pointer to 64-bit prefetchable aperture base address.
+
+  @retval EFI_SUCCESS      Succeed.
+  @retval EFI_NOT_FOUND    Root bridge was not found.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL_GET_RESOURCES_BASES)(
+  IN EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL *This,
+  IN EFI_HANDLE                                       RootBridgeHandle,
+  OUT UINT64                                          *IoBase,
+  OUT UINT64                                          *Mem32Base,
+  OUT UINT64                                          *PMem32Base,
+  OUT UINT64                                          *Mem64Base,
+  OUT UINT64                                          *PMem64Base
+  );
+
 ///
 /// Provides the basic interfaces to abstract a PCI host bridge resource allocation.
 ///
@@ -415,6 +442,12 @@ struct _EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL {
   /// before enumeration.
   ///
   EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL_PREPROCESS_CONTROLLER     PreprocessController;
+
+  ///
+  /// Returns the aligned base addresses of the different resource windows
+  /// of the host bridge. Intended for use before resources are submitted.
+  ///
+  EFI_PCI_HOST_BRIDGE_RESOURCE_ALLOCATION_PROTOCOL_GET_RESOURCES_BASES       GetResourcesBases;
 };
 
 extern EFI_GUID  gEfiPciHostBridgeResourceAllocationProtocolGuid;
-- 
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




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

* [PATCH v2 8/8] MdeModulePkg/PciBusDxe: Handling of pre-populated PCI BARs
  2022-01-20 17:10 [PATCH v2 0/8] Handling of multiple PCI host bridges specified Ojeda Leon, Nicolas
                   ` (6 preceding siblings ...)
  2022-01-20 17:10 ` [PATCH v2 7/8] MdeModulePkg/Pci MdePkg: Create service to retrieve PCI base addresses Ojeda Leon, Nicolas
@ 2022-01-20 17:10 ` Ojeda Leon, Nicolas
  7 siblings, 0 replies; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-20 17:10 UTC (permalink / raw)
  To: devel; +Cc: atugup, Nicolas Ojeda Leon, Alexander Graf, Gerd Hoffmann

Extend the PCI BAR placement logic in order to consider pre-populated
resources first, if indicated by the Pcd token for such purpose.

The PCI_BAR type is augmented by one field for mapping the absolute
address of prepopulated BARs into a root bridge relative offset.
As part of the CreateResourceMap stage of the PCI resource allocation
process, all the resources belonging to a root bridge are analyzed and
if a BaseAddress (absolute address read from the PCI device during
enumeration) is present, it is translated into a root-bridge relative
offset and the space occupied by it is marked as used in the root
bridge's aperture. This process is performed before the regular
placement logic is executed.

The remaining unassigned BARs are then placed according to default
logic starting after the last pre-populated resource.

The motivation behind respecting pre-populated BARs is that the
hypervisor can decide on placement of some resources and the guest must
use the same values to benefit from performance optimizations or
specific requirements.

Cc: Alexander Graf <graf@amazon.de>
Cc: Gerd Hoffmann <kraxel@redhat.com>

Signed-off-by: Nicolas Ojeda Leon <ncoleon@amazon.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h       |   1 +
 .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c  |   5 +-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c       |  11 +
 .../Bus/Pci/PciBusDxe/PciResourceSupport.c    | 277 +++++++++++++++++-
 .../Bus/Pci/PciBusDxe/PciResourceSupport.h    |  20 ++
 5 files changed, 310 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 4b58c3ea9b..9218566bf6 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -94,6 +94,7 @@ typedef enum {
 //
 struct _PCI_BAR {
   UINT64          BaseAddress;
+  UINT64          BaseAddressOffset;
   UINT64          Length;
   UINT64          Alignment;
   PCI_BAR_TYPE    BarType;
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
index 9251388bc2..91caf162e0 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c
@@ -1776,8 +1776,9 @@ PciParseBar (
     return Offset + 4;
   }
 
-  PciIoDevice->PciBar[BarIndex].BarTypeFixed = FALSE;
-  PciIoDevice->PciBar[BarIndex].Offset       = (UINT8)Offset;
+  PciIoDevice->PciBar[BarIndex].BarTypeFixed      = FALSE;
+  PciIoDevice->PciBar[BarIndex].Offset            = (UINT8)Offset;
+  PciIoDevice->PciBar[BarIndex].BaseAddressOffset = 0;
   if ((Value & 0x01) != 0) {
     //
     // Device I/Os
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
index 2ffbc08256..452ca10540 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
@@ -605,6 +605,17 @@ PciHostBridgeResourceAllocator (
         GetResourceFromDevice (RootBridgeDev, IoBridge, Mem32Bridge, PMem32Bridge, Mem64Bridge, PMem64Bridge);
       }
 
+      //
+      // Handle pre-populated resources
+      //
+      PopulateResourceAperture (
+        Mem32Base,
+        PMem32Base,
+        Mem64Base,
+        PMem64Base,
+        RootBridgeDev
+        );
+
       //
       // Create resourcemap by going through all the devices subject to this root bridge
       //
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
index 8ffd05f327..e2a550bf8e 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -324,6 +324,266 @@ CalculateApertureIo16 (
   Bridge->Length = MAX (Bridge->Length, PaddingAperture);
 }
 
+/**
+  Analyze a single BAR and if it is pre-populated (its baseAddress is already
+  set), calculate the offset inside the root bridge.
+
+  @param Bar    PCI BAR to be placed if pre-populated
+  @param Base   Base address of the root bridge
+
+**/
+STATIC
+VOID
+PrePopulateBAR (
+  IN OUT PCI_BAR  *Bar,
+  IN     UINT64   Base
+  )
+{
+  if (Base == gAllOne) {
+    return;
+  }
+
+  //
+  // Pre-populated IO space BARs are not supported
+  //
+  if ((Bar->BarType == PciBarTypeIo16) || (Bar->BarType == PciBarTypeIo32)) {
+    return;
+  }
+
+  if (Bar->BaseAddress < Base) {
+    return;
+  }
+
+  if (Bar->BaseAddress != 0) {
+    Bar->BaseAddressOffset = Bar->BaseAddress - Base;
+  }
+}
+
+/**
+  Calculate and set the address offset inside the root bridge of device
+  BARs that are pre-populated.
+
+  @param Dev      PCI device whose BARs are to be analyzed
+  @param Base     Base address of the root bridge under which the device
+                  is located.
+
+ */
+STATIC
+VOID
+PopulateDeviceBars (
+  IN UINT64         Mem32Base,
+  IN UINT64         PMem32Base,
+  IN UINT64         Mem64Base,
+  IN UINT64         PMem64Base,
+  IN PCI_IO_DEVICE  *Dev
+  )
+{
+  UINTN  Index;
+
+  for (Index = 0; Index < PCI_MAX_BAR; Index++) {
+    switch (Dev->PciBar[Index].BarType) {
+      case PciBarTypeMem32:
+        PrePopulateBAR (&Dev->PciBar[Index], Mem32Base);
+        break;
+
+      case PciBarTypePMem32:
+        PrePopulateBAR (&Dev->PciBar[Index], PMem32Base);
+        break;
+
+      case PciBarTypeMem64:
+        PrePopulateBAR (&Dev->PciBar[Index], Mem64Base);
+        break;
+
+      case PciBarTypePMem64:
+        PrePopulateBAR (&Dev->PciBar[Index], PMem64Base);
+        break;
+
+      default:
+        break;
+    }
+  }
+}
+
+/**
+  Considering the Bridge attributes specifying the supported resources,
+  merge the base addresses to fake the non-suported types belonging to
+  the alternative type.
+
+  @param Mem32Base      32-bit base address
+  @param PMem32Base     32-bit prefetchable base address
+  @param Mem64Base      64-bit base address
+  @param PMem64Base     64-bit prefetchable base address
+  @param Bridge         PCI resource node for given bridge device.
+
+ */
+STATIC
+VOID
+MergeBridgeResourceBases (
+  IN OUT UINT64         *Mem32Base,
+  IN OUT UINT64         *PMem32Base,
+  IN OUT UINT64         *Mem64Base,
+  IN OUT UINT64         *PMem64Base,
+  IN     PCI_IO_DEVICE  *Bridge
+  )
+{
+  //
+  // if root bridge supports combined Pmem Mem decoding
+  // merge these two type of resource
+  //
+  if (BridgeSupportResourceDecode (Bridge, EFI_BRIDGE_PMEM_MEM_COMBINE_SUPPORTED)) {
+    *PMem32Base = *Mem32Base;
+    *PMem64Base = *Mem64Base;
+  }
+
+  //
+  // If bridge doesn't support Pmem32
+  // degrade it to mem32
+  //
+  if (!BridgeSupportResourceDecode (Bridge, EFI_BRIDGE_PMEM32_DECODE_SUPPORTED)) {
+    *PMem32Base = *Mem32Base;
+  }
+
+  //
+  // If firmware is in 32-bit mode,
+  // then degrade PMEM64/MEM64 requests
+  //
+  if (sizeof (UINTN) <= 4) {
+    *Mem64Base  = *Mem32Base;
+    *PMem64Base = *PMem32Base;
+  } else {
+    //
+    // if the bridge does not support MEM64, degrade MEM64 to MEM32
+    //
+    if (!BridgeSupportResourceDecode (Bridge, EFI_BRIDGE_MEM64_DECODE_SUPPORTED)) {
+      *Mem64Base = *Mem32Base;
+    }
+
+    //
+    // if the bridge does not support PMEM64, degrade PMEM64 to PMEM32
+    //
+    if (!BridgeSupportResourceDecode (Bridge, EFI_BRIDGE_PMEM64_DECODE_SUPPORTED)) {
+      *PMem64Base = *PMem32Base;
+    }
+  }
+}
+
+/**
+  Populate a root bridge's resource aperture according to
+  initial conditions ruled by individual pre-populated
+  resources
+
+   @param Bridge    PCI resource node for given bridge device.
+   @param Base      Resource aperture base address
+
+**/
+VOID
+PopulateResourceAperture (
+  IN     UINT64         Mem32Base,
+  IN     UINT64         PMem32Base,
+  IN     UINT64         Mem64Base,
+  IN     UINT64         PMem64Base,
+  IN OUT PCI_IO_DEVICE  *Bridge
+  )
+{
+  PCI_IO_DEVICE  *Temp;
+  LIST_ENTRY     *CurrentLink;
+
+  //
+  // Verify if pre-populated BARs need to be respected, otherwise
+  // there is no need to pre-populate any resource
+  //
+  if (!PcdGetBool (PcdPciPreservePopulatedMappings)) {
+    return;
+  }
+
+  CurrentLink = Bridge->ChildList.ForwardLink;
+
+  //
+  // Merge base addresses of the different types depending on resource
+  // decoding supported by the bridge
+  //
+  MergeBridgeResourceBases (
+    &Mem32Base,
+    &PMem32Base,
+    &Mem64Base,
+    &PMem64Base,
+    Bridge
+    );
+
+  while (CurrentLink != NULL && CurrentLink != &Bridge->ChildList) {
+    Temp = PCI_IO_DEVICE_FROM_LINK (CurrentLink);
+
+    PopulateDeviceBars (
+      Mem32Base,
+      PMem32Base,
+      Mem64Base,
+      PMem64Base,
+      Temp
+      );
+
+    //
+    // Recursive call to analyze the hierarchical tree under a bridge
+    //
+    if (IS_PCI_BRIDGE (&Temp->Pci)) {
+      PopulateResourceAperture (
+        Mem32Base,
+        PMem32Base,
+        Mem64Base,
+        PMem64Base,
+        Temp
+        );
+    }
+
+    CurrentLink = CurrentLink->ForwardLink;
+  }
+}
+
+/**
+  Calculate the current used resource of the bridge provided that some
+  of the resources under it might been pre-populated.
+
+  @param Bridge     PCI resource node for given bridge device.
+
+**/
+STATIC
+UINT64
+GetBridgePopulatedAperture (
+  IN PCI_RESOURCE_NODE  *Bridge
+  )
+{
+  UINT64             Aperture;
+  UINT64             EndAddress;
+  LIST_ENTRY         *CurrentLink;
+  PCI_RESOURCE_NODE  *Node;
+  PCI_BAR            *Bar;
+
+  Aperture = 0;
+
+  //
+  // Analyze all resource nodes looking for the prepopulated with the
+  // highest address used.
+  //
+  for ( CurrentLink = GetFirstNode (&Bridge->ChildList)
+        ; !IsNull (&Bridge->ChildList, CurrentLink)
+        ; CurrentLink = GetNextNode (&Bridge->ChildList, CurrentLink)
+        )
+  {
+    Node = RESOURCE_NODE_FROM_LINK (CurrentLink);
+
+    Bar = &Node->PciDev->PciBar[Node->Bar];
+
+    if (Bar->BaseAddress != 0) {
+      EndAddress = Bar->BaseAddressOffset + Bar->Length;
+
+      if (EndAddress > Aperture) {
+        Aperture = EndAddress;
+      }
+    }
+  }
+
+  return Aperture;
+}
+
 /**
   This function is used to calculate the resource aperture
   for a given bridge device.
@@ -337,6 +597,7 @@ CalculateResourceAperture (
   )
 {
   UINT64             Aperture[2];
+  UINT64             InitialAperture;
   LIST_ENTRY         *CurrentLink;
   PCI_RESOURCE_NODE  *Node;
 
@@ -349,8 +610,14 @@ CalculateResourceAperture (
     return;
   }
 
-  Aperture[PciResUsageTypical] = 0;
-  Aperture[PciResUsagePadding] = 0;
+  //
+  // Initialize apertures at bridge's current resource usage
+  // which might be occupied by pre-populated resources.
+  //
+  InitialAperture              = GetBridgePopulatedAperture (Bridge);
+  Aperture[PciResUsageTypical] = InitialAperture;
+  Aperture[PciResUsagePadding] = InitialAperture;
+
   //
   // Assume the bridge is aligned
   //
@@ -361,6 +628,12 @@ CalculateResourceAperture (
   {
     Node = RESOURCE_NODE_FROM_LINK (CurrentLink);
 
+    PCI_BAR  *Bar = &Node->PciDev->PciBar[Node->Bar];
+    if (PcdGetBool (PcdPciPreservePopulatedMappings) && Bar->BaseAddress) {
+      Node->Offset = Bar->BaseAddressOffset;
+      continue;
+    }
+
     //
     // It's possible for a bridge to contain multiple padding resource
     // nodes due to DegradeResource().
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.h b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.h
index 1527d4eafa..c5ab5f3fd9 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.h
@@ -111,6 +111,26 @@ CalculateApertureIo16 (
   IN PCI_RESOURCE_NODE  *Bridge
   );
 
+/**
+  Populate a root bridge's resource aperture according to
+  initial conditions ruled by individual pre-populated
+  resources
+
+   @note Pre-populated PIO BARs are not supported.
+
+   @param Bridge    PCI resource node for given bridge device.
+   @param Base      Resource aperture base address
+
+**/
+VOID
+PopulateResourceAperture (
+  IN     UINT64         Mem32Base,
+  IN     UINT64         PMem32Base,
+  IN     UINT64         Mem64Base,
+  IN     UINT64         PMem64Base,
+  IN OUT PCI_IO_DEVICE  *Bridge
+  );
+
 /**
   This function is used to calculate the resource aperture
   for a given bridge device.
-- 
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




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

* Re: [PATCH v2 4/8] Ovmf/PlatformPei: Extend 64-bit MMIO range to fit resources
  2022-01-20 17:10 ` [PATCH v2 4/8] Ovmf/PlatformPei: Extend 64-bit MMIO range to fit resources Ojeda Leon, Nicolas
@ 2022-01-21  9:57   ` Gerd Hoffmann
  0 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2022-01-21  9:57 UTC (permalink / raw)
  To: Nicolas Ojeda Leon; +Cc: devel, atugup, Alexander Graf

  hi,

> As an example, if the PcdPciMmio64Size is initialized to 0x800000000
> and the runtime calculated base address produces the value 0x800000000,
> the last 64-bit MMIO address a PCI host bridge may use would be
> 0xFFFFFFFFF. If one of the host-specified bridges defines a high
> memory window 0x1000000000 - 0x17FFFFFFFF (inclusive) it would be
> outside of the valid range defined by the Base address + PcdPciMmio64Size.
> Therefore, parsing the HardwwareInfo resource allows to notice the higher
> requirement and increase the size to 0x1000000000 so that the MMIO
> address space solicited by the host can be met and the range effectively
> extended to 0x17FFFFFFFF.

I think you can completely ignore PcdPciMmio64Size + PcdPciMmio64Base,
you only must make sure mPhysMemAddressWidth is big enough ...

Background story:

On qemu there is no reliable way to figure what the size of the physical
address space is.  The reason for that a rather long story involving
historical reasons, backward compatibility, live migration compatibility
and probably more stuff which I forgot meanwhile.  Those details don't
matter here, the key takeaway is that ovmf tries to minimize it's
physical address space needs because it just doesn't know how much it
can actually use.

PcdPciMmio64Size default is 32G, so a typical virtual machine fits into
64G address space (aka mPhysMemAddressWidth == 36), which is guaranteed
to work on x64.  This is also part of the plan ;)

With the qemu q35 machine types the firmware initializes the host
bridge, i.e. ovmf programs the host bridge to place the windows and the
mmconfig xbar according to the PcdPciMmio64Base + PcdPciMmio64Size
values ovmf figured during memory detection.

The whole logic is pretty much moot though if the location and size of
the mmio windows is dictated by the host and the guest can't change them
anyway ...

take care,
  Gerd


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

* Re: [PATCH v2 3/8] Ovmf/HardwareInfoLib: Add core to parse heterogenous data
  2022-01-20 17:10 ` [PATCH v2 3/8] Ovmf/HardwareInfoLib: Add core to parse heterogenous data Ojeda Leon, Nicolas
@ 2022-01-21 10:09   ` Gerd Hoffmann
  2022-01-21 10:48     ` [edk2-devel] " Ojeda Leon, Nicolas
  0 siblings, 1 reply; 15+ messages in thread
From: Gerd Hoffmann @ 2022-01-21 10:09 UTC (permalink / raw)
  To: Nicolas Ojeda Leon; +Cc: devel, atugup, Alexander Graf

> +++ b/OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.inf

> +[Sources]
> +  HardwareInfoLib.c
> +  HardwareInfoPciHostBridgeLib.c
> +  QemuFwCfgHardwareInfoLib.c

> +  HardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/HardwareInfoLib.inf
>    QemuFwCfgHardwareInfoLib|OvmfPkg/Library/HardwareInfoLib/QemuFwCfgHardwareInfoLib.inf

Hmm, the QemuFwCfgHardwareInfoLib.inf file created by patch #2 has
HardwareInfoPciHostBridgeLib.c + QemuFwCfgHardwareInfoLib.c too.

I'm wondering why you add two inf files in the first place?

take care,
  Gerd


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

* Re: [PATCH v2 1/8] OvmfPkg/Library: Create base HardwareInfoLib for PCI Host Bridges
  2022-01-20 17:10 ` [PATCH v2 1/8] OvmfPkg/Library: Create base HardwareInfoLib for PCI Host Bridges Ojeda Leon, Nicolas
@ 2022-01-21 10:09   ` Gerd Hoffmann
  0 siblings, 0 replies; 15+ messages in thread
From: Gerd Hoffmann @ 2022-01-21 10:09 UTC (permalink / raw)
  To: Nicolas Ojeda Leon; +Cc: devel, atugup, Alexander Graf

On Thu, Jan 20, 2022 at 06:10:14PM +0100, Nicolas Ojeda Leon wrote:
> Create the Hardware Info library base together with the specifics to
> describe PCI Host Bridges.
> 
> The Hardware Info library is intended to be used for disclosing
> non-discoverable hardware information from the host to the guest in
> Ovmf platforms. Core functionality will provide the possibility to
> parse information from a generic BLOB into runtime structures. The
> library is conceived in a generic way so that further hardware
> elements can also be described using it. For such purpose the length
> of the BLOB is not restricted but instead regarded as a sequence of
> header-info elements that allow the parsing during runtime. The first
> type of hardware defined will be PCI host bridges, providing the
> possibility to define multiple and specify the resources each of them
> can use. This enables the guest firmware to configure PCI resources
> properly. Having the size of each individual element favors the reuse
> of a single interface to convey descriptions of an arbitrary number
> of heterogenous hardware elements. Furthermore, flexible access
> mechanisms coupled with the size will grant the possibility of
> interpreting them in a single run.
> 
> Define the base types of the generic Hardware Info library to parse
> heterogeneous data. Also provide the specific changes to support
> PCI host bridges as the first hardware type supported by the
> library.
> Additionally, define the HOST_BRIDGE_INFO structure to describe PCI
> host bridges along with the functionality to parse such information
> into proper structures used by the PCI driver in a centralized manner
> and taking care of versioning.
> 
> As an example and motivation, the library will be used to define
> multiple PCI host bridges for complex platforms that require it.
> The first means of transportation that will be used is going to be
> fw-cfg, over which a stream of bytes will be transferred and later
> parsed by the hardware info library. Accordingly, the PCI driver
> will make use of these host bridges definitions to populate the
> list of Root Bridges and proceed with the configuration and discovery
> of underlying hardware components.
> 
> As mentioned before, the binary data to be parsed by the Hardware
> Info library should be organized as a sequence of Header-element
> pairs in which the header describes the type and size of the associated
> element that comes right after it. As an illustration, to provide
> inforation of 3 host bridges the data, conceptually, would look
> like this:
> 
> Header PCI Host Bridge (type and size) # 1
> PCI Host Bridge info # 1
> Header PCI Host Bridge (type and size) # 2
> PCI Host Bridge info # 2
> Header PCI Host Bridge (type and size) # 3
> PCI Host Bridge info # 3
> 
> Cc: Alexander Graf <graf@amazon.de>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> 
> Signed-off-by: Nicolas Ojeda Leon <ncoleon@amazon.com>

Acked-by: Gerd Hoffmann <kraxel@redhat.com>


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

* Re: [edk2-devel] [PATCH v2 3/8] Ovmf/HardwareInfoLib: Add core to parse heterogenous data
  2022-01-21 10:09   ` Gerd Hoffmann
@ 2022-01-21 10:48     ` Ojeda Leon, Nicolas
  2022-01-21 11:03       ` Gerd Hoffmann
  0 siblings, 1 reply; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-21 10:48 UTC (permalink / raw)
  To: devel@edk2.groups.io, kraxel@redhat.com
  Cc: Gupta, Atul, Graf (AWS), Alexander

Hi Gerd,

> Hmm, the QemuFwCfgHardwareInfoLib.inf file created by patch #2 has HardwareInfoPciHostBridgeLib.c + QemuFwCfgHardwareInfoLib.c too.

> I'm wondering why you add two inf files in the first place?

My idea was to provide 2 variations of the library:

- One offering no dynamic memory and list features and only read functions to, statically, parse data out of a fw-cfg file -> QemuFwCfgHardwareInfo
- The other one being the general one having everything. Both the list and dynamic features as well as the fw-cfg wrappers -> HardwareInfoLib

Now that you mention it one option forward could be to add QemuFwCfgHardwareInfo as a dependency in HardwareInfoLib.
Or we could have a single inf file (the general one) and not use the dynamic functions where not possible, if you think this would be a cleaner solution.

Thank you for your comment.

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




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

* Re: [edk2-devel] [PATCH v2 3/8] Ovmf/HardwareInfoLib: Add core to parse heterogenous data
  2022-01-21 10:48     ` [edk2-devel] " Ojeda Leon, Nicolas
@ 2022-01-21 11:03       ` Gerd Hoffmann
  2022-01-21 11:10         ` Ojeda Leon, Nicolas
  0 siblings, 1 reply; 15+ messages in thread
From: Gerd Hoffmann @ 2022-01-21 11:03 UTC (permalink / raw)
  To: Ojeda Leon, Nicolas
  Cc: devel@edk2.groups.io, Gupta, Atul, Graf (AWS), Alexander

On Fri, Jan 21, 2022 at 10:48:06AM +0000, Ojeda Leon, Nicolas wrote:
> Hi Gerd,
> 
> > Hmm, the QemuFwCfgHardwareInfoLib.inf file created by patch #2 has HardwareInfoPciHostBridgeLib.c + QemuFwCfgHardwareInfoLib.c too.
> 
> > I'm wondering why you add two inf files in the first place?
> 
> My idea was to provide 2 variations of the library:
> 
> - One offering no dynamic memory and list features and only read functions to, statically, parse data out of a fw-cfg file -> QemuFwCfgHardwareInfo
> - The other one being the general one having everything. Both the list and dynamic features as well as the fw-cfg wrappers -> HardwareInfoLib

Why do you need the former?  To figure the highest address used in PEI,
whereas the DXE drivers use the other one?

Usual naming convention for that case would be PeiHardwareInfoLib + DxeHardwareInfoLib ...

take care,
  Gerd


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

* Re: [edk2-devel] [PATCH v2 3/8] Ovmf/HardwareInfoLib: Add core to parse heterogenous data
  2022-01-21 11:03       ` Gerd Hoffmann
@ 2022-01-21 11:10         ` Ojeda Leon, Nicolas
  0 siblings, 0 replies; 15+ messages in thread
From: Ojeda Leon, Nicolas @ 2022-01-21 11:10 UTC (permalink / raw)
  To: kraxel@redhat.com
  Cc: devel@edk2.groups.io, Gupta, Atul, Graf (AWS), Alexander

> Why do you need the former?  To figure the highest address used in PEI, whereas the DXE drivers use the other one?
Yes that's right.

> Usual naming convention for that case would be PeiHardwareInfoLib + DxeHardwareInfoLib ...
Understood, will change the naming to comply with the convention.

Thank you.

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




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

end of thread, other threads:[~2022-01-21 11:10 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-20 17:10 [PATCH v2 0/8] Handling of multiple PCI host bridges specified Ojeda Leon, Nicolas
2022-01-20 17:10 ` [PATCH v2 1/8] OvmfPkg/Library: Create base HardwareInfoLib for PCI Host Bridges Ojeda Leon, Nicolas
2022-01-21 10:09   ` Gerd Hoffmann
2022-01-20 17:10 ` [PATCH v2 2/8] Ovmf/HardwareInfoLib: Parse data directly from fw-cfg Ojeda Leon, Nicolas
2022-01-20 17:10 ` [PATCH v2 3/8] Ovmf/HardwareInfoLib: Add core to parse heterogenous data Ojeda Leon, Nicolas
2022-01-21 10:09   ` Gerd Hoffmann
2022-01-21 10:48     ` [edk2-devel] " Ojeda Leon, Nicolas
2022-01-21 11:03       ` Gerd Hoffmann
2022-01-21 11:10         ` Ojeda Leon, Nicolas
2022-01-20 17:10 ` [PATCH v2 4/8] Ovmf/PlatformPei: Extend 64-bit MMIO range to fit resources Ojeda Leon, Nicolas
2022-01-21  9:57   ` Gerd Hoffmann
2022-01-20 17:10 ` [PATCH v2 5/8] OvmfPkg/PciHostBridgeUtilityLib: Initialize RootBridges apertures with spec Ojeda Leon, Nicolas
2022-01-20 17:10 ` [PATCH v2 6/8] MdeModulePkg, OvmfPkg: Add Pcd token for PCI pre-populated BARs Ojeda Leon, Nicolas
2022-01-20 17:10 ` [PATCH v2 7/8] MdeModulePkg/Pci MdePkg: Create service to retrieve PCI base addresses Ojeda Leon, Nicolas
2022-01-20 17:10 ` [PATCH v2 8/8] MdeModulePkg/PciBusDxe: Handling of pre-populated PCI BARs Ojeda Leon, Nicolas

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