Re: [edk2-devel] [PATCH v2] MdeModulePkg/PciHostBridgeDxe: Add readback after final Cfg-Write

["Joe L" <jlotwo@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3499 bytes --]

++ CC'd


On Mon, Nov 6, 2023 at 6:02 PM Joe Lopez <jlotwo@gmail.com> wrote:

> From: joelopez333 <jlotwo@gmail.com>
>
>     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 <quic_llindhol@quicinc.com>
>     Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>     Cc: Sami Mujawar <sami.mujawar@arm.com>
>     Cc: Jian J Wang <jian.j.wang@intel.com>
>     Cc: Liming Gao <gaoliming@byosoft.com.cn>
>     Cc: Hao A Wu <hao.a.wu@intel.com>
>     Cc: Ray Ni <ray.ni@intel.com>
>     Cc: Pedro Falcato <pedro.falcato@gmail.com>
>     Cc: Michael Brown <mcb30@ipxe.org>
>     Signed-off-by: Joe Lopez <jlotwo2@gmail.com>
> ---
>  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 (#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]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 5331 bytes --]