> -----Original Message----- > From: Richardson, Bruce > Sent: Friday, January 13, 2017 11:47 AM > To: Yigit, Ferruh <ferruh.yi...@intel.com> > Cc: Jerin Jacob <jerin.ja...@caviumnetworks.com>; dev@dpdk.org; Ananyev, > Konstantin <konstantin.anan...@intel.com>; > thomas.monja...@6wind.com; jianbo....@linaro.org; vikto...@rehivetech.com; > santosh.shu...@caviumnetworks.com; Griffin, John > <john.grif...@intel.com>; Trahe, Fiona <fiona.tr...@intel.com>; Jain, Deepak > K <deepak.k.j...@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