From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id EF13E5588 for ; Fri, 13 Jan 2017 12:47:10 +0100 (CET) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga104.fm.intel.com with ESMTP; 13 Jan 2017 03:47:09 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,221,1477983600"; d="scan'208";a="52582803" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.61]) by orsmga005.jf.intel.com with SMTP; 13 Jan 2017 03:47:06 -0800 Received: by (sSMTP sendmail emulation); Fri, 13 Jan 2017 11:47:06 +0000 Date: Fri, 13 Jan 2017 11:47:05 +0000 From: Bruce Richardson To: Ferruh Yigit Cc: Jerin Jacob , dev@dpdk.org, konstantin.ananyev@intel.com, thomas.monjalon@6wind.com, jianbo.liu@linaro.org, viktorin@rehivetech.com, santosh.shukla@caviumnetworks.com, John Griffin , Fiona Trahe , Deepak Kumar Jain Message-ID: <20170113114705.GA188532@bricha3-MOBL3.ger.corp.intel.com> References: <1482832175-27199-1-git-send-email-jerin.jacob@caviumnetworks.com> <1484212646-10338-1-git-send-email-jerin.jacob@caviumnetworks.com> <1484212646-10338-16-git-send-email-jerin.jacob@caviumnetworks.com> <6bb9980b-f546-38d5-044a-63507510f6a5@intel.com> <20170113081641.GA17635@localhost.localdomain> <76c8ede9-54bb-51ba-293b-583681616e0a@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <76c8ede9-54bb-51ba-293b-583681616e0a@intel.com> Organization: Intel Research and =?iso-8859-1?Q?De=ACvel?= =?iso-8859-1?Q?opment?= Ireland Ltd. User-Agent: Mutt/1.7.1 (2016-10-04) Subject: Re: [dpdk-dev] [PATCH v3 15/29] crypto/qat: use eal I/O device memory read/write API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Jan 2017 11:47:11 -0000 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 > >>> + > >>> /* 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. 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