From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mx.groups.io with SMTP id smtpd.web08.1827.1627973594454468389 for ; Mon, 02 Aug 2021 23:53:14 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=iYH5kX/l; spf=pass (domain: kernel.org, ip: 198.145.29.99, mailfrom: ardb@kernel.org) Received: by mail.kernel.org (Postfix) with ESMTPSA id 9E7D860FA0 for ; Tue, 3 Aug 2021 06:53:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1627973593; bh=busRNhQK60jioVYoC6d2lkX0d0VPjuSMwHvAAnJddMM=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=iYH5kX/lWrLlk2itLpd3V07z8M2lAPChyU035lYBy6Pfi773tdyO+Ts3X4tnLxcLB JWOlrRdU4+MnWkCEYgKLB/IVPFmJ5d9S4Mf47sLPWRTlC9Bt9D45fMBKhLxGarQs5f f19O73d4SCMPVhh1ewJy342Kb3u47ESU+KPs93YV88GJWoL50CHH9+5QCXV8lvMiY/ ORK/BQuLzgWcqYxGcY4pcVCT7MiAoIHtN27CSYTKigZav8KrSYORDtfhqY3cyEK5jI NhNbnXSQABwIoEdbJaIz7jWHlwF+ZntABUfLp186olZBAuzTjMo8JlaMhi8tKyHQ3o JQ+sEwXDV+1dA== Received: by mail-ot1-f53.google.com with SMTP id c7-20020a9d27870000b02904d360fbc71bso19782278otb.10 for ; Mon, 02 Aug 2021 23:53:13 -0700 (PDT) X-Gm-Message-State: AOAM530GR0nUbbXoOybiBBv9+eNnuX5w6VqoWGuu+dmRc6upFVCj2l8Q CiVhUrk2IPL+r45nJWQQsXXzx5Ai1VWd8pHc5Cg= X-Google-Smtp-Source: ABdhPJyFmOfT9WAsxzvPaV0ik7P0mZ7aVToVh6mpTDjKL7g7ce5QTCbBhGj6dDCmLrfX8v6xImgW9HdvS0sLiz6BqxA= X-Received: by 2002:a9d:5cb:: with SMTP id 69mr14311969otd.90.1627973592925; Mon, 02 Aug 2021 23:53:12 -0700 (PDT) MIME-Version: 1.0 References: <20210802050051.2831716-1-mw@semihalf.com> <20210802050051.2831716-4-mw@semihalf.com> In-Reply-To: From: "Ard Biesheuvel" Date: Tue, 3 Aug 2021 08:53:01 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-platforms PATCH 3/6] Marvell: Armada7k8kPciHostBridgeLib: Remove ECAM base limitation To: Marcin Wojtas Cc: edk2-devel-groups-io , Leif Lindholm , Ard Biesheuvel , Grzegorz Jaszczyk , Grzegorz Bernacki , upstream@semihalf.com, Samer El-Haj-Mahmoud , Jon Nettleton Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2 Aug 2021 at 19:00, Marcin Wojtas wrote: > > Hi Ard, > > pon., 2 sie 2021 o 10:43 Ard Biesheuvel napisa=C5=82(a)= : > > > > On Mon, 2 Aug 2021 at 07:01, Marcin Wojtas 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 :-) > 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 > > > > > Signed-off-by: Marcin Wojtas > > > --- > > > Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/PciHos= tBridgeLibConstructor.c | 1 - > > > 1 file changed, 1 deletion(-) > > > > > > diff --git a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBrid= geLib/PciHostBridgeLibConstructor.c b/Silicon/Marvell/Armada7k8k/Library/Ar= mada7k8kPciHostBridgeLib/PciHostBridgeLibConstructor.c > > > index 067e57a2dc..87e57aeae3 100644 > > > --- a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/P= ciHostBridgeLibConstructor.c > > > +++ b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kPciHostBridgeLib/P= ciHostBridgeLibConstructor.c > > > @@ -219,7 +219,6 @@ Armada7k8kPciHostBridgeLibConstructor ( > > > PcieController =3D &(BoardPcieDescription->PcieControllers[Index= ]); > > > > > > ASSERT (PcieController->PcieBusMin =3D=3D 0); > > > - ASSERT (PcieController->ConfigSpaceAddress % SIZE_256MB =3D=3D 0= ); > > > > > > if (PcieController->HaveResetGpio =3D=3D TRUE) { > > > /* Reset PCIE slot */ > > > -- > > > 2.29.0 > > >