[edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Joe L]

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

Hello,

During experimentation on an AARCH64 platform with a PCIe endpoint that supports option ROM, it was found that the ordering of transactions between ECAM (Cfg-Write) and MMIO (Mem-Read) is not preserved by the CHI HN-I node. The observed sequence is as follows:
1. EFI issues a Cfg-Write to endpoint Memory Space Enable bit to enable memory access
2. EFI issues a Mem-Read to the endpoint Option ROM memory space and receives back 0xFFFF (unsupported request)
If the memory ordering between the two accesses is explicitly preserved via memory barrier (DMB instruction), 2. will return back valid data instead of UR. Analyzing the transactions with a protocol analyzer, we found that the Mem-Read was being issued before the completion for the Cfg-Write is received.

On this system, the HN-I node is configured to separate the ECAM and MMIO into different SAM regions. Both regions are assigned Decice-nGnRnE attributes. According to this snippet from Arm "DEN0114 PCIe AMBA Integration Guide", the ordering of even Device-nGnRnE memory regions cannot be guaranteed if they belong to separate PCIe address spaces

> 
> 
> 
> *4.8.2*
> 
> Multiple PCIe address spaces mapped as Device-nGnRnE or Device-nGnRE Arm
> memory model does not give any ordering guarantees between accesses to
> different Device-nGnRnE or Device-nGnRE peripherals. Additionally, there
> is no restriction on mapping various PCIe address spaces of the same PCIe
> function as different Device-nGnRnE or Device-nGnRE peripherals.
> Consequently, software cannot assume that program order will be maintained
> between accesses to two different PCIe address spaces, even though both
> spaces are mapped as Device-nGnRnE or Device-nGnRE. Therefore, for maximum
> software portability, ordering requirements between accesses to different
> PCIe address spaces must be handled explicitly in software using
> appropriate ordering instructions."

We requested a comment from an Arm representative and received a similar response, confirming that a memory barrier is needed to ensure ordering between accesses to ECAM and MMIO regions (or between any two ranges that are assigned to a separate SAM address region)

> 
> 
> 
> When they are to two different order regions, the read will not wait for
> the write to complete, and can return data before the write does anything.
> The HN-I only preserves ordering between reads and writes to the same
> Order Region (which implies the same Address Region). Likewise, the HN-I
> will only preserve ordering between multiple reads and between multiple
> writes within the same Order Region, and it accomplishes this by issuing
> the reads with the same ARID and the writes with the same AWID (i.e. it
> relies on the downstream device to follow AXI ordering rules). Issuing a
> CHI request with REQ.Order=EndpointOrder only guarantees ordering to the
> same “endpoint address range,” which the HN-I defines as an Order Region
> (within an Address Region).
> 
> Our CMN TRM showcases an example where ECAM and MMIO are two different
> regions in the HN-I SAM. The implication is that we would expect a DSB
> between the ECAM write and MMIO read. I'm asking our Open Source Software
> group to confirm that standard PCIe software is generally expected to be
> aware of the need for a DSB--but my impression from talking to some of our
> hardware engineers is that that is indeed the expectation.
> 
> 
> 

I am requesting that EDK2 consumes or produces a change to the current PciExpressLib that will ensure ordering on Arm architectures after Cfg-Writes which may or may not have side effects. For example, in MdePkg/Library/BasePciExpressLib/PciExpressLib.c,

UINT8
EFIAPI
PciExpressWrite8 (
 IN      UINTN  Address,
 IN      UINT8  Value
 )
{
 ASSERT_INVALID_PCI_ADDRESS (Address);
 if (Address >= PcdPciExpressBaseSize ()) {
   return (UINT8)-1;
 }

 return MmioWrite8 ((UINTN)GetPciExpressBaseAddress () + Address, Value);
}

should become

UINT8
EFIAPI
PciExpressWrite8 (
 IN      UINTN  Address,
 IN      UINT8  Value
 )
{
 ASSERT_INVALID_PCI_ADDRESS (Address);
 if (Address >= PcdPciExpressBaseSize ()) {
   return (UINT8)-1;
 }
 
 UINT8 ReturnValue = MmioWrite8 ((UINTN)GetPciExpressBaseAddress () + Address, Value);
 #if defined (MDE_CPU_AARCH64)
    MemoryFence (); // DMB sy or DSB
 #endif

 return ReturnValue;
}

