From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web10.11798.1639406286542438869 for ; Mon, 13 Dec 2021 06:38:07 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=DmO5XK4W; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 2FD93B810E0 for ; Mon, 13 Dec 2021 14:38:04 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D775EC34603 for ; Mon, 13 Dec 2021 14:38:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1639406282; bh=+6GE/iD82qW0EFkuNpFdak2/BVXm8fpEwVskayqz3nc=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=DmO5XK4WtvmseFEOMqHKYL50F7ADYwYW4PXXyi0+JB1iviAeVL7DSR7uvsC/XeY9y msDJUkWo/xOfa0TieDd3cyHyj7bAX7un4mKGymPktJ4v1fRkc1LP3YYu8KjvvzomwG /Tc3a8yQJDWkOUd9ye/tTnuK1nGRAi0JgYE82Wyal+RvvmQpiA2FehZ2fHx9RpYtva Wm91P54GiUbmc9RokAQ8la4ldYurgogNY3ltWZMW6DtsAC4ciStD9zcL4Cj1xHZ2M/ j1ViYKGNlUvJFLqt6SKiwrX9AYpZ849JhSerBYDAfMSmNF7K99FKdCSgrB00/JPav7 7XumRUQYoX2SA== Received: by mail-oi1-f180.google.com with SMTP id be32so23338987oib.11 for ; Mon, 13 Dec 2021 06:38:02 -0800 (PST) X-Gm-Message-State: AOAM530LJ5tDdfc+Uvxovhj5XjE/w0MY1PsfGPZEVjIFG9O07o1ysppw UmNGkSaQHuvtddr6oI+IaK6lBsOv4ycZ2ODhhaE= X-Google-Smtp-Source: ABdhPJyJYHofoz29pH7CMwXRonbghWdSG/wk6EA8JPI4pnTA9q9PGjx7Rp5TiaSoV3e2SX7GsoUnZBwPj+jkZU1No8s= X-Received: by 2002:aca:ad95:: with SMTP id w143mr27478894oie.47.1639406282057; Mon, 13 Dec 2021 06:38:02 -0800 (PST) MIME-Version: 1.0 References: <2d695d9d-eb38-f8d0-d7bd-d77dbe609ec3@arm.com> <8622.1638932156872484301@groups.io> In-Reply-To: From: "Ard Biesheuvel" Date: Mon, 13 Dec 2021 15:37:51 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1 07/11] Platform/ARM/Morello: Port PCI Express library To: chandni cherukuri Cc: edk2-devel-groups-io , khasim.mohammed@arm.com, Sami Mujawar , Ard Biesheuvel , Leif Lindholm , Pierre Content-Type: text/plain; charset="UTF-8" On Mon, 13 Dec 2021 at 13:41, chandni cherukuri wrote: > > Hi Ard, > > Thanks for the response. > > I had a look at the Synquacer platform where there is a single custom > PCIe library implemented. > So, is it better to implement a single custom PCIe library instead of > two custom PCIe libraries? > Yes, for two reasons: - there is prior art for this solution, and solving the same problem in two different ways in the same codebase is bad form; - those two libraries are layered one on top of the other, which means that your fix requires changes to two different layers, making the interface between those two layers obviously unfit to represent the hardware in question. > If that is the case then will implement a single custom PCIe library > for Morello. > > Thanks > Chandni > On Fri, Dec 10, 2021 at 4:12 PM Ard Biesheuvel wrote: > > > > On Thu, 9 Dec 2021 at 13:30, chandni cherukuri wrote: > > > > > > Hi Ard, Leif, Sami, Pierre, > > > > > > For Morello platform, we created two custom libraries based on the > > > below two native libraries: > > > - https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BasePciSegmentLibPci > > > - https://github.com/tianocore/edk2/tree/master/MdePkg/Library/BasePciExpressLib > > > > > > because of the following reasons. > > > 1. In Morello platform, the ECAM region of the two root ports > > > are placed in non-contiguous memory as per the memory map architecture > > > of the Morello platform. The native PCI Express Library expects both > > > the ECAM regions for all the root ports to be contiguous. > > > > Do you mean root ports or host bridges? If root ports don't share the > > MMIO resource windows provided by the respective host bridges, they > > are really different host bridges and should be modeled as such. > > > > Note that the Cadence and Synopsys IP usually does not implement a > > root port at all, and gets away with this because it only implements a > > single link. > > > > > 2. IORT and kernel currently require each root port to be in a > > > separate segment. You can refer to the code for more details - > > > > > > > Please take a look at the SynQuaecer platform for inspiration. That > > platform is very similar in this respect. > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/acpi/arm64/iort.c#n307. > > > The native PCI Segment Library supports only a single segment. > > > > > > Because of these reasons, the current patch series adapts the native > > > libraries as follows: > > > > > > The custom PCI Segment Library passes the complete address which is > > > consumed by the custom PCI Express library where based on the Segment > > > number, the base address of the PCI Express is returned. > > > > > > The rationale behind maintaining two separate libraries is that when > > > the software evolves and support for multiple segments is adapted in > > > the native PCI Segment Library the custom library could be removed. > > > Also, we might have other protocols which might try to use the PCI > > > Express library directly. However, there are some other platforms > > > where all the platform specific changes have gone in a single custom > > > PCI library > > > > > > Could you please provide some inputs as to which of two approaches > > > would be better to follow as there is one more platform to follow > > > based on the decision taken? > > > > > > Thanks > > > Chandni > > > > > > On Wed, Dec 8, 2021 at 8:25 AM Khasim Mohammed wrote: > > > > > > > > Hi Sami, Chandni, > > > > > > > > There was a suggestion from Pierre on a similar patch for N1SDP to remove PCIExpressLib.c and move to workarounds to PCISegmentLib.c, > > > > > > > > https://edk2.groups.io/g/devel/message/84165?p=%2C%2C%2C20%2C0%2C0%2C0%3A%3Arecentpostdate%2Fsticky%2C%2Ckhasim%2C20%2C2%2C0%2C87257273 > > > > > > > > I think we should discuss this implementation for both N1SDP and Morello as they have similar implementation. > > > > > > > > Regards, > > > > Khasim > > > >