DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms
@ 2020-08-14  6:03 Ruifeng Wang
  2020-08-14  8:13 ` Bruce Richardson
  2020-08-18 14:36 ` Ferruh Yigit
  0 siblings, 2 replies; 9+ messages in thread
From: Ruifeng Wang @ 2020-08-14  6:03 UTC (permalink / raw)
  To: hemant.agrawal, jerinj, viktorin
  Cc: dev, honnappa.nagarahalli, phil.yang, nd, Ruifeng Wang

Flags are used to distinguish different platform architectures.
These flags can be used to pick different code paths for different
architectures at compile time.
For Arm platforms, there are 3 flags in use: RTE_ARCH_ARM,
RTE_ARCH_ARMv7 and RTE_ARCH_ARM64.
RTE_ARCH_ARM64 is used to flag 64-bit aarch64 platforms,
while RTE_ARCH_ARM & RTE_ARCH_ARMv7 are used to flag 32-bit platforms.
RTE_ARCH_ARMv7 is for ARMv7 platforms as its name suggested.

The issue is that flag name RTE_ARCH_ARM is unclear and could cause
confusion. No info about platform word length is included in the name.
To make the flag names more clear, a naming scheme is proposed.

      RTE_ARCH_ARM
          |
          +----RTE_ARCH_ARM32
          |        |
          |        +----RTE_ARCH_ARMv7
          |        |
          |        +----RTE_ARCH_ARMv8_AARCH32
          |
          +----RTE_ARCH_ARM64

RTE_ARCH_ARM32 will be used for 32-bit Arm platforms.
It includes RTE_ARCH_ARMv7 and RTE_ARCH_ARMv8_AARCH32.
RTE_ARCH_ARMv7 is for ARMv7 platforms.
RTE_ARCH_ARMv8_AARCH32 is for aarch32 state on aarch64 platforms.
RTE_ARCH_ARM64 is for 64-bit Arm platforms.
RTE_ARCH_ARM will be used for all Arm platforms, including RTE_ARCH_ARM32
and RTE_ARCH_ARM64.

To fit into the new naming scheme, current usage of RTE_ARCH_ARM in
project code is mapped to RTE_ARCH_ARM32.

Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 app/test/test_cpuflags.c                 | 2 +-
 app/test/test_xmmt_ops.h                 | 2 +-
 config/arm/meson.build                   | 2 +-
 config/defconfig_arm-armv7a-linuxapp-gcc | 2 +-
 drivers/bus/fslmc/qbman/qbman_sys_decl.h | 2 +-
 drivers/common/dpaax/compat.h            | 2 +-
 drivers/net/i40e/Makefile                | 2 +-
 drivers/net/ixgbe/Makefile               | 2 +-
 drivers/net/ixgbe/ixgbe_rxtx.h           | 4 ++--
 drivers/net/virtio/Makefile              | 2 +-
 drivers/net/virtio/virtio_ethdev.c       | 2 +-
 lib/librte_acl/Makefile                  | 2 +-
 lib/librte_acl/meson.build               | 2 +-
 lib/librte_acl/rte_acl.c                 | 4 ++--
 lib/librte_eal/arm/include/rte_vect.h    | 4 ++--
 lib/librte_eal/common/eal_internal_cfg.h | 2 +-
 lib/librte_lpm/Makefile                  | 2 +-
 lib/librte_lpm/rte_lpm.h                 | 2 +-
 18 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/app/test/test_cpuflags.c b/app/test/test_cpuflags.c
index 06718631f..bf9851393 100644
--- a/app/test/test_cpuflags.c
+++ b/app/test/test_cpuflags.c
@@ -86,7 +86,7 @@ test_cpuflags(void)
 	CHECK_FOR_FLAG(RTE_CPUFLAG_ICACHE_SNOOP);
 #endif
 
-#if defined(RTE_ARCH_ARM)
+#if defined(RTE_ARCH_ARM32)
 	printf("Check for NEON:\t\t");
 	CHECK_FOR_FLAG(RTE_CPUFLAG_NEON);
 #endif
diff --git a/app/test/test_xmmt_ops.h b/app/test/test_xmmt_ops.h
index 8bcf0b261..13333bf6e 100644
--- a/app/test/test_xmmt_ops.h
+++ b/app/test/test_xmmt_ops.h
@@ -7,7 +7,7 @@
 
 #include <rte_vect.h>
 
-#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+#if defined(RTE_ARCH_ARM32) || defined(RTE_ARCH_ARM64)
 
 /* vect_* abstraction implementation using NEON */
 
diff --git a/config/arm/meson.build b/config/arm/meson.build
index 8728051d5..62ee2d108 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -132,7 +132,7 @@ dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
 
 if not dpdk_conf.get('RTE_ARCH_64')
 	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
-	dpdk_conf.set('RTE_ARCH_ARM', 1)
+	dpdk_conf.set('RTE_ARCH_ARM32', 1)
 	dpdk_conf.set('RTE_ARCH_ARMv7', 1)
 	# the minimum architecture supported, armv7-a, needs the following,
 	# mk/machine/armv7a/rte.vars.mk sets it too
diff --git a/config/defconfig_arm-armv7a-linuxapp-gcc b/config/defconfig_arm-armv7a-linuxapp-gcc
index ac9112086..b1a1f8524 100644
--- a/config/defconfig_arm-armv7a-linuxapp-gcc
+++ b/config/defconfig_arm-armv7a-linuxapp-gcc
@@ -6,7 +6,7 @@
 CONFIG_RTE_MACHINE="armv7a"
 
 CONFIG_RTE_ARCH="arm"
-CONFIG_RTE_ARCH_ARM=y
+CONFIG_RTE_ARCH_ARM32=y
 CONFIG_RTE_ARCH_ARMv7=y
 CONFIG_RTE_ARCH_ARM_TUNE="cortex-a9"
 
diff --git a/drivers/bus/fslmc/qbman/qbman_sys_decl.h b/drivers/bus/fslmc/qbman/qbman_sys_decl.h
index a29f5b469..e17cbaefe 100644
--- a/drivers/bus/fslmc/qbman/qbman_sys_decl.h
+++ b/drivers/bus/fslmc/qbman/qbman_sys_decl.h
@@ -32,7 +32,7 @@ static inline void prefetch_for_store(void *p)
 {
 	asm volatile("prfm pstl1keep, [%0, #0]" : : "r" (p));
 }
