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 BA433780091 for ; Fri, 3 Nov 2023 07:19:34 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=JGFNO1VcbiZP+RP1EdUBSkDtNv3/gUtSnXKVW023rGA=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1698995973; v=1; b=DI76MmE7YMBvOosFHiij3LnJUVV1GjcmOTgsC8lkUAhTIFcBPdg0CMPnoC/KDUJbSUaMihi1 ywBbzULmBVpWdlhZXBw8oAXWgpIgrEJIhWiBUipbwoCkbQMmxJllbo1scp7gwCywClYZBanrnUy iTC9fT9/jUr910Nz6tQQuexY= X-Received: by 127.0.0.2 with SMTP id OtcvYY7687511x60XgXxKsay; Fri, 03 Nov 2023 00:19:33 -0700 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.37562.1698995972721797819 for ; Fri, 03 Nov 2023 00:19:32 -0700 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-106-XTSFUExeNCCGAvzyjgbH3A-1; Fri, 03 Nov 2023 03:19:28 -0400 X-MC-Unique: XTSFUExeNCCGAvzyjgbH3A-1 X-Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id D982E810FC0; Fri, 3 Nov 2023 07:19:27 +0000 (UTC) X-Received: from [10.39.192.20] (unknown [10.39.192.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id B97E11C060BA; Fri, 3 Nov 2023 07:19:25 +0000 (UTC) Message-ID: Date: Fri, 3 Nov 2023 08:19:24 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/PciHostBridgeDxe: Add readback after final Cfg-Write To: devel@edk2.groups.io, jlotwo@gmail.com Cc: Leif Lindholm , Ard Biesheuvel , Sami Mujawar , Jian J Wang , Liming Gao , Hao A Wu , Ray Ni , Pedro Falcato , Michael Brown References: <742009f523e8645102f784a3c0df6af870c68804.1698966883.git.jlotwo@gmail.com> From: "Laszlo Ersek" In-Reply-To: <742009f523e8645102f784a3c0df6af870c68804.1698966883.git.jlotwo@gmail.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.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,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: 8Gi1HZn46BIHch2UN6TqLadex7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=DI76MmE7; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.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 On 11/3/23 01:03, Joe L wrote: > From: joelopez333 >=20 > REF:https://edk2.groups.io/g/devel/topic/102310377#110456 >=20 > - Add a read after the final PCI Configuration space write > in RootBridgeIoPciAccess. >=20 > - 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. >=20 > 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 | 7 +++++++ > 1 file changed, 7 insertions(+) >=20 > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c b/Md= eModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > index 157a0ada80..4bc774b574 100644 > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c > @@ -1238,6 +1238,13 @@ RootBridgeIoPciAccess ( > } > } > =20 > + // > + // Perform readback after write to confirm completion was received for= the last write > + // > + if (!Read) { > + PciSegmentRead8 (Address - InStride); > + } > + > return EFI_SUCCESS; > } > =20 (1) I'd like (a) the problem report, and the full reasoning by Ard and Michael to be captured in the commit message, and (b) *minimally* a hint at the possible reordering, and at the PCI spec-based workaround, to be placed in the code comment as well. (2) This is a significant change; please file a new tianocore BZ about it. If we include it in the upcoming stable release, the BZ should be listed here, too: https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planni= ng#proposed-features--bug-fixes (3) I seem to understand that the outcome of the discusson thus far is that reading back any config space register should be without side effects. (In turn, this should be documented in the comment and the commit message! But, my more important point here is:) ... assuming Size is not 1, PciSegmentRead8() will not match the size of the most recently performed PciSegmentWriteBuffer(). Is that OK? I'm unsure that *any* config space register (especially one in extended config space) that is larger than one byte per spec (base PCI spec or particular device spec) *guarantees* that a 1-byte read at the front of that register will behave identically to reading back the entire register. ... What's more, I believe that in the previous discussion, it wasn't the outcome that any config space register at all can be read back without side-effects. RootBridgeIoPciAccess() may well read device-specific registers too, and those can have side-effects upon read, can't they? Laszlo -=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 (#110612): https://edk2.groups.io/g/devel/message/110612 Mute This Topic: https://groups.io/mt/102354842/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-