Please let me know your thoughts and if this is the correct implementation change needed to enforce memory ordering between separate address regions. I would also like to know the preferred memory barrier instruction in this case (DMB or DSB).

Thanks,

Joe


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110456): https://edk2.groups.io/g/devel/message/110456
Mute This Topic: https://groups.io/mt/102310377/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: 6051 bytes --]

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Pedro Falcato]

+CC ARM maintainers

On Wed, Nov 1, 2023 at 12:40 AM Joe L <jlotwo@gmail.com> wrote:
>
> Hello,
>
> During experimentation on an AARCH64 platform with a PCIe endpoint that supports option ROM, it was found that the ordering of transactions between ECAM (Cfg-Write) and MMIO (Mem-Read) is not preserved by the CHI HN-I node. The observed sequence is as follows:
> 1. EFI issues a Cfg-Write to endpoint Memory Space Enable bit to enable memory access
> 2. EFI issues a Mem-Read to the endpoint Option ROM memory space and receives back 0xFFFF (unsupported request)
> If the memory ordering between the two accesses is explicitly preserved via memory barrier (DMB instruction), 2. will return back valid data instead of UR. Analyzing the transactions with a protocol analyzer, we found that the Mem-Read was being issued before the completion for the Cfg-Write is received.
>
> On this system, the HN-I node is configured to separate the ECAM and MMIO into different SAM regions. Both regions are assigned Decice-nGnRnE attributes. According to this snippet from Arm "DEN0114 PCIe AMBA Integration Guide", the ordering of even Device-nGnRnE memory regions cannot be guaranteed if they belong to separate PCIe address spaces
>
> 4.8.2
>
> Multiple PCIe address spaces mapped as Device-nGnRnE or Device-nGnRE Arm memory model does not give any ordering guarantees between accesses to different Device-nGnRnE or Device-nGnRE peripherals. Additionally, there is no restriction on mapping various PCIe address spaces of the same PCIe function as different Device-nGnRnE or Device-nGnRE peripherals. Consequently, software cannot assume that program order will be maintained between accesses to two different PCIe address spaces, even though both spaces are mapped as Device-nGnRnE or Device-nGnRE. Therefore, for maximum software portability, ordering requirements between accesses to different PCIe address spaces must be handled explicitly in software using appropriate ordering instructions."
>
> We requested a comment from an Arm representative and received a similar response, confirming that a memory barrier is needed to ensure ordering between accesses to ECAM and MMIO regions (or between any two ranges that are assigned to a separate SAM address region)
>
> When they are to two different order regions, the read will not wait for the write to complete, and can return data before the write does anything. The HN-I only preserves ordering between reads and writes to the same Order Region (which implies the same Address Region). Likewise, the HN-I will only preserve ordering between multiple reads and between multiple writes within the same Order Region, and it accomplishes this by issuing the reads with the same ARID and the writes with the same AWID (i.e. it relies on the downstream device to follow AXI ordering rules). Issuing a CHI request with REQ.Order=EndpointOrder only guarantees ordering to the same “endpoint address range,” which the HN-I defines as an Order Region (within an Address Region).
>
> Our CMN TRM showcases an example where ECAM and MMIO are two different regions in the HN-I SAM. The implication is that we would expect a DSB between the ECAM write and MMIO read. I'm asking our Open Source Software group to confirm that standard PCIe software is generally expected to be aware of the need for a DSB--but my impression from talking to some of our hardware engineers is that that is indeed the expectation.
>
>
> I am requesting that EDK2 consumes or produces a change to the current PciExpressLib that will ensure ordering on Arm architectures after Cfg-Writes which may or may not have side effects. For example, in MdePkg/Library/BasePciExpressLib/PciExpressLib.c,
>
> UINT8
> EFIAPI
> PciExpressWrite8 (
>   IN      UINTN  Address,
>   IN      UINT8  Value
>   )
> {
>   ASSERT_INVALID_PCI_ADDRESS (Address);
>   if (Address >= PcdPciExpressBaseSize ()) {
>     return (UINT8)-1;
>   }
>
>   return MmioWrite8 ((UINTN)GetPciExpressBaseAddress () + Address, Value);
> }
>
> should become
>
>
>
> UINT8
> EFIAPI
> PciExpressWrite8 (
>   IN      UINTN  Address,
>   IN      UINT8  Value
>   )
> {
>   ASSERT_INVALID_PCI_ADDRESS (Address);
>   if (Address >= PcdPciExpressBaseSize ()) {
>     return (UINT8)-1;
>   }
>
>   UINT8 ReturnValue = MmioWrite8 ((UINTN)GetPciExpressBaseAddress () + Address, Value);
>   #if defined (MDE_CPU_AARCH64)
>      MemoryFence (); // DMB sy or DSB
>   #endif
>
>   return ReturnValue;
> }
>
> Please let me know your thoughts and if this is the correct implementation change needed to enforce memory ordering between separate address regions. I would also like to know the preferred memory barrier instruction in this case (DMB or DSB).