-#elif defined(RTE_ARCH_ARM)
+#elif defined(RTE_ARCH_ARM32)
 #define dcbz(p) memset(p, 0, 64)
 #define lwsync() { asm volatile("dmb st" : : : "memory"); }
 #define dcbf(p)	RTE_SET_USED(p)
diff --git a/drivers/common/dpaax/compat.h b/drivers/common/dpaax/compat.h
index 6793cb256..6ac8a7ea3 100644
--- a/drivers/common/dpaax/compat.h
+++ b/drivers/common/dpaax/compat.h
@@ -163,7 +163,7 @@ static inline void out_be32(volatile void *__p, u32 val)
 		asm volatile("prfm pldl1keep, [%0, #64]" : : "r" (p));	\
 	} while (0)
 
-#elif defined(RTE_ARCH_ARM)
+#elif defined(RTE_ARCH_ARM32)
 #define dcbz(p) memset((p), 0, 32)
 #define dcbz_64(p) memset((p), 0, 64)
 #define dcbf(p)	RTE_SET_USED(p)
diff --git a/drivers/net/i40e/Makefile b/drivers/net/i40e/Makefile
index 43f10941b..314c07dc1 100644
--- a/drivers/net/i40e/Makefile
+++ b/drivers/net/i40e/Makefile
@@ -69,7 +69,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_dcb.c
 
 SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_ethdev.c
 SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_rxtx.c
-ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM) $(CONFIG_RTE_ARCH_ARM64)),)
+ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM32) $(CONFIG_RTE_ARCH_ARM64)),)
 SRCS-$(CONFIG_RTE_LIBRTE_I40E_INC_VECTOR) += i40e_rxtx_vec_neon.c
 else ifeq ($(CONFIG_RTE_ARCH_PPC_64),y)
 SRCS-$(CONFIG_RTE_LIBRTE_I40E_INC_VECTOR) += i40e_rxtx_vec_altivec.c
diff --git a/drivers/net/ixgbe/Makefile b/drivers/net/ixgbe/Makefile
index aebf3b286..4c908220b 100644
--- a/drivers/net/ixgbe/Makefile
+++ b/drivers/net/ixgbe/Makefile
@@ -88,7 +88,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe_ethdev.c
 SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe_fdir.c
 SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe_pf.c
 SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe_flow.c
-ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM) $(CONFIG_RTE_ARCH_ARM64)),)
+ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM32) $(CONFIG_RTE_ARCH_ARM64)),)
 SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe_rxtx_vec_neon.c
 else ifeq ($(CONFIG_RTE_ARCH_X86),y)
 SRCS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += ixgbe_rxtx_vec_sse.c
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.h b/drivers/net/ixgbe/ixgbe_rxtx.h
index 7e09291b2..03b57568b 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.h
+++ b/drivers/net/ixgbe/ixgbe_rxtx.h
@@ -33,7 +33,7 @@
 
 #define RTE_IXGBE_DESCS_PER_LOOP    4
 
-#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) || defined(RTE_ARCH_ARM)
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) || defined(RTE_ARCH_ARM32)
 #define RTE_IXGBE_RXQ_REARM_THRESH      32
 #define RTE_IXGBE_MAX_RX_BURST          RTE_IXGBE_RXQ_REARM_THRESH
 #endif
@@ -117,7 +117,7 @@ struct ixgbe_rx_queue {
 	uint8_t            using_ipsec;
 	/**< indicates that IPsec RX feature is in use */
 #endif
-#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) || defined(RTE_ARCH_ARM)
+#if defined(RTE_ARCH_X86) || defined(RTE_ARCH_ARM64) || defined(RTE_ARCH_ARM32)
 	uint16_t            rxrearm_nb;     /**< number of remaining to be re-armed */
 	uint16_t            rxrearm_start;  /**< the idx we start the re-arming from */
 #endif
diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
index 102b1deab..24c60f8f1 100644
--- a/drivers/net/virtio/Makefile
+++ b/drivers/net/virtio/Makefile
@@ -32,7 +32,7 @@ ifeq ($(CONFIG_RTE_ARCH_X86),y)
 SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_sse.c
 else ifeq ($(CONFIG_RTE_ARCH_PPC_64),y)
 SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_altivec.c
-else ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM) $(CONFIG_RTE_ARCH_ARM64)),)
+else ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM32) $(CONFIG_RTE_ARCH_ARM64)),)
 SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_neon.c
 endif
 
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index dc0093bdf..4495968ba 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -2337,7 +2337,7 @@ virtio_dev_configure(struct rte_eth_dev *dev)
 		}
 
 		if (hw->use_vec_rx) {
-#if defined RTE_ARCH_ARM64 || defined RTE_ARCH_ARM
+#if defined RTE_ARCH_ARM64 || defined RTE_ARCH_ARM32
 			if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON)) {
 				PMD_DRV_LOG(INFO,
 					"disabled split ring vectorized path for requirement not met");
diff --git a/lib/librte_acl/Makefile b/lib/librte_acl/Makefile
index f4332b044..2721bd086 100644
--- a/lib/librte_acl/Makefile
+++ b/lib/librte_acl/Makefile
@@ -20,7 +20,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_ACL) += acl_bld.c
 SRCS-$(CONFIG_RTE_LIBRTE_ACL) += acl_gen.c
 SRCS-$(CONFIG_RTE_LIBRTE_ACL) += acl_run_scalar.c
 
-ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM) $(CONFIG_RTE_ARCH_ARM64)),)
+ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM32) $(CONFIG_RTE_ARCH_ARM64)),)
 SRCS-$(CONFIG_RTE_LIBRTE_ACL) += acl_run_neon.c
 CFLAGS_acl_run_neon.o += -flax-vector-conversions
 ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
diff --git a/lib/librte_acl/meson.build b/lib/librte_acl/meson.build
index d1e2c184c..98496de91 100644
--- a/lib/librte_acl/meson.build
+++ b/lib/librte_acl/meson.build
@@ -27,7 +27,7 @@ if dpdk_conf.has('RTE_ARCH_X86')
 		cflags += '-DCC_AVX2_SUPPORT'
 	endif
 
