[dpdk-dev,v2,1/8] eal: introduce DMA memory barriers

  This commit introduces rte_dma_wmb() and rte_dma_rmb(), in order to
guarantee the ordering of coherent shared memory between the CPU and a DMA
capable device.

Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
---
 lib/librte_eal/common/include/generic/rte_atomic.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
  

The 01/15/2018 17:10, Yongseok Koh wrote:
> This commit introduces rte_dma_wmb() and rte_dma_rmb(), in order to
> guarantee the ordering of coherent shared memory between the CPU and a DMA
> capable device.
>
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>

Acked-by: Jianbo Liu <jianbo.liu@arm.com>

> ---
>  lib/librte_eal/common/include/generic/rte_atomic.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
> index 16af5ca57..2e0503ce6 100644
> --- a/lib/librte_eal/common/include/generic/rte_atomic.h
> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
> @@ -98,6 +98,24 @@ static inline void rte_io_wmb(void);
>   */
>  static inline void rte_io_rmb(void);
>
> +/**
> + * Write memory barrier for coherent memory between lcore and IO device
> + *
> + * Guarantees that the STORE operations on coherent memory that
> + * precede the rte_dma_wmb() call are visible to I/O device before the
> + * STORE operations that follow it.
> + */
> +static inline void rte_dma_wmb(void);
> +
> +/**
> + * Read memory barrier for coherent memory between lcore and IO device
> + *
> + * Guarantees that the LOAD operations on coherent memory updated by
> + * IO device that precede the rte_dma_rmb() call are visible to CPU
> + * before the LOAD operations that follow it.
> + */
> +static inline void rte_dma_rmb(void);
> +
>  #endif /* __DOXYGEN__ */
>
>  /**
> --
> 2.11.0
>

--
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  

On 01/16/2018 04:10 AM, Yongseok Koh wrote:
> This commit introduces rte_dma_wmb() and rte_dma_rmb(), in order to
> guarantee the ordering of coherent shared memory between the CPU and a DMA
> capable device.
>
> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> ---
>   lib/librte_eal/common/include/generic/rte_atomic.h | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
>
> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
> index 16af5ca57..2e0503ce6 100644
> --- a/lib/librte_eal/common/include/generic/rte_atomic.h
> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
> @@ -98,6 +98,24 @@ static inline void rte_io_wmb(void);
>    */
>   static inline void rte_io_rmb(void);
>   
> +/**
> + * Write memory barrier for coherent memory between lcore and IO device
> + *
> + * Guarantees that the STORE operations on coherent memory that
> + * precede the rte_dma_wmb() call are visible to I/O device before the
> + * STORE operations that follow it.
> + */
> +static inline void rte_dma_wmb(void);
> +
> +/**
> + * Read memory barrier for coherent memory between lcore and IO device
> + *
> + * Guarantees that the LOAD operations on coherent memory updated by
> + * IO device that precede the rte_dma_rmb() call are visible to CPU
> + * before the LOAD operations that follow it.
> + */
> +static inline void rte_dma_rmb(void);
> +
>   #endif /* __DOXYGEN__ */
>   
>   /**

I'm not an ARMv8 expert so, my comments could be a bit ignorant.
I'd like to understand the difference between io and added here dma 
barriers.
The difference should be clearly explained. Otherwise we'll constantly hit
on incorrect choice of barrier type.
Also I don't understand why "dma" name is chosen taking into account
that definition is bound to coherent memory between lcore and IO device.
  

The 01/16/2018 10:49, Andrew Rybchenko wrote:
> On 01/16/2018 04:10 AM, Yongseok Koh wrote:
> >This commit introduces rte_dma_wmb() and rte_dma_rmb(), in order to
> >guarantee the ordering of coherent shared memory between the CPU and a DMA
> >capable device.
> >
> >Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> >---
> >  lib/librte_eal/common/include/generic/rte_atomic.h | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> >diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
> >index 16af5ca57..2e0503ce6 100644
> >--- a/lib/librte_eal/common/include/generic/rte_atomic.h
> >+++ b/lib/librte_eal/common/include/generic/rte_atomic.h
> >@@ -98,6 +98,24 @@ static inline void rte_io_wmb(void);
> >   */
> >  static inline void rte_io_rmb(void);
> >+/**
> >+ * Write memory barrier for coherent memory between lcore and IO device
> >+ *
> >+ * Guarantees that the STORE operations on coherent memory that
> >+ * precede the rte_dma_wmb() call are visible to I/O device before the
> >+ * STORE operations that follow it.
> >+ */
> >+static inline void rte_dma_wmb(void);
> >+
> >+/**
> >+ * Read memory barrier for coherent memory between lcore and IO device
> >+ *
> >+ * Guarantees that the LOAD operations on coherent memory updated by
> >+ * IO device that precede the rte_dma_rmb() call are visible to CPU
> >+ * before the LOAD operations that follow it.
> >+ */
> >+static inline void rte_dma_rmb(void);
> >+
> >  #endif /* __DOXYGEN__ */
> >  /**
>
> I'm not an ARMv8 expert so, my comments could be a bit ignorant.
> I'd like to understand the difference between io and added here dma
> barriers.
> The difference should be clearly explained. Otherwise we'll constantly hit
> on incorrect choice of barrier type.
> Also I don't understand why "dma" name is chosen taking into account
> that definition is bound to coherent memory between lcore and IO device.

A good explanation can be found here.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1077fa36f23e259858caf6f269a47393a5aff523

--
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
  

16/01/2018 10:10, Jianbo Liu:
> The 01/16/2018 10:49, Andrew Rybchenko wrote:
> > On 01/16/2018 04:10 AM, Yongseok Koh wrote:
> > >This commit introduces rte_dma_wmb() and rte_dma_rmb(), in order to
> > >guarantee the ordering of coherent shared memory between the CPU and a DMA
> > >capable device.
> > >
> > >Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> > >---
> > >  lib/librte_eal/common/include/generic/rte_atomic.h | 18 ++++++++++++++++++
> > >  1 file changed, 18 insertions(+)
> > >
> > >diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
> > >index 16af5ca57..2e0503ce6 100644
> > >--- a/lib/librte_eal/common/include/generic/rte_atomic.h
> > >+++ b/lib/librte_eal/common/include/generic/rte_atomic.h
> > >@@ -98,6 +98,24 @@ static inline void rte_io_wmb(void);
> > >   */
> > >  static inline void rte_io_rmb(void);
> > >+/**
> > >+ * Write memory barrier for coherent memory between lcore and IO device
> > >+ *
> > >+ * Guarantees that the STORE operations on coherent memory that
> > >+ * precede the rte_dma_wmb() call are visible to I/O device before the
> > >+ * STORE operations that follow it.
> > >+ */
> > >+static inline void rte_dma_wmb(void);
> > >+
> > >+/**
> > >+ * Read memory barrier for coherent memory between lcore and IO device
> > >+ *
> > >+ * Guarantees that the LOAD operations on coherent memory updated by
> > >+ * IO device that precede the rte_dma_rmb() call are visible to CPU
> > >+ * before the LOAD operations that follow it.
> > >+ */
> > >+static inline void rte_dma_rmb(void);
> > >+
> > >  #endif /* __DOXYGEN__ */
> > >  /**
> >
> > I'm not an ARMv8 expert so, my comments could be a bit ignorant.
> > I'd like to understand the difference between io and added here dma
> > barriers.
> > The difference should be clearly explained. Otherwise we'll constantly hit
> > on incorrect choice of barrier type.
> > Also I don't understand why "dma" name is chosen taking into account
> > that definition is bound to coherent memory between lcore and IO device.
> 
> A good explanation can be found here.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1077fa36f23e259858caf6f269a47393a5aff523

I agree that something more is needed to explain when to use rte_io_*.
The only difference between rte_io_* and rte_dma_* is "on coherent memory".
  

> On Jan 17, 2018, at 5:46 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 16/01/2018 10:10, Jianbo Liu:
>> The 01/16/2018 10:49, Andrew Rybchenko wrote:
>>> On 01/16/2018 04:10 AM, Yongseok Koh wrote:
>>>> This commit introduces rte_dma_wmb() and rte_dma_rmb(), in order to
>>>> guarantee the ordering of coherent shared memory between the CPU and a DMA
>>>> capable device.
>>>> 
>>>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>>>> ---
>>>> lib/librte_eal/common/include/generic/rte_atomic.h | 18 ++++++++++++++++++
>>>> 1 file changed, 18 insertions(+)
>>>> 
>>>> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
>>>> index 16af5ca57..2e0503ce6 100644
>>>> --- a/lib/librte_eal/common/include/generic/rte_atomic.h
>>>> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
>>>> @@ -98,6 +98,24 @@ static inline void rte_io_wmb(void);
>>>>  */
>>>> static inline void rte_io_rmb(void);
>>>> +/**
>>>> + * Write memory barrier for coherent memory between lcore and IO device
>>>> + *
>>>> + * Guarantees that the STORE operations on coherent memory that
>>>> + * precede the rte_dma_wmb() call are visible to I/O device before the
>>>> + * STORE operations that follow it.
>>>> + */
>>>> +static inline void rte_dma_wmb(void);
>>>> +
>>>> +/**
>>>> + * Read memory barrier for coherent memory between lcore and IO device
>>>> + *
>>>> + * Guarantees that the LOAD operations on coherent memory updated by
>>>> + * IO device that precede the rte_dma_rmb() call are visible to CPU
>>>> + * before the LOAD operations that follow it.
>>>> + */
>>>> +static inline void rte_dma_rmb(void);
>>>> +
>>>> #endif /* __DOXYGEN__ */
>>>> /**
>>> 
>>> I'm not an ARMv8 expert so, my comments could be a bit ignorant.
>>> I'd like to understand the difference between io and added here dma
>>> barriers.
>>> The difference should be clearly explained. Otherwise we'll constantly hit
>>> on incorrect choice of barrier type.
>>> Also I don't understand why "dma" name is chosen taking into account
>>> that definition is bound to coherent memory between lcore and IO device.
>> 
>> A good explanation can be found here.
>> 
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3D1077fa36f23e259858caf6f269a47393a5aff523&data=02%7C01%7Cyskoh%40mellanox.com%7C7b526265cbf1449f3db208d55db0c55d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636517936183877836&sdata=2%2Fi8Gs2n%2Fnbe9%2FJ3GWr22ndPmQVmvM2Xh12r3j1ZWlg%3D&reserved=0
> 
> I agree that something more is needed to explain when to use rte_io_*.
> The only difference between rte_io_* and rte_dma_* is "on coherent memory".

Okay will add more explanation and send out v3 soon. But, please note that
there's no concrete theory when to use which barrier. Actually, it is mostly
for ARMv8 because it provides more options for barriers. For other archs, as you
can see in the patches, there's no difference from IO barriers.

Thanks,
Yongseok
  

On 01/17/2018 09:39 PM, Yongseok Koh wrote:
>> On Jan 17, 2018, at 5:46 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 16/01/2018 10:10, Jianbo Liu:
>>> The 01/16/2018 10:49, Andrew Rybchenko wrote:
>>>> On 01/16/2018 04:10 AM, Yongseok Koh wrote:
>>>>> This commit introduces rte_dma_wmb() and rte_dma_rmb(), in order to
>>>>> guarantee the ordering of coherent shared memory between the CPU and a DMA
>>>>> capable device.
>>>>>
>>>>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>>>>> ---
>>>>> lib/librte_eal/common/include/generic/rte_atomic.h | 18 ++++++++++++++++++
>>>>> 1 file changed, 18 insertions(+)
>>>>>
>>>>> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
>>>>> index 16af5ca57..2e0503ce6 100644
>>>>> --- a/lib/librte_eal/common/include/generic/rte_atomic.h
>>>>> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
>>>>> @@ -98,6 +98,24 @@ static inline void rte_io_wmb(void);
>>>>>   */
>>>>> static inline void rte_io_rmb(void);
>>>>> +/**
>>>>> + * Write memory barrier for coherent memory between lcore and IO device
>>>>> + *
>>>>> + * Guarantees that the STORE operations on coherent memory that
>>>>> + * precede the rte_dma_wmb() call are visible to I/O device before the
>>>>> + * STORE operations that follow it.
>>>>> + */
>>>>> +static inline void rte_dma_wmb(void);
>>>>> +
>>>>> +/**
>>>>> + * Read memory barrier for coherent memory between lcore and IO device
>>>>> + *
>>>>> + * Guarantees that the LOAD operations on coherent memory updated by
>>>>> + * IO device that precede the rte_dma_rmb() call are visible to CPU
>>>>> + * before the LOAD operations that follow it.
>>>>> + */
>>>>> +static inline void rte_dma_rmb(void);
>>>>> +
>>>>> #endif /* __DOXYGEN__ */
>>>>> /**
>>>> I'm not an ARMv8 expert so, my comments could be a bit ignorant.
>>>> I'd like to understand the difference between io and added here dma
>>>> barriers.
>>>> The difference should be clearly explained. Otherwise we'll constantly hit
>>>> on incorrect choice of barrier type.
>>>> Also I don't understand why "dma" name is chosen taking into account
>>>> that definition is bound to coherent memory between lcore and IO device.
>>> A good explanation can be found here.
>>>
>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3D1077fa36f23e259858caf6f269a47393a5aff523&data=02%7C01%7Cyskoh%40mellanox.com%7C7b526265cbf1449f3db208d55db0c55d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636517936183877836&sdata=2%2Fi8Gs2n%2Fnbe9%2FJ3GWr22ndPmQVmvM2Xh12r3j1ZWlg%3D&reserved=0
>> I agree that something more is needed to explain when to use rte_io_*.
>> The only difference between rte_io_* and rte_dma_* is "on coherent memory".
> Okay will add more explanation and send out v3 soon. But, please note that
> there's no concrete theory when to use which barrier. Actually, it is mostly
> for ARMv8 because it provides more options for barriers. For other archs, as you
> can see in the patches, there's no difference from IO barriers.

Absence of concrete theory does not make choice of the memory barrier 
easier.
I would say it complicates it significantly. I think it is a minimal 
requirement for
the patchset to explain why a new type should be defined instead of just
fixing of the rte_io_* barriers on ARMv8. What's the different? Which 
criteria
should be checked/taken into account to make the right choice?

As far as I can see igb_uio and uio_pci_generic do coherent DMA mapping.
It is not that easy with VFIO since in theory it could be non-coherent if
snooping is not supported by IOMMU. Don't know if it is real.
If so, it makes barrier choice UIO driver-dependent. Sounds bad.
  

> On Jan 18, 2018, at 3:56 AM, Andrew Rybchenko <arybchenko@solarflare.com> wrote:
> 
> On 01/17/2018 09:39 PM, Yongseok Koh wrote:
>>> On Jan 17, 2018, at 5:46 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 
>>> 16/01/2018 10:10, Jianbo Liu:
>>>> The 01/16/2018 10:49, Andrew Rybchenko wrote:
>>>>> On 01/16/2018 04:10 AM, Yongseok Koh wrote:
>>>>>> This commit introduces rte_dma_wmb() and rte_dma_rmb(), in order to
>>>>>> guarantee the ordering of coherent shared memory between the CPU and a DMA
>>>>>> capable device.
>>>>>> 
>>>>>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
>>>>>> ---
>>>>>> lib/librte_eal/common/include/generic/rte_atomic.h | 18 ++++++++++++++++++
>>>>>> 1 file changed, 18 insertions(+)
>>>>>> 
>>>>>> diff --git a/lib/librte_eal/common/include/generic/rte_atomic.h b/lib/librte_eal/common/include/generic/rte_atomic.h
>>>>>> index 16af5ca57..2e0503ce6 100644
>>>>>> --- a/lib/librte_eal/common/include/generic/rte_atomic.h
>>>>>> +++ b/lib/librte_eal/common/include/generic/rte_atomic.h
>>>>>> @@ -98,6 +98,24 @@ static inline void rte_io_wmb(void);
>>>>>>  */
>>>>>> static inline void rte_io_rmb(void);
>>>>>> +/**
>>>>>> + * Write memory barrier for coherent memory between lcore and IO device
>>>>>> + *
>>>>>> + * Guarantees that the STORE operations on coherent memory that
>>>>>> + * precede the rte_dma_wmb() call are visible to I/O device before the
>>>>>> + * STORE operations that follow it.
>>>>>> + */
>>>>>> +static inline void rte_dma_wmb(void);
>>>>>> +
>>>>>> +/**
>>>>>> + * Read memory barrier for coherent memory between lcore and IO device
>>>>>> + *
>>>>>> + * Guarantees that the LOAD operations on coherent memory updated by
>>>>>> + * IO device that precede the rte_dma_rmb() call are visible to CPU
>>>>>> + * before the LOAD operations that follow it.
>>>>>> + */
>>>>>> +static inline void rte_dma_rmb(void);
>>>>>> +
>>>>>> #endif /* __DOXYGEN__ */
>>>>>> /**
>>>>> I'm not an ARMv8 expert so, my comments could be a bit ignorant.
>>>>> I'd like to understand the difference between io and added here dma
>>>>> barriers.
>>>>> The difference should be clearly explained. Otherwise we'll constantly hit
>>>>> on incorrect choice of barrier type.
>>>>> Also I don't understand why "dma" name is chosen taking into account
>>>>> that definition is bound to coherent memory between lcore and IO device.
>>>> A good explanation can be found here.
>>>> 
>>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Fcommit%2F%3Fid%3D1077fa36f23e259858caf6f269a47393a5aff523&data=02%7C01%7Cyskoh%40mellanox.com%7C7b526265cbf1449f3db208d55db0c55d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636517936183877836&sdata=2%2Fi8Gs2n%2Fnbe9%2FJ3GWr22ndPmQVmvM2Xh12r3j1ZWlg%3D&reserved=0
>>> I agree that something more is needed to explain when to use rte_io_*.
>>> The only difference between rte_io_* and rte_dma_* is "on coherent memory".
>> Okay will add more explanation and send out v3 soon. But, please note that
>> there's no concrete theory when to use which barrier. Actually, it is mostly
>> for ARMv8 because it provides more options for barriers. For other archs, as you
>> can see in the patches, there's no difference from IO barriers.
> 
> Absence of concrete theory does not make choice of the memory barrier easier.

I didn't say that? I just explained it can't be super clear.

> I would say it complicates it significantly. I think it is a minimal requirement for
> the patchset to explain why a new type should be defined instead of just
> fixing of the rte_io_* barriers on ARMv8. What's the different? Which criteria
> should be checked/taken into account to make the right choice?

I already said I'll send out v3.

> As far as I can see igb_uio and uio_pci_generic do coherent DMA mapping.
> It is not that easy with VFIO since in theory it could be non-coherent if
> snooping is not supported by IOMMU. Don't know if it is real.
> If so, it makes barrier choice UIO driver-dependent. Sounds bad.

I'm not sure I understand this comment but it is because of relaxed memory
ordering model of some processors. Actually, x86 is the only processor which
has the strong memory ordering model. And the link Jianbo shared could be the
best article to understand it and I'll summarize it in my v3.

Thanks,
Yongseok