From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jerin Jacob Subject: Re: [PATCH 13/28] eal/arm64: override I/O device read/write access for arm64 Date: Fri, 16 Dec 2016 15:55:53 +0530 Message-ID: <20161216102551.GA10515@localhost.localdomain> References: <1481680558-4003-1-git-send-email-jerin.jacob@caviumnetworks.com> <1481680558-4003-14-git-send-email-jerin.jacob@caviumnetworks.com> <20161215100423.GA6712@localhost.localdomain> <20161215110807.GA10881@localhost.localdomain> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Cc: , "Ananyev, Konstantin" , Thomas Monjalon , Bruce Richardson , Jan Viktorin To: Jianbo Liu Return-path: Received: from NAM01-SN1-obe.outbound.protection.outlook.com (mail-sn1nam01on0063.outbound.protection.outlook.com [104.47.32.63]) by dpdk.org (Postfix) with ESMTP id B56C03DC for ; Fri, 16 Dec 2016 11:26:17 +0100 (CET) Content-Disposition: inline In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Fri, Dec 16, 2016 at 06:12:13PM +0800, Jianbo Liu wrote: > On 15 December 2016 at 19:08, Jerin Jacob > wrote: > > On Thu, Dec 15, 2016 at 06:17:32PM +0800, Jianbo Liu wrote: > >> On 15 December 2016 at 18:04, Jerin Jacob > >> wrote: > >> > On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote: > >> >> On 14 December 2016 at 09:55, Jerin Jacob > >> >> wrote: > >> >> > Override the generic I/O device memory read/write access and implement it > >> >> > using armv8 instructions for arm64. > >> >> > > >> >> > Signed-off-by: Jerin Jacob > >> >> > --- > >> >> > lib/librte_eal/common/include/arch/arm/rte_io.h | 4 + > >> >> > lib/librte_eal/common/include/arch/arm/rte_io_64.h | 183 +++++++++++++++++++++ > >> >> > 2 files changed, 187 insertions(+) > >> >> > create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h > >> >> > > >> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io.h b/lib/librte_eal/common/include/arch/arm/rte_io.h > >> >> > index 74c1f2c..9593b42 100644 > >> >> > --- a/lib/librte_eal/common/include/arch/arm/rte_io.h > >> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io.h > >> >> > @@ -38,7 +38,11 @@ > >> >> > extern "C" { > >> >> > #endif > >> >> > > >> >> > +#ifdef RTE_ARCH_64 > >> >> > +#include "rte_io_64.h" > >> >> > +#else > >> >> > #include "generic/rte_io.h" > >> >> > +#endif > >> >> > > >> >> > #ifdef __cplusplus > >> >> > } > >> >> > diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_64.h b/lib/librte_eal/common/include/arch/arm/rte_io_64.h > >> >> > new file mode 100644 > >> >> > index 0000000..09e7a89 > >> >> > --- /dev/null > >> >> > +++ b/lib/librte_eal/common/include/arch/arm/rte_io_64.h > >> >> > @@ -0,0 +1,183 @@ > >> >> > +/* > >> >> > + * BSD LICENSE > >> >> > + * > >> >> > + * Copyright (C) Cavium networks Ltd. 2016. > >> >> > + * > >> >> > + * Redistribution and use in source and binary forms, with or without > >> >> > + * modification, are permitted provided that the following conditions > >> >> > + * are met: > >> >> > + * > >> >> > + * * Redistributions of source code must retain the above copyright > >> >> > + * notice, this list of conditions and the following disclaimer. > >> >> > + * * Redistributions in binary form must reproduce the above copyright > >> >> > + * notice, this list of conditions and the following disclaimer in > >> >> > + * the documentation and/or other materials provided with the > >> >> > + * distribution. > >> >> > + * * Neither the name of Cavium networks nor the names of its > >> >> > + * contributors may be used to endorse or promote products derived > >> >> > + * from this software without specific prior written permission. > >> >> > + * > >> >> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > >> >> > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > >> >> > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > >> >> > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT > >> >> > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > >> >> > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > >> >> > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > >> >> > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > >> >> > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > >> >> > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > >> >> > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > >> >> > + */ > >> >> > + > >> >> > +#ifndef _RTE_IO_ARM64_H_ > >> >> > +#define _RTE_IO_ARM64_H_ > >> >> > + > >> >> > +#ifdef __cplusplus > >> >> > +extern "C" { > >> >> > +#endif > >> >> > + > >> >> > +#include > >> >> > + > >> >> > +#define RTE_OVERRIDE_IO_H > >> >> > + > >> >> > +#include "generic/rte_io.h" > >> >> > +#include "rte_atomic_64.h" > >> >> > + > >> >> > +static inline __attribute__((always_inline)) uint8_t > >> >> > +__rte_arm64_readb(const volatile void *addr) > >> >> > +{ > >> >> > + uint8_t val; > >> >> > + > >> >> > + asm volatile( > >> >> > + "ldrb %w[val], [%x[addr]]" > >> >> > + : [val] "=r" (val) > >> >> > + : [addr] "r" (addr)); > >> >> > + return val; > >> >> > +} > >> >> > + > >> >> > +static inline __attribute__((always_inline)) uint16_t > >> >> > +__rte_arm64_readw(const volatile void *addr) > >> >> > +{ > >> >> > + uint16_t val; > >> >> > + > >> >> > + asm volatile( > >> >> > + "ldrh %w[val], [%x[addr]]" > >> >> > + : [val] "=r" (val) > >> >> > + : [addr] "r" (addr)); > >> >> > + return val; > >> >> > +} > >> >> > + > >> >> > +static inline __attribute__((always_inline)) uint32_t > >> >> > +__rte_arm64_readl(const volatile void *addr) > >> >> > +{ > >> >> > + uint32_t val; > >> >> > + > >> >> > + asm volatile( > >> >> > + "ldr %w[val], [%x[addr]]" > >> >> > + : [val] "=r" (val) > >> >> > + : [addr] "r" (addr)); > >> >> > + return val; > >> >> > +} > >> >> > + > >> >> > +static inline __attribute__((always_inline)) uint64_t > >> >> > +__rte_arm64_readq(const volatile void *addr) > >> >> > +{ > >> >> > + uint64_t val; > >> >> > + > >> >> > + asm volatile( > >> >> > + "ldr %x[val], [%x[addr]]" > >> >> > + : [val] "=r" (val) > >> >> > + : [addr] "r" (addr)); > >> >> > + return val; > >> >> > +} > >> >> > + > >> >> > +static inline __attribute__((always_inline)) void > >> >> > +__rte_arm64_writeb(uint8_t val, volatile void *addr) > >> >> > +{ > >> >> > + asm volatile( > >> >> > + "strb %w[val], [%x[addr]]" > >> >> > + : > >> >> > + : [val] "r" (val), [addr] "r" (addr)); > >> >> > +} > >> >> > + > >> >> > +static inline __attribute__((always_inline)) void > >> >> > +__rte_arm64_writew(uint16_t val, volatile void *addr) > >> >> > +{ > >> >> > + asm volatile( > >> >> > + "strh %w[val], [%x[addr]]" > >> >> > + : > >> >> > + : [val] "r" (val), [addr] "r" (addr)); > >> >> > +} > >> >> > + > >> >> > +static inline __attribute__((always_inline)) void > >> >> > +__rte_arm64_writel(uint32_t val, volatile void *addr) > >> >> > +{ > >> >> > + asm volatile( > >> >> > + "str %w[val], [%x[addr]]" > >> >> > + : > >> >> > + : [val] "r" (val), [addr] "r" (addr)); > >> >> > +} > >> >> > + > >> >> > +static inline __attribute__((always_inline)) void > >> >> > +__rte_arm64_writeq(uint64_t val, volatile void *addr) > >> >> > +{ > >> >> > + asm volatile( > >> >> > + "str %x[val], [%x[addr]]" > >> >> > + : > >> >> > + : [val] "r" (val), [addr] "r" (addr)); > >> >> > +} > >> >> > >> >> I'm not quite sure about these overridings. Can you explain the > >> >> benefit to do so? > >> > > >> > Better to be native if there is option. That all. Do you see any issue? > >> > or what is the real concern? > >> > > >> > >> I think it's the same as the generic c version after compiling. Am I right? > > > > I really don't that is the case for all the scenarios like compiler may > > combine two 16bit reads one 32bit read etc and which will impact on IO > > I wonder which compiler will do that as armv8 is 32/64 bit system? Not specific to armv8. Two consecutive continues 16bits reads one 32bit read for optimization. Any idea why Linux kernel doing explicit instructions for readl/writel? obviously not for fun. > > > register access. > > > > But, I am sure the proposed scheme generates correct instruction in all the cases.