-elif dpdk_conf.has('RTE_ARCH_ARM') or dpdk_conf.has('RTE_ARCH_ARM64')
+elif dpdk_conf.has('RTE_ARCH_ARM32') or dpdk_conf.has('RTE_ARCH_ARM64')
 	cflags += '-flax-vector-conversions'
 	sources += files('acl_run_neon.c')
 elif dpdk_conf.has('RTE_ARCH_PPC_64')
diff --git a/lib/librte_acl/rte_acl.c b/lib/librte_acl/rte_acl.c
index 777ec4d34..7d53f23b0 100644
--- a/lib/librte_acl/rte_acl.c
+++ b/lib/librte_acl/rte_acl.c
@@ -44,7 +44,7 @@ rte_acl_classify_sse(__rte_unused const struct rte_acl_ctx *ctx,
 }
 #endif
 
-#ifndef RTE_ARCH_ARM
+#ifndef RTE_ARCH_ARM32
 #ifndef RTE_ARCH_ARM64
 int
 rte_acl_classify_neon(__rte_unused const struct rte_acl_ctx *ctx,
@@ -111,7 +111,7 @@ RTE_INIT(rte_acl_init)
 
 #if defined(RTE_ARCH_ARM64)
 	alg =  RTE_ACL_CLASSIFY_NEON;
-#elif defined(RTE_ARCH_ARM)
+#elif defined(RTE_ARCH_ARM32)
 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_NEON))
 		alg =  RTE_ACL_CLASSIFY_NEON;
 #elif defined(RTE_ARCH_PPC_64)
diff --git a/lib/librte_eal/arm/include/rte_vect.h b/lib/librte_eal/arm/include/rte_vect.h
index 01c51712a..68e1c6781 100644
--- a/lib/librte_eal/arm/include/rte_vect.h
+++ b/lib/librte_eal/arm/include/rte_vect.h
@@ -28,7 +28,7 @@ typedef union rte_xmm {
 	double   pd[XMM_SIZE / sizeof(double)];
 } __rte_aligned(16) rte_xmm_t;
 
-#ifdef RTE_ARCH_ARM
+#ifdef RTE_ARCH_ARM32
 /* NEON intrinsic vqtbl1q_u8() is not supported in ARMv7-A(AArch32) */
 static __inline uint8x16_t
 vqtbl1q_u8(uint8x16_t a, uint8x16_t b)
@@ -62,7 +62,7 @@ vaddvq_u16(uint16x8_t a)
 
 #endif
 
-#if defined(RTE_ARCH_ARM) || \
+#if defined(RTE_ARCH_ARM32) || \
 (defined(RTE_ARCH_ARM64) && RTE_CC_IS_GNU && (GCC_VERSION < 70000))
 /* NEON intrinsic vcopyq_laneq_u32() is not supported in ARMv7-A(AArch32)
  * On AArch64, this intrinsic is supported since GCC version 7.
diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
index 13f93388a..90d2a6d14 100644
--- a/lib/librte_eal/common/eal_internal_cfg.h
+++ b/lib/librte_eal/common/eal_internal_cfg.h
@@ -15,7 +15,7 @@
 
 #include "eal_thread.h"
 
-#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+#if defined(RTE_ARCH_ARM32) || defined(RTE_ARCH_ARM64)
 #define MAX_HUGEPAGE_SIZES 4  /**< support up to 4 page sizes */
 #else
 #define MAX_HUGEPAGE_SIZES 3  /**< support up to 3 page sizes */
diff --git a/lib/librte_lpm/Makefile b/lib/librte_lpm/Makefile
index 6f06c5c03..5c24bffe5 100644
--- a/lib/librte_lpm/Makefile
+++ b/lib/librte_lpm/Makefile
@@ -18,7 +18,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_LPM) := rte_lpm.c rte_lpm6.c
 # install this header file
 SYMLINK-$(CONFIG_RTE_LIBRTE_LPM)-include := rte_lpm.h rte_lpm6.h
 
-ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM) $(CONFIG_RTE_ARCH_ARM64)),)
+ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM32) $(CONFIG_RTE_ARCH_ARM64)),)
 SYMLINK-$(CONFIG_RTE_LIBRTE_LPM)-include += rte_lpm_neon.h
 else ifeq ($(CONFIG_RTE_ARCH_X86),y)
 SYMLINK-$(CONFIG_RTE_LIBRTE_LPM)-include += rte_lpm_sse.h
diff --git a/lib/librte_lpm/rte_lpm.h b/lib/librte_lpm/rte_lpm.h
index 03da2d37e..b669266c0 100644
--- a/lib/librte_lpm/rte_lpm.h
+++ b/lib/librte_lpm/rte_lpm.h
@@ -420,7 +420,7 @@ static inline void
 rte_lpm_lookupx4(const struct rte_lpm *lpm, xmm_t ip, uint32_t hop[4],
 	uint32_t defv);
 
-#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+#if defined(RTE_ARCH_ARM32) || defined(RTE_ARCH_ARM64)
 #include "rte_lpm_neon.h"
 #elif defined(RTE_ARCH_PPC_64)
 #include "rte_lpm_altivec.h"
