* [PATCH 1/7] MdePkg/IndustryStandard/Pci23: add vendor-specific capability header
2017-09-25 19:58 [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU Laszlo Ersek
@ 2017-09-25 19:58 ` Laszlo Ersek
2017-09-27 6:26 ` Gao, Liming
2017-09-25 19:58 ` [PATCH 2/7] OvmfPkg/IndustryStandard: define PCI Capabilities for QEMU's PCI Bridges Laszlo Ersek
` (6 subsequent siblings)
7 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2017-09-25 19:58 UTC (permalink / raw)
To: edk2-devel-01
Cc: Jordan Justen, Liming Gao, Marcel Apfelbaum, Michael D Kinney,
Ruiyu Ni
Revision 2.2 of the PCI Spec defines Capability IDs 0 through 6,
inclusive, in Appendix H. It reserves IDs 7 through 255.
Revision 2.3 of the PCI Spec adds Capability IDs 7 through 0xC, inclusive,
in Appendix H. Capability ID 9 stands for "Vendor Specific".
Add the EFI_PCI_CAPABILITY_ID_VENDOR macro and the
EFI_PCI_CAPABILITY_VENDOR_HDR structure type to MdePkg/IndustryStandard,
in order to describe this capability header.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
MdePkg/Include/IndustryStandard/Pci23.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/MdePkg/Include/IndustryStandard/Pci23.h b/MdePkg/Include/IndustryStandard/Pci23.h
index 467354429e06..87bd16375c02 100644
--- a/MdePkg/Include/IndustryStandard/Pci23.h
+++ b/MdePkg/Include/IndustryStandard/Pci23.h
@@ -91,8 +91,9 @@
///
/// PCI Capability List IDs and records.
///
#define EFI_PCI_CAPABILITY_ID_PCIX 0x07
+#define EFI_PCI_CAPABILITY_ID_VENDOR 0x09
#pragma pack(1)
///
/// PCI-X Capabilities List,
@@ -115,8 +116,17 @@ typedef struct {
UINT32 SplitTransCtrlRegUp;
UINT32 SplitTransCtrlRegDn;
} EFI_PCI_CAPABILITY_PCIX_BRDG;
+///
+/// Vendor Specific Capability Header
+/// Table H-1: Capability IDs, PCI Local Bus Specification, 2.3
+///
+typedef struct {
+ EFI_PCI_CAPABILITY_HDR Hdr;
+ UINT8 Length;
+} EFI_PCI_CAPABILITY_VENDOR_HDR;
+
#pragma pack()
#define PCI_CODE_TYPE_EFI_IMAGE 0x03
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 1/7] MdePkg/IndustryStandard/Pci23: add vendor-specific capability header
2017-09-25 19:58 ` [PATCH 1/7] MdePkg/IndustryStandard/Pci23: add vendor-specific capability header Laszlo Ersek
@ 2017-09-27 6:26 ` Gao, Liming
0 siblings, 0 replies; 12+ messages in thread
From: Gao, Liming @ 2017-09-27 6:26 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01
Cc: Justen, Jordan L, Marcel Apfelbaum, Kinney, Michael D, Ni, Ruiyu
Reviewed-by: Liming Gao <liming.gao@intel.com>
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, September 26, 2017 3:58 AM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Gao, Liming <liming.gao@intel.com>; Marcel Apfelbaum <marcel@redhat.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH 1/7] MdePkg/IndustryStandard/Pci23: add vendor-specific capability header
>
> Revision 2.2 of the PCI Spec defines Capability IDs 0 through 6,
> inclusive, in Appendix H. It reserves IDs 7 through 255.
>
> Revision 2.3 of the PCI Spec adds Capability IDs 7 through 0xC, inclusive,
> in Appendix H. Capability ID 9 stands for "Vendor Specific".
>
> Add the EFI_PCI_CAPABILITY_ID_VENDOR macro and the
> EFI_PCI_CAPABILITY_VENDOR_HDR structure type to MdePkg/IndustryStandard,
> in order to describe this capability header.
>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> MdePkg/Include/IndustryStandard/Pci23.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/MdePkg/Include/IndustryStandard/Pci23.h b/MdePkg/Include/IndustryStandard/Pci23.h
> index 467354429e06..87bd16375c02 100644
> --- a/MdePkg/Include/IndustryStandard/Pci23.h
> +++ b/MdePkg/Include/IndustryStandard/Pci23.h
> @@ -91,8 +91,9 @@
> ///
> /// PCI Capability List IDs and records.
> ///
> #define EFI_PCI_CAPABILITY_ID_PCIX 0x07
> +#define EFI_PCI_CAPABILITY_ID_VENDOR 0x09
>
> #pragma pack(1)
> ///
> /// PCI-X Capabilities List,
> @@ -115,8 +116,17 @@ typedef struct {
> UINT32 SplitTransCtrlRegUp;
> UINT32 SplitTransCtrlRegDn;
> } EFI_PCI_CAPABILITY_PCIX_BRDG;
>
> +///
> +/// Vendor Specific Capability Header
> +/// Table H-1: Capability IDs, PCI Local Bus Specification, 2.3
> +///
> +typedef struct {
> + EFI_PCI_CAPABILITY_HDR Hdr;
> + UINT8 Length;
> +} EFI_PCI_CAPABILITY_VENDOR_HDR;
> +
> #pragma pack()
>
> #define PCI_CODE_TYPE_EFI_IMAGE 0x03
>
> --
> 2.14.1.3.gb7cf6e02401b
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/7] OvmfPkg/IndustryStandard: define PCI Capabilities for QEMU's PCI Bridges
2017-09-25 19:58 [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU Laszlo Ersek
2017-09-25 19:58 ` [PATCH 1/7] MdePkg/IndustryStandard/Pci23: add vendor-specific capability header Laszlo Ersek
@ 2017-09-25 19:58 ` Laszlo Ersek
2017-09-25 19:58 ` [PATCH 3/7] OvmfPkg/PciHotPlugInitDxe: clean up protocol usage comment Laszlo Ersek
` (5 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-09-25 19:58 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jordan Justen, Marcel Apfelbaum, Ruiyu Ni
QEMU has recently gained the ability to provide various hints about its
PCI bridges. The hints take the form of vendor-specific PCI capabilities.
Define macros and types under "OvmfPkg/Include/IndustryStandard" to
describe these capabilities.
The definitions correspond to "docs/pcie_pci_bridge.txt" in the QEMU tree.
Said documentation was added in the last commit of the following series:
a35fe226558a hw/pci: introduce pcie-pci-bridge device
70e1ee59bb94 hw/pci: introduce bridge-only vendor-specific capability to
provide some hints to firmware
226263fb5cda hw/pci: add QEMU-specific PCI capability to the Generic PCI
Express Root Port
c1800a162765 docs: update documentation considering PCIE-PCI bridge
We are going to parse the Resource Reservation Capability in
OvmfPkg/PciHotPlugInitDxe, and return the reservation requests to
PciBusDxe.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/Include/IndustryStandard/QemuPciBridgeCapabilities.h | 60 ++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/OvmfPkg/Include/IndustryStandard/QemuPciBridgeCapabilities.h b/OvmfPkg/Include/IndustryStandard/QemuPciBridgeCapabilities.h
new file mode 100644
index 000000000000..bf2373c556c4
--- /dev/null
+++ b/OvmfPkg/Include/IndustryStandard/QemuPciBridgeCapabilities.h
@@ -0,0 +1,60 @@
+/** @file
+ Macro and type definitions for QEMU's Red Hat vendor-specific PCI
+ capabilities that provide various hints about PCI Bridges.
+
+ Refer to "docs/pcie_pci_bridge.txt" in the QEMU source directory.
+
+ Copyright (C) 2017, Red Hat, Inc.
+
+ 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_PCI_BRIDGE_CAPABILITIES_H__
+#define __QEMU_PCI_BRIDGE_CAPABILITIES_H__
+
+#include <IndustryStandard/Pci23.h>
+
+//
+// The hints apply to PCI Bridges whose PCI_DEVICE_INDEPENDENT_REGION.VendorId
+// equals the following value.
+//
+#define QEMU_PCI_BRIDGE_VENDOR_ID_REDHAT 0x1B36
+
+//
+// Common capability header for all hints.
+//
+#pragma pack (1)
+typedef struct {
+ EFI_PCI_CAPABILITY_VENDOR_HDR VendorHdr;
+ UINT8 Type;
+} QEMU_PCI_BRIDGE_CAPABILITY_HDR;
+#pragma pack ()
+
+//
+// Values defined for QEMU_PCI_BRIDGE_CAPABILITY_HDR.Type.
+//
+#define QEMU_PCI_BRIDGE_CAPABILITY_TYPE_RESOURCE_RESERVATION 0x01
+
+//
+// PCI Resource Reservation structure for when
+// QEMU_PCI_BRIDGE_CAPABILITY_HDR.Type equals
+// QEMU_PCI_BRIDGE_CAPABILITY_TYPE_RESOURCE_RESERVATION.
+//
+#pragma pack (1)
+typedef struct {
+ QEMU_PCI_BRIDGE_CAPABILITY_HDR BridgeHdr;
+ UINT32 BusNumbers;
+ UINT64 Io;
+ UINT32 NonPrefetchable32BitMmio;
+ UINT32 Prefetchable32BitMmio;
+ UINT64 Prefetchable64BitMmio;
+} QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION;
+#pragma pack ()
+
+#endif
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/7] OvmfPkg/PciHotPlugInitDxe: clean up protocol usage comment
2017-09-25 19:58 [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU Laszlo Ersek
2017-09-25 19:58 ` [PATCH 1/7] MdePkg/IndustryStandard/Pci23: add vendor-specific capability header Laszlo Ersek
2017-09-25 19:58 ` [PATCH 2/7] OvmfPkg/IndustryStandard: define PCI Capabilities for QEMU's PCI Bridges Laszlo Ersek
@ 2017-09-25 19:58 ` Laszlo Ersek
2017-09-25 19:58 ` [PATCH 4/7] OvmfPkg/PciHotPlugInitDxe: clean up addr. range for non-prefetchable MMIO Laszlo Ersek
` (4 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-09-25 19:58 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jordan Justen, Marcel Apfelbaum, Ruiyu Ni
The driver always produces an instance of the
EFI_PCI_HOT_PLUG_INIT_PROTOCOL. The "SOMETIMES_PRODUCES" remark is an
oversight from the original v1->v2 patch update; v2 should have stated
"ALWAYS_PRODUCES":
http://mid.mail-archive.com/1468242274-12686-5-git-send-email-lersek@redhat.com
> Notes:
> v2:
> - drop the PcdPciBusHotplugDeviceSupport check, and the PcdLib
> dependency with it [Jordan]
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Fixes: 8aba40b79267df761bd24d6874ae87f47a7bd3de
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
index 641ee2cad995..ea19206219b7 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
@@ -35,8 +35,8 @@ [LibraryClasses]
UefiBootServicesTableLib
UefiDriverEntryPoint
[Protocols]
- gEfiPciHotPlugInitProtocolGuid ## SOMETIMES_PRODUCES
+ gEfiPciHotPlugInitProtocolGuid ## ALWAYS_PRODUCES
[Depex]
TRUE
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/7] OvmfPkg/PciHotPlugInitDxe: clean up addr. range for non-prefetchable MMIO
2017-09-25 19:58 [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU Laszlo Ersek
` (2 preceding siblings ...)
2017-09-25 19:58 ` [PATCH 3/7] OvmfPkg/PciHotPlugInitDxe: clean up protocol usage comment Laszlo Ersek
@ 2017-09-25 19:58 ` Laszlo Ersek
2017-09-25 19:58 ` [PATCH 5/7] OvmfPkg/PciHotPlugInitDxe: generalize RESOURCE_PADDING composition Laszlo Ersek
` (3 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-09-25 19:58 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jordan Justen, Marcel Apfelbaum, Ruiyu Ni
The non-prefetchable MMIO aperture of a bridge can never fall outside of
the 32-bit address space. Namely, the MemoryBase and MemoryLimit fields in
PCI_BRIDGE_CONTROL_REGISTER have type UINT16, and based on the PCI-to-PCI
Bridge Architecture Spec, Chapter 3.2, the actual MMIO aperture is
determined as in:
NonPrefetchMemoryBase = (((MemoryBase & 0xFFF0u) >> 4) << 20) | 0x00000
NonPrefetchMemoryLimit = (((MemoryLimit & 0xFFF0u) >> 4) << 20) | 0xFFFFF
In "OvmfPkg/PciHotPlugInitDxe", the
"mPadding.MmioPadding.AddrSpaceGranularity" field is currently initialized
to 64. According to the above, this is useless generality: a
non-prefetchable MMIO reservation may only be satisfied from 32-bit
address space. Update the field to 32.
In practice this change makes no difference, because PciBusDxe already
enforces the 32-bit limitation when it sees "non-prefetchable" from
(SpecificFlag==0). Quoting commit 8aba40b79267 ("OvmfPkg: add
PciHotPlugInitDxe", 2016-06-30): "regardless of our request for 64-bit
MMIO reservation, it is downgraded to 32-bit".
(See the Platform Init Spec 1.6, Volume 5,
- Table 8. "ACPI 2.0 & 3.0 QWORD Address Space Descriptor Usage", and
- Table 11. "Memory Resource Flag (Resource Type = 0) Usage",
for an explanation of the "mPadding.MmioPadding" fields.)
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Fixes: 8aba40b79267df761bd24d6874ae87f47a7bd3de
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
index 2265b8c7e12a..5c98f806def6 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
@@ -67,10 +67,10 @@ STATIC CONST RESOURCE_PADDING mPadding = {
0, // GenFlag:
// ignored
0, // SpecificFlag:
// non-prefetchable
- 64, // AddrSpaceGranularity:
- // reserve 64-bit aperture
+ 32, // AddrSpaceGranularity:
+ // reserve 32-bit aperture
0, // AddrRangeMin:
// ignored
SIZE_2MB - 1, // AddrRangeMax:
// align at 2MB
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/7] OvmfPkg/PciHotPlugInitDxe: generalize RESOURCE_PADDING composition
2017-09-25 19:58 [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU Laszlo Ersek
` (3 preceding siblings ...)
2017-09-25 19:58 ` [PATCH 4/7] OvmfPkg/PciHotPlugInitDxe: clean up addr. range for non-prefetchable MMIO Laszlo Ersek
@ 2017-09-25 19:58 ` Laszlo Ersek
2017-09-25 19:58 ` [PATCH 6/7] OvmfPkg/PciHotPlugInitDxe: add helper functions for setting up paddings Laszlo Ersek
` (2 subsequent siblings)
7 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-09-25 19:58 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jordan Justen, Marcel Apfelbaum, Ruiyu Ni
PciHotPlugInitDxe has a static variable called "mPadding" (of type
RESOURCE_PADDING), which describes two constant resource reservations:
- MmioPadding: 2MB of non-prefetchable (hence 32-bit) MMIO space,
- IoPadding: 512B of IO space.
In the GetResourcePadding() member function of
EFI_PCI_HOT_PLUG_INIT_PROTOCOL, the driver outputs a dynamically allocated
verbatim copy of "mPadding", for PciBusDxe to consume in its
ApplyResourcePadding() function.
In a later patch, we're going to compose the set of resource reservations
dynamically, based on QEMU hints. Generalize the RESOURCE_PADDING
structure so that we may generate (or not generate) each resource type
individually:
- Replace the named "MmioPadding" and "IoPadding" fields in
RESOURCE_PADDING with an array of descriptors,
- remove "mPadding",
- in GetResourcePadding(), request the same (default) reservations as
before, as if we attempted and failed to fetch the QEMU hints.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 1 +
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 163 ++++++++++++--------
2 files changed, 99 insertions(+), 65 deletions(-)
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
index ea19206219b7..91729ae1ed04 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
@@ -28,8 +28,9 @@ [Packages]
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
[LibraryClasses]
+ BaseMemoryLib
DebugLib
DevicePathLib
MemoryAllocationLib
UefiBootServicesTableLib
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
index 5c98f806def6..b1b2c5cd8ddc 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
@@ -14,8 +14,9 @@
**/
#include <IndustryStandard/Acpi10.h>
+#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/DevicePathLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/UefiBootServicesTableLib.h>
@@ -41,83 +42,61 @@ STATIC EFI_PCI_HOT_PLUG_INIT_PROTOCOL mPciHotPlugInit;
//
// This structure is interpreted by the ApplyResourcePadding() function in the
// edk2 PCI Bus UEFI_DRIVER.
//
+// We can request padding for at most four resource types, each of which is
+// optional, independently of the others:
+// (a) bus numbers,
+// (b) IO space,
+// (c) non-prefetchable MMIO space (32-bit only),
+// (d) prefetchable MMIO space (either 32-bit or 64-bit, never both).
+//
#pragma pack (1)
typedef struct {
- EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR MmioPadding;
- EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR IoPadding;
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR Padding[4];
EFI_ACPI_END_TAG_DESCRIPTOR EndDesc;
} RESOURCE_PADDING;
#pragma pack ()
-STATIC CONST RESOURCE_PADDING mPadding = {
- //
- // MmioPadding
- //
- {
- ACPI_ADDRESS_SPACE_DESCRIPTOR, // Desc
- (UINT16)( // Len
- sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
- OFFSET_OF (
- EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
- ResType
- )
- ),
- ACPI_ADDRESS_SPACE_TYPE_MEM, // ResType
- 0, // GenFlag:
- // ignored
- 0, // SpecificFlag:
- // non-prefetchable
- 32, // AddrSpaceGranularity:
- // reserve 32-bit aperture
- 0, // AddrRangeMin:
- // ignored
- SIZE_2MB - 1, // AddrRangeMax:
- // align at 2MB
- 0, // AddrTranslationOffset:
- // ignored
- SIZE_2MB // AddrLen:
- // 2MB padding
- },
- //
- // IoPadding
- //
- {
- ACPI_ADDRESS_SPACE_DESCRIPTOR, // Desc
- (UINT16)( // Len
- sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
- OFFSET_OF (
- EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
- ResType
- )
- ),
- ACPI_ADDRESS_SPACE_TYPE_IO,// ResType
- 0, // GenFlag:
- // ignored
- 0, // SpecificFlag:
- // ignored
- 0, // AddrSpaceGranularity:
- // ignored
- 0, // AddrRangeMin:
- // ignored
- 512 - 1, // AddrRangeMax:
- // align at 512 IO ports
- 0, // AddrTranslationOffset:
- // ignored
- 512 // AddrLen:
- // 512 IO ports
- },
+/**
+ Initialize a RESOURCE_PADDING object.
+
+ @param[out] ResourcePadding The caller-allocated RESOURCE_PADDING object to
+ initialize.
+**/
+STATIC
+VOID
+InitializeResourcePadding (
+ OUT RESOURCE_PADDING *ResourcePadding
+ )
+{
+ UINTN Index;
+
+ ZeroMem (ResourcePadding, sizeof *ResourcePadding);
//
- // EndDesc
+ // Fill in the Padding fields that don't vary across resource types.
//
- {
- ACPI_END_TAG_DESCRIPTOR, // Desc
- 0 // Checksum: to be ignored
+ for (Index = 0; Index < ARRAY_SIZE (ResourcePadding->Padding); ++Index) {
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor;
+
+ Descriptor = ResourcePadding->Padding + Index;
+ Descriptor->Desc = ACPI_ADDRESS_SPACE_DESCRIPTOR;
+ Descriptor->Len = (UINT16)(
+ sizeof (EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR) -
+ OFFSET_OF (
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR,
+ ResType
+ )
+ );
}
-};
+
+ //
+ // Fill in the End Tag.
+ //
+ ResourcePadding->EndDesc.Desc = ACPI_END_TAG_DESCRIPTOR;
+}
/**
Returns a list of root Hot Plug Controllers (HPCs) that require
@@ -274,8 +253,13 @@ GetResourcePadding (
OUT VOID **Padding,
OUT EFI_HPC_PADDING_ATTRIBUTES *Attributes
)
{
+ BOOLEAN DefaultIo;
+ BOOLEAN DefaultMmio;
+ RESOURCE_PADDING ReservationRequest;
+ EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *FirstResource;
+
DEBUG_CODE (
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *Address;
CHAR16 *DevicePathString;
@@ -294,9 +278,58 @@ GetResourcePadding (
if (HpcState == NULL || Padding == NULL || Attributes == NULL) {
return EFI_INVALID_PARAMETER;
}
- *Padding = AllocateCopyPool (sizeof mPadding, &mPadding);
+ DefaultIo = TRUE;
+ DefaultMmio = TRUE;
+
+ //
+ // Init ReservationRequest, and point FirstResource one past the last
+ // descriptor entry. We're going to build the entries backwards from
+ // ReservationRequest.EndDesc.
+ //
+ InitializeResourcePadding (&ReservationRequest);
+ FirstResource = ReservationRequest.Padding +
+ ARRAY_SIZE (ReservationRequest.Padding);
+
+ //
+ // (b) Reserve IO space.
+ //
+ if (DefaultIo) {
+ //
+ // Request defaults.
+ //
+ --FirstResource;
+ FirstResource->ResType = ACPI_ADDRESS_SPACE_TYPE_IO;
+ FirstResource->AddrRangeMax = 512 - 1; // align at 512 IO ports
+ FirstResource->AddrLen = 512; // 512 IO ports
+ }
+
+ //
+ // (c) Reserve non-prefetchable MMIO space (32-bit only).
+ //
+ if (DefaultMmio) {
+ //
+ // Request defaults.
+ //
+ --FirstResource;
+ FirstResource->ResType = ACPI_ADDRESS_SPACE_TYPE_MEM;
+ FirstResource->SpecificFlag = 0; // non-prefetchable
+ FirstResource->AddrSpaceGranularity = 32; // 32-bit aperture
+ FirstResource->AddrRangeMax = SIZE_2MB - 1; // align at 2MB
+ FirstResource->AddrLen = SIZE_2MB; // 2MB padding
+ }
+
+ //
+ // Output a copy of ReservationRequest from the lowest-address populated
+ // entry until the end of the structure (including
+ // ReservationRequest.EndDesc). If no reservations are necessary, we'll only
+ // output the End Tag.
+ //
+ *Padding = AllocateCopyPool (
+ (UINT8 *)(&ReservationRequest + 1) - (UINT8 *)FirstResource,
+ FirstResource
+ );
if (*Padding == NULL) {
return EFI_OUT_OF_RESOURCES;
}
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/7] OvmfPkg/PciHotPlugInitDxe: add helper functions for setting up paddings
2017-09-25 19:58 [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU Laszlo Ersek
` (4 preceding siblings ...)
2017-09-25 19:58 ` [PATCH 5/7] OvmfPkg/PciHotPlugInitDxe: generalize RESOURCE_PADDING composition Laszlo Ersek
@ 2017-09-25 19:58 ` Laszlo Ersek
2017-09-25 19:58 ` [PATCH 7/7] OvmfPkg/PciHotPlugInitDxe: translate QEMU's resource reservation hints Laszlo Ersek
2017-10-03 14:09 ` [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU Laszlo Ersek
7 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-09-25 19:58 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jordan Justen, Marcel Apfelbaum, Ruiyu Ni
Extract the SetIoPadding() and SetMmioPadding() functions, so that we can
set EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR fields using parameter names and
values that are more friendly than the original field names and their
expected values.
Introduce the HighBitSetRoundUp32() and HighBitSetRoundUp64() functions
for calculating the last parameter ("SizeExponent") of SetIoPadding() and
SetMmioPadding().
Put the new functions to use when requesting the default reservations. (In
order to be consistent with a later patch, "SizeExponent" is calculated
for SetIoPadding() with HighBitSetRoundUp64().)
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 1 +
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 165 ++++++++++++++++++--
2 files changed, 156 insertions(+), 10 deletions(-)
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
index 91729ae1ed04..e0ec9baae1c2 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
@@ -28,8 +28,9 @@ [Packages]
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
[LibraryClasses]
+ BaseLib
BaseMemoryLib
DebugLib
DevicePathLib
MemoryAllocationLib
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
index b1b2c5cd8ddc..39646973794b 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
@@ -14,8 +14,9 @@
**/
#include <IndustryStandard/Acpi10.h>
+#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/DevicePathLib.h>
#include <Library/MemoryAllocationLib.h>
@@ -97,8 +98,155 @@ InitializeResourcePadding (
ResourcePadding->EndDesc.Desc = ACPI_END_TAG_DESCRIPTOR;
}
+/**
+ Set up a descriptor entry for reserving IO space.
+
+ @param[in,out] Descriptor The descriptor to configure. The caller shall have
+ initialized Descriptor earlier, with
+ InitializeResourcePadding().
+
+ @param[in] SizeExponent The size and natural alignment of the reservation
+ are determined by raising two to this power.
+**/
+STATIC
+VOID
+SetIoPadding (
+ IN OUT EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor,
+ IN UINTN SizeExponent
+ )
+{
+ Descriptor->ResType = ACPI_ADDRESS_SPACE_TYPE_IO;
+ Descriptor->AddrLen = LShiftU64 (1, SizeExponent);
+ Descriptor->AddrRangeMax = Descriptor->AddrLen - 1;
+}
+
+
+/**
+ Set up a descriptor entry for reserving MMIO space.
+
+ @param[in,out] Descriptor The descriptor to configure. The caller shall
+ have initialized Descriptor earlier, with
+ InitializeResourcePadding().
+
+ @param[in] Prefetchable TRUE if the descriptor should reserve
+ prefetchable MMIO space. Pass FALSE for
+ reserving non-prefetchable MMIO space.
+
+ @param[in] ThirtyTwoBitOnly TRUE if the reservation should be limited to
+ 32-bit address space. FALSE if the reservation
+ can be satisfied from 64-bit address space.
+ ThirtyTwoBitOnly is ignored if Prefetchable is
+ FALSE; in that case ThirtyTwoBitOnly is always
+ considered TRUE.
+
+ @param[in] SizeExponent The size and natural alignment of the
+ reservation are determined by raising two to
+ this power.
+**/
+STATIC
+VOID
+SetMmioPadding (
+ IN OUT EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *Descriptor,
+ IN BOOLEAN Prefetchable,
+ IN BOOLEAN ThirtyTwoBitOnly,
+ IN UINTN SizeExponent
+ )
+{
+ Descriptor->ResType = ACPI_ADDRESS_SPACE_TYPE_MEM;
+ if (Prefetchable) {
+ Descriptor->SpecificFlag =
+ EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_CACHEABLE_PREFETCHABLE;
+ Descriptor->AddrSpaceGranularity = ThirtyTwoBitOnly ? 32 : 64;
+ } else {
+ Descriptor->SpecificFlag =
+ EFI_ACPI_MEMORY_RESOURCE_SPECIFIC_FLAG_NON_CACHEABLE;
+ Descriptor->AddrSpaceGranularity = 32;
+ }
+ Descriptor->AddrLen = LShiftU64 (1, SizeExponent);
+ Descriptor->AddrRangeMax = Descriptor->AddrLen - 1;
+}
+
+
+/**
+ Round up a positive 32-bit value to the next whole power of two, and return
+ the bit position of the highest bit set in the result. Equivalent to
+ ceil(log2(x)).
+
+ @param[in] Operand The 32-bit operand to evaluate.
+
+ @retval -1 Operand is zero.
+
+ @retval -1 Operand is positive, not a whole power of two, and rounding it
+ up to the next power of two does not fit into 32 bits.
+
+ @retval 0..31 Otherwise, return ceil(log2(Value)).
+**/
+STATIC
+INTN
+HighBitSetRoundUp32 (
+ IN UINT32 Operand
+ )
+{
+ INTN HighBit;
+
+ HighBit = HighBitSet32 (Operand);
+ if (HighBit == -1) {
+ //
+ // Operand is zero.
+ //
+ return HighBit;
+ }
+ if ((Operand & (Operand - 1)) != 0) {
+ //
+ // Operand is not a whole power of two.
+ //
+ ++HighBit;
+ }
+ return (HighBit < 32) ? HighBit : -1;
+}
+
+
+/**
+ Round up a positive 64-bit value to the next whole power of two, and return
+ the bit position of the highest bit set in the result. Equivalent to
+ ceil(log2(x)).
+
+ @param[in] Operand The 64-bit operand to evaluate.
+
+ @retval -1 Operand is zero.
+
+ @retval -1 Operand is positive, not a whole power of two, and rounding it
+ up to the next power of two does not fit into 64 bits.
+
+ @retval 0..63 Otherwise, return ceil(log2(Value)).
+**/
+STATIC
+INTN
+HighBitSetRoundUp64 (
+ IN UINT64 Operand
+ )
+{
+ INTN HighBit;
+
+ HighBit = HighBitSet64 (Operand);
+ if (HighBit == -1) {
+ //
+ // Operand is zero.
+ //
+ return HighBit;
+ }
+ if ((Operand & (Operand - 1)) != 0) {
+ //
+ // Operand is not a whole power of two.
+ //
+ ++HighBit;
+ }
+ return (HighBit < 64) ? HighBit : -1;
+}
+
+
/**
Returns a list of root Hot Plug Controllers (HPCs) that require
initialization during the boot process.
@@ -297,12 +445,9 @@ GetResourcePadding (
if (DefaultIo) {
//
// Request defaults.
//
- --FirstResource;
- FirstResource->ResType = ACPI_ADDRESS_SPACE_TYPE_IO;
- FirstResource->AddrRangeMax = 512 - 1; // align at 512 IO ports
- FirstResource->AddrLen = 512; // 512 IO ports
+ SetIoPadding (--FirstResource, (UINTN)HighBitSetRoundUp64 (512));
}
//
// (c) Reserve non-prefetchable MMIO space (32-bit only).
@@ -310,14 +455,14 @@ GetResourcePadding (
if (DefaultMmio) {
//
// Request defaults.
//
- --FirstResource;
- FirstResource->ResType = ACPI_ADDRESS_SPACE_TYPE_MEM;
- FirstResource->SpecificFlag = 0; // non-prefetchable
- FirstResource->AddrSpaceGranularity = 32; // 32-bit aperture
- FirstResource->AddrRangeMax = SIZE_2MB - 1; // align at 2MB
- FirstResource->AddrLen = SIZE_2MB; // 2MB padding
+ SetMmioPadding (
+ --FirstResource,
+ FALSE,
+ TRUE,
+ (UINTN)HighBitSetRoundUp32 (SIZE_2MB)
+ );
}
//
// Output a copy of ReservationRequest from the lowest-address populated
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/7] OvmfPkg/PciHotPlugInitDxe: translate QEMU's resource reservation hints
2017-09-25 19:58 [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU Laszlo Ersek
` (5 preceding siblings ...)
2017-09-25 19:58 ` [PATCH 6/7] OvmfPkg/PciHotPlugInitDxe: add helper functions for setting up paddings Laszlo Ersek
@ 2017-09-25 19:58 ` Laszlo Ersek
2017-10-02 17:43 ` Jordan Justen
2017-10-03 14:09 ` [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU Laszlo Ersek
7 siblings, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2017-09-25 19:58 UTC (permalink / raw)
To: edk2-devel-01; +Cc: Jordan Justen, Marcel Apfelbaum, Ruiyu Ni
Parse QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION from the bridges'
conventional config spaces. Translate the fields as follows:
* BusNumbers:
* 0 -- no reservation;
* (-1) -- firmware default, i.e. no reservation;
* otherwise -- reserve the requested value. (NB, bus number reservation
is not supposed to work before
<https://bugzilla.tianocore.org/show_bug.cgi?id=656> is fixed.)
* Io:
* 0 -- no reservation;
* (-1) -- keep our current default (512B);
* otherwise -- round up the requested value and reserve that.
* NonPrefetchable32BitMmio:
* 0 -- no reservation;
* (-1) -- keep our current default (2MB);
* otherwise -- round up the requested value and reserve that.
* Prefetchable32BitMmio:
* 0 -- no reservation, proceed to Prefetchable64BitMmio;
* (-1) -- firmware default, i.e. no reservation, proceed to
Prefetchable64BitMmio;
* otherwise -- round up the requested value and reserve that. (NB, if
Prefetchable32BitMmio is reserved in addition to
NonPrefetchable32BitMmio, then PciBusDxe currently runs into an
assertion failure. Refer to
<https://bugzilla.tianocore.org/show_bug.cgi?id=720>.)
* Prefetchable64BitMmio:
* only reached if Prefetchable32BitMmio was not reserved;
* 0 -- no reservation;
* (-1) -- firmware default, i.e. no reservation;
* otherwise -- round up the requested value and reserve that.
If QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION is missing, plus any
time the rounding fails, fall back to the current defaults.
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 2 +
OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 371 +++++++++++++++++++-
2 files changed, 367 insertions(+), 6 deletions(-)
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
index e0ec9baae1c2..38043986eb67 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
@@ -26,15 +26,17 @@ [Sources]
[Packages]
MdeModulePkg/MdeModulePkg.dec
MdePkg/MdePkg.dec
+ OvmfPkg/OvmfPkg.dec
[LibraryClasses]
BaseLib
BaseMemoryLib
DebugLib
DevicePathLib
MemoryAllocationLib
+ PciLib
UefiBootServicesTableLib
UefiDriverEntryPoint
[Protocols]
diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
index 39646973794b..177e1a62120d 100644
--- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
+++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
@@ -13,14 +13,16 @@
WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
#include <IndustryStandard/Acpi10.h>
+#include <IndustryStandard/QemuPciBridgeCapabilities.h>
#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/DevicePathLib.h>
#include <Library/MemoryAllocationLib.h>
+#include <Library/PciLib.h>
#include <Library/UefiBootServicesTableLib.h>
#include <Protocol/PciHotPlugInit.h>
#include <Protocol/PciRootBridgeIo.h>
@@ -245,8 +247,239 @@ HighBitSetRoundUp64 (
return (HighBit < 64) ? HighBit : -1;
}
+/**
+ Read a slice from conventional PCI config space at the given offset, then
+ advance the offset.
+
+ @param[in] PciAddress The address of the PCI Device -- Bus, Device, Function
+ -- in UEFI (not PciLib) encoding.
+
+ @param[in,out] Offset On input, the offset in conventional PCI config space
+ to start reading from. On output, the offset of the
+ first byte that was not read.
+
+ @param[in] Size The number of bytes to read.
+
+ @param[out] Buffer On output, the bytes read from PCI config space are
+ stored in this object.
+**/
+STATIC
+VOID
+ReadConfigSpace (
+ IN CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *PciAddress,
+ IN OUT UINT8 *Offset,
+ IN UINT8 Size,
+ OUT VOID *Buffer
+ )
+{
+ PciReadBuffer (
+ PCI_LIB_ADDRESS (
+ PciAddress->Bus,
+ PciAddress->Device,
+ PciAddress->Function,
+ *Offset
+ ),
+ Size,
+ Buffer
+ );
+ *Offset += Size;
+}
+
+
+/**
+ Convenience wrapper macro for ReadConfigSpace().
+
+ Given the following conditions:
+
+ - HeaderField is the first field in the structure pointed-to by Struct,
+
+ - Struct->HeaderField has been populated from the conventional PCI config
+ space of the PCI device identified by PciAddress,
+
+ - *Offset points one past HeaderField in the conventional PCI config space of
+ the PCI device identified by PciAddress,
+
+ populate the rest of *Struct from conventional PCI config space, starting at
+ *Offset. Finally, increment *Offset so that it point one past *Struct.
+
+ @param[in] PciAddress The address of the PCI Device -- Bus, Device, Function
+ -- in UEFI (not PciLib) encoding. Type: pointer to
+ CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS.
+
+ @param[in,out] Offset On input, the offset in conventional PCI config space
+ to start reading from; one past Struct->HeaderField.
+ On output, the offset of the first byte that was not
+ read; one past *Struct. Type: pointer to UINT8.
+
+ @param[out] Struct The structure to complete. Type: pointer to structure
+ object.
+
+ @param[in] HeaderField The name of the first field in *Struct, after which
+ *Struct should be populated. Type: structure member
+ identifier.
+**/
+#define COMPLETE_CONFIG_SPACE_STRUCT(PciAddress, Offset, Struct, HeaderField) \
+ ReadConfigSpace ( \
+ (PciAddress), \
+ (Offset), \
+ (UINT8)(sizeof *(Struct) - sizeof ((Struct)->HeaderField)), \
+ &((Struct)->HeaderField) + 1 \
+ )
+
+
+/**
+ Look up the QEMU-specific Resource Reservation capability in the conventional
+ config space of a Hotplug Controller (that is, PCI Bridge).
+
+ This function performs as few config space reads as possible.
+
+ @param[in] HpcPciAddress The address of the PCI Bridge -- Bus, Device,
+ Function -- in UEFI (not PciLib) encoding.
+
+ @param[out] ReservationHint The caller-allocated capability structure to
+ populate from the PCI Bridge's config space.
+
+ @retval EFI_SUCCESS The capability has been found, ReservationHint has
+ been populated.
+
+ @retval EFI_NOT_FOUND The capability is missing. The contents of
+ ReservationHint are now indeterminate.
+**/
+STATIC
+EFI_STATUS
+QueryReservationHint (
+ IN CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *HpcPciAddress,
+ OUT QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION *ReservationHint
+)
+{
+ UINT16 PciVendorId;
+ UINT16 PciStatus;
+ UINT8 PciCapPtr;
+ UINT8 Offset;
+
+ //
+ // Check the vendor identifier.
+ //
+ PciVendorId = PciRead16 (
+ PCI_LIB_ADDRESS (
+ HpcPciAddress->Bus,
+ HpcPciAddress->Device,
+ HpcPciAddress->Function,
+ PCI_VENDOR_ID_OFFSET
+ )
+ );
+ if (PciVendorId != QEMU_PCI_BRIDGE_VENDOR_ID_REDHAT) {
+ return EFI_NOT_FOUND;
+ }
+
+ //
+ // Check the Capabilities List bit in the PCI Status Register.
+ //
+ PciStatus = PciRead16 (
+ PCI_LIB_ADDRESS (
+ HpcPciAddress->Bus,
+ HpcPciAddress->Device,
+ HpcPciAddress->Function,
+ PCI_PRIMARY_STATUS_OFFSET
+ )
+ );
+ if ((PciStatus & EFI_PCI_STATUS_CAPABILITY) == 0) {
+ return EFI_NOT_FOUND;
+ }
+
+ //
+ // Fetch the start of the Capabilities List.
+ //
+ PciCapPtr = PciRead8 (
+ PCI_LIB_ADDRESS (
+ HpcPciAddress->Bus,
+ HpcPciAddress->Device,
+ HpcPciAddress->Function,
+ PCI_CAPBILITY_POINTER_OFFSET
+ )
+ );
+
+ //
+ // Scan the Capabilities List until we find the terminator element, or the
+ // Resource Reservation capability.
+ //
+ for (Offset = PciCapPtr & 0xFC;
+ Offset > 0;
+ Offset = ReservationHint->BridgeHdr.VendorHdr.Hdr.NextItemPtr & 0xFC) {
+ BOOLEAN EnoughRoom;
+
+ //
+ // Check if the Resource Reservation capability would fit into config space
+ // at this offset.
+ //
+ EnoughRoom = (BOOLEAN)(
+ Offset <= PCI_MAX_CONFIG_OFFSET - sizeof *ReservationHint
+ );
+
+ //
+ // Read the standard capability header so we can check the capability ID
+ // (if necessary) and advance to the next capability.
+ //
+ ReadConfigSpace (
+ HpcPciAddress,
+ &Offset,
+ (UINT8)sizeof ReservationHint->BridgeHdr.VendorHdr.Hdr,
+ &ReservationHint->BridgeHdr.VendorHdr.Hdr
+ );
+ if (!EnoughRoom ||
+ (ReservationHint->BridgeHdr.VendorHdr.Hdr.CapabilityID !=
+ EFI_PCI_CAPABILITY_ID_VENDOR)) {
+ continue;
+ }
+
+ //
+ // Read the rest of the vendor capability header so we can check the
+ // capability length.
+ //
+ COMPLETE_CONFIG_SPACE_STRUCT (
+ HpcPciAddress,
+ &Offset,
+ &ReservationHint->BridgeHdr.VendorHdr,
+ Hdr
+ );
+ if (ReservationHint->BridgeHdr.VendorHdr.Length !=
+ sizeof *ReservationHint) {
+ continue;
+ }
+
+ //
+ // Read the rest of the QEMU bridge capability header so we can check the
+ // capability type.
+ //
+ COMPLETE_CONFIG_SPACE_STRUCT (
+ HpcPciAddress,
+ &Offset,
+ &ReservationHint->BridgeHdr,
+ VendorHdr
+ );
+ if (ReservationHint->BridgeHdr.Type !=
+ QEMU_PCI_BRIDGE_CAPABILITY_TYPE_RESOURCE_RESERVATION) {
+ continue;
+ }
+
+ //
+ // Read the body of the reservation hint.
+ //
+ COMPLETE_CONFIG_SPACE_STRUCT (
+ HpcPciAddress,
+ &Offset,
+ ReservationHint,
+ BridgeHdr
+ );
+ return EFI_SUCCESS;
+ }
+
+ return EFI_NOT_FOUND;
+}
+
+
/**
Returns a list of root Hot Plug Controllers (HPCs) that require
initialization during the boot process.
@@ -401,18 +634,21 @@ GetResourcePadding (
OUT VOID **Padding,
OUT EFI_HPC_PADDING_ATTRIBUTES *Attributes
)
{
+ EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *Address;
BOOLEAN DefaultIo;
BOOLEAN DefaultMmio;
RESOURCE_PADDING ReservationRequest;
EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *FirstResource;
+ EFI_STATUS ReservationHintStatus;
+ QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION ReservationHint;
+
+ Address = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *)&HpcPciAddress;
DEBUG_CODE (
- EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *Address;
CHAR16 *DevicePathString;
- Address = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *)&HpcPciAddress;
DevicePathString = ConvertDevicePathToText (HpcDevicePath, FALSE, FALSE);
DEBUG ((EFI_D_VERBOSE, "%a: Address=%02x:%02x.%x DevicePath=%s\n",
__FUNCTION__, Address->Bus, Address->Device, Address->Function,
@@ -439,20 +675,143 @@ GetResourcePadding (
FirstResource = ReservationRequest.Padding +
ARRAY_SIZE (ReservationRequest.Padding);
//
- // (b) Reserve IO space.
+ // Try to get the QEMU-specific Resource Reservation capability.
//
+ ReservationHintStatus = QueryReservationHint (Address, &ReservationHint);
+ if (!EFI_ERROR (ReservationHintStatus)) {
+ INTN HighBit;
+
+ DEBUG ((
+ DEBUG_VERBOSE,
+ "%a: BusNumbers=0x%x Io=0x%Lx NonPrefetchable32BitMmio=0x%x\n"
+ "%a: Prefetchable32BitMmio=0x%x Prefetchable64BitMmio=0x%Lx\n",
+ __FUNCTION__,
+ ReservationHint.BusNumbers,
+ ReservationHint.Io,
+ ReservationHint.NonPrefetchable32BitMmio,
+ __FUNCTION__,
+ ReservationHint.Prefetchable32BitMmio,
+ ReservationHint.Prefetchable64BitMmio
+ ));
+
+ //
+ // (a) Reserve bus numbers.
+ //
+ switch (ReservationHint.BusNumbers) {
+ case 0:
+ //
+ // No reservation needed.
+ //
+ break;
+ case MAX_UINT32:
+ //
+ // Firmware default (unspecified). Treat it as "no reservation needed".
+ //
+ break;
+ default:
+ //
+ // Request the specified amount.
+ //
+ --FirstResource;
+ FirstResource->ResType = ACPI_ADDRESS_SPACE_TYPE_BUS;
+ FirstResource->AddrLen = ReservationHint.BusNumbers;
+ break;
+ }
+
+ //
+ // (b) Reserve IO space.
+ //
+ switch (ReservationHint.Io) {
+ case 0:
+ //
+ // No reservation needed, disable our built-in.
+ //
+ DefaultIo = FALSE;
+ break;
+ case MAX_UINT64:
+ //
+ // Firmware default (unspecified). Stick with our built-in.
+ //
+ break;
+ default:
+ //
+ // Round the specified amount up to the next power of two. If rounding is
+ // successful, reserve the rounded value. Fall back to the default
+ // otherwise.
+ //
+ HighBit = HighBitSetRoundUp64 (ReservationHint.Io);
+ if (HighBit != -1) {
+ SetIoPadding (--FirstResource, (UINTN)HighBit);
+ DefaultIo = FALSE;
+ }
+ break;
+ }
+
+ //
+ // (c) Reserve non-prefetchable MMIO space (32-bit only).
+ //
+ switch (ReservationHint.NonPrefetchable32BitMmio) {
+ case 0:
+ //
+ // No reservation needed, disable our built-in.
+ //
+ DefaultMmio = FALSE;
+ break;
+ case MAX_UINT32:
+ //
+ // Firmware default (unspecified). Stick with our built-in.
+ //
+ break;
+ default:
+ //
+ // Round the specified amount up to the next power of two. If rounding is
+ // successful, reserve the rounded value. Fall back to the default
+ // otherwise.
+ //
+ HighBit = HighBitSetRoundUp32 (ReservationHint.NonPrefetchable32BitMmio);
+ if (HighBit != -1) {
+ SetMmioPadding (--FirstResource, FALSE, TRUE, (UINTN)HighBit);
+ DefaultMmio = FALSE;
+ }
+ break;
+ }
+
+ //
+ // (d) Reserve prefetchable MMIO space (either 32-bit or 64-bit, never
+ // both).
+ //
+ // For either space, we treat 0 as "no reservation needed", and the maximum
+ // value as "firmware default". The latter is unspecified, and we interpret
+ // it as the former.
+ //
+ // Otherwise, round the specified amount up to the next power of two. If
+ // rounding is successful, reserve the rounded value. Do not reserve
+ // prefetchable MMIO space otherwise.
+ //
+ if (ReservationHint.Prefetchable32BitMmio > 0 &&
+ ReservationHint.Prefetchable32BitMmio < MAX_UINT32) {
+ HighBit = HighBitSetRoundUp32 (ReservationHint.Prefetchable32BitMmio);
+ if (HighBit != -1) {
+ SetMmioPadding (--FirstResource, TRUE, TRUE, (UINTN)HighBit);
+ }
+ } else if (ReservationHint.Prefetchable64BitMmio > 0 &&
+ ReservationHint.Prefetchable64BitMmio < MAX_UINT64) {
+ HighBit = HighBitSetRoundUp64 (ReservationHint.Prefetchable64BitMmio);
+ if (HighBit != -1) {
+ SetMmioPadding (--FirstResource, TRUE, FALSE, (UINTN)HighBit);
+ }
+ }
+ }
+
if (DefaultIo) {
//
// Request defaults.
//
SetIoPadding (--FirstResource, (UINTN)HighBitSetRoundUp64 (512));
}
- //
- // (c) Reserve non-prefetchable MMIO space (32-bit only).
- //
if (DefaultMmio) {
//
// Request defaults.
//
--
2.14.1.3.gb7cf6e02401b
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 7/7] OvmfPkg/PciHotPlugInitDxe: translate QEMU's resource reservation hints
2017-09-25 19:58 ` [PATCH 7/7] OvmfPkg/PciHotPlugInitDxe: translate QEMU's resource reservation hints Laszlo Ersek
@ 2017-10-02 17:43 ` Jordan Justen
2017-10-02 20:00 ` Laszlo Ersek
0 siblings, 1 reply; 12+ messages in thread
From: Jordan Justen @ 2017-10-02 17:43 UTC (permalink / raw)
To: Laszlo Ersek, edk2-devel-01; +Cc: Marcel Apfelbaum, Ruiyu Ni
On 2017-09-25 12:58:24, Laszlo Ersek wrote:
> Parse QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION from the bridges'
> conventional config spaces. Translate the fields as follows:
>
> * BusNumbers:
> * 0 -- no reservation;
> * (-1) -- firmware default, i.e. no reservation;
> * otherwise -- reserve the requested value. (NB, bus number reservation
> is not supposed to work before
> <https://bugzilla.tianocore.org/show_bug.cgi?id=656> is fixed.)
>
> * Io:
> * 0 -- no reservation;
> * (-1) -- keep our current default (512B);
> * otherwise -- round up the requested value and reserve that.
>
> * NonPrefetchable32BitMmio:
> * 0 -- no reservation;
> * (-1) -- keep our current default (2MB);
> * otherwise -- round up the requested value and reserve that.
>
> * Prefetchable32BitMmio:
> * 0 -- no reservation, proceed to Prefetchable64BitMmio;
> * (-1) -- firmware default, i.e. no reservation, proceed to
> Prefetchable64BitMmio;
> * otherwise -- round up the requested value and reserve that. (NB, if
> Prefetchable32BitMmio is reserved in addition to
> NonPrefetchable32BitMmio, then PciBusDxe currently runs into an
> assertion failure. Refer to
> <https://bugzilla.tianocore.org/show_bug.cgi?id=720>.)
>
> * Prefetchable64BitMmio:
> * only reached if Prefetchable32BitMmio was not reserved;
> * 0 -- no reservation;
> * (-1) -- firmware default, i.e. no reservation;
> * otherwise -- round up the requested value and reserve that.
>
> If QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION is missing, plus any
> time the rounding fails, fall back to the current defaults.
>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 2 +
> OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 371 +++++++++++++++++++-
> 2 files changed, 367 insertions(+), 6 deletions(-)
>
> diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> index e0ec9baae1c2..38043986eb67 100644
> --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> +++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
> @@ -26,15 +26,17 @@ [Sources]
>
> [Packages]
> MdeModulePkg/MdeModulePkg.dec
> MdePkg/MdePkg.dec
> + OvmfPkg/OvmfPkg.dec
>
> [LibraryClasses]
> BaseLib
> BaseMemoryLib
> DebugLib
> DevicePathLib
> MemoryAllocationLib
> + PciLib
> UefiBootServicesTableLib
> UefiDriverEntryPoint
>
> [Protocols]
> diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
> index 39646973794b..177e1a62120d 100644
> --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
> +++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
> @@ -13,14 +13,16 @@
> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> **/
>
> #include <IndustryStandard/Acpi10.h>
> +#include <IndustryStandard/QemuPciBridgeCapabilities.h>
>
> #include <Library/BaseLib.h>
> #include <Library/BaseMemoryLib.h>
> #include <Library/DebugLib.h>
> #include <Library/DevicePathLib.h>
> #include <Library/MemoryAllocationLib.h>
> +#include <Library/PciLib.h>
> #include <Library/UefiBootServicesTableLib.h>
>
> #include <Protocol/PciHotPlugInit.h>
> #include <Protocol/PciRootBridgeIo.h>
> @@ -245,8 +247,239 @@ HighBitSetRoundUp64 (
> return (HighBit < 64) ? HighBit : -1;
> }
>
>
> +/**
> + Read a slice from conventional PCI config space at the given offset, then
> + advance the offset.
> +
> + @param[in] PciAddress The address of the PCI Device -- Bus, Device, Function
> + -- in UEFI (not PciLib) encoding.
> +
> + @param[in,out] Offset On input, the offset in conventional PCI config space
> + to start reading from. On output, the offset of the
> + first byte that was not read.
> +
> + @param[in] Size The number of bytes to read.
> +
> + @param[out] Buffer On output, the bytes read from PCI config space are
> + stored in this object.
> +**/
> +STATIC
> +VOID
> +ReadConfigSpace (
> + IN CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *PciAddress,
> + IN OUT UINT8 *Offset,
> + IN UINT8 Size,
> + OUT VOID *Buffer
> + )
> +{
> + PciReadBuffer (
> + PCI_LIB_ADDRESS (
> + PciAddress->Bus,
> + PciAddress->Device,
> + PciAddress->Function,
> + *Offset
> + ),
> + Size,
> + Buffer
> + );
> + *Offset += Size;
> +}
> +
> +
> +/**
> + Convenience wrapper macro for ReadConfigSpace().
> +
> + Given the following conditions:
> +
> + - HeaderField is the first field in the structure pointed-to by Struct,
> +
> + - Struct->HeaderField has been populated from the conventional PCI config
> + space of the PCI device identified by PciAddress,
> +
> + - *Offset points one past HeaderField in the conventional PCI config space of
> + the PCI device identified by PciAddress,
> +
> + populate the rest of *Struct from conventional PCI config space, starting at
> + *Offset. Finally, increment *Offset so that it point one past *Struct.
> +
> + @param[in] PciAddress The address of the PCI Device -- Bus, Device, Function
> + -- in UEFI (not PciLib) encoding. Type: pointer to
> + CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS.
> +
> + @param[in,out] Offset On input, the offset in conventional PCI config space
> + to start reading from; one past Struct->HeaderField.
> + On output, the offset of the first byte that was not
> + read; one past *Struct. Type: pointer to UINT8.
> +
> + @param[out] Struct The structure to complete. Type: pointer to structure
> + object.
> +
> + @param[in] HeaderField The name of the first field in *Struct, after which
> + *Struct should be populated. Type: structure member
> + identifier.
> +**/
> +#define COMPLETE_CONFIG_SPACE_STRUCT(PciAddress, Offset, Struct, HeaderField) \
> + ReadConfigSpace ( \
> + (PciAddress), \
> + (Offset), \
> + (UINT8)(sizeof *(Struct) - sizeof ((Struct)->HeaderField)), \
> + &((Struct)->HeaderField) + 1 \
> + )
There is a lot of assumptions about inputs with usage of this macro.
(Even the in/out offset on ReadConfigSpace seems a little unexpected.)
Based on that, I was hoping to come up with some suggestion. The two
thoughts I had seemed more appropriate for a more generic PCI helper
lib than for this driver:
* A 'reader' struct that bundles the state of the pci address, read
buffer and offset read.
* More generically, a capabilities helper lib that could iterate &
read the PCI capability structs.
Anyway, for this series:
Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
> +
> +
> +/**
> + Look up the QEMU-specific Resource Reservation capability in the conventional
> + config space of a Hotplug Controller (that is, PCI Bridge).
> +
> + This function performs as few config space reads as possible.
> +
> + @param[in] HpcPciAddress The address of the PCI Bridge -- Bus, Device,
> + Function -- in UEFI (not PciLib) encoding.
> +
> + @param[out] ReservationHint The caller-allocated capability structure to
> + populate from the PCI Bridge's config space.
> +
> + @retval EFI_SUCCESS The capability has been found, ReservationHint has
> + been populated.
> +
> + @retval EFI_NOT_FOUND The capability is missing. The contents of
> + ReservationHint are now indeterminate.
> +**/
> +STATIC
> +EFI_STATUS
> +QueryReservationHint (
> + IN CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *HpcPciAddress,
> + OUT QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION *ReservationHint
> +)
> +{
> + UINT16 PciVendorId;
> + UINT16 PciStatus;
> + UINT8 PciCapPtr;
> + UINT8 Offset;
> +
> + //
> + // Check the vendor identifier.
> + //
> + PciVendorId = PciRead16 (
> + PCI_LIB_ADDRESS (
> + HpcPciAddress->Bus,
> + HpcPciAddress->Device,
> + HpcPciAddress->Function,
> + PCI_VENDOR_ID_OFFSET
> + )
> + );
> + if (PciVendorId != QEMU_PCI_BRIDGE_VENDOR_ID_REDHAT) {
> + return EFI_NOT_FOUND;
> + }
> +
> + //
> + // Check the Capabilities List bit in the PCI Status Register.
> + //
> + PciStatus = PciRead16 (
> + PCI_LIB_ADDRESS (
> + HpcPciAddress->Bus,
> + HpcPciAddress->Device,
> + HpcPciAddress->Function,
> + PCI_PRIMARY_STATUS_OFFSET
> + )
> + );
> + if ((PciStatus & EFI_PCI_STATUS_CAPABILITY) == 0) {
> + return EFI_NOT_FOUND;
> + }
> +
> + //
> + // Fetch the start of the Capabilities List.
> + //
> + PciCapPtr = PciRead8 (
> + PCI_LIB_ADDRESS (
> + HpcPciAddress->Bus,
> + HpcPciAddress->Device,
> + HpcPciAddress->Function,
> + PCI_CAPBILITY_POINTER_OFFSET
> + )
> + );
> +
> + //
> + // Scan the Capabilities List until we find the terminator element, or the
> + // Resource Reservation capability.
> + //
> + for (Offset = PciCapPtr & 0xFC;
> + Offset > 0;
> + Offset = ReservationHint->BridgeHdr.VendorHdr.Hdr.NextItemPtr & 0xFC) {
> + BOOLEAN EnoughRoom;
> +
> + //
> + // Check if the Resource Reservation capability would fit into config space
> + // at this offset.
> + //
> + EnoughRoom = (BOOLEAN)(
> + Offset <= PCI_MAX_CONFIG_OFFSET - sizeof *ReservationHint
> + );
> +
> + //
> + // Read the standard capability header so we can check the capability ID
> + // (if necessary) and advance to the next capability.
> + //
> + ReadConfigSpace (
> + HpcPciAddress,
> + &Offset,
> + (UINT8)sizeof ReservationHint->BridgeHdr.VendorHdr.Hdr,
> + &ReservationHint->BridgeHdr.VendorHdr.Hdr
> + );
> + if (!EnoughRoom ||
> + (ReservationHint->BridgeHdr.VendorHdr.Hdr.CapabilityID !=
> + EFI_PCI_CAPABILITY_ID_VENDOR)) {
> + continue;
> + }
> +
> + //
> + // Read the rest of the vendor capability header so we can check the
> + // capability length.
> + //
> + COMPLETE_CONFIG_SPACE_STRUCT (
> + HpcPciAddress,
> + &Offset,
> + &ReservationHint->BridgeHdr.VendorHdr,
> + Hdr
> + );
> + if (ReservationHint->BridgeHdr.VendorHdr.Length !=
> + sizeof *ReservationHint) {
> + continue;
> + }
> +
> + //
> + // Read the rest of the QEMU bridge capability header so we can check the
> + // capability type.
> + //
> + COMPLETE_CONFIG_SPACE_STRUCT (
> + HpcPciAddress,
> + &Offset,
> + &ReservationHint->BridgeHdr,
> + VendorHdr
> + );
> + if (ReservationHint->BridgeHdr.Type !=
> + QEMU_PCI_BRIDGE_CAPABILITY_TYPE_RESOURCE_RESERVATION) {
> + continue;
> + }
> +
> + //
> + // Read the body of the reservation hint.
> + //
> + COMPLETE_CONFIG_SPACE_STRUCT (
> + HpcPciAddress,
> + &Offset,
> + ReservationHint,
> + BridgeHdr
> + );
> + return EFI_SUCCESS;
> + }
> +
> + return EFI_NOT_FOUND;
> +}
> +
> +
> /**
> Returns a list of root Hot Plug Controllers (HPCs) that require
> initialization during the boot process.
>
> @@ -401,18 +634,21 @@ GetResourcePadding (
> OUT VOID **Padding,
> OUT EFI_HPC_PADDING_ATTRIBUTES *Attributes
> )
> {
> + EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *Address;
> BOOLEAN DefaultIo;
> BOOLEAN DefaultMmio;
> RESOURCE_PADDING ReservationRequest;
> EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *FirstResource;
> + EFI_STATUS ReservationHintStatus;
> + QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION ReservationHint;
> +
> + Address = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *)&HpcPciAddress;
>
> DEBUG_CODE (
> - EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *Address;
> CHAR16 *DevicePathString;
>
> - Address = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *)&HpcPciAddress;
> DevicePathString = ConvertDevicePathToText (HpcDevicePath, FALSE, FALSE);
>
> DEBUG ((EFI_D_VERBOSE, "%a: Address=%02x:%02x.%x DevicePath=%s\n",
> __FUNCTION__, Address->Bus, Address->Device, Address->Function,
> @@ -439,20 +675,143 @@ GetResourcePadding (
> FirstResource = ReservationRequest.Padding +
> ARRAY_SIZE (ReservationRequest.Padding);
>
> //
> - // (b) Reserve IO space.
> + // Try to get the QEMU-specific Resource Reservation capability.
> //
> + ReservationHintStatus = QueryReservationHint (Address, &ReservationHint);
> + if (!EFI_ERROR (ReservationHintStatus)) {
> + INTN HighBit;
> +
> + DEBUG ((
> + DEBUG_VERBOSE,
> + "%a: BusNumbers=0x%x Io=0x%Lx NonPrefetchable32BitMmio=0x%x\n"
> + "%a: Prefetchable32BitMmio=0x%x Prefetchable64BitMmio=0x%Lx\n",
> + __FUNCTION__,
> + ReservationHint.BusNumbers,
> + ReservationHint.Io,
> + ReservationHint.NonPrefetchable32BitMmio,
> + __FUNCTION__,
> + ReservationHint.Prefetchable32BitMmio,
> + ReservationHint.Prefetchable64BitMmio
> + ));
> +
> + //
> + // (a) Reserve bus numbers.
> + //
> + switch (ReservationHint.BusNumbers) {
> + case 0:
> + //
> + // No reservation needed.
> + //
> + break;
> + case MAX_UINT32:
> + //
> + // Firmware default (unspecified). Treat it as "no reservation needed".
> + //
> + break;
> + default:
> + //
> + // Request the specified amount.
> + //
> + --FirstResource;
> + FirstResource->ResType = ACPI_ADDRESS_SPACE_TYPE_BUS;
> + FirstResource->AddrLen = ReservationHint.BusNumbers;
> + break;
> + }
> +
> + //
> + // (b) Reserve IO space.
> + //
> + switch (ReservationHint.Io) {
> + case 0:
> + //
> + // No reservation needed, disable our built-in.
> + //
> + DefaultIo = FALSE;
> + break;
> + case MAX_UINT64:
> + //
> + // Firmware default (unspecified). Stick with our built-in.
> + //
> + break;
> + default:
> + //
> + // Round the specified amount up to the next power of two. If rounding is
> + // successful, reserve the rounded value. Fall back to the default
> + // otherwise.
> + //
> + HighBit = HighBitSetRoundUp64 (ReservationHint.Io);
> + if (HighBit != -1) {
> + SetIoPadding (--FirstResource, (UINTN)HighBit);
> + DefaultIo = FALSE;
> + }
> + break;
> + }
> +
> + //
> + // (c) Reserve non-prefetchable MMIO space (32-bit only).
> + //
> + switch (ReservationHint.NonPrefetchable32BitMmio) {
> + case 0:
> + //
> + // No reservation needed, disable our built-in.
> + //
> + DefaultMmio = FALSE;
> + break;
> + case MAX_UINT32:
> + //
> + // Firmware default (unspecified). Stick with our built-in.
> + //
> + break;
> + default:
> + //
> + // Round the specified amount up to the next power of two. If rounding is
> + // successful, reserve the rounded value. Fall back to the default
> + // otherwise.
> + //
> + HighBit = HighBitSetRoundUp32 (ReservationHint.NonPrefetchable32BitMmio);
> + if (HighBit != -1) {
> + SetMmioPadding (--FirstResource, FALSE, TRUE, (UINTN)HighBit);
> + DefaultMmio = FALSE;
> + }
> + break;
> + }
> +
> + //
> + // (d) Reserve prefetchable MMIO space (either 32-bit or 64-bit, never
> + // both).
> + //
> + // For either space, we treat 0 as "no reservation needed", and the maximum
> + // value as "firmware default". The latter is unspecified, and we interpret
> + // it as the former.
> + //
> + // Otherwise, round the specified amount up to the next power of two. If
> + // rounding is successful, reserve the rounded value. Do not reserve
> + // prefetchable MMIO space otherwise.
> + //
> + if (ReservationHint.Prefetchable32BitMmio > 0 &&
> + ReservationHint.Prefetchable32BitMmio < MAX_UINT32) {
> + HighBit = HighBitSetRoundUp32 (ReservationHint.Prefetchable32BitMmio);
> + if (HighBit != -1) {
> + SetMmioPadding (--FirstResource, TRUE, TRUE, (UINTN)HighBit);
> + }
> + } else if (ReservationHint.Prefetchable64BitMmio > 0 &&
> + ReservationHint.Prefetchable64BitMmio < MAX_UINT64) {
> + HighBit = HighBitSetRoundUp64 (ReservationHint.Prefetchable64BitMmio);
> + if (HighBit != -1) {
> + SetMmioPadding (--FirstResource, TRUE, FALSE, (UINTN)HighBit);
> + }
> + }
> + }
> +
> if (DefaultIo) {
> //
> // Request defaults.
> //
> SetIoPadding (--FirstResource, (UINTN)HighBitSetRoundUp64 (512));
> }
>
> - //
> - // (c) Reserve non-prefetchable MMIO space (32-bit only).
> - //
> if (DefaultMmio) {
> //
> // Request defaults.
> //
> --
> 2.14.1.3.gb7cf6e02401b
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 7/7] OvmfPkg/PciHotPlugInitDxe: translate QEMU's resource reservation hints
2017-10-02 17:43 ` Jordan Justen
@ 2017-10-02 20:00 ` Laszlo Ersek
0 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-10-02 20:00 UTC (permalink / raw)
To: Jordan Justen, edk2-devel-01; +Cc: Marcel Apfelbaum, Ruiyu Ni
On 10/02/17 19:43, Jordan Justen wrote:
> On 2017-09-25 12:58:24, Laszlo Ersek wrote:
>> Parse QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION from the bridges'
>> conventional config spaces. Translate the fields as follows:
>>
>> * BusNumbers:
>> * 0 -- no reservation;
>> * (-1) -- firmware default, i.e. no reservation;
>> * otherwise -- reserve the requested value. (NB, bus number reservation
>> is not supposed to work before
>> <https://bugzilla.tianocore.org/show_bug.cgi?id=656> is fixed.)
>>
>> * Io:
>> * 0 -- no reservation;
>> * (-1) -- keep our current default (512B);
>> * otherwise -- round up the requested value and reserve that.
>>
>> * NonPrefetchable32BitMmio:
>> * 0 -- no reservation;
>> * (-1) -- keep our current default (2MB);
>> * otherwise -- round up the requested value and reserve that.
>>
>> * Prefetchable32BitMmio:
>> * 0 -- no reservation, proceed to Prefetchable64BitMmio;
>> * (-1) -- firmware default, i.e. no reservation, proceed to
>> Prefetchable64BitMmio;
>> * otherwise -- round up the requested value and reserve that. (NB, if
>> Prefetchable32BitMmio is reserved in addition to
>> NonPrefetchable32BitMmio, then PciBusDxe currently runs into an
>> assertion failure. Refer to
>> <https://bugzilla.tianocore.org/show_bug.cgi?id=720>.)
>>
>> * Prefetchable64BitMmio:
>> * only reached if Prefetchable32BitMmio was not reserved;
>> * 0 -- no reservation;
>> * (-1) -- firmware default, i.e. no reservation;
>> * otherwise -- round up the requested value and reserve that.
>>
>> If QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION is missing, plus any
>> time the rounding fails, fall back to the current defaults.
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Marcel Apfelbaum <marcel@redhat.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>> OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 2 +
>> OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 371 +++++++++++++++++++-
>> 2 files changed, 367 insertions(+), 6 deletions(-)
>>
>> diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
>> index e0ec9baae1c2..38043986eb67 100644
>> --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
>> +++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
>> @@ -26,15 +26,17 @@ [Sources]
>>
>> [Packages]
>> MdeModulePkg/MdeModulePkg.dec
>> MdePkg/MdePkg.dec
>> + OvmfPkg/OvmfPkg.dec
>>
>> [LibraryClasses]
>> BaseLib
>> BaseMemoryLib
>> DebugLib
>> DevicePathLib
>> MemoryAllocationLib
>> + PciLib
>> UefiBootServicesTableLib
>> UefiDriverEntryPoint
>>
>> [Protocols]
>> diff --git a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
>> index 39646973794b..177e1a62120d 100644
>> --- a/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
>> +++ b/OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c
>> @@ -13,14 +13,16 @@
>> WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> **/
>>
>> #include <IndustryStandard/Acpi10.h>
>> +#include <IndustryStandard/QemuPciBridgeCapabilities.h>
>>
>> #include <Library/BaseLib.h>
>> #include <Library/BaseMemoryLib.h>
>> #include <Library/DebugLib.h>
>> #include <Library/DevicePathLib.h>
>> #include <Library/MemoryAllocationLib.h>
>> +#include <Library/PciLib.h>
>> #include <Library/UefiBootServicesTableLib.h>
>>
>> #include <Protocol/PciHotPlugInit.h>
>> #include <Protocol/PciRootBridgeIo.h>
>> @@ -245,8 +247,239 @@ HighBitSetRoundUp64 (
>> return (HighBit < 64) ? HighBit : -1;
>> }
>>
>>
>> +/**
>> + Read a slice from conventional PCI config space at the given offset, then
>> + advance the offset.
>> +
>> + @param[in] PciAddress The address of the PCI Device -- Bus, Device, Function
>> + -- in UEFI (not PciLib) encoding.
>> +
>> + @param[in,out] Offset On input, the offset in conventional PCI config space
>> + to start reading from. On output, the offset of the
>> + first byte that was not read.
>> +
>> + @param[in] Size The number of bytes to read.
>> +
>> + @param[out] Buffer On output, the bytes read from PCI config space are
>> + stored in this object.
>> +**/
>> +STATIC
>> +VOID
>> +ReadConfigSpace (
>> + IN CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *PciAddress,
>> + IN OUT UINT8 *Offset,
>> + IN UINT8 Size,
>> + OUT VOID *Buffer
>> + )
>> +{
>> + PciReadBuffer (
>> + PCI_LIB_ADDRESS (
>> + PciAddress->Bus,
>> + PciAddress->Device,
>> + PciAddress->Function,
>> + *Offset
>> + ),
>> + Size,
>> + Buffer
>> + );
>> + *Offset += Size;
>> +}
>> +
>> +
>> +/**
>> + Convenience wrapper macro for ReadConfigSpace().
>> +
>> + Given the following conditions:
>> +
>> + - HeaderField is the first field in the structure pointed-to by Struct,
>> +
>> + - Struct->HeaderField has been populated from the conventional PCI config
>> + space of the PCI device identified by PciAddress,
>> +
>> + - *Offset points one past HeaderField in the conventional PCI config space of
>> + the PCI device identified by PciAddress,
>> +
>> + populate the rest of *Struct from conventional PCI config space, starting at
>> + *Offset. Finally, increment *Offset so that it point one past *Struct.
>> +
>> + @param[in] PciAddress The address of the PCI Device -- Bus, Device, Function
>> + -- in UEFI (not PciLib) encoding. Type: pointer to
>> + CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS.
>> +
>> + @param[in,out] Offset On input, the offset in conventional PCI config space
>> + to start reading from; one past Struct->HeaderField.
>> + On output, the offset of the first byte that was not
>> + read; one past *Struct. Type: pointer to UINT8.
>> +
>> + @param[out] Struct The structure to complete. Type: pointer to structure
>> + object.
>> +
>> + @param[in] HeaderField The name of the first field in *Struct, after which
>> + *Struct should be populated. Type: structure member
>> + identifier.
>> +**/
>> +#define COMPLETE_CONFIG_SPACE_STRUCT(PciAddress, Offset, Struct, HeaderField) \
>> + ReadConfigSpace ( \
>> + (PciAddress), \
>> + (Offset), \
>> + (UINT8)(sizeof *(Struct) - sizeof ((Struct)->HeaderField)), \
>> + &((Struct)->HeaderField) + 1 \
>> + )
>
> There is a lot of assumptions about inputs with usage of this macro.
> (Even the in/out offset on ReadConfigSpace seems a little unexpected.)
>
> Based on that, I was hoping to come up with some suggestion. The two
> thoughts I had seemed more appropriate for a more generic PCI helper
> lib than for this driver:
>
> * A 'reader' struct that bundles the state of the pci address, read
> buffer and offset read.
>
> * More generically, a capabilities helper lib that could iterate &
> read the PCI capability structs.
*Absolutely* this ^^^
I've been much missing a canned function that just searches the config
space (conventional or extended, as requested) for a capability, and
returns an offset. Possibly taking an expected size parameter too, for
safety. PciBusDxe has such function(s), but they are not factored out.
I wondered if such a function (or a family of functions) should be part
of the PciLib class, but seeing the number of PciLib implementations, I
didn't even try. :/
I don't know if a standalone PciCapabilityLib class would be acceptable,
if it had (say) 3-4 functions in total. The IoFifo experience doesn't
seem to support that -- the IoFifo functions were ultimately added to
the IoLib class.
Also, I think I've implemented two config space scanning functions in my
career :) ; we'd probably want someone with more such experience to
*design* the lib class.
>
> Anyway, for this series:
>
> Reviewed-by: Jordan Justen <jordan.l.justen@intel.com>
Thank you!
Laszlo
>> +
>> +
>> +/**
>> + Look up the QEMU-specific Resource Reservation capability in the conventional
>> + config space of a Hotplug Controller (that is, PCI Bridge).
>> +
>> + This function performs as few config space reads as possible.
>> +
>> + @param[in] HpcPciAddress The address of the PCI Bridge -- Bus, Device,
>> + Function -- in UEFI (not PciLib) encoding.
>> +
>> + @param[out] ReservationHint The caller-allocated capability structure to
>> + populate from the PCI Bridge's config space.
>> +
>> + @retval EFI_SUCCESS The capability has been found, ReservationHint has
>> + been populated.
>> +
>> + @retval EFI_NOT_FOUND The capability is missing. The contents of
>> + ReservationHint are now indeterminate.
>> +**/
>> +STATIC
>> +EFI_STATUS
>> +QueryReservationHint (
>> + IN CONST EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *HpcPciAddress,
>> + OUT QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION *ReservationHint
>> +)
>> +{
>> + UINT16 PciVendorId;
>> + UINT16 PciStatus;
>> + UINT8 PciCapPtr;
>> + UINT8 Offset;
>> +
>> + //
>> + // Check the vendor identifier.
>> + //
>> + PciVendorId = PciRead16 (
>> + PCI_LIB_ADDRESS (
>> + HpcPciAddress->Bus,
>> + HpcPciAddress->Device,
>> + HpcPciAddress->Function,
>> + PCI_VENDOR_ID_OFFSET
>> + )
>> + );
>> + if (PciVendorId != QEMU_PCI_BRIDGE_VENDOR_ID_REDHAT) {
>> + return EFI_NOT_FOUND;
>> + }
>> +
>> + //
>> + // Check the Capabilities List bit in the PCI Status Register.
>> + //
>> + PciStatus = PciRead16 (
>> + PCI_LIB_ADDRESS (
>> + HpcPciAddress->Bus,
>> + HpcPciAddress->Device,
>> + HpcPciAddress->Function,
>> + PCI_PRIMARY_STATUS_OFFSET
>> + )
>> + );
>> + if ((PciStatus & EFI_PCI_STATUS_CAPABILITY) == 0) {
>> + return EFI_NOT_FOUND;
>> + }
>> +
>> + //
>> + // Fetch the start of the Capabilities List.
>> + //
>> + PciCapPtr = PciRead8 (
>> + PCI_LIB_ADDRESS (
>> + HpcPciAddress->Bus,
>> + HpcPciAddress->Device,
>> + HpcPciAddress->Function,
>> + PCI_CAPBILITY_POINTER_OFFSET
>> + )
>> + );
>> +
>> + //
>> + // Scan the Capabilities List until we find the terminator element, or the
>> + // Resource Reservation capability.
>> + //
>> + for (Offset = PciCapPtr & 0xFC;
>> + Offset > 0;
>> + Offset = ReservationHint->BridgeHdr.VendorHdr.Hdr.NextItemPtr & 0xFC) {
>> + BOOLEAN EnoughRoom;
>> +
>> + //
>> + // Check if the Resource Reservation capability would fit into config space
>> + // at this offset.
>> + //
>> + EnoughRoom = (BOOLEAN)(
>> + Offset <= PCI_MAX_CONFIG_OFFSET - sizeof *ReservationHint
>> + );
>> +
>> + //
>> + // Read the standard capability header so we can check the capability ID
>> + // (if necessary) and advance to the next capability.
>> + //
>> + ReadConfigSpace (
>> + HpcPciAddress,
>> + &Offset,
>> + (UINT8)sizeof ReservationHint->BridgeHdr.VendorHdr.Hdr,
>> + &ReservationHint->BridgeHdr.VendorHdr.Hdr
>> + );
>> + if (!EnoughRoom ||
>> + (ReservationHint->BridgeHdr.VendorHdr.Hdr.CapabilityID !=
>> + EFI_PCI_CAPABILITY_ID_VENDOR)) {
>> + continue;
>> + }
>> +
>> + //
>> + // Read the rest of the vendor capability header so we can check the
>> + // capability length.
>> + //
>> + COMPLETE_CONFIG_SPACE_STRUCT (
>> + HpcPciAddress,
>> + &Offset,
>> + &ReservationHint->BridgeHdr.VendorHdr,
>> + Hdr
>> + );
>> + if (ReservationHint->BridgeHdr.VendorHdr.Length !=
>> + sizeof *ReservationHint) {
>> + continue;
>> + }
>> +
>> + //
>> + // Read the rest of the QEMU bridge capability header so we can check the
>> + // capability type.
>> + //
>> + COMPLETE_CONFIG_SPACE_STRUCT (
>> + HpcPciAddress,
>> + &Offset,
>> + &ReservationHint->BridgeHdr,
>> + VendorHdr
>> + );
>> + if (ReservationHint->BridgeHdr.Type !=
>> + QEMU_PCI_BRIDGE_CAPABILITY_TYPE_RESOURCE_RESERVATION) {
>> + continue;
>> + }
>> +
>> + //
>> + // Read the body of the reservation hint.
>> + //
>> + COMPLETE_CONFIG_SPACE_STRUCT (
>> + HpcPciAddress,
>> + &Offset,
>> + ReservationHint,
>> + BridgeHdr
>> + );
>> + return EFI_SUCCESS;
>> + }
>> +
>> + return EFI_NOT_FOUND;
>> +}
>> +
>> +
>> /**
>> Returns a list of root Hot Plug Controllers (HPCs) that require
>> initialization during the boot process.
>>
>> @@ -401,18 +634,21 @@ GetResourcePadding (
>> OUT VOID **Padding,
>> OUT EFI_HPC_PADDING_ATTRIBUTES *Attributes
>> )
>> {
>> + EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *Address;
>> BOOLEAN DefaultIo;
>> BOOLEAN DefaultMmio;
>> RESOURCE_PADDING ReservationRequest;
>> EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *FirstResource;
>> + EFI_STATUS ReservationHintStatus;
>> + QEMU_PCI_BRIDGE_CAPABILITY_RESOURCE_RESERVATION ReservationHint;
>> +
>> + Address = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *)&HpcPciAddress;
>>
>> DEBUG_CODE (
>> - EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *Address;
>> CHAR16 *DevicePathString;
>>
>> - Address = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_PCI_ADDRESS *)&HpcPciAddress;
>> DevicePathString = ConvertDevicePathToText (HpcDevicePath, FALSE, FALSE);
>>
>> DEBUG ((EFI_D_VERBOSE, "%a: Address=%02x:%02x.%x DevicePath=%s\n",
>> __FUNCTION__, Address->Bus, Address->Device, Address->Function,
>> @@ -439,20 +675,143 @@ GetResourcePadding (
>> FirstResource = ReservationRequest.Padding +
>> ARRAY_SIZE (ReservationRequest.Padding);
>>
>> //
>> - // (b) Reserve IO space.
>> + // Try to get the QEMU-specific Resource Reservation capability.
>> //
>> + ReservationHintStatus = QueryReservationHint (Address, &ReservationHint);
>> + if (!EFI_ERROR (ReservationHintStatus)) {
>> + INTN HighBit;
>> +
>> + DEBUG ((
>> + DEBUG_VERBOSE,
>> + "%a: BusNumbers=0x%x Io=0x%Lx NonPrefetchable32BitMmio=0x%x\n"
>> + "%a: Prefetchable32BitMmio=0x%x Prefetchable64BitMmio=0x%Lx\n",
>> + __FUNCTION__,
>> + ReservationHint.BusNumbers,
>> + ReservationHint.Io,
>> + ReservationHint.NonPrefetchable32BitMmio,
>> + __FUNCTION__,
>> + ReservationHint.Prefetchable32BitMmio,
>> + ReservationHint.Prefetchable64BitMmio
>> + ));
>> +
>> + //
>> + // (a) Reserve bus numbers.
>> + //
>> + switch (ReservationHint.BusNumbers) {
>> + case 0:
>> + //
>> + // No reservation needed.
>> + //
>> + break;
>> + case MAX_UINT32:
>> + //
>> + // Firmware default (unspecified). Treat it as "no reservation needed".
>> + //
>> + break;
>> + default:
>> + //
>> + // Request the specified amount.
>> + //
>> + --FirstResource;
>> + FirstResource->ResType = ACPI_ADDRESS_SPACE_TYPE_BUS;
>> + FirstResource->AddrLen = ReservationHint.BusNumbers;
>> + break;
>> + }
>> +
>> + //
>> + // (b) Reserve IO space.
>> + //
>> + switch (ReservationHint.Io) {
>> + case 0:
>> + //
>> + // No reservation needed, disable our built-in.
>> + //
>> + DefaultIo = FALSE;
>> + break;
>> + case MAX_UINT64:
>> + //
>> + // Firmware default (unspecified). Stick with our built-in.
>> + //
>> + break;
>> + default:
>> + //
>> + // Round the specified amount up to the next power of two. If rounding is
>> + // successful, reserve the rounded value. Fall back to the default
>> + // otherwise.
>> + //
>> + HighBit = HighBitSetRoundUp64 (ReservationHint.Io);
>> + if (HighBit != -1) {
>> + SetIoPadding (--FirstResource, (UINTN)HighBit);
>> + DefaultIo = FALSE;
>> + }
>> + break;
>> + }
>> +
>> + //
>> + // (c) Reserve non-prefetchable MMIO space (32-bit only).
>> + //
>> + switch (ReservationHint.NonPrefetchable32BitMmio) {
>> + case 0:
>> + //
>> + // No reservation needed, disable our built-in.
>> + //
>> + DefaultMmio = FALSE;
>> + break;
>> + case MAX_UINT32:
>> + //
>> + // Firmware default (unspecified). Stick with our built-in.
>> + //
>> + break;
>> + default:
>> + //
>> + // Round the specified amount up to the next power of two. If rounding is
>> + // successful, reserve the rounded value. Fall back to the default
>> + // otherwise.
>> + //
>> + HighBit = HighBitSetRoundUp32 (ReservationHint.NonPrefetchable32BitMmio);
>> + if (HighBit != -1) {
>> + SetMmioPadding (--FirstResource, FALSE, TRUE, (UINTN)HighBit);
>> + DefaultMmio = FALSE;
>> + }
>> + break;
>> + }
>> +
>> + //
>> + // (d) Reserve prefetchable MMIO space (either 32-bit or 64-bit, never
>> + // both).
>> + //
>> + // For either space, we treat 0 as "no reservation needed", and the maximum
>> + // value as "firmware default". The latter is unspecified, and we interpret
>> + // it as the former.
>> + //
>> + // Otherwise, round the specified amount up to the next power of two. If
>> + // rounding is successful, reserve the rounded value. Do not reserve
>> + // prefetchable MMIO space otherwise.
>> + //
>> + if (ReservationHint.Prefetchable32BitMmio > 0 &&
>> + ReservationHint.Prefetchable32BitMmio < MAX_UINT32) {
>> + HighBit = HighBitSetRoundUp32 (ReservationHint.Prefetchable32BitMmio);
>> + if (HighBit != -1) {
>> + SetMmioPadding (--FirstResource, TRUE, TRUE, (UINTN)HighBit);
>> + }
>> + } else if (ReservationHint.Prefetchable64BitMmio > 0 &&
>> + ReservationHint.Prefetchable64BitMmio < MAX_UINT64) {
>> + HighBit = HighBitSetRoundUp64 (ReservationHint.Prefetchable64BitMmio);
>> + if (HighBit != -1) {
>> + SetMmioPadding (--FirstResource, TRUE, FALSE, (UINTN)HighBit);
>> + }
>> + }
>> + }
>> +
>> if (DefaultIo) {
>> //
>> // Request defaults.
>> //
>> SetIoPadding (--FirstResource, (UINTN)HighBitSetRoundUp64 (512));
>> }
>>
>> - //
>> - // (c) Reserve non-prefetchable MMIO space (32-bit only).
>> - //
>> if (DefaultMmio) {
>> //
>> // Request defaults.
>> //
>> --
>> 2.14.1.3.gb7cf6e02401b
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU
2017-09-25 19:58 [PATCH 0/7] OvmfPkg/PciHotPlugInitDxe: obey PCI resource reservation hints from QEMU Laszlo Ersek
` (6 preceding siblings ...)
2017-09-25 19:58 ` [PATCH 7/7] OvmfPkg/PciHotPlugInitDxe: translate QEMU's resource reservation hints Laszlo Ersek
@ 2017-10-03 14:09 ` Laszlo Ersek
7 siblings, 0 replies; 12+ messages in thread
From: Laszlo Ersek @ 2017-10-03 14:09 UTC (permalink / raw)
To: edk2-devel-01
Cc: Marcel Apfelbaum, Jordan Justen, Michael D Kinney, Ruiyu Ni,
Liming Gao
On 09/25/17 21:58, Laszlo Ersek wrote:
> Repo: https://github.com/lersek/edk2.git
> Branch: pci_resource_reserve
>
> "OvmfPkg/PciHotPlugInitDxe" implements EFI_PCI_HOT_PLUG_INIT_PROTOCOL
> from the PI spec. Currently it returns fixed resource reservations (512B
> IO space and 2MB non-prefetchable 32-bit MMIO space) for all bridges.
> This is not flexible enough:
>
> (1) For PCI Express bridges (such as root ports and downstream ports),
> reserving any IO space is wasteful, and limits the number of ports.
> (This is because each such port can only accept one (multi-function)
> device, and because the 512B reservation is rounded up to 4KB, and
> because the IO space under OVMF/Q35 consists of 10 * 4KB.)
>
> (2) For hot-plugging PCI Express devices with large MMIO BARs into such
> ports, the reservation of only 2MB MMIO (and even that limited to
> non-prefetchable, hence 32-bit, address space) is insufficient.
>
> QEMU has recently gained the ability to control these reservation sizes
> on the command line. At this point the hints are only supported for the
> "pcie-root-port" device -- which implies the Q35 machine type, given
> that i440fx only supports conventional PCI, not PCI Express --, with the
> following properties:
>
> - pcie-root-port.bus-reserve=uint32
> - pcie-root-port.io-reserve=size
> - pcie-root-port.mem-reserve=size
> - pcie-root-port.pref32-reserve=size
> - pcie-root-port.pref64-reserve=size
>
> The hints are exposed to the guest in the conventional config space of
> the device, using a vendor-specific capability (which is documented in
> the QEMU source tree; see all references in the individual patches). It
> is expected that, if any future hotplug controllers in QEMU gain the
> same ability, they will reuse the mechanism identically.
>
> This series generally implements the parsing of this capability in
> OvmfPkg/PciHotPlugInitDxe, and the translation thereof to the ACPI
> address space descriptor format that the Platform Init spec defines for
> EFI_PCI_HOT_PLUG_INIT_PROTOCOL.GetResourcePadding().
>
> The first patch is for "MdePkg/IndustryStandard"; it introduces the
> vendor-specific capability header, from the PCI 2.3 spec. The rest of
> the patches is for OvmfPkg.
>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Marcel Apfelbaum <marcel@redhat.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>
> Thanks
> Laszlo
>
> Laszlo Ersek (7):
> MdePkg/IndustryStandard/Pci23: add vendor-specific capability header
> OvmfPkg/IndustryStandard: define PCI Capabilities for QEMU's PCI
> Bridges
> OvmfPkg/PciHotPlugInitDxe: clean up protocol usage comment
> OvmfPkg/PciHotPlugInitDxe: clean up addr. range for non-prefetchable
> MMIO
> OvmfPkg/PciHotPlugInitDxe: generalize RESOURCE_PADDING composition
> OvmfPkg/PciHotPlugInitDxe: add helper functions for setting up
> paddings
> OvmfPkg/PciHotPlugInitDxe: translate QEMU's resource reservation hints
>
> MdePkg/Include/IndustryStandard/Pci23.h | 10 +
> OvmfPkg/Include/IndustryStandard/QemuPciBridgeCapabilities.h | 60 ++
> OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 661 ++++++++++++++++++--
> OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf | 6 +-
> 4 files changed, 674 insertions(+), 63 deletions(-)
> create mode 100644 OvmfPkg/Include/IndustryStandard/QemuPciBridgeCapabilities.h
>
Thanks All for the feedback, pushed as commit range
9425b34925d0..fe4049471bdf.
Laszlo
^ permalink raw reply [flat|nested] 12+ messages in thread