Re: [edk2-platforms PATCH 3/6] Marvell: Armada7k8kPciHostBridgeLib: Remove ECAM base limitation


Marcin Wojtas
 

Hi Ard,

pon., 2 sie 2021 o 10:43 Ard Biesheuvel <ardb@...> napisaƂ(a):

On Mon, 2 Aug 2021 at 07:01, Marcin Wojtas <mw@...> wrote:

On CN913x-based platforms it is possible to have up to 9 PCIE
root complexes. In such case it may be necessary to configure
more configuration spaces with smaller bus count, so that
to fit the memory layout constraints. For that purpose remove
forcing ECAM base to be divisible by SIZE_256MB.
There is one subtlety here that we need to take into account: IIUC,
PCIe requires that the ECAM start address of bus N equals N MB modulo
256 MB. In other words, if your ECAM range lives at 1 GB + 128 MB, the
bus range has to start at bus 128.

I think OSes are usually quite lax about this, but it is something to
double check regardless, even for existing platforms
I tested a wide range of OSs (various Linux distributions, Win10 PE,
FreeBSD, OpenBSD and of course EDK2) and with 7 ECAMs, of which 6 are
squeezed within 256MB memory chunk together with their mmio32 and no
issue was observed. Moreover, if you recall, contrary to the EDK2,
where the full bus number is used, in ACPI we expose a single 1MB
space with the ECAM base address aligned to 0x8000.

Do you wish to change the assertion in EDK2 instead of removing?

Thanks,
Marcin


Signed-off-by: Marcin Wojtas <mw@...>
---
Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.c b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.c
index 067e57a2dc..87e57aeae3 100644
--- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.c
+++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.c
@@ -219,7 +219,6 @@ Armada7k8kPciHostBridgeLibConstructor (
PcieController = &(BoardPcieDescription->PcieControllers[Index]);

ASSERT (PcieController->PcieBusMin == 0);
- ASSERT (PcieController->ConfigSpaceAddress % SIZE_256MB == 0);

if (PcieController->HaveResetGpio == TRUE) {
/* Reset PCIE slot */
--
2.29.0

Join devel@edk2.groups.io to automatically receive all group messages.