DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ray Kinsella <mdr@ashroe.eu>
To: Kevin Traynor <ktraynor@redhat.com>, akhil.goyal@nxp.com, dev@dpdk.org
Cc: hemant.agrawal@nxp.com, david.marchand@redhat.com,
	thomas@monjalon.net, stable@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] drivers/crypto: fix gcc 10 no-common errors
Date: Thu, 14 May 2020 08:42:32 +0100	[thread overview]
Message-ID: <deded474-9dcc-10fc-e6c0-0297803bcd4a@ashroe.eu> (raw)
In-Reply-To: <0a3c0209-8438-2586-e300-4dbee671e563@redhat.com>


Tidy solution.

On 13/05/2020 17:40, Kevin Traynor wrote:
> On 13/05/2020 14:50, akhil.goyal@nxp.com wrote:
>> From: Akhil Goyal <akhil.goyal@nxp.com>
>>
>> gcc 10 defaults to -fno-common and as a result when linking
>> with crypto drivers:
>>
>> drivers/librte_pmd_dpaa_sec.a(crypto_dpaa_sec_dpaa_sec.c.o):
>> (.bss+0x4): multiple definition of `rta_sec_era';
>> drivers/librte_pmd_caam_jr.a(crypto_caam_jr_caam_jr.c.o):
>> (.bss+0x0): first defined here
>>
>> drivers/librte_pmd_dpaa2_sec.a(crypto_dpaa2_sec_dpaa2_sec_dpseci.c.o):
>> (.data+0x0): multiple definition of `rta_sec_era';
>> drivers/librte_pmd_caam_jr.a(crypto_caam_jr_caam_jr.c.o):
>> (.bss+0x0): first defined here
>>
>> This patch fixes the blunt workaround in the following commit.
>>
>> Fixes: 50b03f3b8eaf ("drivers/crypto: disable gcc 10 no-common errors")
>>
>> Bugzilla ID: 469
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
>> ---
>> This patch is not tested on GCC 10.
>> @kevin Please verify on your setup if it is fixing the GCC 10
>> compilation.
>>
> 
> Passing with gcc 10 on my setup. Internal symbol part looks ok to me,
> but might be best to get review from Ray.
> 
> Tested-by: Kevin Traynor <ktraynor@redhat.com>
> 
>>
>>  drivers/common/dpaax/Makefile                    |  3 ++-
>>  drivers/common/dpaax/caamflib.c                  | 16 ++++++++++++++++
>>  drivers/common/dpaax/meson.build                 |  4 +++-
>>  .../common/dpaax/rte_common_dpaax_version.map    |  4 ++++
>>  drivers/crypto/caam_jr/Makefile                  |  7 -------
>>  drivers/crypto/caam_jr/caam_jr.c                 |  2 --
>>  drivers/crypto/caam_jr/meson.build               |  5 -----
>>  drivers/crypto/dpaa2_sec/Makefile                |  7 -------
>>  drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c      |  4 ++--
>>  drivers/crypto/dpaa2_sec/meson.build             |  5 -----
>>  drivers/crypto/dpaa_sec/Makefile                 |  7 -------
>>  drivers/crypto/dpaa_sec/dpaa_sec.c               |  2 --
>>  drivers/crypto/dpaa_sec/meson.build              |  5 -----
>>  13 files changed, 27 insertions(+), 44 deletions(-)
>>  create mode 100644 drivers/common/dpaax/caamflib.c
>>
>> diff --git a/drivers/common/dpaax/Makefile b/drivers/common/dpaax/Makefile
>> index 15b0b38d0..2f4b924fd 100644
>> --- a/drivers/common/dpaax/Makefile
>> +++ b/drivers/common/dpaax/Makefile
>> @@ -15,6 +15,7 @@ CFLAGS += -Wno-pointer-arith
>>  CFLAGS += -Wno-cast-qual
>>  
>>  CFLAGS += -I$(RTE_SDK)/drivers/common/dpaax
>> +CFLAGS += -I$(RTE_SDK)/drivers/common/dpaax/caamflib
>>  
>>  # versioning export map
>>  EXPORT_MAP := rte_common_dpaax_version.map
>> @@ -22,7 +23,7 @@ EXPORT_MAP := rte_common_dpaax_version.map
>>  #
>>  # all source are stored in SRCS-y
>>  #
>> -SRCS-y += dpaax_iova_table.c dpaa_of.c
>> +SRCS-y += dpaax_iova_table.c dpaa_of.c caamflib.c
>>  
>>  LDLIBS += -lrte_eal
>>  
>> diff --git a/drivers/common/dpaax/caamflib.c b/drivers/common/dpaax/caamflib.c
>> new file mode 100644
>> index 000000000..55e20281c
>> --- /dev/null
>> +++ b/drivers/common/dpaax/caamflib.c
>> @@ -0,0 +1,16 @@
>> +/* SPDX-License-Identifier: (BSD-3-Clause OR GPL-2.0)
>> + *
>> + * Copyright 2020 NXP
>> + *
>> + */
>> +
>> +#include <rta.h>
>> +
>> +/*
>> + * SEC HW block revision.
>> + *
>> + * This *must not be confused with SEC version*:
>> + * - SEC HW block revision format is "v"
>> + * - SEC revision format is "x.y"
>> + */
>> +enum rta_sec_era rta_sec_era;
>> diff --git a/drivers/common/dpaax/meson.build b/drivers/common/dpaax/meson.build
>> index ff2d1a507..0b8bf7bd5 100644
>> --- a/drivers/common/dpaax/meson.build
>> +++ b/drivers/common/dpaax/meson.build
>> @@ -6,7 +6,9 @@ if not is_linux
>>  	reason = 'only supported on linux'
>>  endif
>>  
>> -sources = files('dpaax_iova_table.c', 'dpaa_of.c')
>> +sources = files('dpaax_iova_table.c', 'dpaa_of.c', 'caamflib.c')
>> +
>> +includes += include_directories('caamflib')
>>  
>>  cflags += ['-D_GNU_SOURCE']
>>  if cc.has_argument('-Wno-cast-qual')
>> diff --git a/drivers/common/dpaax/rte_common_dpaax_version.map b/drivers/common/dpaax/rte_common_dpaax_version.map
>> index f72eba761..837ce01af 100644
>> --- a/drivers/common/dpaax/rte_common_dpaax_version.map
>> +++ b/drivers/common/dpaax/rte_common_dpaax_version.map
>> @@ -21,3 +21,7 @@ DPDK_20.0 {
>>  
>>  	local: *;
>>  };
>> +
>> +INTERNAL {
>> +	rta_sec_era;
>> +};
>> diff --git a/drivers/crypto/caam_jr/Makefile b/drivers/crypto/caam_jr/Makefile
>> index 10848884c..89d323817 100644
>> --- a/drivers/crypto/caam_jr/Makefile
>> +++ b/drivers/crypto/caam_jr/Makefile
>> @@ -14,13 +14,6 @@ CFLAGS += -D _GNU_SOURCE
>>  CFLAGS += -O3
>>  CFLAGS += $(WERROR_FLAGS)
>>  
>> -# FIXME: temporary solution for Bugzilla 469
>> -ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
>> -ifeq ($(shell test $(GCC_VERSION) -ge 100 && echo 1), 1)
>> -CFLAGS += -fcommon
>> -endif
>> -endif
>> -
>>  CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa/include
>>  CFLAGS += -I$(RTE_SDK)/drivers/common/dpaax
>>  CFLAGS += -I$(RTE_SDK)/drivers/common/dpaax/caamflib/
>> diff --git a/drivers/crypto/caam_jr/caam_jr.c b/drivers/crypto/caam_jr/caam_jr.c
>> index 5a29dd169..45003ba25 100644
>> --- a/drivers/crypto/caam_jr/caam_jr.c
>> +++ b/drivers/crypto/caam_jr/caam_jr.c
>> @@ -37,8 +37,6 @@
>>  static uint8_t cryptodev_driver_id;
>>  int caam_jr_logtype;
>>  
>> -enum rta_sec_era rta_sec_era;
>> -
>>  /* Lists the states possible for the SEC user space driver. */
>>  enum sec_driver_state_e {
>>  	SEC_DRIVER_STATE_IDLE,		/* Driver not initialized */
>> diff --git a/drivers/crypto/caam_jr/meson.build b/drivers/crypto/caam_jr/meson.build
>> index 551b13632..f8b5250a9 100644
>> --- a/drivers/crypto/caam_jr/meson.build
>> +++ b/drivers/crypto/caam_jr/meson.build
>> @@ -12,11 +12,6 @@ sources = files('caam_jr_capabilities.c',
>>  		'caam_jr_uio.c',
>>  		'caam_jr.c')
>>  
>> -# FIXME: temporary solution for Bugzilla 469
>> -if (toolchain == 'gcc' and cc.version().version_compare('>=10.0.0'))
>> -	cflags += '-fcommon'
>> -endif
>> -
>>  includes += include_directories('../../bus/dpaa/include/')
>>  includes += include_directories('../../common/dpaax/')
>>  includes += include_directories('../../common/dpaax/caamflib/')
>> diff --git a/drivers/crypto/dpaa2_sec/Makefile b/drivers/crypto/dpaa2_sec/Makefile
>> index 8ce637db6..a0a279557 100644
>> --- a/drivers/crypto/dpaa2_sec/Makefile
>> +++ b/drivers/crypto/dpaa2_sec/Makefile
>> @@ -19,13 +19,6 @@ CFLAGS += -Wno-implicit-fallthrough
>>  endif
>>  endif
>>  
>> -# FIXME: temporary solution for Bugzilla 469
>> -ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
>> -ifeq ($(shell test $(GCC_VERSION) -ge 100 && echo 1), 1)
>> -CFLAGS += -fcommon
>> -endif
>> -endif
>> -
>>  CFLAGS += -I$(RTE_SDK)/drivers/common/dpaax
>>  CFLAGS += -I$(RTE_SDK)/drivers/common/dpaax/caamflib
>>  CFLAGS += -I$(RTE_SDK)/drivers/crypto/dpaa2_sec/
>> diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
>> index fe34e644c..6459a025b 100644
>> --- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
>> +++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
>> @@ -56,8 +56,6 @@
>>  #define SEC_FLC_DHR_OUTBOUND	-114
>>  #define SEC_FLC_DHR_INBOUND	0
>>  
>> -enum rta_sec_era rta_sec_era = RTA_SEC_ERA_8;
>> -
>>  static uint8_t cryptodev_driver_id;
>>  
>>  int dpaa2_logtype_sec;
>> @@ -3870,6 +3868,8 @@ cryptodev_dpaa2_sec_probe(struct rte_dpaa2_driver *dpaa2_drv __rte_unused,
>>  
>>  	if (dpaa2_svr_family == SVR_LX2160A)
>>  		rta_set_sec_era(RTA_SEC_ERA_10);
>> +	else
>> +		rta_set_sec_era(RTA_SEC_ERA_8);
>>  
>>  	DPAA2_SEC_INFO("2-SEC ERA is %d", rta_get_sec_era());
>>  
>> diff --git a/drivers/crypto/dpaa2_sec/meson.build b/drivers/crypto/dpaa2_sec/meson.build
>> index 505ad9414..cb1c2d049 100644
>> --- a/drivers/crypto/dpaa2_sec/meson.build
>> +++ b/drivers/crypto/dpaa2_sec/meson.build
>> @@ -10,9 +10,4 @@ deps += ['security', 'mempool_dpaa2']
>>  sources = files('dpaa2_sec_dpseci.c',
>>  		'mc/dpseci.c')
>>  
>> -# FIXME: temporary solution for Bugzilla 469
>> -if (toolchain == 'gcc' and cc.version().version_compare('>=10.0.0'))
>> -	cflags += '-fcommon'
>> -endif
>> -
>>  includes += include_directories('mc', '../../common/dpaax', '../../common/dpaax/caamflib')
>> diff --git a/drivers/crypto/dpaa_sec/Makefile b/drivers/crypto/dpaa_sec/Makefile
>> index 6cf392cb3..ea266962a 100644
>> --- a/drivers/crypto/dpaa_sec/Makefile
>> +++ b/drivers/crypto/dpaa_sec/Makefile
>> @@ -13,13 +13,6 @@ LIB = librte_pmd_dpaa_sec.a
>>  CFLAGS += -O3
>>  CFLAGS += $(WERROR_FLAGS)
>>  
>> -# FIXME: temporary solution for Bugzilla 469
>> -ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
>> -ifeq ($(shell test $(GCC_VERSION) -ge 100 && echo 1), 1)
>> -CFLAGS += -fcommon
>> -endif
>> -endif
>> -
>>  CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa
>>  CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa/include
>>  CFLAGS += -I$(RTE_SDK)/drivers/bus/dpaa/base/qbman
>> diff --git a/drivers/crypto/dpaa_sec/dpaa_sec.c b/drivers/crypto/dpaa_sec/dpaa_sec.c
>> index 25dcbd259..a9bfb8685 100644
>> --- a/drivers/crypto/dpaa_sec/dpaa_sec.c
>> +++ b/drivers/crypto/dpaa_sec/dpaa_sec.c
>> @@ -43,8 +43,6 @@
>>  #include <dpaa_sec_log.h>
>>  #include <dpaax_iova_table.h>
>>  
>> -enum rta_sec_era rta_sec_era;
>> -
>>  int dpaa_logtype_sec;
>>  
>>  static uint8_t cryptodev_driver_id;
>> diff --git a/drivers/crypto/dpaa_sec/meson.build b/drivers/crypto/dpaa_sec/meson.build
>> index f5e660457..7d422d8d5 100644
>> --- a/drivers/crypto/dpaa_sec/meson.build
>> +++ b/drivers/crypto/dpaa_sec/meson.build
>> @@ -9,11 +9,6 @@ endif
>>  deps += ['bus_dpaa', 'mempool_dpaa', 'security']
>>  sources = files('dpaa_sec.c')
>>  
>> -# FIXME: temporary solution for Bugzilla 469
>> -if (toolchain == 'gcc' and cc.version().version_compare('>=10.0.0'))
>> -	cflags += '-fcommon'
>> -endif
>> -
>>  includes += include_directories('../../bus/dpaa/include')
>>  includes += include_directories('../../common/dpaax')
>>  includes += include_directories('../../common/dpaax/caamflib/')
>>
> 

Acked-by: Ray Kinsella <mdr@ashroe.eu>

  reply	other threads:[~2020-05-14  7:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-13 13:50 akhil.goyal
2020-05-13 16:40 ` Kevin Traynor
2020-05-14  7:42   ` Ray Kinsella [this message]
2020-05-15 18:11     ` Akhil Goyal
2020-05-13 18:43 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2020-05-14  6:09   ` Hemant Agrawal (OSS)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=deded474-9dcc-10fc-e6c0-0297803bcd4a@ashroe.eu \
    --to=mdr@ashroe.eu \
    --cc=akhil.goyal@nxp.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    --cc=ktraynor@redhat.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).