From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=156.151.31.86; helo=userp2130.oracle.com; envelope-from=liran.alon@oracle.com; receiver=edk2-devel@lists.01.org Received: from userp2130.oracle.com (userp2130.oracle.com [156.151.31.86]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id C2488210F4BBC for ; Fri, 7 Sep 2018 10:01:56 -0700 (PDT) Received: from pps.filterd (userp2130.oracle.com [127.0.0.1]) by userp2130.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w87GxMwp042277; Fri, 7 Sep 2018 17:01:54 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=content-type : mime-version : subject : from : in-reply-to : date : cc : content-transfer-encoding : message-id : references : to; s=corp-2018-07-02; bh=hkvH1/yZ9UkUTmvJRm/AsdVv1nMDWwmu7ctUDMu7H+s=; b=qlrcfd9xwh2C288toggBOrQzr5GKTatfs0WqDUykFRWhSl53u427DkgAiCKYGjMa+sl1 kN4NRud0jbGIyor/F8M/68TZ8KfnEUh//RtI33AE0aUyRd3lmfEYDDBlL5vwu/lpIVqw W+wpY3BkOQHO+fxIZumyjTixdfYSY5cAoWYO6rPKDIAflDMn5ltyXoQTYsc3be4tk3bd Hi+NFsVhodOE3G8dII63YEHS2MYw0t5E/CRkbO/CcsDOVv66+LOiJ9IdkT+EVW8UeiAh wuz7Q+Of8Fv1KDkL3YSkFup+iTgdUhZqzuFaVIG6VHYKr3tVGoBRasqe6sfy6n53BRt/ AA== Received: from userv0021.oracle.com (userv0021.oracle.com [156.151.31.71]) by userp2130.oracle.com with ESMTP id 2m7j6u3t07-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 07 Sep 2018 17:01:54 +0000 Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by userv0021.oracle.com (8.14.4/8.14.4) with ESMTP id w87H1swK003894 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 7 Sep 2018 17:01:54 GMT Received: from abhmp0010.oracle.com (abhmp0010.oracle.com [141.146.116.16]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id w87H1rHR005053; Fri, 7 Sep 2018 17:01:53 GMT Received: from [192.168.14.112] (/79.181.249.233) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Fri, 07 Sep 2018 10:01:53 -0700 Mime-Version: 1.0 (Mac OS X Mail 11.1 \(3445.4.7\)) From: Liran Alon In-Reply-To: <2fdb6059-86a4-a8d5-a46c-286c62e17864@redhat.com> Date: Fri, 7 Sep 2018 20:01:49 +0300 Cc: Nikita Leshenko , edk2-devel@lists.01.org, Ard Biesheuvel Message-Id: <8A63AC46-22FF-480A-A109-9902B7076464@oracle.com> References: <2fdb6059-86a4-a8d5-a46c-286c62e17864@redhat.com> To: Laszlo Ersek X-Mailer: Apple Mail (2.3445.4.7) X-Proofpoint-Virus-Version: vendor=nai engine=5900 definitions=9009 signatures=668708 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 malwarescore=0 phishscore=0 bulkscore=0 spamscore=0 mlxscore=0 mlxlogscore=925 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1807170000 definitions=main-1809070169 Subject: Re: PciBusDxe: PCI-Express bug with dynamic PcdPciExpressBaseAddress X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Sep 2018 17:01:57 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > On 7 Sep 2018, at 11:44, Laszlo Ersek wrote: >=20 > (+Ard) >=20 > On 09/06/18 21:08, Nikita Leshenko wrote: >> Hi, >>=20 >> We ran into a bug in EDK2 relating to PCI-Express in PciBusDxe. = Here's the flow >> of the bug: >>=20 >> 1. PciBusDxe/PciEnumeratorSupport.c: Function BarExisted probes a = BAR. It raises >> TPL to TPL_HIGH_LEVEL to avoid timer interrupts while probing the = BAR and >> calls PciIo->Pci.Write. >> 2. BasePciExpressLib/PciExpressLib.c: The write reaches = PciExpressWrite32, which >> calls GetPciExpressBaseAddress. >> 3. GetPciExpressBaseAddress retrieves the address from = PcdPciExpressBaseAddress. >> 4. Reading the PCD calls DxePcdGet64 -> GetWorker -> >> EfiAcquireLock(&mPcdDatabaseLock), which is at TPL_NOTIFY level. = This crashes >> the firmware because step 1 raised the TPL to TPL_HIGH_LEVEL. >>=20 >> This doesn't happen when PcdPciExpressBaseAddress is fixed at build = (because >> then the read is optimized to a static global variable), but when the = PCD is >> dynamic PCI-Express is broken. >>=20 >> Does anybody have a suggestion for fixing it? >>=20 >> Options we thought about: >> - Change mPcdDatabaseLock.Tpl to TPL_HIGH_LEVEL >> - Don't use a PCD for the base address, put it in a static global = variable and >> create functions to set and retrieve it. >=20 > In the ArmVirtPkg platforms, we also set "PcdPciExpressBaseAddress" > dynamically. And, we implemented your second option above; see: >=20 > ArmVirtPkg/Library/BaseCachingPciExpressLib/ >=20 > Relevant commits: >=20 > - ad3359eb43a9 ("ArmVirtualizationPkg: clone BasePciExpressLib, cache > PCIe config base", 2015-02-23) > - a06d0bb58eb9 ("ArmVirtPkg/BaseCachingPciExpressLib: depend on > PciPcdProducerLib", 2016-04-12) >=20 > (In fact, commit ad3359eb43a9 documents the exact issue you report = here.) >=20 > Thanks > Laszlo Thanks Lazlo. Weren=E2=80=99t aware of this solution in the ArmVirtPkg = platforms. However, I wonder why the solution was to clone the = MdePkg/Library/BasePciExpressLib rather than change the original library itself? That is, what was the reason for not just adding a library constructor = to MdePkg/Library/BasePciExpressLib to cache PcdPciExpressBaseAddress in a global var? This seem to solve = the issues for all platforms. If PcdPciExpressBaseAddress is fixed and doesn=E2=80=99t change = dynamically, then the caching of the value in a global var should be harmless (besides adding an extra read from global-var). If it = does change dynamically,=20 MdePkg/Library/BasePciExpressLib have the issue discussed in this email = thread. Thanks, -Liran