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: Thu, 15 Dec 2016 16:38:09 +0530 Message-ID: <20161215110807.GA10881@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> 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 NAM03-DM3-obe.outbound.protection.outlook.com (mail-dm3nam03on0085.outbound.protection.outlook.com [104.47.41.85]) by dpdk.org (Postfix) with ESMTP id 2B90B282 for ; Thu, 15 Dec 2016 12:08:32 +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 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 register access. But, I am sure the proposed scheme generates correct instruction in all the cases.