From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH v2] i40e: Fix eth_i40e_dev_init sequence on ThunderX Date: Tue, 22 Nov 2016 03:46:38 +0530 Message-ID: <20161121221638.GA17380@svelivela-lt.caveonetworks.com> References: <1479473533-9393-1-git-send-email-skoteshwar@caviumnetworks.com> <2601191342CEEE43887BDE71AB9772583F0DE265@irsmsx105.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Cc: Satha Rao , "Zhang, Helin" , "Wu, Jingjing" , "jianbo.liu@linaro.org" , "dev@dpdk.org" To: "Ananyev, Konstantin" Return-path: Received: from NAM03-DM3-obe.outbound.protection.outlook.com (mail-dm3nam03on0060.outbound.protection.outlook.com [104.47.41.60]) by dpdk.org (Postfix) with ESMTP id DFDF829D6 for ; Mon, 21 Nov 2016 23:16:47 +0100 (CET) Content-Disposition: inline In-Reply-To: <2601191342CEEE43887BDE71AB9772583F0DE265@irsmsx105.ger.corp.intel.com> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Sun, Nov 20, 2016 at 11:21:43PM +0000, Ananyev, Konstantin wrote: > Hi > > > > i40e_asq_send_command: rd32 & wr32 under ThunderX gives unpredictable > > results. To solve this include rte memory barriers > > > > Signed-off-by: Satha Rao > > --- > > drivers/net/i40e/base/i40e_osdep.h | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/drivers/net/i40e/base/i40e_osdep.h b/drivers/net/i40e/base/i40e_osdep.h > > index 38e7ba5..ffa3160 100644 > > --- a/drivers/net/i40e/base/i40e_osdep.h > > +++ b/drivers/net/i40e/base/i40e_osdep.h > > @@ -158,7 +158,13 @@ do { \ > > ((volatile uint32_t *)((char *)(a)->hw_addr + (reg))) > > static inline uint32_t i40e_read_addr(volatile void *addr) > > { > > +#if defined(RTE_ARCH_ARM64) > > + uint32_t val = rte_le_to_cpu_32(I40E_PCI_REG(addr)); > > + rte_rmb(); > > + return val; > > If you really need an rmb/wmb with MMIO read/writes on ARM, > I think you can avoid #ifdefs here and use rte_smp_rmb/rte_smp_wmb. > BTW, I suppose if you need it for i40e, you would need it for other devices too. Yes. ARM would need for all devices(typically, the devices on external PCI bus). I guess rte_smp_rmb may not be the correct abstraction. So we need more of rte_rmb() as we need only non smp variant on IO side. I guess then it make sense to create new abstraction in eal with following variants so that each arch gets opportunity to make what it makes sense that specific platform rte_readb_relaxed rte_readw_relaxed rte_readl_relaxed rte_readq_relaxed rte_writeb_relaxed rte_writew_relaxed rte_writel_relaxed rte_writeq_relaxed rte_readb rte_readw rte_readl rte_readq rte_writeb rte_writew rte_writel rte_writeq Thoughts ? Jerin > Konstantin > > > +#else > > return rte_le_to_cpu_32(I40E_PCI_REG(addr)); > > +#endif > > } > > #define I40E_PCI_REG_WRITE(reg, value) \ > > do { I40E_PCI_REG((reg)) = rte_cpu_to_le_32(value); } while (0) > > @@ -171,8 +177,16 @@ static inline uint32_t i40e_read_addr(volatile void *addr) > > I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((hw), (reg)), (value)) > > > > #define rd32(a, reg) i40e_read_addr(I40E_PCI_REG_ADDR((a), (reg))) > > +#if defined(RTE_ARCH_ARM64) > > +#define wr32(a, reg, value) \ > > + do { \ > > + I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value)); \ > > + rte_wmb(); \ > > + } while (0) > > +#else > > #define wr32(a, reg, value) \ > > I40E_PCI_REG_WRITE(I40E_PCI_REG_ADDR((a), (reg)), (value)) > > +#endif > > #define flush(a) i40e_read_addr(I40E_PCI_REG_ADDR((a), (I40E_GLGEN_STAT))) > > > > #define ARRAY_SIZE(arr) (sizeof(arr)/sizeof(arr[0])) > > -- > > 2.7.4 >