From: "Stanisław Kardach" <kda@semihalf.com>
To: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
Cc: Ruifeng Wang <ruifeng.wang@arm.com>,
Min Zhou <zhoumin@loongson.cn>,
David Christensen <drc@linux.vnet.ibm.com>,
Bruce Richardson <bruce.richardson@intel.com>,
Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>,
dev <dev@dpdk.org>
Subject: Re: [PATCH v2 2/2] eal: remove NUMFLAGS enumeration
Date: Tue, 15 Aug 2023 08:10:17 +0200 [thread overview]
Message-ID: <CALVGJWJoCsnOk_b4+uXCjYEs1krN-s=wEerpJWuTFnaY_nsrLQ@mail.gmail.com> (raw)
In-Reply-To: <20230811060755.481572-2-sivaprasad.tummala@amd.com>
[-- Attachment #1: Type: text/plain, Size: 9614 bytes --]
On Fri, Aug 11, 2023, 08:08 Sivaprasad Tummala <sivaprasad.tummala@amd.com>
wrote:
> This patch removes RTE_CPUFLAG_NUMFLAGS to allow new CPU
> features without breaking ABI each time.
>
> Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
> ---
> lib/eal/arm/include/rte_cpuflags_32.h | 1 -
> lib/eal/arm/include/rte_cpuflags_64.h | 1 -
> lib/eal/arm/rte_cpuflags.c | 7 +++++--
> lib/eal/loongarch/include/rte_cpuflags.h | 1 -
> lib/eal/loongarch/rte_cpuflags.c | 7 +++++--
> lib/eal/ppc/include/rte_cpuflags.h | 1 -
> lib/eal/ppc/rte_cpuflags.c | 7 +++++--
> lib/eal/riscv/include/rte_cpuflags.h | 1 -
> lib/eal/riscv/rte_cpuflags.c | 7 +++++--
> lib/eal/x86/include/rte_cpuflags.h | 1 -
> lib/eal/x86/rte_cpuflags.c | 7 +++++--
> 11 files changed, 25 insertions(+), 16 deletions(-)
>
> diff --git a/lib/eal/arm/include/rte_cpuflags_32.h
> b/lib/eal/arm/include/rte_cpuflags_32.h
> index 4e254428a2..41ab0d5f21 100644
> --- a/lib/eal/arm/include/rte_cpuflags_32.h
> +++ b/lib/eal/arm/include/rte_cpuflags_32.h
> @@ -43,7 +43,6 @@ enum rte_cpu_flag_t {
> RTE_CPUFLAG_V7L,
> RTE_CPUFLAG_V8L,
> /* The last item */
> - RTE_CPUFLAG_NUMFLAGS,/**< This should always be the last! */
> };
>
Since there is no description of V1 to V2 changes, could you point me to
what has changed?
Also I see you're still removing the RTE_CPUFLAG_NUMFLAGS (what I call a
last element canary). Why? If you're concerned with ABI, then we're talking
about an application linking dynamically with DPDK or talking via some RPC
channel with another DPDK application. So clashing with this definition
does not come into question. One should rather use
rte_cpu_get_flag_enabled().
Also if you want to introduce new features, one would add them yo the
rte_cpuflags headers, unless you'd like to not add those and keep an
undocumented list "above" the last defined element.
Could you explain a bit more Your use-case?
>
> #include "generic/rte_cpuflags.h"
> diff --git a/lib/eal/arm/include/rte_cpuflags_64.h
> b/lib/eal/arm/include/rte_cpuflags_64.h
> index aa7a56d491..ea5193e510 100644
> --- a/lib/eal/arm/include/rte_cpuflags_64.h
> +++ b/lib/eal/arm/include/rte_cpuflags_64.h
> @@ -37,7 +37,6 @@ enum rte_cpu_flag_t {
> RTE_CPUFLAG_SVEBF16,
> RTE_CPUFLAG_AARCH64,
> /* The last item */
> - RTE_CPUFLAG_NUMFLAGS,/**< This should always be the last! */
> };
>
> #include "generic/rte_cpuflags.h"
> diff --git a/lib/eal/arm/rte_cpuflags.c b/lib/eal/arm/rte_cpuflags.c
> index 56e7b2e689..f33fee242b 100644
> --- a/lib/eal/arm/rte_cpuflags.c
> +++ b/lib/eal/arm/rte_cpuflags.c
> @@ -139,8 +139,9 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
> {
> const struct feature_entry *feat;
> hwcap_registers_t regs = {0};
> + unsigned int num_flags = RTE_DIM(rte_cpu_feature_table);
>
> - if (feature >= RTE_CPUFLAG_NUMFLAGS)
> + if ((unsigned int)feature >= num_flags)
> return -ENOENT;
>
> feat = &rte_cpu_feature_table[feature];
> @@ -154,7 +155,9 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
> const char *
> rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
> {
> - if (feature >= RTE_CPUFLAG_NUMFLAGS)
> + unsigned int num_flags = RTE_DIM(rte_cpu_feature_table);
> +
> + if ((unsigned int)feature >= num_flags)
> return NULL;
> return rte_cpu_feature_table[feature].name;
> }
> diff --git a/lib/eal/loongarch/include/rte_cpuflags.h
> b/lib/eal/loongarch/include/rte_cpuflags.h
> index 1c80779262..9ff8baaa3c 100644
> --- a/lib/eal/loongarch/include/rte_cpuflags.h
> +++ b/lib/eal/loongarch/include/rte_cpuflags.h
> @@ -27,7 +27,6 @@ enum rte_cpu_flag_t {
> RTE_CPUFLAG_LBT_ARM,
> RTE_CPUFLAG_LBT_MIPS,
> /* The last item */
> - RTE_CPUFLAG_NUMFLAGS /**< This should always be the last! */
> };
>
> #include "generic/rte_cpuflags.h"
> diff --git a/lib/eal/loongarch/rte_cpuflags.c
> b/lib/eal/loongarch/rte_cpuflags.c
> index 0a75ca58d4..73b53b8a3a 100644
> --- a/lib/eal/loongarch/rte_cpuflags.c
> +++ b/lib/eal/loongarch/rte_cpuflags.c
> @@ -66,8 +66,9 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
> {
> const struct feature_entry *feat;
> hwcap_registers_t regs = {0};
> + unsigned int num_flags = RTE_DIM(rte_cpu_feature_table);
>
> - if (feature >= RTE_CPUFLAG_NUMFLAGS)
> + if ((unsigned int)feature >= num_flags)
> return -ENOENT;
>
> feat = &rte_cpu_feature_table[feature];
> @@ -81,7 +82,9 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
> const char *
> rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
> {
> - if (feature >= RTE_CPUFLAG_NUMFLAGS)
> + unsigned int num_flags = RTE_DIM(rte_cpu_feature_table);
> +
> + if ((unsigned int)feature >= num_flags)
> return NULL;
> return rte_cpu_feature_table[feature].name;
> }
> diff --git a/lib/eal/ppc/include/rte_cpuflags.h
> b/lib/eal/ppc/include/rte_cpuflags.h
> index a88355d170..b74e7a73ee 100644
> --- a/lib/eal/ppc/include/rte_cpuflags.h
> +++ b/lib/eal/ppc/include/rte_cpuflags.h
> @@ -49,7 +49,6 @@ enum rte_cpu_flag_t {
> RTE_CPUFLAG_HTM,
> RTE_CPUFLAG_ARCH_2_07,
> /* The last item */
> - RTE_CPUFLAG_NUMFLAGS,/**< This should always be the last! */
> };
>
> #include "generic/rte_cpuflags.h"
> diff --git a/lib/eal/ppc/rte_cpuflags.c b/lib/eal/ppc/rte_cpuflags.c
> index 61db5c216d..a173c62631 100644
> --- a/lib/eal/ppc/rte_cpuflags.c
> +++ b/lib/eal/ppc/rte_cpuflags.c
> @@ -90,8 +90,9 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
> {
> const struct feature_entry *feat;
> hwcap_registers_t regs = {0};
> + unsigned int num_flags = RTE_DIM(rte_cpu_feature_table);
>
> - if (feature >= RTE_CPUFLAG_NUMFLAGS)
> + if ((unsigned int)feature >= num_flags)
> return -ENOENT;
>
> feat = &rte_cpu_feature_table[feature];
> @@ -105,7 +106,9 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
> const char *
> rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
> {
> - if (feature >= RTE_CPUFLAG_NUMFLAGS)
> + unsigned int num_flags = RTE_DIM(rte_cpu_feature_table);
> +
> + if ((unsigned int)feature >= num_flags)
> return NULL;
> return rte_cpu_feature_table[feature].name;
> }
> diff --git a/lib/eal/riscv/include/rte_cpuflags.h
> b/lib/eal/riscv/include/rte_cpuflags.h
> index 66e787f898..803c3655ae 100644
> --- a/lib/eal/riscv/include/rte_cpuflags.h
> +++ b/lib/eal/riscv/include/rte_cpuflags.h
> @@ -43,7 +43,6 @@ enum rte_cpu_flag_t {
> RTE_CPUFLAG_RISCV_ISA_Y, /* Reserved */
> RTE_CPUFLAG_RISCV_ISA_Z, /* Reserved */
> /* The last item */
> - RTE_CPUFLAG_NUMFLAGS,/**< This should always be the last! */
> };
>
> #include "generic/rte_cpuflags.h"
> diff --git a/lib/eal/riscv/rte_cpuflags.c b/lib/eal/riscv/rte_cpuflags.c
> index 4f6d29b947..6d3f8f16cc 100644
> --- a/lib/eal/riscv/rte_cpuflags.c
> +++ b/lib/eal/riscv/rte_cpuflags.c
> @@ -95,8 +95,9 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
> {
> const struct feature_entry *feat;
> hwcap_registers_t regs = {0};
> + unsigned int num_flags = RTE_DIM(rte_cpu_feature_table);
>
> - if (feature >= RTE_CPUFLAG_NUMFLAGS)
> + if ((unsigned int)feature >= num_flags)
> return -ENOENT;
>
> feat = &rte_cpu_feature_table[feature];
> @@ -110,7 +111,9 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
> const char *
> rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
> {
> - if (feature >= RTE_CPUFLAG_NUMFLAGS)
> + unsigned int num_flags = RTE_DIM(rte_cpu_feature_table);
> +
> + if ((unsigned int)feature >= num_flags)
> return NULL;
> return rte_cpu_feature_table[feature].name;
> }
> diff --git a/lib/eal/x86/include/rte_cpuflags.h
> b/lib/eal/x86/include/rte_cpuflags.h
> index 92e90fb6e0..7fc6117243 100644
> --- a/lib/eal/x86/include/rte_cpuflags.h
> +++ b/lib/eal/x86/include/rte_cpuflags.h
> @@ -135,7 +135,6 @@ enum rte_cpu_flag_t {
> RTE_CPUFLAG_WAITPKG, /**< UMONITOR/UMWAIT/TPAUSE */
>
> /* The last item */
> - RTE_CPUFLAG_NUMFLAGS, /**< This should always be the
> last! */
> };
>
> #include "generic/rte_cpuflags.h"
> diff --git a/lib/eal/x86/rte_cpuflags.c b/lib/eal/x86/rte_cpuflags.c
> index d6b518251b..22061cb6d3 100644
> --- a/lib/eal/x86/rte_cpuflags.c
> +++ b/lib/eal/x86/rte_cpuflags.c
> @@ -149,8 +149,9 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
> const struct feature_entry *feat;
> cpuid_registers_t regs;
> unsigned int maxleaf;
> + unsigned int num_flags = RTE_DIM(rte_cpu_feature_table);
>
> - if (feature >= RTE_CPUFLAG_NUMFLAGS)
> + if ((unsigned int)feature >= num_flags)
> /* Flag does not match anything in the feature tables */
> return -ENOENT;
>
> @@ -176,7 +177,9 @@ rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature)
> const char *
> rte_cpu_get_flag_name(enum rte_cpu_flag_t feature)
> {
> - if (feature >= RTE_CPUFLAG_NUMFLAGS)
> + unsigned int num_flags = RTE_DIM(rte_cpu_feature_table);
> +
> + if ((unsigned int)feature >= num_flags)
> return NULL;
> return rte_cpu_feature_table[feature].name;
> }
> --
> 2.34.1
>
>
[-- Attachment #2: Type: text/html, Size: 11579 bytes --]
next prev parent reply other threads:[~2023-08-15 6:10 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-02 21:11 [PATCH 1/2] eal: remove RTE_CPUFLAG_NUMFLAGS Sivaprasad Tummala
2023-08-02 21:11 ` [PATCH 2/2] test/cpuflags: " Sivaprasad Tummala
2023-08-02 23:50 ` [PATCH 1/2] eal: " Stanisław Kardach
2023-08-11 4:02 ` Tummala, Sivaprasad
2023-08-11 6:07 ` [PATCH v2 1/2] test/cpuflags: removed test for NUMFLAGS Sivaprasad Tummala
2023-08-11 6:07 ` [PATCH v2 2/2] eal: remove NUMFLAGS enumeration Sivaprasad Tummala
2023-08-15 6:10 ` Stanisław Kardach [this message]
2023-09-19 14:47 ` David Marchand
2023-09-20 6:00 ` Stanisław Kardach
2023-09-20 7:34 ` David Marchand
2023-09-21 13:18 ` Tummala, Sivaprasad
2023-09-21 14:49 ` Stanisław Kardach
2023-09-27 11:55 ` Ferruh Yigit
2023-09-27 13:48 ` Stanisław Kardach
2023-09-27 14:09 ` Ferruh Yigit
2023-09-27 15:03 ` Stanisław Kardach
2023-09-27 16:13 ` Ferruh Yigit
2023-09-27 13:13 ` Ferruh Yigit
2023-09-28 9:31 ` Tummala, Sivaprasad
2023-10-06 8:27 ` David Marchand
2023-10-06 11:19 ` Ferruh Yigit
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='CALVGJWJoCsnOk_b4+uXCjYEs1krN-s=wEerpJWuTFnaY_nsrLQ@mail.gmail.com' \
--to=kda@semihalf.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=drc@linux.vnet.ibm.com \
--cc=konstantin.v.ananyev@yandex.ru \
--cc=ruifeng.wang@arm.com \
--cc=sivaprasad.tummala@amd.com \
--cc=zhoumin@loongson.cn \
/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).