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 2029B941543 for ; Mon, 13 Nov 2023 11:37:25 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=ZS2sVFsjXXYezw81i+/dJ7nPCTICj6jHpX2hujseR+s=; 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=1699875444; v=1; b=rMqc58PjLD56Fp/jAnrJQg9fqAtIJY6jxpBVSD2bgSXV8SI7R1auK+MI76u1SGUhdQ7v9NrR zIN6g45KYc2F5SwrW/0u88XGioDjVUr9J/EMjWcf6xxPYuu5kgkj8w2jmmtLnSfwxgoFHgpUffq tR6MyOoKsRPMAoSfXmdeUSIw= X-Received: by 127.0.0.2 with SMTP id O0LuYY7687511x6V70gMTF2r; Mon, 13 Nov 2023 03:37:24 -0800 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.34840.1699875444081258616 for ; Mon, 13 Nov 2023 03:37:24 -0800 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-412-gjt5iubxNbyuAOH6KWWxKg-1; Mon, 13 Nov 2023 06:37:17 -0500 X-MC-Unique: gjt5iubxNbyuAOH6KWWxKg-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 245BE1018800; Mon, 13 Nov 2023 11:37:17 +0000 (UTC) X-Received: from [10.39.192.220] (unknown [10.39.192.220]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 759861C060AE; Mon, 13 Nov 2023 11:37:13 +0000 (UTC) Message-ID: Date: Mon, 13 Nov 2023 12:37:12 +0100 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v2] MdeModulePkg/PciHostBridgeDxe: Add readback after final Cfg-Write To: devel@edk2.groups.io, michael.d.kinney@intel.com, Jose Lopez Cc: Leif Lindholm , Ard Biesheuvel , "Gao, Liming" , Michael Brown , Pedro Falcato , "Ni, Ray" , "Wu, Hao A" , "Wang, Jian J" , Sami Mujawar References: <34167f0f873c01654aee0e9c8629221dad241529.1699322498.git.jlotwo@gmail.com> From: "Laszlo Ersek" In-Reply-To: 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: n1krwyaYLF7pHlNJeZeUxYlGx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=rMqc58Pj; 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/8/23 02:06, Michael D Kinney wrote: > Hi Jose, > >   > > 1. This logic needs to move into an AARCH64 specific directory/file.  > Other architectures handle this in other ways. > 2. There are many places throughout edk2 sources that perform PCI > config write operations.  You are only fixing it in a single > location.  You may want to look at the MdePkg PciLibs to see if it > can be addressed there with an AARCH64 specific dir/file, but that > still may not address all possible PCI config write accesses.  Fill > analysis of the target platform sources may be required to make sure > it is fixes for all locations. Hm. Perhaps if this change is pushed down as much as possible, into a special AARCH64 PciLib instance, so that the readback or the DSB (IIRC) is performed after every PCI config space write, then that might actually suffice. Because, ultimately, PciHostBridgeDxe too depends on PciSegmentLib for the access sycles. (Previously I suggested extending PciHostBridgeLib with a new API, but that wasn't right, per your second point -- I didn't realize the same issue must exist (for example) in the PEI phase, before the PCI-related protocols even exist.) Laszlo > >   > > Mike > >   > > *From:* Jose Lopez > *Sent:* Tuesday, November 7, 2023 10:59 AM > *To:* devel@edk2.groups.io > *Cc:* Leif Lindholm ; Ard Biesheuvel > ; Gao, Liming ; > Michael Brown ; Pedro Falcato ; > Ni, Ray ; Wu, Hao A ; Wang, Jian J > ; Sami Mujawar ; > lersek@redhat.com; Kinney, Michael D > *Subject:* Re: [PATCH v2] MdeModulePkg/PciHostBridgeDxe: Add readback > after final Cfg-Write > >   > > ++ Laszlo and Michael > >   > > On Tue, Nov 7, 2023 at 10:54 AM Jose Lopez > wrote: > > ++ CC'd > >   > >   > > On Mon, Nov 6, 2023 at 6:02 PM 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 > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#111145): https://edk2.groups.io/g/devel/message/111145 Mute This Topic: https://groups.io/mt/102435564/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-