From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ua1-f48.google.com (mail-ua1-f48.google.com [209.85.222.48]) by mx.groups.io with SMTP id smtpd.web08.14360.1639417462886777602 for ; Mon, 13 Dec 2021 09:44:23 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=DuGz/zrw; spf=pass (domain: gmail.com, ip: 209.85.222.48, mailfrom: ch.chandni@gmail.com) Received: by mail-ua1-f48.google.com with SMTP id t13so30506204uad.9 for ; Mon, 13 Dec 2021 09:44:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=rmF3bBObxSm5tJLaDf9FofT62rCrvFSNA4FAMVVAUEI=; b=DuGz/zrwnE+0ktP4jMqZJMX+Eb5LyVPvt8T8mNW2wEOUj9MDUi6jfzGIPg0bT3QUlk MB1TuWW8+d0+hjwV+Zy38ho7BYKBqVanFyzPj2n8yfhGgDYpDqYjYLvQzRgz5J471mGg 7x3bRT29LbWVkxY99nEylWT/GuVzC1kutLVCyLn4JPGyWNyVar71rEf64Jh1I0o66gfi q4FFKUPLql/13YJvTuu6mwjRnHptMn3/5zBphvIks+bOgz3Osm3KleTDVg9NnrDoxwBC Budk3bNYH0CvIYzgaALSI7J+KsncYgOJRRtfjjvfarWuTmAA6RfrACx8gsCfBbroOSOP mCTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=rmF3bBObxSm5tJLaDf9FofT62rCrvFSNA4FAMVVAUEI=; b=WOM65Ycwdf4KAAYow2DF3iWUqN/QF2Kkyu8UCEy8sKFAntWQphr/eRWnT3ImE2CInP 9uTABgYDhfWGmsOt7/ahjPiDDbeqW0Yb38vRV3rREldAV3cQUQLQb7kMBSwvHttz+1Sn 2+WTYz8KfLD8nvMnq+movvnBrepZQcpTjYW2ZZTnu/mF+Li+0Tif7hguuZtT21b9usNf gXnXWt6hUPSH12jFJ+KsEuX9Ocnd5dO8R2cBtFQMlmzkW3LTb6yiWuJmAwrvYIhSTOeN EyZHypFLbiZqoBd3KKir2Tpg4k73+bH1u1yL65wm/um56KibDlVR2LRfH+SB/INj+/Jq vUrg== X-Gm-Message-State: AOAM533uq+Z/CtPUD3RrbNqFZLTgSiPRksCdzrodj/ps7VJgDeop6Wyk YS1m1MCBTsFFt61/aS+SuUwXao8JIe81ph0iPNA= X-Google-Smtp-Source: ABdhPJzZ16KOSTZnFPU2LHBWrZm/L+XXMzGXDpsAjXGaSzO0HWTlEr8ECNkYu23Bup3ULkl6tO3Kraf3Et7K1QyTCYo= X-Received: by 2002:a67:745:: with SMTP id 66mr18660vsh.43.1639417460341; Mon, 13 Dec 2021 09:44:20 -0800 (PST) MIME-Version: 1.0 References: <2d695d9d-eb38-f8d0-d7bd-d77dbe609ec3@arm.com> <8622.1638932156872484301@groups.io> In-Reply-To: From: "chandni cherukuri" Date: Mon, 13 Dec 2021 23:14:09 +0530 Message-ID: Subject: Re: [edk2-devel] [edk2-platforms][PATCH V1 07/11] Platform/ARM/Morello: Port PCI Express library To: Ard Biesheuvel , edk2-devel-groups-io Cc: khasim.mohammed@arm.com, Sami Mujawar , Ard Biesheuvel , Leif Lindholm , Pierre Content-Type: text/plain; charset="UTF-8" On Mon, Dec 13, 2021 at 8:08 PM Ard Biesheuvel wrote: > > 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. > > Thanks for the response. Will implement a single custom PCIe library and post a new patchset. Thanks Chandni > > 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 > > > > >