From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id DBB3F740035 for ; Tue, 7 Nov 2023 18:54:25 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=GSjw5Clhnl/OmrusAV8Umzu9lqiXaB2Abya3cXNc+T8=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1699383264; v=1; b=AvJntoo/tP2SLAutXmjx+rzpKTvrNzHe7bineiK6d0NyYzya0JfQtUWInU6Y+GXmbAkIebDQ ovlT3yp0naDHwXydTyU75AfiUFLp+bCVXFBNkufqx00nLJ/xC3iRyjt4kx6SjlqV3c9w1NnMZPV Bqz23kAjf7jTpphv/DxCRrrA= X-Received: by 127.0.0.2 with SMTP id 7ReDYY7687511xbPHrajwgzM; Tue, 07 Nov 2023 10:54:24 -0800 X-Received: from mail-yb1-f175.google.com (mail-yb1-f175.google.com [209.85.219.175]) by mx.groups.io with SMTP id smtpd.web10.3051.1699383263947523958 for ; Tue, 07 Nov 2023 10:54:24 -0800 X-Received: by mail-yb1-f175.google.com with SMTP id 3f1490d57ef6-d9ac3b4f42cso6235387276.0 for ; Tue, 07 Nov 2023 10:54:23 -0800 (PST) X-Gm-Message-State: FQZN4cP1TZWTz6GStWxnvhRHx7686176AA= X-Google-Smtp-Source: AGHT+IG0wutkCZfTUEBZfyyf0O12ZZVEzTZHE4s1Y9sOEyFLCV4haOh/DOBjOnaMoLnXLrsPlbe+5QT21L28Kvcr+m0= X-Received: by 2002:a25:4209:0:b0:da0:454d:cf57 with SMTP id p9-20020a254209000000b00da0454dcf57mr2863960yba.16.1699383262932; Tue, 07 Nov 2023 10:54:22 -0800 (PST) MIME-Version: 1.0 References: <34167f0f873c01654aee0e9c8629221dad241529.1699322498.git.jlotwo@gmail.com> In-Reply-To: <34167f0f873c01654aee0e9c8629221dad241529.1699322498.git.jlotwo@gmail.com> From: "Joe L" Date: Tue, 7 Nov 2023 10:54:11 -0800 Message-ID: Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/PciHostBridgeDxe: Add readback after final Cfg-Write To: devel@edk2.groups.io Cc: Leif Lindholm , Ard Biesheuvel , Liming Gao , Michael Brown , Pedro Falcato , Ray Ni , Hao A Wu , Jian J Wang , Sami Mujawar Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,jlotwo@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="000000000000b98fe30609947fcf" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b="AvJntoo/"; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --000000000000b98fe30609947fcf Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable ++ CC'd On Mon, Nov 6, 2023 at 6:02=E2=80=AFPM Joe Lopez wrote: > From: joelopez333 > > REF:https://edk2.groups.io/g/devel/topic/102310377#110456 > > Problem Report: > > On AARCH64, there is no ordering guarantee between configuration > space (ECAM) writes and memory space reads (MMIO). ARM AMBA CHI > only guarantees ordering for reads and writes within a single address > region, > however, on some systems MMIO and ECAM may be split into separate > address regions. > > A problem may arise when an ECAM write is issued a completion before = a > subsequent > MMIO read is issued and receives a completion. > > For example, a typical PCI software flow is the following: > > 1. ECAM write to device command register to enable memory space > 2. MMIO read from device memory space for which access was enabled > in step 1. > > There is no guarantee that step 2. will not begin before the > completion of step 1. > on systems where ECAM/MMIO are specified as separate address regions, > even > if both spaces have the memory attributes device-nGnRnE. > > Fix: > > - Add a read after the final PCI Configuration space write > in RootBridgeIoPciAccess. > > - When configuration space is strongly ordered, this ensures > that program execution cannot continue until the completion > is received for the previous Cfg-Write, which may have side-effects= . > > - Risk of reading a "write-only" register and causing a CA which > leaves the device > unresponsive. The expectation based on the PCI Base Spec v6.1 > section 7.4 is that > all PCI Spec-defined registers will be readable, however, there may > exist > design-specific registers that fall into this category. > > Cc: Leif Lindholm > Cc: Ard Biesheuvel > Cc: Sami Mujawar > Cc: Jian J Wang > Cc: Liming Gao > Cc: Hao A Wu > Cc: Ray Ni > Cc: Pedro Falcato > Cc: Michael Brown > Signed-off-by: Joe Lopez > --- > MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > index 157a0ada80..c2dc2018d6 100644 > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > @@ -1238,6 +1238,14 @@ RootBridgeIoPciAccess ( > } > } > > + // > + // Perform readback after write to confirm completion was received for > the last write > + // before subsequent memory operations can be issued. > + // > + if (!Read) { > + PciSegmentRead8 (Address - InStride); > + } > + > return EFI_SUCCESS; > } > > -- > 2.25.1 > > -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110874): https://edk2.groups.io/g/devel/message/110874 Mute This Topic: https://groups.io/mt/102435564/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --000000000000b98fe30609947fcf Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Mon, Nov 6, 2023 at 6:02=E2=80= =AFPM Joe Lopez <j= lotwo@gmail.com> wrote:
From: joelopez333 <jlotwo@gmail.com>