-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms
  2020-08-14  6:03 [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms Ruifeng Wang
@ 2020-08-14  8:13 ` Bruce Richardson
  2020-08-14  9:05   ` Ruifeng Wang
  2020-08-18 14:36 ` Ferruh Yigit
  1 sibling, 1 reply; 9+ messages in thread
From: Bruce Richardson @ 2020-08-14  8:13 UTC (permalink / raw)
  To: Ruifeng Wang
  Cc: hemant.agrawal, jerinj, viktorin, dev, honnappa.nagarahalli,
	phil.yang, nd

On Fri, Aug 14, 2020 at 02:03:20PM +0800, Ruifeng Wang wrote:
> Flags are used to distinguish different platform architectures.
> These flags can be used to pick different code paths for different
> architectures at compile time.
> For Arm platforms, there are 3 flags in use: RTE_ARCH_ARM,
> RTE_ARCH_ARMv7 and RTE_ARCH_ARM64.
> RTE_ARCH_ARM64 is used to flag 64-bit aarch64 platforms,
> while RTE_ARCH_ARM & RTE_ARCH_ARMv7 are used to flag 32-bit platforms.
> RTE_ARCH_ARMv7 is for ARMv7 platforms as its name suggested.
> 
> The issue is that flag name RTE_ARCH_ARM is unclear and could cause
> confusion. No info about platform word length is included in the name.
> To make the flag names more clear, a naming scheme is proposed.
> 
>       RTE_ARCH_ARM
>           |
>           +----RTE_ARCH_ARM32
>           |        |
>           |        +----RTE_ARCH_ARMv7
>           |        |
>           |        +----RTE_ARCH_ARMv8_AARCH32
>           |
>           +----RTE_ARCH_ARM64
> 
> RTE_ARCH_ARM32 will be used for 32-bit Arm platforms.
> It includes RTE_ARCH_ARMv7 and RTE_ARCH_ARMv8_AARCH32.
> RTE_ARCH_ARMv7 is for ARMv7 platforms.
> RTE_ARCH_ARMv8_AARCH32 is for aarch32 state on aarch64 platforms.
> RTE_ARCH_ARM64 is for 64-bit Arm platforms.
> RTE_ARCH_ARM will be used for all Arm platforms, including RTE_ARCH_ARM32
> and RTE_ARCH_ARM64.
> 
> To fit into the new naming scheme, current usage of RTE_ARCH_ARM in
> project code is mapped to RTE_ARCH_ARM32.
> 
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
Just to note that for all architectures there is the RTE_ARCH_64 define
which is set if the system is 64-bit. That could be used instead if you
didn't want to have to specially define ARM32 and ARM64 macros.

/Bruce

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms
  2020-08-14  8:13 ` Bruce Richardson
@ 2020-08-14  9:05   ` Ruifeng Wang
  2020-08-14 10:01     ` Bruce Richardson
  0 siblings, 1 reply; 9+ messages in thread
From: Ruifeng Wang @ 2020-08-14  9:05 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: hemant.agrawal, jerinj, viktorin, dev, Honnappa Nagarahalli,
	Phil Yang, nd, nd


> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, August 14, 2020 4:13 PM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: hemant.agrawal@nxp.com; jerinj@marvell.com;
> viktorin@rehivetech.com; dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Phil Yang <Phil.Yang@arm.com>; nd
> <nd@arm.com>
> Subject: Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm
> platforms
> 
> On Fri, Aug 14, 2020 at 02:03:20PM +0800, Ruifeng Wang wrote:
> > Flags are used to distinguish different platform architectures.
> > These flags can be used to pick different code paths for different
> > architectures at compile time.
> > For Arm platforms, there are 3 flags in use: RTE_ARCH_ARM,
> > RTE_ARCH_ARMv7 and RTE_ARCH_ARM64.
> > RTE_ARCH_ARM64 is used to flag 64-bit aarch64 platforms, while
> > RTE_ARCH_ARM & RTE_ARCH_ARMv7 are used to flag 32-bit platforms.
> > RTE_ARCH_ARMv7 is for ARMv7 platforms as its name suggested.
> >
> > The issue is that flag name RTE_ARCH_ARM is unclear and could cause
> > confusion. No info about platform word length is included in the name.
> > To make the flag names more clear, a naming scheme is proposed.
> >
> >       RTE_ARCH_ARM
> >           |
> >           +----RTE_ARCH_ARM32
> >           |        |
> >           |        +----RTE_ARCH_ARMv7
> >           |        |
> >           |        +----RTE_ARCH_ARMv8_AARCH32
> >           |
> >           +----RTE_ARCH_ARM64
> >
> > RTE_ARCH_ARM32 will be used for 32-bit Arm platforms.
> > It includes RTE_ARCH_ARMv7 and RTE_ARCH_ARMv8_AARCH32.
> > RTE_ARCH_ARMv7 is for ARMv7 platforms.
> > RTE_ARCH_ARMv8_AARCH32 is for aarch32 state on aarch64 platforms.
> > RTE_ARCH_ARM64 is for 64-bit Arm platforms.
> > RTE_ARCH_ARM will be used for all Arm platforms, including
> > RTE_ARCH_ARM32 and RTE_ARCH_ARM64.
> >
> > To fit into the new naming scheme, current usage of RTE_ARCH_ARM in
> > project code is mapped to RTE_ARCH_ARM32.
> >
> > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > ---
> Just to note that for all architectures there is the RTE_ARCH_64 define which
> is set if the system is 64-bit. That could be used instead if you didn't want to
> have to specially define ARM32 and ARM64 macros.
> 
Yes. Thanks for the note. 
RTE_ARCH_ARM64 is used in architecture specific cases. For example, when a processing 
path is not implemented by some of 64-bit architectures, RTE_ARCH_64 is not sufficient.

> /Bruce

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms
  2020-08-14  9:05   ` Ruifeng Wang
@ 2020-08-14 10:01     ` Bruce Richardson
  2020-08-14 10:42       ` Ruifeng Wang
  0 siblings, 1 reply; 9+ messages in thread
From: Bruce Richardson @ 2020-08-14 10:01 UTC (permalink / raw)
  To: Ruifeng Wang
  Cc: hemant.agrawal, jerinj, viktorin, dev, Honnappa Nagarahalli,
	Phil Yang, nd

On Fri, Aug 14, 2020 at 09:05:23AM +0000, Ruifeng Wang wrote:
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Friday, August 14, 2020 4:13 PM
> > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > Cc: hemant.agrawal@nxp.com; jerinj@marvell.com;
> > viktorin@rehivetech.com; dev@dpdk.org; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; Phil Yang <Phil.Yang@arm.com>; nd
> > <nd@arm.com>
> > Subject: Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm
> > platforms
> > 
> > On Fri, Aug 14, 2020 at 02:03:20PM +0800, Ruifeng Wang wrote:
> > > Flags are used to distinguish different platform architectures.
> > > These flags can be used to pick different code paths for different
> > > architectures at compile time.
> > > For Arm platforms, there are 3 flags in use: RTE_ARCH_ARM,
> > > RTE_ARCH_ARMv7 and RTE_ARCH_ARM64.
> > > RTE_ARCH_ARM64 is used to flag 64-bit aarch64 platforms, while
> > > RTE_ARCH_ARM & RTE_ARCH_ARMv7 are used to flag 32-bit platforms.
> > > RTE_ARCH_ARMv7 is for ARMv7 platforms as its name suggested.
> > >
> > > The issue is that flag name RTE_ARCH_ARM is unclear and could cause
> > > confusion. No info about platform word length is included in the name.
> > > To make the flag names more clear, a naming scheme is proposed.
> > >
> > >       RTE_ARCH_ARM
> > >           |
> > >           +----RTE_ARCH_ARM32
> > >           |        |
> > >           |        +----RTE_ARCH_ARMv7
> > >           |        |
> > >           |        +----RTE_ARCH_ARMv8_AARCH32
> > >           |
> > >           +----RTE_ARCH_ARM64
> > >
> > > RTE_ARCH_ARM32 will be used for 32-bit Arm platforms.
> > > It includes RTE_ARCH_ARMv7 and RTE_ARCH_ARMv8_AARCH32.
> > > RTE_ARCH_ARMv7 is for ARMv7 platforms.
> > > RTE_ARCH_ARMv8_AARCH32 is for aarch32 state on aarch64 platforms.
> > > RTE_ARCH_ARM64 is for 64-bit Arm platforms.
> > > RTE_ARCH_ARM will be used for all Arm platforms, including
> > > RTE_ARCH_ARM32 and RTE_ARCH_ARM64.
> > >
> > > To fit into the new naming scheme, current usage of RTE_ARCH_ARM in
> > > project code is mapped to RTE_ARCH_ARM32.
> > >
> > > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > > ---
> > Just to note that for all architectures there is the RTE_ARCH_64 define which
> > is set if the system is 64-bit. That could be used instead if you didn't want to
> > have to specially define ARM32 and ARM64 macros.
> > 
> Yes. Thanks for the note. 
> RTE_ARCH_ARM64 is used in architecture specific cases. For example, when a processing 
> path is not implemented by some of 64-bit architectures, RTE_ARCH_64 is not sufficient.
> 
Yes, but is RTE_ARCH_ARM64 not identical to RTE_ARCH_ARM && RTE_ARCH_64?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms
  2020-08-14 10:01     ` Bruce Richardson
@ 2020-08-14 10:42       ` Ruifeng Wang
  0 siblings, 0 replies; 9+ messages in thread
From: Ruifeng Wang @ 2020-08-14 10:42 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: hemant.agrawal, jerinj, viktorin, dev, Honnappa Nagarahalli,
	Phil Yang, nd, nd


> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, August 14, 2020 6:01 PM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> Cc: hemant.agrawal@nxp.com; jerinj@marvell.com;
> viktorin@rehivetech.com; dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Phil Yang <Phil.Yang@arm.com>; nd
> <nd@arm.com>
> Subject: Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm
> platforms
> 
> On Fri, Aug 14, 2020 at 09:05:23AM +0000, Ruifeng Wang wrote:
> >
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Friday, August 14, 2020 4:13 PM
> > > To: Ruifeng Wang <Ruifeng.Wang@arm.com>
> > > Cc: hemant.agrawal@nxp.com; jerinj@marvell.com;
> > > viktorin@rehivetech.com; dev@dpdk.org; Honnappa Nagarahalli
> > > <Honnappa.Nagarahalli@arm.com>; Phil Yang <Phil.Yang@arm.com>; nd
> > > <nd@arm.com>
> > > Subject: Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm
> > > platforms
> > >
> > > On Fri, Aug 14, 2020 at 02:03:20PM +0800, Ruifeng Wang wrote:
> > > > Flags are used to distinguish different platform architectures.
> > > > These flags can be used to pick different code paths for different
> > > > architectures at compile time.
> > > > For Arm platforms, there are 3 flags in use: RTE_ARCH_ARM,
> > > > RTE_ARCH_ARMv7 and RTE_ARCH_ARM64.
> > > > RTE_ARCH_ARM64 is used to flag 64-bit aarch64 platforms, while
> > > > RTE_ARCH_ARM & RTE_ARCH_ARMv7 are used to flag 32-bit platforms.
> > > > RTE_ARCH_ARMv7 is for ARMv7 platforms as its name suggested.
> > > >
> > > > The issue is that flag name RTE_ARCH_ARM is unclear and could
> > > > cause confusion. No info about platform word length is included in the
> name.
> > > > To make the flag names more clear, a naming scheme is proposed.
> > > >
> > > >       RTE_ARCH_ARM
> > > >           |
> > > >           +----RTE_ARCH_ARM32
> > > >           |        |
> > > >           |        +----RTE_ARCH_ARMv7
> > > >           |        |
> > > >           |        +----RTE_ARCH_ARMv8_AARCH32
> > > >           |
> > > >           +----RTE_ARCH_ARM64
> > > >
> > > > RTE_ARCH_ARM32 will be used for 32-bit Arm platforms.
> > > > It includes RTE_ARCH_ARMv7 and RTE_ARCH_ARMv8_AARCH32.
> > > > RTE_ARCH_ARMv7 is for ARMv7 platforms.
> > > > RTE_ARCH_ARMv8_AARCH32 is for aarch32 state on aarch64 platforms.
> > > > RTE_ARCH_ARM64 is for 64-bit Arm platforms.
> > > > RTE_ARCH_ARM will be used for all Arm platforms, including
> > > > RTE_ARCH_ARM32 and RTE_ARCH_ARM64.
> > > >
> > > > To fit into the new naming scheme, current usage of RTE_ARCH_ARM
> > > > in project code is mapped to RTE_ARCH_ARM32.
> > > >
> > > > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > > > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > > > ---
> > > Just to note that for all architectures there is the RTE_ARCH_64
> > > define which is set if the system is 64-bit. That could be used
> > > instead if you didn't want to have to specially define ARM32 and ARM64
> macros.
> > >
> > Yes. Thanks for the note.
> > RTE_ARCH_ARM64 is used in architecture specific cases. For example,
> > when a processing path is not implemented by some of 64-bit architectures,
> RTE_ARCH_64 is not sufficient.
> >
> Yes, but is RTE_ARCH_ARM64 not identical to RTE_ARCH_ARM &&
> RTE_ARCH_64?

My thought is:
1. RTE_ARCH_ARM64 is a direct macro, so with better readability.
2. RTE_ARCH_ARM64 is already used widely in the project.
So I think it is better not to replace the occurrences with combination of other macros.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms
  2020-08-14  6:03 [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms Ruifeng Wang
  2020-08-14  8:13 ` Bruce Richardson
@ 2020-08-18 14:36 ` Ferruh Yigit
  2020-08-18 14:53   ` Bruce Richardson
  2020-08-19  8:01   ` Ruifeng Wang
  1 sibling, 2 replies; 9+ messages in thread
From: Ferruh Yigit @ 2020-08-18 14:36 UTC (permalink / raw)
  To: Ruifeng Wang, hemant.agrawal, jerinj, viktorin
  Cc: dev, honnappa.nagarahalli, phil.yang, nd

On 8/14/2020 7:03 AM, Ruifeng Wang wrote:
> Flags are used to distinguish different platform architectures.
> These flags can be used to pick different code paths for different
> architectures at compile time.
> For Arm platforms, there are 3 flags in use: RTE_ARCH_ARM,
> RTE_ARCH_ARMv7 and RTE_ARCH_ARM64.
> RTE_ARCH_ARM64 is used to flag 64-bit aarch64 platforms,
> while RTE_ARCH_ARM & RTE_ARCH_ARMv7 are used to flag 32-bit platforms.
> RTE_ARCH_ARMv7 is for ARMv7 platforms as its name suggested.
> 
> The issue is that flag name RTE_ARCH_ARM is unclear and could cause
> confusion. No info about platform word length is included in the name.
> To make the flag names more clear, a naming scheme is proposed.
> 
>       RTE_ARCH_ARM
>           |
>           +----RTE_ARCH_ARM32
>           |        |
>           |        +----RTE_ARCH_ARMv7
>           |        |
>           |        +----RTE_ARCH_ARMv8_AARCH32
>           |
>           +----RTE_ARCH_ARM64
> 
> RTE_ARCH_ARM32 will be used for 32-bit Arm platforms.
> It includes RTE_ARCH_ARMv7 and RTE_ARCH_ARMv8_AARCH32.
> RTE_ARCH_ARMv7 is for ARMv7 platforms.
> RTE_ARCH_ARMv8_AARCH32 is for aarch32 state on aarch64 platforms.
> RTE_ARCH_ARM64 is for 64-bit Arm platforms.
> RTE_ARCH_ARM will be used for all Arm platforms, including RTE_ARCH_ARM32
> and RTE_ARCH_ARM64.
> 
> To fit into the new naming scheme, current usage of RTE_ARCH_ARM in
> project code is mapped to RTE_ARCH_ARM32.
> 
> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---

<...>

> @@ -6,7 +6,7 @@
>  CONFIG_RTE_MACHINE="armv7a"
>  
>  CONFIG_RTE_ARCH="arm"
> -CONFIG_RTE_ARCH_ARM=y
> +CONFIG_RTE_ARCH_ARM32=y
>  CONFIG_RTE_ARCH_ARMv7=y
>  CONFIG_RTE_ARCH_ARM_TUNE="cortex-a9"

According commit log message I thought 'RTE_ARCH_ARM' will be always set, isn't
it the case?

Is below wrong:
aarch64  -> ARM | ARM64 | ARCH_64
armv7a   -> ARM | ARM32 | ARMv7
aarch32  -> ARM | ARM32 | ARMv8_AARCH32

If so some of the 'defined(RTE_ARCH_ARM32) || defined(RTE_ARCH_ARM64)' checks
can be simplified as 'defined(RTE_ARCH_ARM)'


Also currently missing 'ARCH_64' flag implies the 32bit support, for all
architectures, what about having a common 'ARCH_32' flag and use for all arch,
instead of 'ARM32'? So something like below:
aarch64  -> ARM | ARM64 | ARCH_64
armv7a   -> ARM | ARMv7 | ARCH_32
aarch32  -> ARM | ARMv8_AARCH32 | ARCH_32




Just to record matching flags for other archs are:

x86_64  -> X86 | X86_64 | ARCH_64
i686    -> X86 | I686
x86_32  -> X86 | X86_32

ppc_64  -> PPC_64 | ARCH_64


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms
  2020-08-18 14:36 ` Ferruh Yigit
@ 2020-08-18 14:53   ` Bruce Richardson
  2020-08-18 18:48     ` Ferruh Yigit
  2020-08-19  8:01   ` Ruifeng Wang
  1 sibling, 1 reply; 9+ messages in thread
From: Bruce Richardson @ 2020-08-18 14:53 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Ruifeng Wang, hemant.agrawal, jerinj, viktorin, dev,
	honnappa.nagarahalli, phil.yang, nd

On Tue, Aug 18, 2020 at 03:36:00PM +0100, Ferruh Yigit wrote:
> On 8/14/2020 7:03 AM, Ruifeng Wang wrote:
> > Flags are used to distinguish different platform architectures.
> > These flags can be used to pick different code paths for different
> > architectures at compile time.
> > For Arm platforms, there are 3 flags in use: RTE_ARCH_ARM,
> > RTE_ARCH_ARMv7 and RTE_ARCH_ARM64.
> > RTE_ARCH_ARM64 is used to flag 64-bit aarch64 platforms,
> > while RTE_ARCH_ARM & RTE_ARCH_ARMv7 are used to flag 32-bit platforms.
> > RTE_ARCH_ARMv7 is for ARMv7 platforms as its name suggested.
> > 
> > The issue is that flag name RTE_ARCH_ARM is unclear and could cause
> > confusion. No info about platform word length is included in the name.
> > To make the flag names more clear, a naming scheme is proposed.
> > 
> >       RTE_ARCH_ARM
> >           |
> >           +----RTE_ARCH_ARM32
> >           |        |
> >           |        +----RTE_ARCH_ARMv7
> >           |        |
> >           |        +----RTE_ARCH_ARMv8_AARCH32
> >           |
> >           +----RTE_ARCH_ARM64
> > 
> > RTE_ARCH_ARM32 will be used for 32-bit Arm platforms.
> > It includes RTE_ARCH_ARMv7 and RTE_ARCH_ARMv8_AARCH32.
> > RTE_ARCH_ARMv7 is for ARMv7 platforms.
> > RTE_ARCH_ARMv8_AARCH32 is for aarch32 state on aarch64 platforms.
> > RTE_ARCH_ARM64 is for 64-bit Arm platforms.
> > RTE_ARCH_ARM will be used for all Arm platforms, including RTE_ARCH_ARM32
> > and RTE_ARCH_ARM64.
> > 
> > To fit into the new naming scheme, current usage of RTE_ARCH_ARM in
> > project code is mapped to RTE_ARCH_ARM32.
> > 
> > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > ---
> 
> <...>
> 
> > @@ -6,7 +6,7 @@
> >  CONFIG_RTE_MACHINE="armv7a"
> >  
> >  CONFIG_RTE_ARCH="arm"
> > -CONFIG_RTE_ARCH_ARM=y
> > +CONFIG_RTE_ARCH_ARM32=y
> >  CONFIG_RTE_ARCH_ARMv7=y
> >  CONFIG_RTE_ARCH_ARM_TUNE="cortex-a9"
> 
> According commit log message I thought 'RTE_ARCH_ARM' will be always set, isn't
> it the case?
> 
> Is below wrong:
> aarch64  -> ARM | ARM64 | ARCH_64
> armv7a   -> ARM | ARM32 | ARMv7
> aarch32  -> ARM | ARM32 | ARMv8_AARCH32
> 
> If so some of the 'defined(RTE_ARCH_ARM32) || defined(RTE_ARCH_ARM64)' checks
> can be simplified as 'defined(RTE_ARCH_ARM)'
> 
> 
> Also currently missing 'ARCH_64' flag implies the 32bit support, for all
> architectures, what about having a common 'ARCH_32' flag and use for all arch,
> instead of 'ARM32'? So something like below:
> aarch64  -> ARM | ARM64 | ARCH_64
> armv7a   -> ARM | ARMv7 | ARCH_32
> aarch32  -> ARM | ARMv8_AARCH32 | ARCH_32
> 
Not sure why you would need ARCH_32, since it's basically just !ARCH_64.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms
  2020-08-18 14:53   ` Bruce Richardson
@ 2020-08-18 18:48     ` Ferruh Yigit
  0 siblings, 0 replies; 9+ messages in thread
From: Ferruh Yigit @ 2020-08-18 18:48 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Ruifeng Wang, hemant.agrawal, jerinj, viktorin, dev,
	honnappa.nagarahalli, phil.yang, nd

On 8/18/2020 3:53 PM, Bruce Richardson wrote:
> On Tue, Aug 18, 2020 at 03:36:00PM +0100, Ferruh Yigit wrote:
>> On 8/14/2020 7:03 AM, Ruifeng Wang wrote:
>>> Flags are used to distinguish different platform architectures.
>>> These flags can be used to pick different code paths for different
>>> architectures at compile time.
>>> For Arm platforms, there are 3 flags in use: RTE_ARCH_ARM,
>>> RTE_ARCH_ARMv7 and RTE_ARCH_ARM64.
>>> RTE_ARCH_ARM64 is used to flag 64-bit aarch64 platforms,
>>> while RTE_ARCH_ARM & RTE_ARCH_ARMv7 are used to flag 32-bit platforms.
>>> RTE_ARCH_ARMv7 is for ARMv7 platforms as its name suggested.
>>>
>>> The issue is that flag name RTE_ARCH_ARM is unclear and could cause
>>> confusion. No info about platform word length is included in the name.
>>> To make the flag names more clear, a naming scheme is proposed.
>>>
>>>       RTE_ARCH_ARM
>>>           |
>>>           +----RTE_ARCH_ARM32
>>>           |        |
>>>           |        +----RTE_ARCH_ARMv7
>>>           |        |
>>>           |        +----RTE_ARCH_ARMv8_AARCH32
>>>           |
>>>           +----RTE_ARCH_ARM64
>>>
>>> RTE_ARCH_ARM32 will be used for 32-bit Arm platforms.
>>> It includes RTE_ARCH_ARMv7 and RTE_ARCH_ARMv8_AARCH32.
>>> RTE_ARCH_ARMv7 is for ARMv7 platforms.
>>> RTE_ARCH_ARMv8_AARCH32 is for aarch32 state on aarch64 platforms.
>>> RTE_ARCH_ARM64 is for 64-bit Arm platforms.
>>> RTE_ARCH_ARM will be used for all Arm platforms, including RTE_ARCH_ARM32
>>> and RTE_ARCH_ARM64.
>>>
>>> To fit into the new naming scheme, current usage of RTE_ARCH_ARM in
>>> project code is mapped to RTE_ARCH_ARM32.
>>>
>>> Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>> Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>> ---
>>
>> <...>
>>
>>> @@ -6,7 +6,7 @@
>>>  CONFIG_RTE_MACHINE="armv7a"
>>>  
>>>  CONFIG_RTE_ARCH="arm"
>>> -CONFIG_RTE_ARCH_ARM=y
>>> +CONFIG_RTE_ARCH_ARM32=y
>>>  CONFIG_RTE_ARCH_ARMv7=y
>>>  CONFIG_RTE_ARCH_ARM_TUNE="cortex-a9"
>>
>> According commit log message I thought 'RTE_ARCH_ARM' will be always set, isn't
>> it the case?
>>
>> Is below wrong:
>> aarch64  -> ARM | ARM64 | ARCH_64
>> armv7a   -> ARM | ARM32 | ARMv7
>> aarch32  -> ARM | ARM32 | ARMv8_AARCH32
>>
>> If so some of the 'defined(RTE_ARCH_ARM32) || defined(RTE_ARCH_ARM64)' checks
>> can be simplified as 'defined(RTE_ARCH_ARM)'
>>
>>
>> Also currently missing 'ARCH_64' flag implies the 32bit support, for all
>> architectures, what about having a common 'ARCH_32' flag and use for all arch,
>> instead of 'ARM32'? So something like below:
>> aarch64  -> ARM | ARM64 | ARCH_64
>> armv7a   -> ARM | ARMv7 | ARCH_32
>> aarch32  -> ARM | ARMv8_AARCH32 | ARCH_32
>>
> Not sure why you would need ARCH_32, since it's basically just !ARCH_64.
> 

Just to be more explicit, other than that same as '!ARCH_64'.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms
  2020-08-18 14:36 ` Ferruh Yigit
  2020-08-18 14:53   ` Bruce Richardson
@ 2020-08-19  8:01   ` Ruifeng Wang
  1 sibling, 0 replies; 9+ messages in thread
From: Ruifeng Wang @ 2020-08-19  8:01 UTC (permalink / raw)
  To: Ferruh Yigit, hemant.agrawal, jerinj, viktorin
  Cc: dev, Honnappa Nagarahalli, Phil Yang, nd, nd


> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, August 18, 2020 10:36 PM
> To: Ruifeng Wang <Ruifeng.Wang@arm.com>; hemant.agrawal@nxp.com;
> jerinj@marvell.com; viktorin@rehivetech.com
> Cc: dev@dpdk.org; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Phil Yang <Phil.Yang@arm.com>; nd
> <nd@arm.com>
> Subject: Re: [dpdk-dev] [RFC PATCH] config: remap flags used for Arm
> platforms
> 
> On 8/14/2020 7:03 AM, Ruifeng Wang wrote:
> > Flags are used to distinguish different platform architectures.
> > These flags can be used to pick different code paths for different
> > architectures at compile time.
> > For Arm platforms, there are 3 flags in use: RTE_ARCH_ARM,
> > RTE_ARCH_ARMv7 and RTE_ARCH_ARM64.
> > RTE_ARCH_ARM64 is used to flag 64-bit aarch64 platforms, while
> > RTE_ARCH_ARM & RTE_ARCH_ARMv7 are used to flag 32-bit platforms.
> > RTE_ARCH_ARMv7 is for ARMv7 platforms as its name suggested.
> >
> > The issue is that flag name RTE_ARCH_ARM is unclear and could cause
> > confusion. No info about platform word length is included in the name.
> > To make the flag names more clear, a naming scheme is proposed.
> >
> >       RTE_ARCH_ARM
> >           |
> >           +----RTE_ARCH_ARM32
> >           |        |
> >           |        +----RTE_ARCH_ARMv7
> >           |        |
> >           |        +----RTE_ARCH_ARMv8_AARCH32
> >           |
> >           +----RTE_ARCH_ARM64
> >
> > RTE_ARCH_ARM32 will be used for 32-bit Arm platforms.
> > It includes RTE_ARCH_ARMv7 and RTE_ARCH_ARMv8_AARCH32.
> > RTE_ARCH_ARMv7 is for ARMv7 platforms.
> > RTE_ARCH_ARMv8_AARCH32 is for aarch32 state on aarch64 platforms.
> > RTE_ARCH_ARM64 is for 64-bit Arm platforms.
> > RTE_ARCH_ARM will be used for all Arm platforms, including
> > RTE_ARCH_ARM32 and RTE_ARCH_ARM64.
> >
> > To fit into the new naming scheme, current usage of RTE_ARCH_ARM in
> > project code is mapped to RTE_ARCH_ARM32.
> >
> > Suggested-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Signed-off-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > ---
> 
> <...>
> 
> > @@ -6,7 +6,7 @@
> >  CONFIG_RTE_MACHINE="armv7a"
> >
> >  CONFIG_RTE_ARCH="arm"
> > -CONFIG_RTE_ARCH_ARM=y
> > +CONFIG_RTE_ARCH_ARM32=y
> >  CONFIG_RTE_ARCH_ARMv7=y
> >  CONFIG_RTE_ARCH_ARM_TUNE="cortex-a9"
> 
> According commit log message I thought 'RTE_ARCH_ARM' will be always set,
> isn't it the case?
> 
> Is below wrong:
> aarch64  -> ARM | ARM64 | ARCH_64
> armv7a   -> ARM | ARM32 | ARMv7
> aarch32  -> ARM | ARM32 | ARMv8_AARCH32
> 
Yes, it is.
This is correct.

> If so some of the 'defined(RTE_ARCH_ARM32) ||
> defined(RTE_ARCH_ARM64)' checks can be simplified as
> 'defined(RTE_ARCH_ARM)'
> 
Will do the change when converting this RFC to a patch.

> 
> Also currently missing 'ARCH_64' flag implies the 32bit support, for all
> architectures, what about having a common 'ARCH_32' flag and use for all
> arch, instead of 'ARM32'? So something like below:
> aarch64  -> ARM | ARM64 | ARCH_64
> armv7a   -> ARM | ARMv7 | ARCH_32
> aarch32  -> ARM | ARMv8_AARCH32 | ARCH_32
> 
Having a common 'ARCH_32' flag for all arch is good.

Then for Arm we will have:
RTE_ARCH_ARM
    |
    +----RTE_ARCH_32
    |        |
    |        +----RTE_ARCH_ARMv7
    |        |
    |        +----RTE_ARCH_ARMv8_AARCH32
    |
    +----RTE_ARCH_64
             |
             +----RTE_ARCH_ARM64

For x86 we will have:
RTE_ARCH_X86
    |
    +----RTE_ARCH_32
    |        |
    |        +----RTE_ARCH_I686
    |        |
    |        +----RTE_ARCH_X86_X32
    |
    +----RTE_ARCH_64
             |
             +----RTE_ARCH_X86_64

For PowerPC we will have: RTE_ARCH_PPC_64

> 
> 
> 
> Just to record matching flags for other archs are:
> 
> x86_64  -> X86 | X86_64 | ARCH_64
> i686    -> X86 | I686
> x86_32  -> X86 | X86_32
> 
> ppc_64  -> PPC_64 | ARCH_64


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-08-19  8:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-14  6:03 [dpdk-dev] [RFC PATCH] config: remap flags used for Arm platforms Ruifeng Wang
2020-08-14  8:13 ` Bruce Richardson
2020-08-14  9:05   ` Ruifeng Wang
2020-08-14 10:01     ` Bruce Richardson
2020-08-14 10:42       ` Ruifeng Wang
2020-08-18 14:36 ` Ferruh Yigit
2020-08-18 14:53   ` Bruce Richardson
2020-08-18 18:48     ` Ferruh Yigit
2020-08-19  8:01   ` Ruifeng Wang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).