diff mbox

[dpdk-dev,13/28] eal/arm64: override I/O device read/write access for arm64

Message ID 1481680558-4003-14-git-send-email-jerin.jacob@caviumnetworks.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel compilation success Compilation OK

Commit Message

Jerin Jacob Dec. 14, 2016, 1:55 a.m. UTC 
  Override the generic I/O device memory read/write access and implement it
using armv8 instructions for arm64.

Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
---
 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

Comments

Jianbo Liu Dec. 15, 2016, 9:53 a.m. UTC | #1 
On 14 December 2016 at 09:55, Jerin Jacob
<jerin.jacob@caviumnetworks.com> wrote:
> Override the generic I/O device memory read/write access and implement it
> using armv8 instructions for arm64.
>
> Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> ---
>  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 <stdint.h>
> +
> +#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?

> +
> +#define rte_readb_relaxed(addr) \
> +       ({ uint8_t __v = __rte_arm64_readb(addr); __v; })
> +
> +#define rte_readw_relaxed(addr) \
> +       ({ uint16_t __v = __rte_arm64_readw(addr); __v; })
> +
> +#define rte_readl_relaxed(addr) \
> +       ({ uint32_t __v = __rte_arm64_readl(addr); __v; })
> +
> +#define rte_readq_relaxed(addr) \
> +       ({ uint64_t __v = __rte_arm64_readq(addr); __v; })
> +
> +#define rte_writeb_relaxed(value, addr) \
> +       ({ __rte_arm64_writeb(value, addr); })
> +
> +#define rte_writew_relaxed(value, addr) \
> +       ({ __rte_arm64_writew(value, addr); })
> +
> +#define rte_writel_relaxed(value, addr) \
> +       ({ __rte_arm64_writel(value, addr); })
> +
> +#define rte_writeq_relaxed(value, addr) \
> +       ({ __rte_arm64_writeq(value, addr); })
> +
> +#define rte_readb(addr) \
> +       ({ uint8_t __v = __rte_arm64_readb(addr); rte_io_rmb(); __v; })
> +
> +#define rte_readw(addr) \
> +       ({ uint16_t __v = __rte_arm64_readw(addr); rte_io_rmb(); __v; })
> +
> +#define rte_readl(addr) \
> +       ({ uint32_t __v = __rte_arm64_readl(addr); rte_io_rmb(); __v; })
> +
> +#define rte_readq(addr) \
> +       ({ uint64_t __v = __rte_arm64_readq(addr); rte_io_rmb(); __v; })
> +
> +#define rte_writeb(value, addr) \
> +       ({ rte_io_wmb(); rte_writeb_relaxed(value, addr); })
> +
> +#define rte_writew(value, addr) \
> +       ({ rte_io_wmb(); rte_writew_relaxed(value, addr); })
> +
> +#define rte_writel(value, addr) \
> +       ({ rte_io_wmb(); rte_writel_relaxed(value, addr); })
> +
> +#define rte_writeq(value, addr) \
> +       ({ rte_io_wmb(); rte_writeq_relaxed(value, addr); })
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_IO_ARM64_H_ */
> --
> 2.5.5
>
Jerin Jacob Dec. 15, 2016, 10:04 a.m. UTC | #2 
On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
> On 14 December 2016 at 09:55, Jerin Jacob
> <jerin.jacob@caviumnetworks.com> wrote:
> > Override the generic I/O device memory read/write access and implement it
> > using armv8 instructions for arm64.
> >
> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> > ---
> >  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 <stdint.h>
> > +
> > +#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?
Jianbo Liu Dec. 15, 2016, 10:17 a.m. UTC | #3 
On 15 December 2016 at 18:04, Jerin Jacob
<jerin.jacob@caviumnetworks.com> wrote:
> On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
>> On 14 December 2016 at 09:55, Jerin Jacob
>> <jerin.jacob@caviumnetworks.com> wrote:
>> > Override the generic I/O device memory read/write access and implement it
>> > using armv8 instructions for arm64.
>> >
>> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> > ---
>> >  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 <stdint.h>
>> > +
>> > +#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?
If there is no apparent benefit, I don't think we need the overriding.
Jerin Jacob Dec. 15, 2016, 11:08 a.m. UTC | #4 
On Thu, Dec 15, 2016 at 06:17:32PM +0800, Jianbo Liu wrote:
> On 15 December 2016 at 18:04, Jerin Jacob
> <jerin.jacob@caviumnetworks.com> wrote:
> > On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
> >> On 14 December 2016 at 09:55, Jerin Jacob
> >> <jerin.jacob@caviumnetworks.com> wrote:
> >> > Override the generic I/O device memory read/write access and implement it
> >> > using armv8 instructions for arm64.
> >> >
> >> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> >> > ---
> >> >  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 <stdint.h>
> >> > +
> >> > +#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.
Jianbo Liu Dec. 16, 2016, 10:12 a.m. UTC | #5 
On 15 December 2016 at 19:08, Jerin Jacob
<jerin.jacob@caviumnetworks.com> wrote:
> On Thu, Dec 15, 2016 at 06:17:32PM +0800, Jianbo Liu wrote:
>> On 15 December 2016 at 18:04, Jerin Jacob
>> <jerin.jacob@caviumnetworks.com> wrote:
>> > On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
>> >> On 14 December 2016 at 09:55, Jerin Jacob
>> >> <jerin.jacob@caviumnetworks.com> wrote:
>> >> > Override the generic I/O device memory read/write access and implement it
>> >> > using armv8 instructions for arm64.
>> >> >
>> >> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
>> >> > ---
>> >> >  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 <stdint.h>
>> >> > +
>> >> > +#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?

