Re: [dpdk-dev] [PATCH v3 15/29] crypto/qat: use eal I/O device memory read/write API

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Friday, January 13, 2017 11:47 AM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: Jerin Jacob <jerin.jacob@caviumnetworks.com>; dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> thomas.monjalon@6wind.com; jianbo.liu@linaro.org; viktorin@rehivetech.com; santosh.shukla@caviumnetworks.com; Griffin, John
> <john.griffin@intel.com>; Trahe, Fiona <fiona.trahe@intel.com>; Jain, Deepak K <deepak.k.jain@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v3 15/29] crypto/qat: use eal I/O device memory read/write API
> 
> On Fri, Jan 13, 2017 at 11:40:06AM +0000, Ferruh Yigit wrote:
> > On 1/13/2017 8:17 AM, Jerin Jacob wrote:
> > > On Thu, Jan 12, 2017 at 07:09:22PM +0000, Ferruh Yigit wrote:
> > >> Hi Jerin,
> > >>
> > >> On 1/12/2017 9:17 AM, Jerin Jacob wrote:
> > >> <...>
> > >>
> > >>> +#include <rte_io.h>
> > >>> +
> > >>>  /* CSR write macro */
> > >>> -#define ADF_CSR_WR(csrAddr, csrOffset, val) \
> > >>> -	(void)((*((volatile uint32_t *)(((uint8_t *)csrAddr) + csrOffset)) \
> > >>> -			= (val)))
> > >>> +#define ADF_CSR_WR(csrAddr, csrOffset, val)		\
> > >>> +	rte_write32(val, (((uint8_t *)csrAddr) + csrOffset))
> > >>
> > >> For IA, this update introduces an extra compiler barrier (rte_io_wmb()),
> > >> which is indeed not a must, is this correct?
> > >
> > > AFAIK, Compiler barrier is required for IA. I am not an IA expert, if
> > > someone thinks it needs to changed then I can fix it in following commit
> > > in this patch series by making rte_io_wmb() and rte_io_rmb() as empty.
> > >
> > > Let me know.
> >
> > I don't know, but what I know is this was working for IA without
> > compiler barrier before.
> >
> > Bruce or Konstantin can help here.
> >
> I think having a compiler barrier is safer. If all data being written
> before the actual register write to the device is volatile, none is
> needed, but also in that case, the compiler barrier should have no
> effect. 

+1

>If some of the data is not volatile, then a barrier is needed
> for correctness. IMHO, unless there is a performance regression by doing
> so, I think having the IO register writes have compiler barriers on IA
> is a good thing. It should save individual drivers from having to worry
> about the barriers themselves in most cases.
> 
> Regards,
> /Bruce