On Wed, 1 Nov 2023 at 14:24, Michael Brown <mc...@ipxe.org> wrote: > > On 01/11/2023 12:51, Ard Biesheuvel wrote: > > On Wed, 1 Nov 2023 at 13:25, Michael Brown <mc...@ipxe.org> wrote: > >> By my reading, the PCIe specification seems to therefore require > >> something stronger than an ordering guarantee: it requires the ability > >> for software to make a standalone determination that the write has > >> *completed*, independent of the existence of any subsequent I/O operations. > > > > indeed, thanks for bringing this up. > > > >> The PCIe specification does not mandate that any particular mechanism be > >> used, but it does require that the processor and/or host bridge provides > >> *some* mechanism for software to determine that the ECAM write has > >> completed. > >> > >> What mechanism does ARM (or the host bridge) provide to determine > >> completion of an ECAM write? > > > > A MMIO read of the same location should ensure that the MMIO write has > > completed by the time the read returns. Not sure whether or not there > > are any other requirements (e.g., wrt.the size of the read vs the size > > of the write). > > That seems to suggest that a logical PCIe configuration space write > operation using ECAM should probably always comprise: > > 1. ECAM write > 2. ECAM read from the same location (using the same size) > > If reads are not allowed to have side effects (e.g. read-clear > registers) then this seems safe. The PCIe specification "Configuration > Register Types" list comprises (in version 3.0, at least): > > HwInit - read-only, no read side effects > > RO - read-only, no read side effects > > RW - read-write, no read side effects > > RW1C - write 1 to clear bits, no read side effects > > ROS - read-only, no read side effects > > RWS - read-write, no read side effects > > RW1CS - write 1 to clear bits, no read side effects > > RsvdP - read-write, no read side effects > > RsvdZ - read-write, no read side effects > > So, unless newer versions of the PCIe specification have allowed for the > existence of configuration register types with read side effects, then > the approach of always reading back from ECAM seems to be safe for any > conforming PCIe device. > > I would therefore suggest that all ECAM driver implementation functions > in EDK2 (e.g. PciExpressWrite32(), PciExpressOr32(), > PciSegmentWrite32(), etc) be updated to add the relevant ECAM read > following the write operation. > > PCI configuration space writes are never fast-path operations (in any > sane hardware), and so the delay introduced by the read should not be > significant. > > Does this seem like a sensible solution? >
I think that would be reasonable, but it needs to be implemented at the correct level of abstraction. We have some plumbing to iterate over ranges to do what basically comes down to memcpy and memset on MMIO ranges, and I don't think we want the readback on every store in a sequence of multiple. RootBridgeIoPciAccess() looks like a reasonable location to insert a readback of the last access if it was a write. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110486): https://edk2.groups.io/g/devel/message/110486 Mute This Topic: https://groups.io/mt/102310377/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