Hrmmm, I think the current implementation of MmioRead/Write only
offers TSO instead of "full serialization". For instance:

//
//  Reads an 8-bit MMIO register.
//
//  Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
//  returned. This function must guarantee that all MMIO read and write
//  operations are serialized.
//
//  @param  Address The MMIO register to read.
//
//  @return The value read.
//
ASM_PFX(MmioRead8Internal):
  AARCH64_BTI(c)
  ldrb    w0, [x0]
  dmb     ld
  ret

//
//  Writes an 8-bit MMIO register.
//
//  Writes the 8-bit MMIO register specified by Address with the value specified
//  by Value and returns Value. This function must guarantee that all MMIO read
//  and write operations are serialized.
//
//  @param  Address The MMIO register to write.
//  @param  Value   The value to write to the MMIO register.
//
ASM_PFX(MmioWrite8Internal):
  AARCH64_BTI(c)
  dmb     st
  strb    w1, [x0]
  ret


So prior reads are ordered against reads/stores (through dmb ld after
the load). Prior writes are ordered against writes only (through dmb
st before the store). So one can easily devise a problem here where

  dmb     st /* orders against prior writes */
  strb    w1, [x0] /* write */
[...]

  ldrb    w0, [x0] /* load */
  dmb     ld /* orders this load against later reads/stores, but not
against prior writes (!!) */

where the CPU can (AFAIK) freely reorder the write after the load in
the ARM64 case.

Do we want dmb st after the store instead?

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110462): https://edk2.groups.io/g/devel/message/110462
Mute This Topic: https://groups.io/mt/102310377/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Ard Biesheuvel]

(cc Ray)

