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 09731AC191B for ; Tue, 7 Nov 2023 18:58:56 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=CVdfWPTijiKY3NRIQ7vldzWyMsarJfOZYXn7uZBbIbk=; 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=1699383535; v=1; b=VUm+wdklgrdbJUy++V6qx96UOKFjazKnT5lH6nxQbol/aMQAJSOr83wzHatcVhi2ra3GaJ6R zgP5JGn6VtPbT22sR5tzsxJOn7D6/D68Dg5qcBuD+fxC/BPR0CZnTh/ij7xI4JKPPZu1/QPZ/Zi xgqPAghzh6SvbaMsVaw33mm0= X-Received: by 127.0.0.2 with SMTP id GdjNYY7687511xmNXz6JO7jj; Tue, 07 Nov 2023 10:58:55 -0800 X-Received: from mail-yw1-f181.google.com (mail-yw1-f181.google.com [209.85.128.181]) by mx.groups.io with SMTP id smtpd.web11.3132.1699383535109170435 for ; Tue, 07 Nov 2023 10:58:55 -0800 X-Received: by mail-yw1-f181.google.com with SMTP id 00721157ae682-5a8ada42c2aso70198027b3.3 for ; Tue, 07 Nov 2023 10:58:55 -0800 (PST) X-Gm-Message-State: Z4y5ngJqwExBnmYx4zkZ5VTwx7686176AA= X-Google-Smtp-Source: AGHT+IHILA3RLmKEbMm9TvdTDl3Ozmsy1uUIFlUKAphtWHv8J7mXbhd65LIRtMAS1EPYyitQcWTXFE+iALF5SgidAgQ= X-Received: by 2002:a81:920e:0:b0:5a7:bbc2:4e12 with SMTP id j14-20020a81920e000000b005a7bbc24e12mr15602066ywg.4.1699383534110; Tue, 07 Nov 2023 10:58:54 -0800 (PST) MIME-Version: 1.0 References: <34167f0f873c01654aee0e9c8629221dad241529.1699322498.git.jlotwo@gmail.com> In-Reply-To: From: "Joe L" Date: Tue, 7 Nov 2023 10:58:42 -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 , lersek@redhat.com, michael.d.kinney@intel.com 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="000000000000e34e580609948f4b" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=VUm+wdkl; 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 --000000000000e34e580609948f4b Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable ++ Laszlo and Michael On Tue, Nov 7, 2023 at 10:54=E2=80=AFAM Jose Lopez wrote= : > ++ 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 addres= s >> 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-effect= s. >> >> - 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 ma= y >> 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 fo= r >> 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 (#110876): https://edk2.groups.io/g/devel/message/110876 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- --000000000000e34e580609948f4b Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
=
On Tue, No= v 7, 2023 at 10:54=E2=80=AFAM Jose Lopez <jlotwo@gmail.com> wrote:

<= br>
On Mon,= Nov 6, 2023 at 6:02=E2=80=AFPM Joe Lopez <jlotwo@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 (#110876) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--000000000000e34e580609948f4b--