=C2=A0 =C2=A0 REF:https://edk2.groups.io/g/deve= l/topic/102310377#110456

=C2=A0 =C2=A0 Problem Report:

=C2=A0 =C2=A0 On AARCH64, there is no ordering guarantee between configurat= ion
=C2=A0 =C2=A0 space (ECAM) writes and memory space reads (MMIO). ARM AMBA C= HI
=C2=A0 =C2=A0 only guarantees ordering for reads and writes within a single= address region,
=C2=A0 =C2=A0 however, on some systems MMIO and ECAM may be split into sepa= rate
=C2=A0 =C2=A0 address regions.

=C2=A0 =C2=A0 A problem may arise when an ECAM write is issued a completion= before a subsequent
=C2=A0 =C2=A0 MMIO read is issued and receives a completion.

=C2=A0 =C2=A0 For example, a typical PCI software flow is the following:
=C2=A0 =C2=A0 1. ECAM write to device command register to enable memory spa= ce
=C2=A0 =C2=A0 2. MMIO read from device memory space for which access was en= abled
=C2=A0 =C2=A0 in step 1.

=C2=A0 =C2=A0 There is no guarantee that step 2. will not begin before the = completion of step 1.
=C2=A0 =C2=A0 on systems where ECAM/MMIO are specified as separate address = regions, even
=C2=A0 =C2=A0 if both spaces have the memory attributes device-nGnRnE.

=C2=A0 =C2=A0 Fix:

=C2=A0 =C2=A0 - Add a read after the final PCI Configuration space write =C2=A0 =C2=A0 =C2=A0 in RootBridgeIoPciAccess.

=C2=A0 =C2=A0 - When configuration space is strongly ordered, this ensures<= br> =C2=A0 =C2=A0 =C2=A0 that program execution cannot continue until the compl= etion
=C2=A0 =C2=A0 =C2=A0 is received for the previous Cfg-Write, which may have= side-effects.

=C2=A0 =C2=A0 - Risk of reading a "write-only" register and causi= ng a CA which leaves the device
=C2=A0 =C2=A0 =C2=A0 unresponsive. The expectation based on the PCI Base Sp= ec v6.1 section 7.4 is that
=C2=A0 =C2=A0 =C2=A0 all PCI Spec-defined registers will be readable, howev= er, there may exist
=C2=A0 =C2=A0 =C2=A0 design-specific registers that fall into this category= .

=C2=A0 =C2=A0 Cc: Leif Lindholm <quic_llindhol@quicinc.com>
=C2=A0 =C2=A0 Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
=C2=A0 =C2=A0 Cc: Sami Mujawar <sami.mujawar@arm.com>
=C2=A0 =C2=A0 Cc: Jian J Wang <jian.j.wang@intel.com>
=C2=A0 =C2=A0 Cc: Liming Gao <gaoliming@byosoft.com.cn>
=C2=A0 =C2=A0 Cc: Hao A Wu <hao.a.wu@intel.com>
=C2=A0 =C2=A0 Cc: Ray Ni <ray.ni@intel.com>
=C2=A0 =C2=A0 Cc: Pedro Falcato <pedro.falcato@gmail.com>
=C2=A0 =C2=A0 Cc: Michael Brown <mcb30@ipxe.org>
=C2=A0 =C2=A0 Signed-off-by: Joe Lopez <jlotwo2@gmail.com>
---
=C2=A0MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 8 ++++++++<= br> =C2=A01 file changed, 8 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/MdeM= odulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index 157a0ada80..c2dc2018d6 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -1238,6 +1238,14 @@ RootBridgeIoPciAccess (
=C2=A0 =C2=A0 =C2=A0}
=C2=A0 =C2=A0}

+=C2=A0 //
+=C2=A0 // Perform readback after write to confirm completion was received = for the last write
+=C2=A0 // before subsequent memory operations can be issued.
+=C2=A0 //
+=C2=A0 if (!Read) {
+=C2=A0 =C2=A0 PciSegmentRead8 (Address - InStride);
+=C2=A0 }
+
=C2=A0 =C2=A0return EFI_SUCCESS;
=C2=A0}

--
2.25.1

_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#110874) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--000000000000b98fe30609947fcf--