> register access.
>
> But, I am sure the proposed scheme generates correct instruction in all the cases.
Jerin Jacob Dec. 16, 2016, 10:25 a.m. UTC | #6 
On Fri, Dec 16, 2016 at 06:12:13PM +0800, Jianbo Liu wrote:
> On 15 December 2016 at 19:08, Jerin Jacob
> <jerin.jacob@caviumnetworks.com> wrote:
> > On Thu, Dec 15, 2016 at 06:17:32PM +0800, Jianbo Liu wrote:
> >> On 15 December 2016 at 18:04, Jerin Jacob
> >> <jerin.jacob@caviumnetworks.com> wrote:
> >> > On Thu, Dec 15, 2016 at 05:53:05PM +0800, Jianbo Liu wrote:
> >> >> On 14 December 2016 at 09:55, Jerin Jacob
> >> >> <jerin.jacob@caviumnetworks.com> wrote:
> >> >> > Override the generic I/O device memory read/write access and implement it
> >> >> > using armv8 instructions for arm64.
> >> >> >
> >> >> > Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
> >> >> > ---
> >> >> >  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 <stdint.h>
> >> >> > +
> >> >> > +#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.
diff mbox

Patch

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 <stdint.h>
+
+#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));
+}
+
+#define rte_readb_relaxed(addr) \
+	({ uint8_t __v = __rte_arm64_readb(addr); __v; })
+
+#define rte_readw_relaxed(addr) \
+	({ uint16_t __v = __rte_arm64_readw(addr); __v; })
+
+#define rte_readl_relaxed(addr) \
+	({ uint32_t __v = __rte_arm64_readl(addr); __v; })
+
+#define rte_readq_relaxed(addr) \
+	({ uint64_t __v = __rte_arm64_readq(addr); __v; })
+
+#define rte_writeb_relaxed(value, addr) \
+	({ __rte_arm64_writeb(value, addr); })
+
+#define rte_writew_relaxed(value, addr) \
+	({ __rte_arm64_writew(value, addr); })
+
+#define rte_writel_relaxed(value, addr) \
+	({ __rte_arm64_writel(value, addr); })
+
+#define rte_writeq_relaxed(value, addr) \
+	({ __rte_arm64_writeq(value, addr); })
+
+#define rte_readb(addr) \
+	({ uint8_t __v = __rte_arm64_readb(addr); rte_io_rmb(); __v; })
+
+#define rte_readw(addr) \
+	({ uint16_t __v = __rte_arm64_readw(addr); rte_io_rmb(); __v; })
+
+#define rte_readl(addr) \
+	({ uint32_t __v = __rte_arm64_readl(addr); rte_io_rmb(); __v; })
+
+#define rte_readq(addr) \
+	({ uint64_t __v = __rte_arm64_readq(addr); rte_io_rmb(); __v; })
+
+#define rte_writeb(value, addr) \
+	({ rte_io_wmb(); rte_writeb_relaxed(value, addr); })
+
+#define rte_writew(value, addr) \
+	({ rte_io_wmb(); rte_writew_relaxed(value, addr); })
+
+#define rte_writel(value, addr) \
+	({ rte_io_wmb(); rte_writel_relaxed(value, addr); })
+
+#define rte_writeq(value, addr) \
+	({ rte_io_wmb(); rte_writeq_relaxed(value, addr); })
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_IO_ARM64_H_ */