On Wed, 1 Nov 2023 at 03:09, Pedro Falcato <pedro.falcato@gmail.com> wrote:
>
> +CC ARM maintainers
>
> On Wed, Nov 1, 2023 at 12:40 AM Joe L <jlotwo@gmail.com> wrote:
> >
> > Hello,
> >
> > During experimentation on an AARCH64 platform with a PCIe endpoint that supports option ROM, it was found that the ordering of transactions between ECAM (Cfg-Write) and MMIO (Mem-Read) is not preserved by the CHI HN-I node. The observed sequence is as follows:
> > 1. EFI issues a Cfg-Write to endpoint Memory Space Enable bit to enable memory access
> > 2. EFI issues a Mem-Read to the endpoint Option ROM memory space and receives back 0xFFFF (unsupported request)
> > If the memory ordering between the two accesses is explicitly preserved via memory barrier (DMB instruction), 2. will return back valid data instead of UR. Analyzing the transactions with a protocol analyzer, we found that the Mem-Read was being issued before the completion for the Cfg-Write is received.
> >
> > On this system, the HN-I node is configured to separate the ECAM and MMIO into different SAM regions. Both regions are assigned Decice-nGnRnE attributes. According to this snippet from Arm "DEN0114 PCIe AMBA Integration Guide", the ordering of even Device-nGnRnE memory regions cannot be guaranteed if they belong to separate PCIe address spaces
> >
> > 4.8.2
> >
> > Multiple PCIe address spaces mapped as Device-nGnRnE or Device-nGnRE Arm memory model does not give any ordering guarantees between accesses to different Device-nGnRnE or Device-nGnRE peripherals. Additionally, there is no restriction on mapping various PCIe address spaces of the same PCIe function as different Device-nGnRnE or Device-nGnRE peripherals. Consequently, software cannot assume that program order will be maintained between accesses to two different PCIe address spaces, even though both spaces are mapped as Device-nGnRnE or Device-nGnRE. Therefore, for maximum software portability, ordering requirements between accesses to different PCIe address spaces must be handled explicitly in software using appropriate ordering instructions."
> >
> > We requested a comment from an Arm representative and received a similar response, confirming that a memory barrier is needed to ensure ordering between accesses to ECAM and MMIO regions (or between any two ranges that are assigned to a separate SAM address region)
> >
> > When they are to two different order regions, the read will not wait for the write to complete, and can return data before the write does anything. The HN-I only preserves ordering between reads and writes to the same Order Region (which implies the same Address Region). Likewise, the HN-I will only preserve ordering between multiple reads and between multiple writes within the same Order Region, and it accomplishes this by issuing the reads with the same ARID and the writes with the same AWID (i.e. it relies on the downstream device to follow AXI ordering rules). Issuing a CHI request with REQ.Order=EndpointOrder only guarantees ordering to the same “endpoint address range,” which the HN-I defines as an Order Region (within an Address Region).
> >
> > Our CMN TRM showcases an example where ECAM and MMIO are two different regions in the HN-I SAM. The implication is that we would expect a DSB between the ECAM write and MMIO read. I'm asking our Open Source Software group to confirm that standard PCIe software is generally expected to be aware of the need for a DSB--but my impression from talking to some of our hardware engineers is that that is indeed the expectation.
> >
> >
> > I am requesting that EDK2 consumes or produces a change to the current PciExpressLib that will ensure ordering on Arm architectures after Cfg-Writes which may or may not have side effects. For example, in MdePkg/Library/BasePciExpressLib/PciExpressLib.c,
> >
> > UINT8
> > EFIAPI
> > PciExpressWrite8 (
> >   IN      UINTN  Address,
> >   IN      UINT8  Value
> >   )
> > {
> >   ASSERT_INVALID_PCI_ADDRESS (Address);
> >   if (Address >= PcdPciExpressBaseSize ()) {
> >     return (UINT8)-1;
> >   }
> >
> >   return MmioWrite8 ((UINTN)GetPciExpressBaseAddress () + Address, Value);
> > }
> >
> > should become
> >
> >
> >
> > UINT8
> > EFIAPI
> > PciExpressWrite8 (
> >   IN      UINTN  Address,
> >   IN      UINT8  Value
> >   )
> > {
> >   ASSERT_INVALID_PCI_ADDRESS (Address);
> >   if (Address >= PcdPciExpressBaseSize ()) {
> >     return (UINT8)-1;
> >   }
> >
> >   UINT8 ReturnValue = MmioWrite8 ((UINTN)GetPciExpressBaseAddress () + Address, Value);
> >   #if defined (MDE_CPU_AARCH64)
> >      MemoryFence (); // DMB sy or DSB
> >   #endif
> >
> >   return ReturnValue;
> > }
> >
> > Please let me know your thoughts and if this is the correct implementation change needed to enforce memory ordering between separate address regions. I would also like to know the preferred memory barrier instruction in this case (DMB or DSB).
>
> Hrmmm, I think the current implementation of MmioRead/Write only
> offers TSO instead of "full serialization". For instance:
>
> //
> //  Reads an 8-bit MMIO register.
> //
> //  Reads the 8-bit MMIO register specified by Address. The 8-bit read value is
> //  returned. This function must guarantee that all MMIO read and write
> //  operations are serialized.
> //
> //  @param  Address The MMIO register to read.
> //
> //  @return The value read.
> //
> ASM_PFX(MmioRead8Internal):
>   AARCH64_BTI(c)
>   ldrb    w0, [x0]
>   dmb     ld
>   ret
>
> //
> //  Writes an 8-bit MMIO register.
> //
> //  Writes the 8-bit MMIO register specified by Address with the value specified
> //  by Value and returns Value. This function must guarantee that all MMIO read
> //  and write operations are serialized.
> //
> //  @param  Address The MMIO register to write.
> //  @param  Value   The value to write to the MMIO register.
> //
> ASM_PFX(MmioWrite8Internal):
>   AARCH64_BTI(c)
>   dmb     st
>   strb    w1, [x0]
>   ret
>
>
> So prior reads are ordered against reads/stores (through dmb ld after
> the load). Prior writes are ordered against writes only (through dmb
> st before the store). So one can easily devise a problem here where
>
>   dmb     st /* orders against prior writes */
>   strb    w1, [x0] /* write */
> [...]
>
>   ldrb    w0, [x0] /* load */
>   dmb     ld /* orders this load against later reads/stores, but not
> against prior writes (!!) */
>
> where the CPU can (AFAIK) freely reorder the write after the load in
> the ARM64 case.
>

