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

["Marcin Wojtas" <mw@semihalf.com>]

Hi Ard,

pon., 2 sie 2021 o 10:43 Ard Biesheuvel <ardb@kernel.org> napisał(a):
>
> On Mon, 2 Aug 2021 at 07:01, Marcin Wojtas <mw@semihalf.com> 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@semihalf.com>
> > ---
> >  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
> >