On Fri, Dec 20, 2019 at 9:03 AM Jerin Jacob <jerinjac...@gmail.com> wrote: > > On Fri, Dec 20, 2019 at 8:40 AM Gavin Hu <gavin...@arm.com> wrote: > > > > Armv8's peripheral coherence order is a total order on all reads and writes > > to that peripheral.[1] > > > > The peripheral coherence order for a memory-mapped peripheral signifies the > > order in which accesses arrive at the endpoint. For a read or a write RW1 > > and a read or a write RW2 to the same peripheral, then RW1 will appear in > > the peripheral coherence order for the peripheral before RW2 if either of > > the following cases apply: > > 1. RW1 and RW2 are accesses using Non-cacheable or Device attributes and > > RW1 is Ordered-before RW2. > > 2. RW1 and RW2 are accesses using Device-nGnRE or Device-nGnRnE attributes > > and RW1 appears in program order before RW2. > > > This is true if RW1 and RW2 addresses are device memory. i.e the > registers in the PCI bar address. > If RW1 is DDR address which is been used by the controller(say NIC > ring descriptor) then there will be an issue. > For example Intel i40e driver, the admin queue update in Host DDR > memory and it updates the doorbell. > In such a case, this patch will create an issue. Correct? Have you > checked this patch with ARM64 + XL710 controllers? > > Some of the legacy code is missing such barriers, that's the reason > for adding rte_io_* barrier.
More details: https://dev.dpdk.narkive.com/DpIRqDuy/dpdk-dev-patch-v2-i40e-fix-eth-i40e-dev-init-sequence-on-thunderx > > > > > On arm platforms, all the PCI resources are mapped to nGnRE device memory > > [2], the above case 2 holds true, that means the peripheral coherence order > > applies here and just a compiler barrier is sufficient for rte io barriers. > > > > [1] Section B2.3.4 of ARMARM, https://developer.arm.com/docs/ddi0487/lates > > t/arm-architecture-reference-manual-armv8-for-armv8-a-architecture-profile > > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ > > tree/drivers/pci/pci-sysfs.c#n1204 > > > > Signed-off-by: Gavin Hu <gavin...@arm.com> > > Reviewed-by: Steve Capper <steve.cap...@arm.com> > > Reviewed-by: Phil Yang <phil.y...@arm.com> > > --- > > lib/librte_eal/common/include/arch/arm/rte_atomic_64.h | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h > > b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h > > index 859ae12..fd63956 100644 > > --- a/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h > > +++ b/lib/librte_eal/common/include/arch/arm/rte_atomic_64.h > > @@ -34,11 +34,11 @@ extern "C" { > > > > #define rte_smp_rmb() dmb(ishld) > > > > -#define rte_io_mb() rte_mb() > > +#define rte_io_mb() rte_compiler_barrier() > > > > -#define rte_io_wmb() rte_wmb() > > +#define rte_io_wmb() rte_compiler_barrier() > > > > -#define rte_io_rmb() rte_rmb() > > +#define rte_io_rmb() rte_compiler_barrier() > > > > #define rte_cio_wmb() dmb(oshst) > > > > -- > > 2.7.4 > >
