Including rte_byteorder.h may fail for ARM builds with 'Platform must be built with RTE_FORCE_INTRINSICS' if rte_config.h is not included before. Include rte_config.h from rte_byteorder.h to solve the issue. Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> --- lib/eal/arm/include/rte_byteorder.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/eal/arm/include/rte_byteorder.h b/lib/eal/arm/include/rte_byteorder.h index df2f1d87ba..1f90db9943 100644 --- a/lib/eal/arm/include/rte_byteorder.h +++ b/lib/eal/arm/include/rte_byteorder.h @@ -5,18 +5,19 @@ #ifndef _RTE_BYTEORDER_ARM_H_ #define _RTE_BYTEORDER_ARM_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif #include <stdint.h> #include <rte_common.h> +#include <rte_config.h> #include "generic/rte_byteorder.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) -- 2.31.1
On Fri, Jun 4, 2021 at 5:18 PM Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> wrote: > > Including rte_byteorder.h may fail for ARM builds with 'Platform must > be built with RTE_FORCE_INTRINSICS' if rte_config.h is not included > before. Include rte_config.h from rte_byteorder.h to solve the issue. > > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> Since -include rte_config.h added by meson, This issue is not coming with in tree build. IMO, Fix is correct. Please send v2 with - Add Fixes: tag and Cc: stable@dpdk.org so that i can be backported to stable tree. - Fix following checkformat.sh warning Wrong headline format: eal: add include for rte_byteorder on ARM Wrong headline case: "eal: add include for rte_byteorder on ARM": ARM --> Arm - Subject could be "eal: arm: fix out of tree build" or so. With above change: Acked-by: Jerin Jacob <jerinj@marvell.com> > --- > lib/eal/arm/include/rte_byteorder.h | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/lib/eal/arm/include/rte_byteorder.h b/lib/eal/arm/include/rte_byteorder.h > index df2f1d87ba..1f90db9943 100644 > --- a/lib/eal/arm/include/rte_byteorder.h > +++ b/lib/eal/arm/include/rte_byteorder.h > @@ -5,18 +5,19 @@ > #ifndef _RTE_BYTEORDER_ARM_H_ > #define _RTE_BYTEORDER_ARM_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS > -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > > #include <stdint.h> > #include <rte_common.h> > +#include <rte_config.h> > #include "generic/rte_byteorder.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS > +#endif > + > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ > #if !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) > > -- > 2.31.1 >
Including rte_byteorder.h may fail for ARM builds with 'Platform must be built with RTE_FORCE_INTRINSICS' if rte_config.h is not included before. Include rte_config.h from rte_byteorder.h to solve the issue. Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM") Cc: kosar@rehivetech.com Cc: stable@dpdk.org Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> --- v3: * Proper in-reply-to. v2: * Fixed subject line. * Add "Fixes:" and CC to stable. lib/eal/arm/include/rte_byteorder.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/eal/arm/include/rte_byteorder.h b/lib/eal/arm/include/rte_byteorder.h index df2f1d87ba..1f90db9943 100644 --- a/lib/eal/arm/include/rte_byteorder.h +++ b/lib/eal/arm/include/rte_byteorder.h @@ -5,18 +5,19 @@ #ifndef _RTE_BYTEORDER_ARM_H_ #define _RTE_BYTEORDER_ARM_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif #include <stdint.h> #include <rte_common.h> +#include <rte_config.h> #include "generic/rte_byteorder.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) -- 2.31.1
> -----Original Message----- > From: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> > Sent: Monday, June 7, 2021 2:53 PM > To: Jan Viktorin <viktorin@rehivetech.com>; Ruifeng Wang > <Ruifeng.Wang@arm.com>; jerinj@marvell.com > Cc: dev@dpdk.org; Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>; > kosar@rehivetech.com; stable@dpdk.org > Subject: [PATCH v3] eal: arm: fix out of tree build > > Including rte_byteorder.h may fail for ARM builds with 'Platform must be > built with RTE_FORCE_INTRINSICS' if rte_config.h is not included before. > Include rte_config.h from rte_byteorder.h to solve the issue. > > Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM") > Cc: kosar@rehivetech.com > Cc: stable@dpdk.org > > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> > --- > v3: > * Proper in-reply-to. > > v2: > * Fixed subject line. > * Add "Fixes:" and CC to stable. > > lib/eal/arm/include/rte_byteorder.h | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/lib/eal/arm/include/rte_byteorder.h > b/lib/eal/arm/include/rte_byteorder.h > index df2f1d87ba..1f90db9943 100644 > --- a/lib/eal/arm/include/rte_byteorder.h > +++ b/lib/eal/arm/include/rte_byteorder.h > @@ -5,18 +5,19 @@ > #ifndef _RTE_BYTEORDER_ARM_H_ > #define _RTE_BYTEORDER_ARM_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif > - Moving of this section solves the problem. > #ifdef __cplusplus > extern "C" { > #endif > > #include <stdint.h> > #include <rte_common.h> > +#include <rte_config.h> rte_config.h is included by rte_common.h in above line. > #include "generic/rte_byteorder.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS #endif > + > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if !(__GNUC__ > > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) > > -- > 2.31.1 I see similar issue in rte_atomic32.h/rte_atomic64.h/rte_spinlock.h/rte_ticketlock.h/rte_mcslock.h in lib/eal/arm/include. Do you want to fix all these? Thanks.
Including various headers may fail for ARM builds with 'Platform must be built with RTE_FORCE_INTRINSICS' if rte_config.h is not included before. Move the error message after the includes to ensure rte_config.h is always included. Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM") Cc: kosar@rehivetech.com Cc: stable@dpdk.org Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> --- v4: * Apply to fix to rte_atomic_{32,64}.h, rte_{mcs,spin,ticket}lock.h as well. * Remove unnecessary include of rte_config.h. * Adjust commit message. v3: * Proper in-reply-to. v2: * Fixed subject line. lib/eal/arm/include/rte_atomic_32.h | 8 ++++---- lib/eal/arm/include/rte_atomic_64.h | 8 ++++---- lib/eal/arm/include/rte_byteorder.h | 8 ++++---- lib/eal/arm/include/rte_mcslock.h | 8 ++++---- lib/eal/arm/include/rte_spinlock.h | 8 ++++---- lib/eal/arm/include/rte_ticketlock.h | 8 ++++---- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/lib/eal/arm/include/rte_atomic_32.h b/lib/eal/arm/include/rte_atomic_32.h index c00ab78dba..138abd3a8b 100644 --- a/lib/eal/arm/include/rte_atomic_32.h +++ b/lib/eal/arm/include/rte_atomic_32.h @@ -5,16 +5,16 @@ #ifndef _RTE_ATOMIC_ARM32_H_ #define _RTE_ATOMIC_ARM32_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif #include "generic/rte_atomic.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + #define rte_mb() __sync_synchronize() #define rte_wmb() do { asm volatile ("dmb st" : : : "memory"); } while (0) diff --git a/lib/eal/arm/include/rte_atomic_64.h b/lib/eal/arm/include/rte_atomic_64.h index fa6f334c0d..2907a95196 100644 --- a/lib/eal/arm/include/rte_atomic_64.h +++ b/lib/eal/arm/include/rte_atomic_64.h @@ -6,10 +6,6 @@ #ifndef _RTE_ATOMIC_ARM64_H_ #define _RTE_ATOMIC_ARM64_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif @@ -19,6 +15,10 @@ extern "C" { #include <rte_compat.h> #include <rte_debug.h> +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + #define rte_mb() asm volatile("dmb osh" : : : "memory") #define rte_wmb() asm volatile("dmb oshst" : : : "memory") diff --git a/lib/eal/arm/include/rte_byteorder.h b/lib/eal/arm/include/rte_byteorder.h index df2f1d87ba..c00bebf680 100644 --- a/lib/eal/arm/include/rte_byteorder.h +++ b/lib/eal/arm/include/rte_byteorder.h @@ -5,10 +5,6 @@ #ifndef _RTE_BYTEORDER_ARM_H_ #define _RTE_BYTEORDER_ARM_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif @@ -17,6 +13,10 @@ extern "C" { #include <rte_common.h> #include "generic/rte_byteorder.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) diff --git a/lib/eal/arm/include/rte_mcslock.h b/lib/eal/arm/include/rte_mcslock.h index 896d678073..915a3ed647 100644 --- a/lib/eal/arm/include/rte_mcslock.h +++ b/lib/eal/arm/include/rte_mcslock.h @@ -5,16 +5,16 @@ #ifndef _RTE_MCSLOCK_ARM_H_ #define _RTE_MCSLOCK_ARM_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif #include "generic/rte_mcslock.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + #ifdef __cplusplus } #endif diff --git a/lib/eal/arm/include/rte_spinlock.h b/lib/eal/arm/include/rte_spinlock.h index a973763c23..e29ddab89a 100644 --- a/lib/eal/arm/include/rte_spinlock.h +++ b/lib/eal/arm/include/rte_spinlock.h @@ -5,10 +5,6 @@ #ifndef _RTE_SPINLOCK_ARM_H_ #define _RTE_SPINLOCK_ARM_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif @@ -16,6 +12,10 @@ extern "C" { #include <rte_common.h> #include "generic/rte_spinlock.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + static inline int rte_tm_supported(void) { return 0; diff --git a/lib/eal/arm/include/rte_ticketlock.h b/lib/eal/arm/include/rte_ticketlock.h index e66beefbe2..22eb40fb66 100644 --- a/lib/eal/arm/include/rte_ticketlock.h +++ b/lib/eal/arm/include/rte_ticketlock.h @@ -5,16 +5,16 @@ #ifndef _RTE_TICKETLOCK_ARM_H_ #define _RTE_TICKETLOCK_ARM_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif #include "generic/rte_ticketlock.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + #ifdef __cplusplus } #endif -- 2.32.0
> -----Original Message----- > From: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> > Sent: Tuesday, June 8, 2021 6:29 PM > To: Jan Viktorin <viktorin@rehivetech.com>; Ruifeng Wang > <Ruifeng.Wang@arm.com>; jerinj@marvell.com > Cc: dev@dpdk.org; Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>; > kosar@rehivetech.com; stable@dpdk.org > Subject: [PATCH v4] eal: arm: fix out of tree build > > Including various headers may fail for ARM builds with 'Platform must be built > with RTE_FORCE_INTRINSICS' if rte_config.h is not included before. Move > the error message after the includes to ensure rte_config.h is always > included. > > Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM") Thanks for the patch. More fix lines should be added: Fixes: 17d5fa0fa90d ("eal/arm: add atomic operations for ARMv7") Fixes: d708f01b7102 ("eal/arm: add atomic operations for ARMv8") Fixes: 2173f3333b61 ("mcslock: add MCS queued lock implementation") Fixes: 7860c3965483 ("eal/arm: add spinlock operations for ARM") Fixes: ca49b92079df ("ticketlock: enable generic ticketlock on all arch") With the change: Acked-by: Ruifeng Wang <ruifeng.wang@arm.com> > Cc: kosar@rehivetech.com > Cc: stable@dpdk.org > > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> > --- > v4: > * Apply to fix to rte_atomic_{32,64}.h, rte_{mcs,spin,ticket}lock.h as > well. > * Remove unnecessary include of rte_config.h. > * Adjust commit message. > > v3: > * Proper in-reply-to. > > v2: > * Fixed subject line. > > lib/eal/arm/include/rte_atomic_32.h | 8 ++++---- > lib/eal/arm/include/rte_atomic_64.h | 8 ++++---- > lib/eal/arm/include/rte_byteorder.h | 8 ++++---- > lib/eal/arm/include/rte_mcslock.h | 8 ++++---- > lib/eal/arm/include/rte_spinlock.h | 8 ++++---- > lib/eal/arm/include/rte_ticketlock.h | 8 ++++---- > 6 files changed, 24 insertions(+), 24 deletions(-) > > diff --git a/lib/eal/arm/include/rte_atomic_32.h > b/lib/eal/arm/include/rte_atomic_32.h > index c00ab78dba..138abd3a8b 100644 > --- a/lib/eal/arm/include/rte_atomic_32.h > +++ b/lib/eal/arm/include/rte_atomic_32.h > @@ -5,16 +5,16 @@ > #ifndef _RTE_ATOMIC_ARM32_H_ > #define _RTE_ATOMIC_ARM32_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > > #include "generic/rte_atomic.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS #endif > + > #define rte_mb() __sync_synchronize() > > #define rte_wmb() do { asm volatile ("dmb st" : : : "memory"); } while > (0) > diff --git a/lib/eal/arm/include/rte_atomic_64.h > b/lib/eal/arm/include/rte_atomic_64.h > index fa6f334c0d..2907a95196 100644 > --- a/lib/eal/arm/include/rte_atomic_64.h > +++ b/lib/eal/arm/include/rte_atomic_64.h > @@ -6,10 +6,6 @@ > #ifndef _RTE_ATOMIC_ARM64_H_ > #define _RTE_ATOMIC_ARM64_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > @@ -19,6 +15,10 @@ extern "C" { > #include <rte_compat.h> > #include <rte_debug.h> > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS #endif > + > #define rte_mb() asm volatile("dmb osh" : : : "memory") > > #define rte_wmb() asm volatile("dmb oshst" : : : "memory") diff --git > a/lib/eal/arm/include/rte_byteorder.h > b/lib/eal/arm/include/rte_byteorder.h > index df2f1d87ba..c00bebf680 100644 > --- a/lib/eal/arm/include/rte_byteorder.h > +++ b/lib/eal/arm/include/rte_byteorder.h > @@ -5,10 +5,6 @@ > #ifndef _RTE_BYTEORDER_ARM_H_ > #define _RTE_BYTEORDER_ARM_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > @@ -17,6 +13,10 @@ extern "C" { > #include <rte_common.h> > #include "generic/rte_byteorder.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS #endif > + > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if !(__GNUC__ > > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) > > diff --git a/lib/eal/arm/include/rte_mcslock.h > b/lib/eal/arm/include/rte_mcslock.h > index 896d678073..915a3ed647 100644 > --- a/lib/eal/arm/include/rte_mcslock.h > +++ b/lib/eal/arm/include/rte_mcslock.h > @@ -5,16 +5,16 @@ > #ifndef _RTE_MCSLOCK_ARM_H_ > #define _RTE_MCSLOCK_ARM_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > > #include "generic/rte_mcslock.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS #endif > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/eal/arm/include/rte_spinlock.h > b/lib/eal/arm/include/rte_spinlock.h > index a973763c23..e29ddab89a 100644 > --- a/lib/eal/arm/include/rte_spinlock.h > +++ b/lib/eal/arm/include/rte_spinlock.h > @@ -5,10 +5,6 @@ > #ifndef _RTE_SPINLOCK_ARM_H_ > #define _RTE_SPINLOCK_ARM_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > @@ -16,6 +12,10 @@ extern "C" { > #include <rte_common.h> > #include "generic/rte_spinlock.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS #endif > + > static inline int rte_tm_supported(void) { > return 0; > diff --git a/lib/eal/arm/include/rte_ticketlock.h > b/lib/eal/arm/include/rte_ticketlock.h > index e66beefbe2..22eb40fb66 100644 > --- a/lib/eal/arm/include/rte_ticketlock.h > +++ b/lib/eal/arm/include/rte_ticketlock.h > @@ -5,16 +5,16 @@ > #ifndef _RTE_TICKETLOCK_ARM_H_ > #define _RTE_TICKETLOCK_ARM_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > > #include "generic/rte_ticketlock.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS #endif > + > #ifdef __cplusplus > } > #endif > -- > 2.32.0
Including various headers may fail for ARM builds with 'Platform must be built with RTE_FORCE_INTRINSICS' if rte_config.h is not included before. Move the error message after the includes to ensure rte_config.h is always included. Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM") Fixes: 17d5fa0fa90d ("eal/arm: add atomic operations for ARMv7") Fixes: d708f01b7102 ("eal/arm: add atomic operations for ARMv8") Fixes: 2173f3333b61 ("mcslock: add MCS queued lock implementation") Fixes: 7860c3965483 ("eal/arm: add spinlock operations for ARM") Fixes: ca49b92079df ("ticketlock: enable generic ticketlock on all arch") Cc: stable@dpdk.org Cc: kosar@rehivetech.com Cc: phil.yang@arm.com Cc: joyce.kong@arm.com Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> --- v5: * Add Fixes: tag. v4: * Apply to fix to rte_atomic_{32,64}.h, rte_{mcs,spin,ticket}lock.h as well. * Remove unnecessary include of rte_config.h. * Adjust commit message. v3: * Proper in-reply-to. v2: * Fixed subject line. lib/eal/arm/include/rte_atomic_32.h | 8 ++++---- lib/eal/arm/include/rte_atomic_64.h | 8 ++++---- lib/eal/arm/include/rte_byteorder.h | 8 ++++---- lib/eal/arm/include/rte_mcslock.h | 8 ++++---- lib/eal/arm/include/rte_spinlock.h | 8 ++++---- lib/eal/arm/include/rte_ticketlock.h | 8 ++++---- 6 files changed, 24 insertions(+), 24 deletions(-) diff --git a/lib/eal/arm/include/rte_atomic_32.h b/lib/eal/arm/include/rte_atomic_32.h index c00ab78dba..138abd3a8b 100644 --- a/lib/eal/arm/include/rte_atomic_32.h +++ b/lib/eal/arm/include/rte_atomic_32.h @@ -5,16 +5,16 @@ #ifndef _RTE_ATOMIC_ARM32_H_ #define _RTE_ATOMIC_ARM32_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif #include "generic/rte_atomic.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + #define rte_mb() __sync_synchronize() #define rte_wmb() do { asm volatile ("dmb st" : : : "memory"); } while (0) diff --git a/lib/eal/arm/include/rte_atomic_64.h b/lib/eal/arm/include/rte_atomic_64.h index fa6f334c0d..2907a95196 100644 --- a/lib/eal/arm/include/rte_atomic_64.h +++ b/lib/eal/arm/include/rte_atomic_64.h @@ -6,10 +6,6 @@ #ifndef _RTE_ATOMIC_ARM64_H_ #define _RTE_ATOMIC_ARM64_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif @@ -19,6 +15,10 @@ extern "C" { #include <rte_compat.h> #include <rte_debug.h> +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + #define rte_mb() asm volatile("dmb osh" : : : "memory") #define rte_wmb() asm volatile("dmb oshst" : : : "memory") diff --git a/lib/eal/arm/include/rte_byteorder.h b/lib/eal/arm/include/rte_byteorder.h index df2f1d87ba..c00bebf680 100644 --- a/lib/eal/arm/include/rte_byteorder.h +++ b/lib/eal/arm/include/rte_byteorder.h @@ -5,10 +5,6 @@ #ifndef _RTE_BYTEORDER_ARM_H_ #define _RTE_BYTEORDER_ARM_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif @@ -17,6 +13,10 @@ extern "C" { #include <rte_common.h> #include "generic/rte_byteorder.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + /* fix missing __builtin_bswap16 for gcc older then 4.8 */ #if !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) diff --git a/lib/eal/arm/include/rte_mcslock.h b/lib/eal/arm/include/rte_mcslock.h index 896d678073..915a3ed647 100644 --- a/lib/eal/arm/include/rte_mcslock.h +++ b/lib/eal/arm/include/rte_mcslock.h @@ -5,16 +5,16 @@ #ifndef _RTE_MCSLOCK_ARM_H_ #define _RTE_MCSLOCK_ARM_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif #include "generic/rte_mcslock.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + #ifdef __cplusplus } #endif diff --git a/lib/eal/arm/include/rte_spinlock.h b/lib/eal/arm/include/rte_spinlock.h index a973763c23..e29ddab89a 100644 --- a/lib/eal/arm/include/rte_spinlock.h +++ b/lib/eal/arm/include/rte_spinlock.h @@ -5,10 +5,6 @@ #ifndef _RTE_SPINLOCK_ARM_H_ #define _RTE_SPINLOCK_ARM_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif @@ -16,6 +12,10 @@ extern "C" { #include <rte_common.h> #include "generic/rte_spinlock.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + static inline int rte_tm_supported(void) { return 0; diff --git a/lib/eal/arm/include/rte_ticketlock.h b/lib/eal/arm/include/rte_ticketlock.h index e66beefbe2..22eb40fb66 100644 --- a/lib/eal/arm/include/rte_ticketlock.h +++ b/lib/eal/arm/include/rte_ticketlock.h @@ -5,16 +5,16 @@ #ifndef _RTE_TICKETLOCK_ARM_H_ #define _RTE_TICKETLOCK_ARM_H_ -#ifndef RTE_FORCE_INTRINSICS -# error Platform must be built with RTE_FORCE_INTRINSICS -#endif - #ifdef __cplusplus extern "C" { #endif #include "generic/rte_ticketlock.h" +#ifndef RTE_FORCE_INTRINSICS +# error Platform must be built with RTE_FORCE_INTRINSICS +#endif + #ifdef __cplusplus } #endif -- 2.32.0
> -----Original Message-----
> From: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> Sent: Wednesday, June 9, 2021 6:18 PM
> To: Jan Viktorin <viktorin@rehivetech.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; jerinj@marvell.com
> Cc: dev@dpdk.org; Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>;
> stable@dpdk.org; kosar@rehivetech.com; Phil Yang <Phil.Yang@arm.com>;
> Joyce Kong <Joyce.Kong@arm.com>
> Subject: [PATCH v5] eal: arm: fix out of tree build
>
> Including various headers may fail for ARM builds with 'Platform must be built
> with RTE_FORCE_INTRINSICS' if rte_config.h is not included before. Move
> the error message after the includes to ensure rte_config.h is always
> included.
>
> Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM")
> Fixes: 17d5fa0fa90d ("eal/arm: add atomic operations for ARMv7")
> Fixes: d708f01b7102 ("eal/arm: add atomic operations for ARMv8")
> Fixes: 2173f3333b61 ("mcslock: add MCS queued lock implementation")
> Fixes: 7860c3965483 ("eal/arm: add spinlock operations for ARM")
> Fixes: ca49b92079df ("ticketlock: enable generic ticketlock on all arch")
> Cc: stable@dpdk.org
> Cc: kosar@rehivetech.com
> Cc: phil.yang@arm.com
> Cc: joyce.kong@arm.com
>
> Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> ---
Acked-by: Ruifeng Wang <ruifeng.wang@arm.com>
On Wed, Jun 9, 2021 at 3:48 PM Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> wrote: > > Including various headers may fail for ARM builds with 'Platform must > be built with RTE_FORCE_INTRINSICS' if rte_config.h is not included > before. Move the error message after the includes to ensure rte_config.h > is always included. > > Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM") > Fixes: 17d5fa0fa90d ("eal/arm: add atomic operations for ARMv7") > Fixes: d708f01b7102 ("eal/arm: add atomic operations for ARMv8") > Fixes: 2173f3333b61 ("mcslock: add MCS queued lock implementation") > Fixes: 7860c3965483 ("eal/arm: add spinlock operations for ARM") > Fixes: ca49b92079df ("ticketlock: enable generic ticketlock on all arch") > Cc: stable@dpdk.org > Cc: kosar@rehivetech.com > Cc: phil.yang@arm.com > Cc: joyce.kong@arm.com > > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> Acked-by: Jerin Jacob <jerinj@marvell.com> > --- > v5: > * Add Fixes: tag. > > v4: > * Apply to fix to rte_atomic_{32,64}.h, rte_{mcs,spin,ticket}lock.h as > well. > * Remove unnecessary include of rte_config.h. > * Adjust commit message. > > v3: > * Proper in-reply-to. > > v2: > * Fixed subject line. > > lib/eal/arm/include/rte_atomic_32.h | 8 ++++---- > lib/eal/arm/include/rte_atomic_64.h | 8 ++++---- > lib/eal/arm/include/rte_byteorder.h | 8 ++++---- > lib/eal/arm/include/rte_mcslock.h | 8 ++++---- > lib/eal/arm/include/rte_spinlock.h | 8 ++++---- > lib/eal/arm/include/rte_ticketlock.h | 8 ++++---- > 6 files changed, 24 insertions(+), 24 deletions(-) > > diff --git a/lib/eal/arm/include/rte_atomic_32.h b/lib/eal/arm/include/rte_atomic_32.h > index c00ab78dba..138abd3a8b 100644 > --- a/lib/eal/arm/include/rte_atomic_32.h > +++ b/lib/eal/arm/include/rte_atomic_32.h > @@ -5,16 +5,16 @@ > #ifndef _RTE_ATOMIC_ARM32_H_ > #define _RTE_ATOMIC_ARM32_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS > -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > > #include "generic/rte_atomic.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS > +#endif > + > #define rte_mb() __sync_synchronize() > > #define rte_wmb() do { asm volatile ("dmb st" : : : "memory"); } while (0) > diff --git a/lib/eal/arm/include/rte_atomic_64.h b/lib/eal/arm/include/rte_atomic_64.h > index fa6f334c0d..2907a95196 100644 > --- a/lib/eal/arm/include/rte_atomic_64.h > +++ b/lib/eal/arm/include/rte_atomic_64.h > @@ -6,10 +6,6 @@ > #ifndef _RTE_ATOMIC_ARM64_H_ > #define _RTE_ATOMIC_ARM64_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS > -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > @@ -19,6 +15,10 @@ extern "C" { > #include <rte_compat.h> > #include <rte_debug.h> > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS > +#endif > + > #define rte_mb() asm volatile("dmb osh" : : : "memory") > > #define rte_wmb() asm volatile("dmb oshst" : : : "memory") > diff --git a/lib/eal/arm/include/rte_byteorder.h b/lib/eal/arm/include/rte_byteorder.h > index df2f1d87ba..c00bebf680 100644 > --- a/lib/eal/arm/include/rte_byteorder.h > +++ b/lib/eal/arm/include/rte_byteorder.h > @@ -5,10 +5,6 @@ > #ifndef _RTE_BYTEORDER_ARM_H_ > #define _RTE_BYTEORDER_ARM_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS > -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > @@ -17,6 +13,10 @@ extern "C" { > #include <rte_common.h> > #include "generic/rte_byteorder.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS > +#endif > + > /* fix missing __builtin_bswap16 for gcc older then 4.8 */ > #if !(__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 8)) > > diff --git a/lib/eal/arm/include/rte_mcslock.h b/lib/eal/arm/include/rte_mcslock.h > index 896d678073..915a3ed647 100644 > --- a/lib/eal/arm/include/rte_mcslock.h > +++ b/lib/eal/arm/include/rte_mcslock.h > @@ -5,16 +5,16 @@ > #ifndef _RTE_MCSLOCK_ARM_H_ > #define _RTE_MCSLOCK_ARM_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS > -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > > #include "generic/rte_mcslock.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS > +#endif > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/eal/arm/include/rte_spinlock.h b/lib/eal/arm/include/rte_spinlock.h > index a973763c23..e29ddab89a 100644 > --- a/lib/eal/arm/include/rte_spinlock.h > +++ b/lib/eal/arm/include/rte_spinlock.h > @@ -5,10 +5,6 @@ > #ifndef _RTE_SPINLOCK_ARM_H_ > #define _RTE_SPINLOCK_ARM_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS > -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > @@ -16,6 +12,10 @@ extern "C" { > #include <rte_common.h> > #include "generic/rte_spinlock.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS > +#endif > + > static inline int rte_tm_supported(void) > { > return 0; > diff --git a/lib/eal/arm/include/rte_ticketlock.h b/lib/eal/arm/include/rte_ticketlock.h > index e66beefbe2..22eb40fb66 100644 > --- a/lib/eal/arm/include/rte_ticketlock.h > +++ b/lib/eal/arm/include/rte_ticketlock.h > @@ -5,16 +5,16 @@ > #ifndef _RTE_TICKETLOCK_ARM_H_ > #define _RTE_TICKETLOCK_ARM_H_ > > -#ifndef RTE_FORCE_INTRINSICS > -# error Platform must be built with RTE_FORCE_INTRINSICS > -#endif > - > #ifdef __cplusplus > extern "C" { > #endif > > #include "generic/rte_ticketlock.h" > > +#ifndef RTE_FORCE_INTRINSICS > +# error Platform must be built with RTE_FORCE_INTRINSICS > +#endif > + > #ifdef __cplusplus > } > #endif > -- > 2.32.0 >
Hello Bruce,
On Wed, Jun 9, 2021 at 12:18 PM Michael Pfeiffer
<michael.pfeiffer@tu-ilmenau.de> wrote:
>
> Including various headers may fail for ARM builds with 'Platform must
> be built with RTE_FORCE_INTRINSICS' if rte_config.h is not included
> before. Move the error message after the includes to ensure rte_config.h
> is always included.
>
> Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM")
> Fixes: 17d5fa0fa90d ("eal/arm: add atomic operations for ARMv7")
> Fixes: d708f01b7102 ("eal/arm: add atomic operations for ARMv8")
> Fixes: 2173f3333b61 ("mcslock: add MCS queued lock implementation")
> Fixes: 7860c3965483 ("eal/arm: add spinlock operations for ARM")
> Fixes: ca49b92079df ("ticketlock: enable generic ticketlock on all arch")
> Cc: stable@dpdk.org
> Cc: kosar@rehivetech.com
> Cc: phil.yang@arm.com
> Cc: joyce.kong@arm.com
>
> Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
The header check currently compiles all headers with an implicit
"-include rte_config.h".
I suppose this is because it comes from the project level meson configuration.
Would there be a way to detect the issue fixed by this patch?
--
David Marchand
On Fri, Jun 11, 2021 at 11:54:25AM +0200, David Marchand wrote:
> Hello Bruce,
>
> On Wed, Jun 9, 2021 at 12:18 PM Michael Pfeiffer
> <michael.pfeiffer@tu-ilmenau.de> wrote:
> >
> > Including various headers may fail for ARM builds with 'Platform must
> > be built with RTE_FORCE_INTRINSICS' if rte_config.h is not included
> > before. Move the error message after the includes to ensure rte_config.h
> > is always included.
> >
> > Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM")
> > Fixes: 17d5fa0fa90d ("eal/arm: add atomic operations for ARMv7")
> > Fixes: d708f01b7102 ("eal/arm: add atomic operations for ARMv8")
> > Fixes: 2173f3333b61 ("mcslock: add MCS queued lock implementation")
> > Fixes: 7860c3965483 ("eal/arm: add spinlock operations for ARM")
> > Fixes: ca49b92079df ("ticketlock: enable generic ticketlock on all arch")
> > Cc: stable@dpdk.org
> > Cc: kosar@rehivetech.com
> > Cc: phil.yang@arm.com
> > Cc: joyce.kong@arm.com
> >
> > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
>
> The header check currently compiles all headers with an implicit
> "-include rte_config.h".
> I suppose this is because it comes from the project level meson configuration.
>
> Would there be a way to detect the issue fixed by this patch?
>
I'm not convinced that there is an issue here. For DPDK compiles
rte_config.h must always be included first, which is why it's included in
the cflags reported by pkg-config. If we do want to move away from having
rte_config as an omnipresent first include, we need to update many DPDK
headers to explicitly include it.
/Bruce
Hi there,
On Fri, 2021-06-11 at 10:59 +0100, Bruce Richardson wrote:
> On Fri, Jun 11, 2021 at 11:54:25AM +0200, David Marchand wrote:
> > Hello Bruce,
> >
> > On Wed, Jun 9, 2021 at 12:18 PM Michael Pfeiffer
> > <michael.pfeiffer@tu-ilmenau.de> wrote:
> > >
> > > Including various headers may fail for ARM builds with 'Platform must
> > > be built with RTE_FORCE_INTRINSICS' if rte_config.h is not included
> > > before. Move the error message after the includes to ensure rte_config.h
> > > is always included.
> > >
> > > Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM")
> > > Fixes: 17d5fa0fa90d ("eal/arm: add atomic operations for ARMv7")
> > > Fixes: d708f01b7102 ("eal/arm: add atomic operations for ARMv8")
> > > Fixes: 2173f3333b61 ("mcslock: add MCS queued lock implementation")
> > > Fixes: 7860c3965483 ("eal/arm: add spinlock operations for ARM")
> > > Fixes: ca49b92079df ("ticketlock: enable generic ticketlock on all arch")
> > > Cc: stable@dpdk.org
> > > Cc: kosar@rehivetech.com
> > > Cc: phil.yang@arm.com
> > > Cc: joyce.kong@arm.com
> > >
> > > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de>
> >
> > The header check currently compiles all headers with an implicit
> > "-include rte_config.h".
> > I suppose this is because it comes from the project level meson
> > configuration.
> >
> > Would there be a way to detect the issue fixed by this patch?
> >
> I'm not convinced that there is an issue here. For DPDK compiles
> rte_config.h must always be included first, which is why it's included in
> the cflags reported by pkg-config. If we do want to move away from having
> rte_config as an omnipresent first include, we need to update many DPDK
> headers to explicitly include it.
the issue came up in our (rather large) DPDK app. Our build process uses a
hand-crafted build system (based on ninja) for native and cross builds, i.e.
pkg-config is not involved. It basically throws "-isystem <our-target-
prefix>/include/dpdk" into the compiler and omits linking some of the DPDK libs
not used by us.
This worked fine for us, and has no issues for x86 targets. I guess because
most headers do include rte_config (or rte_common) at least transitively before
doing anything else. Anyway, if you regard "-include rte_config.h" as the way
to go, this is fine for me. Its a one-liner for us and we can probably drop the
patch then.
Regards
Michael
> -----Original Message----- > From: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> > Sent: Friday, June 11, 2021 6:28 PM > To: Bruce Richardson <bruce.richardson@intel.com>; David Marchand > <david.marchand@redhat.com> > Cc: Jan Viktorin <viktorin@rehivetech.com>; Ruifeng Wang > <Ruifeng.Wang@arm.com>; jerinj@marvell.com; dev <dev@dpdk.org>; > dpdk stable <stable@dpdk.org>; kosar@rehivetech.com; Phil Yang > <Phil.Yang@arm.com>; Joyce Kong <Joyce.Kong@arm.com> > Subject: Re: [dpdk-stable] [PATCH v5] eal: arm: fix out of tree build > > Hi there, > > On Fri, 2021-06-11 at 10:59 +0100, Bruce Richardson wrote: > > On Fri, Jun 11, 2021 at 11:54:25AM +0200, David Marchand wrote: > > > Hello Bruce, > > > > > > On Wed, Jun 9, 2021 at 12:18 PM Michael Pfeiffer > > > <michael.pfeiffer@tu-ilmenau.de> wrote: > > > > > > > > Including various headers may fail for ARM builds with 'Platform > > > > must be built with RTE_FORCE_INTRINSICS' if rte_config.h is not > > > > included before. Move the error message after the includes to > > > > ensure rte_config.h is always included. > > > > > > > > Fixes: de966ccdcd7f ("eal/arm: add byte order operations for ARM") > > > > Fixes: 17d5fa0fa90d ("eal/arm: add atomic operations for ARMv7") > > > > Fixes: d708f01b7102 ("eal/arm: add atomic operations for ARMv8") > > > > Fixes: 2173f3333b61 ("mcslock: add MCS queued lock > > > > implementation") > > > > Fixes: 7860c3965483 ("eal/arm: add spinlock operations for ARM") > > > > Fixes: ca49b92079df ("ticketlock: enable generic ticketlock on all > > > > arch") > > > > Cc: stable@dpdk.org > > > > Cc: kosar@rehivetech.com > > > > Cc: phil.yang@arm.com > > > > Cc: joyce.kong@arm.com > > > > > > > > Signed-off-by: Michael Pfeiffer <michael.pfeiffer@tu-ilmenau.de> > > > > > > The header check currently compiles all headers with an implicit > > > "-include rte_config.h". > > > I suppose this is because it comes from the project level meson > > > configuration. > > > > > > Would there be a way to detect the issue fixed by this patch? > > > > > I'm not convinced that there is an issue here. For DPDK compiles > > rte_config.h must always be included first, which is why it's included > > in the cflags reported by pkg-config. If we do want to move away from > > having rte_config as an omnipresent first include, we need to update > > many DPDK headers to explicitly include it. > > the issue came up in our (rather large) DPDK app. Our build process uses a > hand-crafted build system (based on ninja) for native and cross builds, i.e. > pkg-config is not involved. It basically throws "-isystem <our-target- > prefix>/include/dpdk" into the compiler and omits linking some of the > prefix>DPDK libs > not used by us. > > This worked fine for us, and has no issues for x86 targets. I guess because > most headers do include rte_config (or rte_common) at least transitively As what I can find, no issues for x86 because it has been fixed in commit: 0d440d081ca1 ("lib: fix missing includes in exported headers") > before doing anything else. Anyway, if you regard "-include rte_config.h" as > the way to go, this is fine for me. Its a one-liner for us and we can probably > drop the patch then. > > Regards > Michael