Not if the memory type is device/strongly ordered, which is the
assumption for regions these accessors operate on. And even with
cached normal memory, only /other/ observers might see these take
effect in a different order.

The barriers here are supposed to manage concurrent accesses, which
are mostly from coherent masters other than secondary CPUs in the
context that EDK2 typically operates in.

For instance, the DMB ST in MmioWrite8Internal() is supposed to ensure
that any prior writes to a DMA buffer are not reordered with the MMIO
write that kicks of the DMA transaction.


> Do we want dmb st after the store instead?
>

I think we should separate the two cases here:

1) as per the architecture (as interpreted by the ARM architects), a
DSB is required to ensure that the side effects of enabling a MMIO BAR
in the PCI config space are sufficiently observable to memory accesses
to that BAR that appear after the PCI config space access in the
program.

2) the MMIO accessors are completely undocumented, and it would make
sense to put some of the (alleged) rationale in the source code for
posterity.


As for 1), I don't think dropping a MemoryFence() (which issues a DBM
not a DSB on arm64) in a shared source file somewhere is the right
approach here. I'd much prefer handling this in
RootBridgeIoPciAccess(), and using/introducing a generic helper other
than MemoryFence() to convey the existence of a potential side effect
that may turn entire regions of memory on or off (including ones that
the program/compiler may consider 'ordinary' memory)

On ARM/AARCH64, we already have DataSynchronizationBarrier () for this
purpose. Alternatively, we might introduce a PciExpressLib instance in
ArmPkg that uses this.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110473): https://edk2.groups.io/g/devel/message/110473
Mute This Topic: https://groups.io/mt/102310377/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Michael Brown]

On 01/11/2023 09:56, Ard Biesheuvel wrote:
> On Wed, 1 Nov 2023 at 03:09, Pedro Falcato <pedro.falcato@gmail.com> wrote:
>> On Wed, Nov 1, 2023 at 12:40 AM Joe L <jlotwo@gmail.com> wrote:
>>> Our CMN TRM showcases an example where ECAM and MMIO are two different regions in the HN-I SAM. The implication is that we would expect a DSB between the ECAM write and MMIO read. I'm asking our Open Source Software group to confirm that standard PCIe software is generally expected to be aware of the need for a DSB--but my impression from talking to some of our hardware engineers is that that is indeed the expectation.
> 
> <snip>
> 
> 1) as per the architecture (as interpreted by the ARM architects), a
> DSB is required to ensure that the side effects of enabling a MMIO BAR
> in the PCI config space are sufficiently observable to memory accesses
> to that BAR that appear after the PCI config space access in the
> program.

It's possibly worth mentioning what the PCIe specification requires in 
terms of ECAM ordering:

   "As an example, software may wish to configure a device Function’s
    Base Address register by writing to the device using the ECAM,
    and then read a location in the memory-mapped range described by
    this Base Address register. If the software were to issue the
    memory-mapped read before the ECAM write was completed, it would
    be possible for the memory-mapped read to be re-ordered and arrive
    at the device before the Configuration Write Request, thus causing
    unpredictable results.

    To avoid this problem, processor and host bridge implementations must
    ensure that a method exists for the software to determine when the
    write using the ECAM is completed by the completer."

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.

