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

[Marcin Wojtas]

wt., 3 sie 2021 o 08:53 Ard Biesheuvel <ardb at kernel.org> napisał(a):
>
> On Mon, 2 Aug 2021 at 19:00, Marcin Wojtas <mw at semihalf.com> wrote:
> >
> > Hi Ard,
> >
> > pon., 2 sie 2021 o 10:43 Ard Biesheuvel <ardb at kernel.org> napisał(a):
> > >
> > > On Mon, 2 Aug 2021 at 07:01, Marcin Wojtas <mw at 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.
> >
>
> Ah yes, I had forgotten about that hack :-)

A great one though.

>
> > Do you wish to change the assertion in EDK2 instead of removing?
> >
>
> No worries - if all those OSes are fine with this, I don't see a point
> in being pedantic. I will note, however, that you can still comply
> with this requirement by changing the bus ranges: each RC only uses a
> single bus, but that bus number could be (ECAM base address / 1M) %
> 256
>

For OS's there is indeed only bus0 exposed, but I plan to make it
tunable, so that to use entire range (e.g. for FreeBSD). In EDK2 there
is full coverage. FYI, in the platform I plan to submit after this
patchset there 7 RC's: 1 with 255 and 6 with 15 busses (the last 1 MB
in each case is used for IO space).

Best regards,
Marcin

>
> > >
> > > > Signed-off-by: Marcin Wojtas <mw at 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
> > > >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78586): https://edk2.groups.io/g/devel/message/78586
Mute This Topic: https://groups.io/mt/84605051/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-