As a practical example of when this might be relevant: software could be 
writing to device configuration space to disable bus mastering as part 
of a reset or shutdown sequence, in order to guarantee that the device 
will initiate no further DMA operations and that any DMA buffers 
allocated to the device can be freed and reused.  In this situation, 
there may be no subsequent MMIO read or write to the device, and so 
there is no way to rely upon an ordering guarantee to satisfy the 
requirement.

Any solution involving ordering guarantees can therefore mask the 
problem in some situations, but cannot solve it.

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?

Thanks,

Michael



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110477): https://edk2.groups.io/g/devel/message/110477
Mute This Topic: https://groups.io/mt/102310377/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Ard Biesheuvel]

On Wed, 1 Nov 2023 at 13:25, Michael Brown <mcb30@ipxe.org> wrote:
>
> On 01/11/2023 09:56, Ard Biesheuvel wrote:
> > On Wed, 1 Nov 2023 at 03:09, Pedro Falcato <pedro.falcato@gmail.com> wrote:
> >> On Wed, Nov 1, 2023 at 12:40 AM Joe L <jlotwo@gmail.com> wrote:
> >>> Our CMN TRM showcases an example where ECAM and MMIO are two different regions in the HN-I SAM. The implication is that we would expect a DSB between the ECAM write and MMIO read. I'm asking our Open Source Software group to confirm that standard PCIe software is generally expected to be aware of the need for a DSB--but my impression from talking to some of our hardware engineers is that that is indeed the expectation.
> >
> > <snip>
> >
> > 1) as per the architecture (as interpreted by the ARM architects), a
> > DSB is required to ensure that the side effects of enabling a MMIO BAR
> > in the PCI config space are sufficiently observable to memory accesses
> > to that BAR that appear after the PCI config space access in the
> > program.
>
> It's possibly worth mentioning what the PCIe specification requires in
> terms of ECAM ordering:
>
>    "As an example, software may wish to configure a device Function’s
>     Base Address register by writing to the device using the ECAM,
>     and then read a location in the memory-mapped range described by
>     this Base Address register. If the software were to issue the
>     memory-mapped read before the ECAM write was completed, it would
>     be possible for the memory-mapped read to be re-ordered and arrive
>     at the device before the Configuration Write Request, thus causing
>     unpredictable results.
>
>     To avoid this problem, processor and host bridge implementations must
>     ensure that a method exists for the software to determine when the
>     write using the ECAM is completed by the completer."
>
> 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.

> As a practical example of when this might be relevant: software could be
> writing to device configuration space to disable bus mastering as part
> of a reset or shutdown sequence, in order to guarantee that the device
> will initiate no further DMA operations and that any DMA buffers
> allocated to the device can be freed and reused.  In this situation,
> there may be no subsequent MMIO read or write to the device, and so
> there is no way to rely upon an ordering guarantee to satisfy the
> requirement.
>
> Any solution involving ordering guarantees can therefore mask the
> problem in some situations, but cannot solve it.
>
> 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).


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110478): https://edk2.groups.io/g/devel/message/110478
Mute This Topic: https://groups.io/mt/102310377/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Michael Brown]

On 01/11/2023 12:51, Ard Biesheuvel wrote:
> On Wed, 1 Nov 2023 at 13:25, Michael Brown <mcb30@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?

Thanks,

Michael



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110480): https://edk2.groups.io/g/devel/message/110480
Mute This Topic: https://groups.io/mt/102310377/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Ard Biesheuvel]

On Wed, 1 Nov 2023 at 14:24, Michael Brown <mcb30@ipxe.org> wrote:
>
> On 01/11/2023 12:51, Ard Biesheuvel wrote:
> > On Wed, 1 Nov 2023 at 13:25, Michael Brown <mcb30@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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Joe L]

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

Thanks for the discussion,

Are we saying that DataSynchronizationBarrier () before the return from RootBridgeIoPciAccess() is not strong enough to determine that the final ECAM write would have completed? According to Learn the architecture - ARMv8-A memory systems ( https://developer.arm.com/documentation/100941/0101/Barriers ) the DSB should block "execution of any further instructions, not just loads or stores, until synchronization is complete". To me this means that for Arm the DSB will stall any subsequent instructions until the completion for the ECAM write is received by the processor.

Though if an architecture-agnostic solution is desired the readback before returning from RootBridgeIoPciAccess() does make sense. If the access spanned multiple DWORDs then should a read from the final aligned DWORD in the "buffer" be sufficient?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110489): https://edk2.groups.io/g/devel/message/110489
Mute This Topic: https://groups.io/mt/102310377/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: 1851 bytes --]

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Michael Brown]

On 01/11/2023 20:17, Joe L wrote:
> Thanks for the discussion,
> 
> Are we saying that DataSynchronizationBarrier () before the return 
> from RootBridgeIoPciAccess() is not strong enough to determine that 
> the final ECAM write would have completed? According to Learn the 
> architecture - ARMv8-A memory systems 
> <https://developer.arm.com/documentation/100941/0101/Barriers> the 
> DSB should block "execution of any further instructions, not just 
> loads or stores, until synchronization is complete". To me this means
> that for Arm the DSB will stall any subsequent instructions until the
> completion for the ECAM write is received by the processor.

I honestly can't tell from the wording there whether or not DSB would
guarantee that the configuration space write has completed all the way 
to the target PCIe device (as opposed to e.g. completing as far as the 
host bridge, or to an intermediate PCIe bridge).

I don't know the answer, and it feels like the kind of messy area where 
adding a barrier will definitely reduce the probability of the issue 
occurring but might not guarantee a fix, which is a bad situation to be 
left in.

> Though if an architecture-agnostic solution is desired the readback 
> before returning from RootBridgeIoPciAccess() does make sense.

Personally I like having an architecture-agnostic solution, and I'll be
updating the ECAM driver in iPXE to use the readback approach.

> If the access spanned multiple DWORDs then should a read from the 
> final aligned DWORD in the "buffer" be sufficient?

Quite possibly.  Given that PCIe configuration space accesses are never 
performance-critical, I'm not sure it's worth the optimisation, but I'll 
defer to the people who actually have to implement it in EDK2.

Thanks,

Michael


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110493): https://edk2.groups.io/g/devel/message/110493
Mute This Topic: https://groups.io/mt/102310377/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC] Ordering of Arm PCI ECAM and MMIO operations

[Pedro Falcato]

On Wed, Nov 1, 2023 at 9:51 PM Michael Brown <mcb30@ipxe.org> wrote:
>
> On 01/11/2023 20:17, Joe L wrote:
> > Thanks for the discussion,
> >
> > Are we saying that DataSynchronizationBarrier () before the return
> > from RootBridgeIoPciAccess() is not strong enough to determine that
> > the final ECAM write would have completed? According to Learn the
> > architecture - ARMv8-A memory systems
> > <https://developer.arm.com/documentation/100941/0101/Barriers> the
> > DSB should block "execution of any further instructions, not just
> > loads or stores, until synchronization is complete". To me this means
> > that for Arm the DSB will stall any subsequent instructions until the
> > completion for the ECAM write is received by the processor.
>
> I honestly can't tell from the wording there whether or not DSB would
> guarantee that the configuration space write has completed all the way
> to the target PCIe device (as opposed to e.g. completing as far as the
> host bridge, or to an intermediate PCIe bridge).
>
> I don't know the answer, and it feels like the kind of messy area where
> adding a barrier will definitely reduce the probability of the issue
> occurring but might not guarantee a fix, which is a bad situation to be
> left in.

Given we are working with Device nGnRnE memory, and the documentation
says stuff like

" This determines whether an intermediate write buffer between the
processor and the slave device being accessed is allowed to send an
acknowledgement of a write completion. If the address is marked as non
Early Write Acknowledgement (nE), then the write response must come
from the peripheral. If the address is marked as Early Write
Acknowledgement (E), then it is permissible for a buffer in the
interconnect logic to signal write acceptance, in advance of the write
actually being received by the end device. This is essentially a
message to the external memory system."

I would expect the write ACK to come from the device itself (I can't
wait for the obscure ARM doc that proves me wrong!)

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110494): https://edk2.groups.io/g/devel/message/110494
Mute This Topic: https://groups.io/mt/102310377/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-