[v3,01/18] eal: add max SIMD bitwidth
Checks
Commit Message
This patch adds a max SIMD bitwidth EAL configuration. The API allows
for an app to set this value. It can also be set using EAL argument
--force-max-simd-bitwidth, which will lock the value and override any
modifications made by the app.
Signed-off-by: Ciara Power <ciara.power@intel.com>
---
v3:
- Added enum value to essentially disable using max SIMD to choose
paths, intended for use by ARM SVE.
- Fixed parsing bitwidth argument to return an error for values
greater than uint16_t.
v2: Added to Doxygen comment for API.
---
lib/librte_eal/common/eal_common_options.c | 64 ++++++++++++++++++++++
lib/librte_eal/common/eal_internal_cfg.h | 8 +++
lib/librte_eal/common/eal_options.h | 2 +
lib/librte_eal/include/rte_eal.h | 33 +++++++++++
lib/librte_eal/rte_eal_version.map | 4 ++
5 files changed, 111 insertions(+)
Comments
Hi Ciara
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ciara Power
<snip>
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h
> b/lib/librte_eal/common/eal_internal_cfg.h
> index 13f93388a7..367e0cc19e 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -33,6 +33,12 @@ struct hugepage_info {
> int lock_descriptor; /**< file descriptor for hugepage dir */
> };
>
> +struct simd_bitwidth {
> + /**< flag indicating if bitwidth is locked from further modification */
> + bool locked;
> + uint16_t bitwidth; /**< bitwidth value */ };
[DC] The doxygen comment on 'locked' flag uses '/**<' so should come after the field.
Having the comment after the field seems to be the way it's done in this file so I'd move
the comment as opposed to removing the '<'
> +
> /**
> * internal configuration
> */
> @@ -85,6 +91,8 @@ struct internal_config {
> volatile unsigned int init_complete;
> /**< indicates whether EAL has completed initialization */
> unsigned int no_telemetry; /**< true to disable Telemetry */
> + /** max simd bitwidth path to use */
> + struct simd_bitwidth max_simd_bitwidth;
[DC] Again the doxygen comments seem to come after the struct fields in this file
so I'd move the comment for max_simd_bitwidth to after it and add the '<'
> };
>
> void eal_reset_internal_config(struct internal_config *internal_cfg); diff --git
<snip>
>
> diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> index ddcf6a2e7a..fb739f3474 100644
> --- a/lib/librte_eal/include/rte_eal.h
> +++ b/lib/librte_eal/include/rte_eal.h
> @@ -43,6 +43,14 @@ enum rte_proc_type_t {
> RTE_PROC_INVALID
> };
>
> +enum rte_max_simd_t {
> + RTE_NO_SIMD = 64,
> + RTE_MAX_128_SIMD = 128,
> + RTE_MAX_256_SIMD = 256,
> + RTE_MAX_512_SIMD = 512,
> + RTE_MAX_SIMD_DISABLE = UINT16_MAX,
> +};
[DC] Add doxygen comments on enum rte_max_simd_t and each of it's values
> +
> /**
> * Get the process type in a multi-process setup
> *
> @@ -51,6 +59,31 @@ enum rte_proc_type_t {
> */
> enum rte_proc_type_t rte_eal_process_type(void);
>
> +/**
> + * Get the supported SIMD bitwidth.
> + *
> + * @return
> + * uint16_t bitwidth.
> + */
> +__rte_experimental
> +uint16_t rte_get_max_simd_bitwidth(void);
> +
> +/**
> + * Set the supported SIMD bitwidth.
> + * This API should only be called once at initialization, before EAL init.
> + *
> + * @param bitwidth
> + * uint16_t bitwidth.
> + * @return
> + * 0 on success.
> + * @return
> + * -EINVAL on invalid bitwidth parameter.
> + * @return
> + * -EPERM if bitwidth is locked.
[DC] Minor thing.. normally there's just 1 @return tag with all of the return values under
it as a bullet list
> + */
> +__rte_experimental
> +int rte_set_max_simd_bitwidth(uint16_t bitwidth);
> +
> /**
> * Request iopl privilege for all RPL.
> *
> diff --git a/lib/librte_eal/rte_eal_version.map
> b/lib/librte_eal/rte_eal_version.map
> index c32461c663..17a7195a3d 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -397,6 +397,10 @@ EXPERIMENTAL {
> rte_service_lcore_may_be_active;
> rte_thread_register;
> rte_thread_unregister;
> +
> + # added in 20.11
> + rte_get_max_simd_bitwidth;
> + rte_set_max_simd_bitwidth;
> };
[DC] rte_get_max_simd_bitwidth is called from rte_net_crc (and other libraries) so this
symbol possibly needs to be added to librte_eal/rte_eal_exports.def file too.
This is the windows symbol export file, used on windows build.
This has caught us out on the AVX512 CRC patchset https://patchwork.dpdk.org/project/dpdk/list/?series=12596
where a windows build failed in the 'ci/iol-testing' checks in patchwork because
rte_net_crc couldn't find the symbol rte_cpu_get_flag_enabled, which also comes
from rte_eal. We have to add this symbol to rte_eal_exports.def to fix this.
The 'ci/iol-testing' check has not run for your patchset so I can't say for certain if the
windows build would have failed for you, but I think it would
>
> INTERNAL {
> --
> 2.17.1
Hi Ciara,
Please find some comments below.
On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote:
> This patch adds a max SIMD bitwidth EAL configuration. The API allows
> for an app to set this value. It can also be set using EAL argument
> --force-max-simd-bitwidth, which will lock the value and override any
> modifications made by the app.
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
>
> ---
> v3:
> - Added enum value to essentially disable using max SIMD to choose
> paths, intended for use by ARM SVE.
> - Fixed parsing bitwidth argument to return an error for values
> greater than uint16_t.
> v2: Added to Doxygen comment for API.
> ---
> lib/librte_eal/common/eal_common_options.c | 64 ++++++++++++++++++++++
> lib/librte_eal/common/eal_internal_cfg.h | 8 +++
> lib/librte_eal/common/eal_options.h | 2 +
> lib/librte_eal/include/rte_eal.h | 33 +++++++++++
> lib/librte_eal/rte_eal_version.map | 4 ++
> 5 files changed, 111 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index a5426e1234..e9117a96af 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -102,6 +102,7 @@ eal_long_options[] = {
> {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
> {OPT_TELEMETRY, 0, NULL, OPT_TELEMETRY_NUM },
> {OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
> + {OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
> {0, 0, NULL, 0 }
> };
>
> @@ -1309,6 +1310,34 @@ eal_parse_iova_mode(const char *name)
> return 0;
> }
>
> +static int
> +eal_parse_simd_bitwidth(const char *arg, bool locked)
> +{
> + char *end;
> + unsigned long bitwidth;
> + int ret;
> + struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> +
> + if (arg == NULL || arg[0] == '\0')
> + return -1;
> +
> + errno = 0;
> + bitwidth = strtoul(arg, &end, 0);
> +
> + /* check for errors */
> + if (bitwidth > UINT16_MAX || errno != 0 || end == NULL || *end != '\0')
> + return -1;
> +
> + if (bitwidth == 0)
> + bitwidth = UINT16_MAX;
> + ret = rte_set_max_simd_bitwidth(bitwidth);
> + if (ret < 0)
> + return -1;
> + internal_conf->max_simd_bitwidth.locked = locked;
> + return 0;
> +}
> +
> static int
> eal_parse_base_virtaddr(const char *arg)
> {
> @@ -1707,6 +1736,13 @@ eal_parse_common_option(int opt, const char *optarg,
> case OPT_NO_TELEMETRY_NUM:
> conf->no_telemetry = 1;
> break;
> + case OPT_FORCE_MAX_SIMD_BITWIDTH_NUM:
> + if (eal_parse_simd_bitwidth(optarg, 1) < 0) {
> + RTE_LOG(ERR, EAL, "invalid parameter for --"
> + OPT_FORCE_MAX_SIMD_BITWIDTH "\n");
> + return -1;
> + }
> + break;
>
> /* don't know what to do, leave this to caller */
> default:
> @@ -1903,6 +1939,33 @@ eal_check_common_options(struct internal_config *internal_cfg)
> return 0;
> }
>
> +uint16_t
> +rte_get_max_simd_bitwidth(void)
> +{
> + const struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> + return internal_conf->max_simd_bitwidth.bitwidth;
> +}
Should the return value be enum rte_max_simd_t?
If not, do we really need the enum definition?
> +
> +int
> +rte_set_max_simd_bitwidth(uint16_t bitwidth)
> +{
> + struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> + if (internal_conf->max_simd_bitwidth.locked) {
> + RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user runtime override enabled");
> + return -EPERM;
> + }
> +
> + if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth < RTE_NO_SIMD ||
> + !rte_is_power_of_2(bitwidth))) {
> + RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
> + return -EINVAL;
> + }
> + internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
> + return 0;
> +}
Same question, should the parameter be enum rte_max_simd_t?
> +
> void
> eal_common_usage(void)
> {
> @@ -1981,6 +2044,7 @@ eal_common_usage(void)
> " --"OPT_BASE_VIRTADDR" Base virtual address\n"
> " --"OPT_TELEMETRY" Enable telemetry support (on by default)\n"
> " --"OPT_NO_TELEMETRY" Disable telemetry support\n"
> + " --"OPT_FORCE_MAX_SIMD_BITWIDTH" Force the max SIMD bitwidth\n"
> "\nEAL options for DEBUG use only:\n"
> " --"OPT_HUGE_UNLINK" Unlink hugepage files after init\n"
> " --"OPT_NO_HUGE" Use malloc instead of hugetlbfs\n"
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index 13f93388a7..367e0cc19e 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -33,6 +33,12 @@ struct hugepage_info {
> int lock_descriptor; /**< file descriptor for hugepage dir */
> };
>
> +struct simd_bitwidth {
> + /**< flag indicating if bitwidth is locked from further modification */
> + bool locked;
> + uint16_t bitwidth; /**< bitwidth value */
> +};
> +
> /**
> * internal configuration
> */
> @@ -85,6 +91,8 @@ struct internal_config {
> volatile unsigned int init_complete;
> /**< indicates whether EAL has completed initialization */
> unsigned int no_telemetry; /**< true to disable Telemetry */
> + /** max simd bitwidth path to use */
> + struct simd_bitwidth max_simd_bitwidth;
> };
>
> void eal_reset_internal_config(struct internal_config *internal_cfg);
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index 89769d48b4..ef33979664 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -85,6 +85,8 @@ enum {
> OPT_TELEMETRY_NUM,
> #define OPT_NO_TELEMETRY "no-telemetry"
> OPT_NO_TELEMETRY_NUM,
> +#define OPT_FORCE_MAX_SIMD_BITWIDTH "force-max-simd-bitwidth"
> + OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
> OPT_LONG_MAX_NUM
> };
>
> diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> index ddcf6a2e7a..fb739f3474 100644
> --- a/lib/librte_eal/include/rte_eal.h
> +++ b/lib/librte_eal/include/rte_eal.h
> @@ -43,6 +43,14 @@ enum rte_proc_type_t {
> RTE_PROC_INVALID
> };
>
> +enum rte_max_simd_t {
> + RTE_NO_SIMD = 64,
> + RTE_MAX_128_SIMD = 128,
> + RTE_MAX_256_SIMD = 256,
> + RTE_MAX_512_SIMD = 512,
> + RTE_MAX_SIMD_DISABLE = UINT16_MAX,
> +};
What is the difference between RTE_NO_SIMD and RTE_MAX_SIMD_DISABLE?
The default value in internal_config is 0, so in my understanding
rte_get_max_simd_bitwidth() will return 0 if --force-max-simd-bitwidth
is not passed. Is it expected?
Maybe I'm missing something, but I don't understand why the value in
internal_config is not set to the maximum supported SIMD bitwidth by
default, and optionally overriden by the command line argument, or by
the API.
> +
> /**
> * Get the process type in a multi-process setup
> *
> @@ -51,6 +59,31 @@ enum rte_proc_type_t {
> */
> enum rte_proc_type_t rte_eal_process_type(void);
>
> +/**
> + * Get the supported SIMD bitwidth.
> + *
> + * @return
> + * uint16_t bitwidth.
> + */
> +__rte_experimental
> +uint16_t rte_get_max_simd_bitwidth(void);
> +
> +/**
> + * Set the supported SIMD bitwidth.
> + * This API should only be called once at initialization, before EAL init.
> + *
> + * @param bitwidth
> + * uint16_t bitwidth.
> + * @return
> + * 0 on success.
> + * @return
> + * -EINVAL on invalid bitwidth parameter.
> + * @return
> + * -EPERM if bitwidth is locked.
> + */
> +__rte_experimental
> +int rte_set_max_simd_bitwidth(uint16_t bitwidth);
> +
> /**
> * Request iopl privilege for all RPL.
> *
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index c32461c663..17a7195a3d 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -397,6 +397,10 @@ EXPERIMENTAL {
> rte_service_lcore_may_be_active;
> rte_thread_register;
> rte_thread_unregister;
> +
> + # added in 20.11
> + rte_get_max_simd_bitwidth;
> + rte_set_max_simd_bitwidth;
> };
>
> INTERNAL {
> --
> 2.17.1
>
On 9/30/20 3:03 PM, Ciara Power wrote:
> This patch adds a max SIMD bitwidth EAL configuration. The API allows
> for an app to set this value. It can also be set using EAL argument
> --force-max-simd-bitwidth, which will lock the value and override any
> modifications made by the app.
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
>
> ---
> v3:
> - Added enum value to essentially disable using max SIMD to choose
> paths, intended for use by ARM SVE.
> - Fixed parsing bitwidth argument to return an error for values
> greater than uint16_t.
> v2: Added to Doxygen comment for API.
> ---
> lib/librte_eal/common/eal_common_options.c | 64 ++++++++++++++++++++++
> lib/librte_eal/common/eal_internal_cfg.h | 8 +++
> lib/librte_eal/common/eal_options.h | 2 +
> lib/librte_eal/include/rte_eal.h | 33 +++++++++++
> lib/librte_eal/rte_eal_version.map | 4 ++
> 5 files changed, 111 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index a5426e1234..e9117a96af 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -102,6 +102,7 @@ eal_long_options[] = {
> {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
> {OPT_TELEMETRY, 0, NULL, OPT_TELEMETRY_NUM },
> {OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
> + {OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
> {0, 0, NULL, 0 }
> };
>
> @@ -1309,6 +1310,34 @@ eal_parse_iova_mode(const char *name)
> return 0;
> }
>
> +static int
> +eal_parse_simd_bitwidth(const char *arg, bool locked)
> +{
> + char *end;
> + unsigned long bitwidth;
> + int ret;
> + struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> +
> + if (arg == NULL || arg[0] == '\0')
> + return -1;
> +
> + errno = 0;
> + bitwidth = strtoul(arg, &end, 0);
> +
> + /* check for errors */
> + if (bitwidth > UINT16_MAX || errno != 0 || end == NULL || *end != '\0')
> + return -1;
> +
> + if (bitwidth == 0)
> + bitwidth = UINT16_MAX;
(unsigned long)RTE_MAX_SIMD_DISABLE instead?
> + ret = rte_set_max_simd_bitwidth(bitwidth);
> + if (ret < 0)
> + return -1;
> + internal_conf->max_simd_bitwidth.locked = locked;
> + return 0;
> +}
> +
> static int
> eal_parse_base_virtaddr(const char *arg)
> {
> @@ -1707,6 +1736,13 @@ eal_parse_common_option(int opt, const char *optarg,
> case OPT_NO_TELEMETRY_NUM:
> conf->no_telemetry = 1;
> break;
> + case OPT_FORCE_MAX_SIMD_BITWIDTH_NUM:
> + if (eal_parse_simd_bitwidth(optarg, 1) < 0) {
> + RTE_LOG(ERR, EAL, "invalid parameter for --"
> + OPT_FORCE_MAX_SIMD_BITWIDTH "\n");
> + return -1;
> + }
> + break;
>
> /* don't know what to do, leave this to caller */
> default:
> @@ -1903,6 +1939,33 @@ eal_check_common_options(struct internal_config *internal_cfg)
> return 0;
> }
>
> +uint16_t
shouldn't it return an enum rte_max_simd_t?
> +rte_get_max_simd_bitwidth(void)
> +{
> + const struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> + return internal_conf->max_simd_bitwidth.bitwidth;
What is the default value if not set?
> +}
> +
> +int
> +rte_set_max_simd_bitwidth(uint16_t bitwidth)
> +{
> + struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> + if (internal_conf->max_simd_bitwidth.locked) {
> + RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user runtime override enabled");
> + return -EPERM;
> + }
> +
> + if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth < RTE_NO_SIMD ||
> + !rte_is_power_of_2(bitwidth))) {
> + RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
> + return -EINVAL;
> + }
> + internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
> + return 0;
> +}
> +
> void
> eal_common_usage(void)
> {
> @@ -1981,6 +2044,7 @@ eal_common_usage(void)
> " --"OPT_BASE_VIRTADDR" Base virtual address\n"
> " --"OPT_TELEMETRY" Enable telemetry support (on by default)\n"
> " --"OPT_NO_TELEMETRY" Disable telemetry support\n"
> + " --"OPT_FORCE_MAX_SIMD_BITWIDTH" Force the max SIMD bitwidth\n"
> "\nEAL options for DEBUG use only:\n"
> " --"OPT_HUGE_UNLINK" Unlink hugepage files after init\n"
> " --"OPT_NO_HUGE" Use malloc instead of hugetlbfs\n"
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index 13f93388a7..367e0cc19e 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -33,6 +33,12 @@ struct hugepage_info {
> int lock_descriptor; /**< file descriptor for hugepage dir */
> };
>
> +struct simd_bitwidth {
> + /**< flag indicating if bitwidth is locked from further modification */
> + bool locked;
> + uint16_t bitwidth; /**< bitwidth value */
> +};
> +
> /**
> * internal configuration
> */
> @@ -85,6 +91,8 @@ struct internal_config {
> volatile unsigned int init_complete;
> /**< indicates whether EAL has completed initialization */
> unsigned int no_telemetry; /**< true to disable Telemetry */
> + /** max simd bitwidth path to use */
> + struct simd_bitwidth max_simd_bitwidth;
> };
>
> void eal_reset_internal_config(struct internal_config *internal_cfg);
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index 89769d48b4..ef33979664 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -85,6 +85,8 @@ enum {
> OPT_TELEMETRY_NUM,
> #define OPT_NO_TELEMETRY "no-telemetry"
> OPT_NO_TELEMETRY_NUM,
> +#define OPT_FORCE_MAX_SIMD_BITWIDTH "force-max-simd-bitwidth"
> + OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
> OPT_LONG_MAX_NUM
> };
>
> diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> index ddcf6a2e7a..fb739f3474 100644
> --- a/lib/librte_eal/include/rte_eal.h
> +++ b/lib/librte_eal/include/rte_eal.h
> @@ -43,6 +43,14 @@ enum rte_proc_type_t {
> RTE_PROC_INVALID
> };
>
> +enum rte_max_simd_t {
> + RTE_NO_SIMD = 64,
> + RTE_MAX_128_SIMD = 128,
> + RTE_MAX_256_SIMD = 256,
> + RTE_MAX_512_SIMD = 512,
> + RTE_MAX_SIMD_DISABLE = UINT16_MAX,
> +};
> +
> /**
> * Get the process type in a multi-process setup
> *
> @@ -51,6 +59,31 @@ enum rte_proc_type_t {
> */
> enum rte_proc_type_t rte_eal_process_type(void);
>
> +/**
> + * Get the supported SIMD bitwidth.
> + *
> + * @return
> + * uint16_t bitwidth.
> + */
> +__rte_experimental
> +uint16_t rte_get_max_simd_bitwidth(void);
> +
> +/**
> + * Set the supported SIMD bitwidth.
> + * This API should only be called once at initialization, before EAL init.
> + *
> + * @param bitwidth
> + * uint16_t bitwidth.
> + * @return
> + * 0 on success.
> + * @return
> + * -EINVAL on invalid bitwidth parameter.
> + * @return
> + * -EPERM if bitwidth is locked.
> + */
> +__rte_experimental
> +int rte_set_max_simd_bitwidth(uint16_t bitwidth);
> +
> /**
> * Request iopl privilege for all RPL.
> *
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index c32461c663..17a7195a3d 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -397,6 +397,10 @@ EXPERIMENTAL {
> rte_service_lcore_may_be_active;
> rte_thread_register;
> rte_thread_unregister;
> +
> + # added in 20.11
> + rte_get_max_simd_bitwidth;
> + rte_set_max_simd_bitwidth;
> };
>
> INTERNAL {
>
Hi Olivier,
Thanks for reviewing, some comments below.
>-----Original Message-----
>From: Olivier Matz <olivier.matz@6wind.com>
>Sent: Tuesday 6 October 2020 10:32
>To: Power, Ciara <ciara.power@intel.com>
>Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
><nhorman@tuxdriver.com>
>Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>
>Hi Ciara,
>
>Please find some comments below.
>
>On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote:
>> This patch adds a max SIMD bitwidth EAL configuration. The API allows
>> for an app to set this value. It can also be set using EAL argument
>> --force-max-simd-bitwidth, which will lock the value and override any
>> modifications made by the app.
>>
>> Signed-off-by: Ciara Power <ciara.power@intel.com>
>>
>> ---
>> v3:
>> - Added enum value to essentially disable using max SIMD to choose
>> paths, intended for use by ARM SVE.
>> - Fixed parsing bitwidth argument to return an error for values
>> greater than uint16_t.
>> v2: Added to Doxygen comment for API.
>> ---
<snip>
>>
>> +uint16_t
>> +rte_get_max_simd_bitwidth(void)
>> +{
>> + const struct internal_config *internal_conf =
>> + eal_get_internal_configuration();
>> + return internal_conf->max_simd_bitwidth.bitwidth;
>> +}
>
>Should the return value be enum rte_max_simd_t?
>If not, do we really need the enum definition?
>
I kept the return value and param value below as uint16_t to allow for arbitrary values,
and will allow it be more flexible for future additions as new enums won't need to be added.
For the set function below, this is used when a user passes the EAL command line flag, which
passes an integer value rather than an enum one.
The enums are useful when checking the max_simd_bitwidth in drivers/libs, for example using
"RTE_MAX_256_SIMD" instead of "256" in the condition checks.
>> +
>> +int
>> +rte_set_max_simd_bitwidth(uint16_t bitwidth) {
>> + struct internal_config *internal_conf =
>> + eal_get_internal_configuration();
>> + if (internal_conf->max_simd_bitwidth.locked) {
>> + RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user
>runtime override enabled");
>> + return -EPERM;
>> + }
>> +
>> + if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth <
>RTE_NO_SIMD ||
>> + !rte_is_power_of_2(bitwidth))) {
>> + RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
>> + return -EINVAL;
>> + }
>> + internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
>> + return 0;
>> +}
>
>Same question, should the parameter be enum rte_max_simd_t?
>
<snip>
>> +enum rte_max_simd_t {
>> + RTE_NO_SIMD = 64,
>> + RTE_MAX_128_SIMD = 128,
>> + RTE_MAX_256_SIMD = 256,
>> + RTE_MAX_512_SIMD = 512,
>> + RTE_MAX_SIMD_DISABLE = UINT16_MAX,
>> +};
>
>What is the difference between RTE_NO_SIMD and
>RTE_MAX_SIMD_DISABLE?
RTE_NO_SIMD has value 64 to limit paths to scalar only.
RTE_MAX_SIMD_DISABLE sets the highest value possible,
so essentially disables the limit affecting which vector paths are taken.
This disable option was added to allow for ARM SVE which will be later added,
Discussed with Honnappa on a previous version: https://patchwork.dpdk.org/patch/76097/
>The default value in internal_config is 0, so in my understanding
>rte_get_max_simd_bitwidth() will return 0 if --force-max-simd-bitwidth is
>not passed. Is it expected?
>
>Maybe I'm missing something, but I don't understand why the value in
>internal_config is not set to the maximum supported SIMD bitwidth by
>default, and optionally overriden by the command line argument, or by the
>API.
>
The default value for max_simd_bitwidth is set depending on the architecture, 256 for x86/ppc,
and UINT16_MAX for ARM. So for example the default on x86 allows for AVX2 and under.
The defaults can be seen in patch 2: https://patchwork.dpdk.org/patch/79339/
<snip>
Thanks,
Ciara
On Wed, Oct 07, 2020 at 11:47:34AM +0100, Power, Ciara wrote:
> Hi Olivier,
>
> Thanks for reviewing, some comments below.
>
>
> >-----Original Message-----
> >From: Olivier Matz <olivier.matz@6wind.com>
> >Sent: Tuesday 6 October 2020 10:32
> >To: Power, Ciara <ciara.power@intel.com>
> >Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
> ><nhorman@tuxdriver.com>
> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
> >
> >Hi Ciara,
> >
> >Please find some comments below.
> >
> >On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote:
> >> This patch adds a max SIMD bitwidth EAL configuration. The API allows
> >> for an app to set this value. It can also be set using EAL argument
> >> --force-max-simd-bitwidth, which will lock the value and override any
> >> modifications made by the app.
> >>
> >> Signed-off-by: Ciara Power <ciara.power@intel.com>
> >>
> >> ---
> >> v3:
> >> - Added enum value to essentially disable using max SIMD to choose
> >> paths, intended for use by ARM SVE.
> >> - Fixed parsing bitwidth argument to return an error for values
> >> greater than uint16_t.
> >> v2: Added to Doxygen comment for API.
> >> ---
>
> <snip>
>
> >>
> >> +uint16_t
> >> +rte_get_max_simd_bitwidth(void)
> >> +{
> >> +const struct internal_config *internal_conf =
> >> +eal_get_internal_configuration();
> >> +return internal_conf->max_simd_bitwidth.bitwidth;
> >> +}
> >
> >Should the return value be enum rte_max_simd_t?
> >If not, do we really need the enum definition?
> >
>
> I kept the return value and param value below as uint16_t to allow for arbitrary values,
> and will allow it be more flexible for future additions as new enums won't need to be added.
> For the set function below, this is used when a user passes the EAL command line flag, which
> passes an integer value rather than an enum one.
> The enums are useful when checking the max_simd_bitwidth in drivers/libs, for example using
> "RTE_MAX_256_SIMD" instead of "256" in the condition checks.
>
So basically these enum values are #defines for readability, just in enum
form, right?
Hi Maxime,
Thanks for reviewing, some comments below.
>-----Original Message-----
>From: Maxime Coquelin <maxime.coquelin@redhat.com>
>Sent: Tuesday 6 October 2020 12:50
>To: Power, Ciara <ciara.power@intel.com>; dev@dpdk.org
>Cc: Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
>Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>
>
>
>On 9/30/20 3:03 PM, Ciara Power wrote:
>> This patch adds a max SIMD bitwidth EAL configuration. The API allows
>> for an app to set this value. It can also be set using EAL argument
>> --force-max-simd-bitwidth, which will lock the value and override any
>> modifications made by the app.
>>
>> Signed-off-by: Ciara Power <ciara.power@intel.com>
>>
>> ---
>> v3:
>> - Added enum value to essentially disable using max SIMD to choose
>> paths, intended for use by ARM SVE.
>> - Fixed parsing bitwidth argument to return an error for values
>> greater than uint16_t.
>> v2: Added to Doxygen comment for API.
>> ---
<snip>
>> @@ -1903,6 +1939,33 @@ eal_check_common_options(struct
>internal_config *internal_cfg)
>> return 0;
>> }
>>
>> +uint16_t
>
>shouldn't it return an enum rte_max_simd_t?
I kept the return value as uint16_t to allow for arbitrary values,
and will allow it be more flexible for future additions as new enums won't need to be added.
The enums are really used for readability when checking the bitwidth limit in drivers/libs, so
essentially #defines in enum form.
>
>> +rte_get_max_simd_bitwidth(void)
>> +{
>> + const struct internal_config *internal_conf =
>> + eal_get_internal_configuration();
>> + return internal_conf->max_simd_bitwidth.bitwidth;
>
>What is the default value if not set?
>
The default value for max_simd_bitwidth is set depending on the architecture, 256 for x86/ppc,
and UINT16_MAX for ARM. So for example the default on x86 allows for AVX2 and under.
The defaults can be seen in patch 2: https://patchwork.dpdk.org/patch/79339/
<snip>
Thanks,
Ciara
Hi Bruce,
>-----Original Message-----
>From: Bruce Richardson <bruce.richardson@intel.com>
>Sent: Wednesday 7 October 2020 11:52
>To: Power, Ciara <ciara.power@intel.com>
>Cc: Olivier Matz <olivier.matz@6wind.com>; dev@dpdk.org; Ray Kinsella
><mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
>Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>
>On Wed, Oct 07, 2020 at 11:47:34AM +0100, Power, Ciara wrote:
>> Hi Olivier,
>>
>> Thanks for reviewing, some comments below.
>>
>>
>> >-----Original Message-----
>> >From: Olivier Matz <olivier.matz@6wind.com>
>> >Sent: Tuesday 6 October 2020 10:32
>> >To: Power, Ciara <ciara.power@intel.com>
>> >Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
>> ><nhorman@tuxdriver.com>
>> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>> >
>> >Hi Ciara,
>> >
>> >Please find some comments below.
>> >
>> >On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote:
>> >> This patch adds a max SIMD bitwidth EAL configuration. The API
>> >> allows for an app to set this value. It can also be set using EAL
>> >> argument --force-max-simd-bitwidth, which will lock the value and
>> >> override any modifications made by the app.
>> >>
>> >> Signed-off-by: Ciara Power <ciara.power@intel.com>
>> >>
>> >> ---
>> >> v3:
>> >> - Added enum value to essentially disable using max SIMD to choose
>> >> paths, intended for use by ARM SVE.
>> >> - Fixed parsing bitwidth argument to return an error for values
>> >> greater than uint16_t.
>> >> v2: Added to Doxygen comment for API.
>> >> ---
>>
>> <snip>
>>
>> >>
>> >> +uint16_t
>> >> +rte_get_max_simd_bitwidth(void)
>> >> +{
>> >> +const struct internal_config *internal_conf =
>> >> +eal_get_internal_configuration();
>> >> +return internal_conf->max_simd_bitwidth.bitwidth;
>> >> +}
>> >
>> >Should the return value be enum rte_max_simd_t?
>> >If not, do we really need the enum definition?
>> >
>>
>> I kept the return value and param value below as uint16_t to allow for
>> arbitrary values, and will allow it be more flexible for future additions as
>new enums won't need to be added.
>> For the set function below, this is used when a user passes the EAL
>> command line flag, which passes an integer value rather than an enum one.
>> The enums are useful when checking the max_simd_bitwidth in
>> drivers/libs, for example using "RTE_MAX_256_SIMD" instead of "256" in
>the condition checks.
>>
>So basically these enum values are #defines for readability, just in enum
>form, right?
Yes, that's exactly right.
Thanks,
Ciara
Hi Ciara,
On Wed, Oct 07, 2020 at 10:47:34AM +0000, Power, Ciara wrote:
> Hi Olivier,
>
> Thanks for reviewing, some comments below.
>
>
> >-----Original Message-----
> >From: Olivier Matz <olivier.matz@6wind.com>
> >Sent: Tuesday 6 October 2020 10:32
> >To: Power, Ciara <ciara.power@intel.com>
> >Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
> ><nhorman@tuxdriver.com>
> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
> >
> >Hi Ciara,
> >
> >Please find some comments below.
> >
> >On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote:
> >> This patch adds a max SIMD bitwidth EAL configuration. The API allows
> >> for an app to set this value. It can also be set using EAL argument
> >> --force-max-simd-bitwidth, which will lock the value and override any
> >> modifications made by the app.
> >>
> >> Signed-off-by: Ciara Power <ciara.power@intel.com>
> >>
> >> ---
> >> v3:
> >> - Added enum value to essentially disable using max SIMD to choose
> >> paths, intended for use by ARM SVE.
> >> - Fixed parsing bitwidth argument to return an error for values
> >> greater than uint16_t.
> >> v2: Added to Doxygen comment for API.
> >> ---
>
> <snip>
>
> >>
> >> +uint16_t
> >> +rte_get_max_simd_bitwidth(void)
> >> +{
> >> + const struct internal_config *internal_conf =
> >> + eal_get_internal_configuration();
> >> + return internal_conf->max_simd_bitwidth.bitwidth;
> >> +}
> >
> >Should the return value be enum rte_max_simd_t?
> >If not, do we really need the enum definition?
> >
>
> I kept the return value and param value below as uint16_t to allow for arbitrary values,
> and will allow it be more flexible for future additions as new enums won't need to be added.
> For the set function below, this is used when a user passes the EAL command line flag, which
> passes an integer value rather than an enum one.
> The enums are useful when checking the max_simd_bitwidth in drivers/libs, for example using
> "RTE_MAX_256_SIMD" instead of "256" in the condition checks.
>
> >> +
> >> +int
> >> +rte_set_max_simd_bitwidth(uint16_t bitwidth) {
> >> + struct internal_config *internal_conf =
> >> + eal_get_internal_configuration();
> >> + if (internal_conf->max_simd_bitwidth.locked) {
> >> + RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user
> >runtime override enabled");
> >> + return -EPERM;
> >> + }
> >> +
> >> + if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth <
> >RTE_NO_SIMD ||
> >> + !rte_is_power_of_2(bitwidth))) {
> >> + RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
> >> + return -EINVAL;
> >> + }
> >> + internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
> >> + return 0;
> >> +}
> >
> >Same question, should the parameter be enum rte_max_simd_t?
> >
>
> <snip>
>
> >> +enum rte_max_simd_t {
> >> + RTE_NO_SIMD = 64,
> >> + RTE_MAX_128_SIMD = 128,
> >> + RTE_MAX_256_SIMD = 256,
> >> + RTE_MAX_512_SIMD = 512,
> >> + RTE_MAX_SIMD_DISABLE = UINT16_MAX,
> >> +};
> >
> >What is the difference between RTE_NO_SIMD and
> >RTE_MAX_SIMD_DISABLE?
>
> RTE_NO_SIMD has value 64 to limit paths to scalar only.
> RTE_MAX_SIMD_DISABLE sets the highest value possible,
> so essentially disables the limit affecting which vector paths are taken.
> This disable option was added to allow for ARM SVE which will be later added,
> Discussed with Honnappa on a previous version: https://patchwork.dpdk.org/patch/76097/
Ok, so RTE_MAX_SIMD_DISABLE means "disable the max limit", right?
I feel the name is a bit confusing. What about something like this:
enum rte_simd {
RTE_SIMD_DISABLED = 0,
RTE_SIMD_128 = 128,
RTE_SIMD_256 = 256,
RTE_SIMD_512 = 512,
RTE_SIMD_MAX = UINT16_MAX,
};
>
> >The default value in internal_config is 0, so in my understanding
> >rte_get_max_simd_bitwidth() will return 0 if --force-max-simd-bitwidth is
> >not passed. Is it expected?
> >
> >Maybe I'm missing something, but I don't understand why the value in
> >internal_config is not set to the maximum supported SIMD bitwidth by
> >default, and optionally overriden by the command line argument, or by the
> >API.
> >
>
> The default value for max_simd_bitwidth is set depending on the architecture, 256 for x86/ppc,
> and UINT16_MAX for ARM. So for example the default on x86 allows for AVX2 and under.
> The defaults can be seen in patch 2: https://patchwork.dpdk.org/patch/79339/
Ok, I was expecting to have a runtime check for this. For instance, on
intel architecture, it is not known at compilation, it depends on the
target which can support up to AVX, AVX2, or AVX512.
>
> <snip>
>
> Thanks,
> Ciara
Hi Olivier,
>-----Original Message-----
>From: Olivier Matz <olivier.matz@6wind.com>
>Sent: Wednesday 7 October 2020 12:18
>To: Power, Ciara <ciara.power@intel.com>
>Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
><nhorman@tuxdriver.com>; Richardson, Bruce <bruce.richardson@intel.com>
>Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>
>Hi Ciara,
>
>On Wed, Oct 07, 2020 at 10:47:34AM +0000, Power, Ciara wrote:
>> Hi Olivier,
>>
>> Thanks for reviewing, some comments below.
>>
>>
>> >-----Original Message-----
>> >From: Olivier Matz <olivier.matz@6wind.com>
>> >Sent: Tuesday 6 October 2020 10:32
>> >To: Power, Ciara <ciara.power@intel.com>
>> >Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
>> ><nhorman@tuxdriver.com>
>> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>> >
>> >Hi Ciara,
>> >
>> >Please find some comments below.
>> >
>> >On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote:
>> >> This patch adds a max SIMD bitwidth EAL configuration. The API
>> >> allows for an app to set this value. It can also be set using EAL
>> >> argument --force-max-simd-bitwidth, which will lock the value and
>> >> override any modifications made by the app.
>> >>
>> >> Signed-off-by: Ciara Power <ciara.power@intel.com>
>> >>
>> >> ---
>> >> v3:
>> >> - Added enum value to essentially disable using max SIMD to choose
>> >> paths, intended for use by ARM SVE.
>> >> - Fixed parsing bitwidth argument to return an error for values
>> >> greater than uint16_t.
>> >> v2: Added to Doxygen comment for API.
>> >> ---
>>
>> <snip>
>>
>> >>
>> >> +uint16_t
>> >> +rte_get_max_simd_bitwidth(void)
>> >> +{
>> >> + const struct internal_config *internal_conf =
>> >> + eal_get_internal_configuration();
>> >> + return internal_conf->max_simd_bitwidth.bitwidth;
>> >> +}
>> >
>> >Should the return value be enum rte_max_simd_t?
>> >If not, do we really need the enum definition?
>> >
>>
>> I kept the return value and param value below as uint16_t to allow for
>> arbitrary values, and will allow it be more flexible for future additions as
>new enums won't need to be added.
>> For the set function below, this is used when a user passes the EAL
>> command line flag, which passes an integer value rather than an enum one.
>> The enums are useful when checking the max_simd_bitwidth in
>> drivers/libs, for example using "RTE_MAX_256_SIMD" instead of "256" in
>the condition checks.
>>
>> >> +
>> >> +int
>> >> +rte_set_max_simd_bitwidth(uint16_t bitwidth) {
>> >> + struct internal_config *internal_conf =
>> >> + eal_get_internal_configuration();
>> >> + if (internal_conf->max_simd_bitwidth.locked) {
>> >> + RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user
>> >runtime override enabled");
>> >> + return -EPERM;
>> >> + }
>> >> +
>> >> + if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth <
>> >RTE_NO_SIMD ||
>> >> + !rte_is_power_of_2(bitwidth))) {
>> >> + RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
>> >> + return -EINVAL;
>> >> + }
>> >> + internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
>> >> + return 0;
>> >> +}
>> >
>> >Same question, should the parameter be enum rte_max_simd_t?
>> >
>>
>> <snip>
>>
>> >> +enum rte_max_simd_t {
>> >> + RTE_NO_SIMD = 64,
>> >> + RTE_MAX_128_SIMD = 128,
>> >> + RTE_MAX_256_SIMD = 256,
>> >> + RTE_MAX_512_SIMD = 512,
>> >> + RTE_MAX_SIMD_DISABLE = UINT16_MAX, };
>> >
>> >What is the difference between RTE_NO_SIMD and
>RTE_MAX_SIMD_DISABLE?
>>
>> RTE_NO_SIMD has value 64 to limit paths to scalar only.
>> RTE_MAX_SIMD_DISABLE sets the highest value possible, so essentially
>> disables the limit affecting which vector paths are taken.
>> This disable option was added to allow for ARM SVE which will be later
>> added, Discussed with Honnappa on a previous version:
>> https://patchwork.dpdk.org/patch/76097/
>
>Ok, so RTE_MAX_SIMD_DISABLE means "disable the max limit", right?
>
>I feel the name is a bit confusing. What about something like this:
>
>enum rte_simd {
> RTE_SIMD_DISABLED = 0,
> RTE_SIMD_128 = 128,
> RTE_SIMD_256 = 256,
> RTE_SIMD_512 = 512,
> RTE_SIMD_MAX = UINT16_MAX,
>};
>
>
Sure, I can rename these. Although will implement with RTE_SIMD_DISABLED=64 to allow for scalar path only.
>>
>> >The default value in internal_config is 0, so in my understanding
>> >rte_get_max_simd_bitwidth() will return 0 if
>> >--force-max-simd-bitwidth is not passed. Is it expected?
>> >
>> >Maybe I'm missing something, but I don't understand why the value in
>> >internal_config is not set to the maximum supported SIMD bitwidth by
>> >default, and optionally overriden by the command line argument, or by
>> >the API.
>> >
>>
>> The default value for max_simd_bitwidth is set depending on the
>> architecture, 256 for x86/ppc, and UINT16_MAX for ARM. So for example
>the default on x86 allows for AVX2 and under.
>> The defaults can be seen in patch 2:
>> https://patchwork.dpdk.org/patch/79339/
>
>Ok, I was expecting to have a runtime check for this. For instance, on intel
>architecture, it is not known at compilation, it depends on the target which
>can support up to AVX, AVX2, or AVX512.
>
Yes, the actual support will vary, but this max SIMD bitwidth is only an upper limit on what paths can be taken.
So for example with x86 default at 256, the path will still be chosen based on what the target can support, but it must be AVX2 or a lesser path.
This allows for AVX512 to be enabled at runtime, by increasing the max SIMD bitwidth to 512, allowing for that path to be taken where supported.
Thanks,
Ciara
Hi Ciara,
On Thu, Oct 08, 2020 at 09:25:42AM +0000, Power, Ciara wrote:
> Hi Olivier,
>
>
> >-----Original Message-----
> >From: Olivier Matz <olivier.matz@6wind.com>
> >Sent: Wednesday 7 October 2020 12:18
> >To: Power, Ciara <ciara.power@intel.com>
> >Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
> ><nhorman@tuxdriver.com>; Richardson, Bruce <bruce.richardson@intel.com>
> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
> >
> >Hi Ciara,
> >
> >On Wed, Oct 07, 2020 at 10:47:34AM +0000, Power, Ciara wrote:
> >> Hi Olivier,
> >>
> >> Thanks for reviewing, some comments below.
> >>
> >>
> >> >-----Original Message-----
> >> >From: Olivier Matz <olivier.matz@6wind.com>
> >> >Sent: Tuesday 6 October 2020 10:32
> >> >To: Power, Ciara <ciara.power@intel.com>
> >> >Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
> >> ><nhorman@tuxdriver.com>
> >> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
> >> >
> >> >Hi Ciara,
> >> >
> >> >Please find some comments below.
> >> >
> >> >On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote:
> >> >> This patch adds a max SIMD bitwidth EAL configuration. The API
> >> >> allows for an app to set this value. It can also be set using EAL
> >> >> argument --force-max-simd-bitwidth, which will lock the value and
> >> >> override any modifications made by the app.
> >> >>
> >> >> Signed-off-by: Ciara Power <ciara.power@intel.com>
> >> >>
> >> >> ---
> >> >> v3:
> >> >> - Added enum value to essentially disable using max SIMD to choose
> >> >> paths, intended for use by ARM SVE.
> >> >> - Fixed parsing bitwidth argument to return an error for values
> >> >> greater than uint16_t.
> >> >> v2: Added to Doxygen comment for API.
> >> >> ---
> >>
> >> <snip>
> >>
> >> >>
> >> >> +uint16_t
> >> >> +rte_get_max_simd_bitwidth(void)
> >> >> +{
> >> >> + const struct internal_config *internal_conf =
> >> >> + eal_get_internal_configuration();
> >> >> + return internal_conf->max_simd_bitwidth.bitwidth;
> >> >> +}
> >> >
> >> >Should the return value be enum rte_max_simd_t?
> >> >If not, do we really need the enum definition?
> >> >
> >>
> >> I kept the return value and param value below as uint16_t to allow for
> >> arbitrary values, and will allow it be more flexible for future additions as
> >new enums won't need to be added.
> >> For the set function below, this is used when a user passes the EAL
> >> command line flag, which passes an integer value rather than an enum one.
> >> The enums are useful when checking the max_simd_bitwidth in
> >> drivers/libs, for example using "RTE_MAX_256_SIMD" instead of "256" in
> >the condition checks.
> >>
> >> >> +
> >> >> +int
> >> >> +rte_set_max_simd_bitwidth(uint16_t bitwidth) {
> >> >> + struct internal_config *internal_conf =
> >> >> + eal_get_internal_configuration();
> >> >> + if (internal_conf->max_simd_bitwidth.locked) {
> >> >> + RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user
> >> >runtime override enabled");
> >> >> + return -EPERM;
> >> >> + }
> >> >> +
> >> >> + if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth <
> >> >RTE_NO_SIMD ||
> >> >> + !rte_is_power_of_2(bitwidth))) {
> >> >> + RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
> >> >> + return -EINVAL;
> >> >> + }
> >> >> + internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
> >> >> + return 0;
> >> >> +}
> >> >
> >> >Same question, should the parameter be enum rte_max_simd_t?
> >> >
> >>
> >> <snip>
> >>
> >> >> +enum rte_max_simd_t {
> >> >> + RTE_NO_SIMD = 64,
> >> >> + RTE_MAX_128_SIMD = 128,
> >> >> + RTE_MAX_256_SIMD = 256,
> >> >> + RTE_MAX_512_SIMD = 512,
> >> >> + RTE_MAX_SIMD_DISABLE = UINT16_MAX, };
> >> >
> >> >What is the difference between RTE_NO_SIMD and
> >RTE_MAX_SIMD_DISABLE?
> >>
> >> RTE_NO_SIMD has value 64 to limit paths to scalar only.
> >> RTE_MAX_SIMD_DISABLE sets the highest value possible, so essentially
> >> disables the limit affecting which vector paths are taken.
> >> This disable option was added to allow for ARM SVE which will be later
> >> added, Discussed with Honnappa on a previous version:
> >> https://patchwork.dpdk.org/patch/76097/
> >
> >Ok, so RTE_MAX_SIMD_DISABLE means "disable the max limit", right?
> >
> >I feel the name is a bit confusing. What about something like this:
> >
> >enum rte_simd {
> > RTE_SIMD_DISABLED = 0,
> > RTE_SIMD_128 = 128,
> > RTE_SIMD_256 = 256,
> > RTE_SIMD_512 = 512,
> > RTE_SIMD_MAX = UINT16_MAX,
> >};
> >
> >
>
> Sure, I can rename these. Although will implement with RTE_SIMD_DISABLED=64 to allow for scalar path only.
Out of curiosity, why 64? I thought 0 was a good value for "disabled".
> >>
> >> >The default value in internal_config is 0, so in my understanding
> >> >rte_get_max_simd_bitwidth() will return 0 if
> >> >--force-max-simd-bitwidth is not passed. Is it expected?
> >> >
> >> >Maybe I'm missing something, but I don't understand why the value in
> >> >internal_config is not set to the maximum supported SIMD bitwidth by
> >> >default, and optionally overriden by the command line argument, or by
> >> >the API.
> >> >
> >>
> >> The default value for max_simd_bitwidth is set depending on the
> >> architecture, 256 for x86/ppc, and UINT16_MAX for ARM. So for example
> >the default on x86 allows for AVX2 and under.
> >> The defaults can be seen in patch 2:
> >> https://patchwork.dpdk.org/patch/79339/
> >
> >Ok, I was expecting to have a runtime check for this. For instance, on intel
> >architecture, it is not known at compilation, it depends on the target which
> >can support up to AVX, AVX2, or AVX512.
> >
>
> Yes, the actual support will vary, but this max SIMD bitwidth is only an upper limit on what paths can be taken.
> So for example with x86 default at 256, the path will still be chosen based on what the target can support, but it must be AVX2 or a lesser path.
> This allows for AVX512 to be enabled at runtime, by increasing the max SIMD bitwidth to 512, allowing for that path to be taken where supported.
Ah, this means that AVX512 won't be enabled by default on machine that
support it? Is there a reason for that?
Another question: if the default value for max-simd-bitwidth is 256 on
Intel, and we are running on a target that does not support AVX2, will
the value be updated to 128 at initialization? In other word, is it
still up to the dpdk libraries doing vector code to check the
availability of vector instructions?
Thanks,
Olivier
Hi Olivier,
>-----Original Message-----
>From: Olivier Matz <olivier.matz@6wind.com>
>Sent: Thursday 8 October 2020 11:04
>To: Power, Ciara <ciara.power@intel.com>
>Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
><nhorman@tuxdriver.com>; Richardson, Bruce <bruce.richardson@intel.com>
>Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>
>Hi Ciara,
>
>On Thu, Oct 08, 2020 at 09:25:42AM +0000, Power, Ciara wrote:
>> Hi Olivier,
>>
>>
>> >-----Original Message-----
>> >From: Olivier Matz <olivier.matz@6wind.com>
>> >Sent: Wednesday 7 October 2020 12:18
>> >To: Power, Ciara <ciara.power@intel.com>
>> >Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
>> ><nhorman@tuxdriver.com>; Richardson, Bruce
>> ><bruce.richardson@intel.com>
>> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>> >
>> >Hi Ciara,
>> >
>> >On Wed, Oct 07, 2020 at 10:47:34AM +0000, Power, Ciara wrote:
>> >> Hi Olivier,
>> >>
>> >> Thanks for reviewing, some comments below.
>> >>
>> >>
>> >> >-----Original Message-----
>> >> >From: Olivier Matz <olivier.matz@6wind.com>
>> >> >Sent: Tuesday 6 October 2020 10:32
>> >> >To: Power, Ciara <ciara.power@intel.com>
>> >> >Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>; Neil Horman
>> >> ><nhorman@tuxdriver.com>
>> >> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD
>> >> >bitwidth
>> >> >
>> >> >Hi Ciara,
>> >> >
>> >> >Please find some comments below.
>> >> >
>> >> >On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote:
>> >> >> This patch adds a max SIMD bitwidth EAL configuration. The API
>> >> >> allows for an app to set this value. It can also be set using
>> >> >> EAL argument --force-max-simd-bitwidth, which will lock the
>> >> >> value and override any modifications made by the app.
>> >> >>
<snip>
>> >>
>> >> >> +enum rte_max_simd_t {
>> >> >> + RTE_NO_SIMD = 64,
>> >> >> + RTE_MAX_128_SIMD = 128,
>> >> >> + RTE_MAX_256_SIMD = 256,
>> >> >> + RTE_MAX_512_SIMD = 512,
>> >> >> + RTE_MAX_SIMD_DISABLE = UINT16_MAX, };
>> >> >
>> >> >What is the difference between RTE_NO_SIMD and
>> >RTE_MAX_SIMD_DISABLE?
>> >>
>> >> RTE_NO_SIMD has value 64 to limit paths to scalar only.
>> >> RTE_MAX_SIMD_DISABLE sets the highest value possible, so
>> >> essentially disables the limit affecting which vector paths are taken.
>> >> This disable option was added to allow for ARM SVE which will be
>> >> later added, Discussed with Honnappa on a previous version:
>> >> https://patchwork.dpdk.org/patch/76097/
>> >
>> >Ok, so RTE_MAX_SIMD_DISABLE means "disable the max limit", right?
>> >
>> >I feel the name is a bit confusing. What about something like this:
>> >
>> >enum rte_simd {
>> > RTE_SIMD_DISABLED = 0,
>> > RTE_SIMD_128 = 128,
>> > RTE_SIMD_256 = 256,
>> > RTE_SIMD_512 = 512,
>> > RTE_SIMD_MAX = UINT16_MAX,
>> >};
>> >
>> >
>>
>> Sure, I can rename these. Although will implement with
>RTE_SIMD_DISABLED=64 to allow for scalar path only.
>
>Out of curiosity, why 64? I thought 0 was a good value for "disabled".
>
64 was chosen because it represents the max bitwidth for the scalar path, 64 bits.
Currently, we use 0 on the command-line to represent the RTE_SIMD_MAX = UINT16_MAX,
as it is more user friendly to pass "--force-max-simd-bitwidth=0" rather than a max value, the
0 is then internally converted to the max value option. This would not be possible if we have
the scalar option as 0 value.
>> >>
>> >> >The default value in internal_config is 0, so in my understanding
>> >> >rte_get_max_simd_bitwidth() will return 0 if
>> >> >--force-max-simd-bitwidth is not passed. Is it expected?
>> >> >
>> >> >Maybe I'm missing something, but I don't understand why the value
>> >> >in internal_config is not set to the maximum supported SIMD
>> >> >bitwidth by default, and optionally overriden by the command line
>> >> >argument, or by the API.
>> >> >
>> >>
>> >> The default value for max_simd_bitwidth is set depending on the
>> >> architecture, 256 for x86/ppc, and UINT16_MAX for ARM. So for
>> >> example
>> >the default on x86 allows for AVX2 and under.
>> >> The defaults can be seen in patch 2:
>> >> https://patchwork.dpdk.org/patch/79339/
>> >
>> >Ok, I was expecting to have a runtime check for this. For instance,
>> >on intel architecture, it is not known at compilation, it depends on
>> >the target which can support up to AVX, AVX2, or AVX512.
>> >
>>
>> Yes, the actual support will vary, but this max SIMD bitwidth is only an
>upper limit on what paths can be taken.
>> So for example with x86 default at 256, the path will still be chosen based
>on what the target can support, but it must be AVX2 or a lesser path.
>> This allows for AVX512 to be enabled at runtime, by increasing the max
>SIMD bitwidth to 512, allowing for that path to be taken where supported.
>
>Ah, this means that AVX512 won't be enabled by default on machine that
>support it? Is there a reason for that?
>
We can't enable the AVX512 by default because it can cause CPU frequency slowdowns,
But this will allow runtime enabling to take that path if the app/user finds it is the best choice for their use,
by setting the max SIMD bitwidth to 512.
>Another question: if the default value for max-simd-bitwidth is 256 on Intel,
>and we are running on a target that does not support AVX2, will the value be
>updated to 128 at initialization? In other word, is it still up to the dpdk
>libraries doing vector code to check the availability of vector instructions?
>
>Thanks,
>Olivier
No the value is not updated depending on the support, it is just a limit.
Libraries still do the checks they had done previously to check what is supported, and once
that supported path is within the max SIMD bitwidth limit, it is okay to go ahead,
otherwise it will need to choose a lesser path.
For example, if a library supports AVX2, SSE and a scalar path, but the max SIMD bitwidth is set at 128 by app/user,
although the library supports AVX2, it will be limited to choosing the SSE path.
Whereas if for example a library supports only SSE and a scalar path, and the default max SIMD bitwidth is used (256),
the library can still choose SSE as it is below the 256 bit limit, and the limit remains at 256.
Thanks,
Ciara
On Thu, Oct 08, 2020 at 11:58:08AM +0100, Power, Ciara wrote:
> Hi Olivier,
>
>
> >-----Original Message----- From: Olivier Matz <olivier.matz@6wind.com>
> >Sent: Thursday 8 October 2020 11:04 To: Power, Ciara
> ><ciara.power@intel.com> Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>;
> >Neil Horman <nhorman@tuxdriver.com>; Richardson, Bruce
> ><bruce.richardson@intel.com> Subject: Re: [dpdk-dev] [PATCH v3 01/18]
> >eal: add max SIMD bitwidth
> >
> >Hi Ciara,
> >
> >On Thu, Oct 08, 2020 at 09:25:42AM +0000, Power, Ciara wrote:
> >> Hi Olivier,
> >>
> >>
> >> >-----Original Message----- From: Olivier Matz
> >> ><olivier.matz@6wind.com> Sent: Wednesday 7 October 2020 12:18 To:
> >> >Power, Ciara <ciara.power@intel.com> Cc: dev@dpdk.org; Ray Kinsella
> >> ><mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Richardson,
> >> >Bruce <bruce.richardson@intel.com> Subject: Re: [dpdk-dev] [PATCH v3
> >> >01/18] eal: add max SIMD bitwidth
> >> >
> >> >Hi Ciara,
> >> >
> >> >On Wed, Oct 07, 2020 at 10:47:34AM +0000, Power, Ciara wrote:
> >> >> Hi Olivier,
> >> >>
> >> >> Thanks for reviewing, some comments below.
> >> >>
> >> >>
> >> >> >-----Original Message----- From: Olivier Matz
> >> >> ><olivier.matz@6wind.com> Sent: Tuesday 6 October 2020 10:32 To:
> >> >> >Power, Ciara <ciara.power@intel.com> Cc: dev@dpdk.org; Ray
> >> >> >Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
> >> >> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD
> >> >> >bitwidth
> >> >> >
> >> >> >Hi Ciara,
> >> >> >
> >> >> >Please find some comments below.
> >> >> >
> >> >> >On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote:
> >> >> >> This patch adds a max SIMD bitwidth EAL configuration. The API
> >> >> >> allows for an app to set this value. It can also be set using
> >> >> >> EAL argument --force-max-simd-bitwidth, which will lock the
> >> >> >> value and override any modifications made by the app.
> >> >> >>
> <snip>
> >> >>
> >> >> >> +enum rte_max_simd_t { +RTE_NO_SIMD = 64, +RTE_MAX_128_SIMD =
> >> >> >> 128, +RTE_MAX_256_SIMD = 256, +RTE_MAX_512_SIMD = 512,
> >> >> >> +RTE_MAX_SIMD_DISABLE = UINT16_MAX, };
> >> >> >
> >> >> >What is the difference between RTE_NO_SIMD and
> >> >RTE_MAX_SIMD_DISABLE?
> >> >>
> >> >> RTE_NO_SIMD has value 64 to limit paths to scalar only.
> >> >> RTE_MAX_SIMD_DISABLE sets the highest value possible, so
> >> >> essentially disables the limit affecting which vector paths are
> >> >> taken. This disable option was added to allow for ARM SVE which
> >> >> will be later added, Discussed with Honnappa on a previous version:
> >> >> https://patchwork.dpdk.org/patch/76097/
> >> >
> >> >Ok, so RTE_MAX_SIMD_DISABLE means "disable the max limit", right?
> >> >
> >> >I feel the name is a bit confusing. What about something like this:
> >> >
> >> >enum rte_simd { RTE_SIMD_DISABLED = 0, RTE_SIMD_128 = 128,
> >> >RTE_SIMD_256 = 256, RTE_SIMD_512 = 512, RTE_SIMD_MAX = UINT16_MAX, };
> >> >
> >> >
> >>
> >> Sure, I can rename these. Although will implement with
> >RTE_SIMD_DISABLED=64 to allow for scalar path only.
> >
> >Out of curiosity, why 64? I thought 0 was a good value for "disabled".
> >
>
> 64 was chosen because it represents the max bitwidth for the scalar path,
> 64 bits. Currently, we use 0 on the command-line to represent the
> RTE_SIMD_MAX = UINT16_MAX, as it is more user friendly to pass
> "--force-max-simd-bitwidth=0" rather than a max value, the 0 is then
> internally converted to the max value option. This would not be possible
> if we have the scalar option as 0 value.
>
> >> >>
> >> >> >The default value in internal_config is 0, so in my understanding
> >> >> >rte_get_max_simd_bitwidth() will return 0 if
> >> >> >--force-max-simd-bitwidth is not passed. Is it expected?
> >> >> >
> >> >> >Maybe I'm missing something, but I don't understand why the value
> >> >> >in internal_config is not set to the maximum supported SIMD
> >> >> >bitwidth by default, and optionally overriden by the command line
> >> >> >argument, or by the API.
> >> >> >
> >> >>
> >> >> The default value for max_simd_bitwidth is set depending on the
> >> >> architecture, 256 for x86/ppc, and UINT16_MAX for ARM. So for
> >> >> example
> >> >the default on x86 allows for AVX2 and under.
> >> >> The defaults can be seen in patch 2:
> >> >> https://patchwork.dpdk.org/patch/79339/
> >> >
> >> >Ok, I was expecting to have a runtime check for this. For instance,
> >> >on intel architecture, it is not known at compilation, it depends on
> >> >the target which can support up to AVX, AVX2, or AVX512.
> >> >
> >>
> >> Yes, the actual support will vary, but this max SIMD bitwidth is only
> >> an
> >upper limit on what paths can be taken.
> >> So for example with x86 default at 256, the path will still be chosen
> >> based
> >on what the target can support, but it must be AVX2 or a lesser path.
> >> This allows for AVX512 to be enabled at runtime, by increasing the max
> >SIMD bitwidth to 512, allowing for that path to be taken where
> >supported.
> >
> >Ah, this means that AVX512 won't be enabled by default on machine that
> >support it? Is there a reason for that?
> >
>
> We can't enable the AVX512 by default because it can cause CPU frequency
> slowdowns, But this will allow runtime enabling to take that path if the
> app/user finds it is the best choice for their use, by setting the max
> SIMD bitwidth to 512.
>
> >Another question: if the default value for max-simd-bitwidth is 256 on
> >Intel, and we are running on a target that does not support AVX2, will
> >the value be updated to 128 at initialization? In other word, is it
> >still up to the dpdk libraries doing vector code to check the
> >availability of vector instructions?
> >
> >Thanks, Olivier
>
> No the value is not updated depending on the support, it is just a limit.
> Libraries still do the checks they had done previously to check what is
> supported, and once that supported path is within the max SIMD bitwidth
> limit, it is okay to go ahead, otherwise it will need to choose a lesser
> path. For example, if a library supports AVX2, SSE and a scalar path,
> but the max SIMD bitwidth is set at 128 by app/user, although the library
> supports AVX2, it will be limited to choosing the SSE path. Whereas if
> for example a library supports only SSE and a scalar path, and the
> default max SIMD bitwidth is used (256), the library can still choose SSE
> as it is below the 256 bit limit, and the limit remains at 256.
>
Just to note too that the reason for keeping this separation is that the
actual code path selection in each library is going to have to be
architecture specific, e.g. to choose SSE or NEON for 128-bit width,
whether or not the max-bitwidth functions take the running system into
account. By leaving the libs/drivers to check the CPU support themselves,
it keeps the max-bitwidth functions generic and saves having
architecture-specific code in two places for this.
On Thu, Oct 08, 2020 at 12:48:47PM +0100, Bruce Richardson wrote:
> On Thu, Oct 08, 2020 at 11:58:08AM +0100, Power, Ciara wrote:
> > Hi Olivier,
> >
> >
> > >-----Original Message----- From: Olivier Matz <olivier.matz@6wind.com>
> > >Sent: Thursday 8 October 2020 11:04 To: Power, Ciara
> > ><ciara.power@intel.com> Cc: dev@dpdk.org; Ray Kinsella <mdr@ashroe.eu>;
> > >Neil Horman <nhorman@tuxdriver.com>; Richardson, Bruce
> > ><bruce.richardson@intel.com> Subject: Re: [dpdk-dev] [PATCH v3 01/18]
> > >eal: add max SIMD bitwidth
> > >
> > >Hi Ciara,
> > >
> > >On Thu, Oct 08, 2020 at 09:25:42AM +0000, Power, Ciara wrote:
> > >> Hi Olivier,
> > >>
> > >>
> > >> >-----Original Message----- From: Olivier Matz
> > >> ><olivier.matz@6wind.com> Sent: Wednesday 7 October 2020 12:18 To:
> > >> >Power, Ciara <ciara.power@intel.com> Cc: dev@dpdk.org; Ray Kinsella
> > >> ><mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Richardson,
> > >> >Bruce <bruce.richardson@intel.com> Subject: Re: [dpdk-dev] [PATCH v3
> > >> >01/18] eal: add max SIMD bitwidth
> > >> >
> > >> >Hi Ciara,
> > >> >
> > >> >On Wed, Oct 07, 2020 at 10:47:34AM +0000, Power, Ciara wrote:
> > >> >> Hi Olivier,
> > >> >>
> > >> >> Thanks for reviewing, some comments below.
> > >> >>
> > >> >>
> > >> >> >-----Original Message----- From: Olivier Matz
> > >> >> ><olivier.matz@6wind.com> Sent: Tuesday 6 October 2020 10:32 To:
> > >> >> >Power, Ciara <ciara.power@intel.com> Cc: dev@dpdk.org; Ray
> > >> >> >Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
> > >> >> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD
> > >> >> >bitwidth
> > >> >> >
> > >> >> >Hi Ciara,
> > >> >> >
> > >> >> >Please find some comments below.
> > >> >> >
> > >> >> >On Wed, Sep 30, 2020 at 02:03:57PM +0100, Ciara Power wrote:
> > >> >> >> This patch adds a max SIMD bitwidth EAL configuration. The API
> > >> >> >> allows for an app to set this value. It can also be set using
> > >> >> >> EAL argument --force-max-simd-bitwidth, which will lock the
> > >> >> >> value and override any modifications made by the app.
> > >> >> >>
> > <snip>
> > >> >>
> > >> >> >> +enum rte_max_simd_t { +RTE_NO_SIMD = 64, +RTE_MAX_128_SIMD =
> > >> >> >> 128, +RTE_MAX_256_SIMD = 256, +RTE_MAX_512_SIMD = 512,
> > >> >> >> +RTE_MAX_SIMD_DISABLE = UINT16_MAX, };
> > >> >> >
> > >> >> >What is the difference between RTE_NO_SIMD and
> > >> >RTE_MAX_SIMD_DISABLE?
> > >> >>
> > >> >> RTE_NO_SIMD has value 64 to limit paths to scalar only.
> > >> >> RTE_MAX_SIMD_DISABLE sets the highest value possible, so
> > >> >> essentially disables the limit affecting which vector paths are
> > >> >> taken. This disable option was added to allow for ARM SVE which
> > >> >> will be later added, Discussed with Honnappa on a previous version:
> > >> >> https://patchwork.dpdk.org/patch/76097/
> > >> >
> > >> >Ok, so RTE_MAX_SIMD_DISABLE means "disable the max limit", right?
> > >> >
> > >> >I feel the name is a bit confusing. What about something like this:
> > >> >
> > >> >enum rte_simd { RTE_SIMD_DISABLED = 0, RTE_SIMD_128 = 128,
> > >> >RTE_SIMD_256 = 256, RTE_SIMD_512 = 512, RTE_SIMD_MAX = UINT16_MAX, };
> > >> >
> > >> >
> > >>
> > >> Sure, I can rename these. Although will implement with
> > >RTE_SIMD_DISABLED=64 to allow for scalar path only.
> > >
> > >Out of curiosity, why 64? I thought 0 was a good value for "disabled".
> > >
> >
> > 64 was chosen because it represents the max bitwidth for the scalar path,
> > 64 bits. Currently, we use 0 on the command-line to represent the
> > RTE_SIMD_MAX = UINT16_MAX, as it is more user friendly to pass
> > "--force-max-simd-bitwidth=0" rather than a max value, the 0 is then
> > internally converted to the max value option. This would not be possible
> > if we have the scalar option as 0 value.
> >
> > >> >>
> > >> >> >The default value in internal_config is 0, so in my understanding
> > >> >> >rte_get_max_simd_bitwidth() will return 0 if
> > >> >> >--force-max-simd-bitwidth is not passed. Is it expected?
> > >> >> >
> > >> >> >Maybe I'm missing something, but I don't understand why the value
> > >> >> >in internal_config is not set to the maximum supported SIMD
> > >> >> >bitwidth by default, and optionally overriden by the command line
> > >> >> >argument, or by the API.
> > >> >> >
> > >> >>
> > >> >> The default value for max_simd_bitwidth is set depending on the
> > >> >> architecture, 256 for x86/ppc, and UINT16_MAX for ARM. So for
> > >> >> example
> > >> >the default on x86 allows for AVX2 and under.
> > >> >> The defaults can be seen in patch 2:
> > >> >> https://patchwork.dpdk.org/patch/79339/
> > >> >
> > >> >Ok, I was expecting to have a runtime check for this. For instance,
> > >> >on intel architecture, it is not known at compilation, it depends on
> > >> >the target which can support up to AVX, AVX2, or AVX512.
> > >> >
> > >>
> > >> Yes, the actual support will vary, but this max SIMD bitwidth is only
> > >> an
> > >upper limit on what paths can be taken.
> > >> So for example with x86 default at 256, the path will still be chosen
> > >> based
> > >on what the target can support, but it must be AVX2 or a lesser path.
> > >> This allows for AVX512 to be enabled at runtime, by increasing the max
> > >SIMD bitwidth to 512, allowing for that path to be taken where
> > >supported.
> > >
> > >Ah, this means that AVX512 won't be enabled by default on machine that
> > >support it? Is there a reason for that?
> > >
> >
> > We can't enable the AVX512 by default because it can cause CPU frequency
> > slowdowns, But this will allow runtime enabling to take that path if the
> > app/user finds it is the best choice for their use, by setting the max
> > SIMD bitwidth to 512.
> >
> > >Another question: if the default value for max-simd-bitwidth is 256 on
> > >Intel, and we are running on a target that does not support AVX2, will
> > >the value be updated to 128 at initialization? In other word, is it
> > >still up to the dpdk libraries doing vector code to check the
> > >availability of vector instructions?
> > >
> > >Thanks, Olivier
> >
> > No the value is not updated depending on the support, it is just a limit.
> > Libraries still do the checks they had done previously to check what is
> > supported, and once that supported path is within the max SIMD bitwidth
> > limit, it is okay to go ahead, otherwise it will need to choose a lesser
> > path. For example, if a library supports AVX2, SSE and a scalar path,
> > but the max SIMD bitwidth is set at 128 by app/user, although the library
> > supports AVX2, it will be limited to choosing the SSE path. Whereas if
> > for example a library supports only SSE and a scalar path, and the
> > default max SIMD bitwidth is used (256), the library can still choose SSE
> > as it is below the 256 bit limit, and the limit remains at 256.
> >
> Just to note too that the reason for keeping this separation is that the
> actual code path selection in each library is going to have to be
> architecture specific, e.g. to choose SSE or NEON for 128-bit width,
> whether or not the max-bitwidth functions take the running system into
> account. By leaving the libs/drivers to check the CPU support themselves,
> it keeps the max-bitwidth functions generic and saves having
> architecture-specific code in two places for this.
Ok, that's clearer to me, thanks Ciara and Bruce.
> This patch adds a max SIMD bitwidth EAL configuration. The API allows
> for an app to set this value. It can also be set using EAL argument
> --force-max-simd-bitwidth, which will lock the value and override any
> modifications made by the app.
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
>
> ---
> v3:
> - Added enum value to essentially disable using max SIMD to choose
> paths, intended for use by ARM SVE.
> - Fixed parsing bitwidth argument to return an error for values
> greater than uint16_t.
> v2: Added to Doxygen comment for API.
> ---
> lib/librte_eal/common/eal_common_options.c | 64 ++++++++++++++++++++++
> lib/librte_eal/common/eal_internal_cfg.h | 8 +++
> lib/librte_eal/common/eal_options.h | 2 +
> lib/librte_eal/include/rte_eal.h | 33 +++++++++++
> lib/librte_eal/rte_eal_version.map | 4 ++
> 5 files changed, 111 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index a5426e1234..e9117a96af 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -102,6 +102,7 @@ eal_long_options[] = {
> {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
> {OPT_TELEMETRY, 0, NULL, OPT_TELEMETRY_NUM },
> {OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
> + {OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
> {0, 0, NULL, 0 }
> };
>
> @@ -1309,6 +1310,34 @@ eal_parse_iova_mode(const char *name)
> return 0;
> }
>
> +static int
> +eal_parse_simd_bitwidth(const char *arg, bool locked)
> +{
> + char *end;
> + unsigned long bitwidth;
> + int ret;
> + struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> +
> + if (arg == NULL || arg[0] == '\0')
> + return -1;
> +
> + errno = 0;
> + bitwidth = strtoul(arg, &end, 0);
> +
> + /* check for errors */
> + if (bitwidth > UINT16_MAX || errno != 0 || end == NULL || *end != '\0')
> + return -1;
> +
> + if (bitwidth == 0)
> + bitwidth = UINT16_MAX;
> + ret = rte_set_max_simd_bitwidth(bitwidth);
> + if (ret < 0)
> + return -1;
> + internal_conf->max_simd_bitwidth.locked = locked;
> + return 0;
> +}
> +
> static int
> eal_parse_base_virtaddr(const char *arg)
> {
> @@ -1707,6 +1736,13 @@ eal_parse_common_option(int opt, const char *optarg,
> case OPT_NO_TELEMETRY_NUM:
> conf->no_telemetry = 1;
> break;
> + case OPT_FORCE_MAX_SIMD_BITWIDTH_NUM:
> + if (eal_parse_simd_bitwidth(optarg, 1) < 0) {
> + RTE_LOG(ERR, EAL, "invalid parameter for --"
> + OPT_FORCE_MAX_SIMD_BITWIDTH "\n");
> + return -1;
> + }
> + break;
>
> /* don't know what to do, leave this to caller */
> default:
> @@ -1903,6 +1939,33 @@ eal_check_common_options(struct internal_config *internal_cfg)
> return 0;
> }
>
> +uint16_t
> +rte_get_max_simd_bitwidth(void)
> +{
> + const struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> + return internal_conf->max_simd_bitwidth.bitwidth;
> +}
> +
> +int
> +rte_set_max_simd_bitwidth(uint16_t bitwidth)
> +{
> + struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> + if (internal_conf->max_simd_bitwidth.locked) {
> + RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user runtime override enabled");
> + return -EPERM;
> + }
> +
> + if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth < RTE_NO_SIMD ||
> + !rte_is_power_of_2(bitwidth))) {
> + RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
> + return -EINVAL;
> + }
> + internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
> + return 0;
> +}
> +
> void
> eal_common_usage(void)
> {
> @@ -1981,6 +2044,7 @@ eal_common_usage(void)
> " --"OPT_BASE_VIRTADDR" Base virtual address\n"
> " --"OPT_TELEMETRY" Enable telemetry support (on by default)\n"
> " --"OPT_NO_TELEMETRY" Disable telemetry support\n"
> + " --"OPT_FORCE_MAX_SIMD_BITWIDTH" Force the max SIMD bitwidth\n"
> "\nEAL options for DEBUG use only:\n"
> " --"OPT_HUGE_UNLINK" Unlink hugepage files after init\n"
> " --"OPT_NO_HUGE" Use malloc instead of hugetlbfs\n"
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index 13f93388a7..367e0cc19e 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -33,6 +33,12 @@ struct hugepage_info {
> int lock_descriptor; /**< file descriptor for hugepage dir */
> };
>
> +struct simd_bitwidth {
> + /**< flag indicating if bitwidth is locked from further modification */
> + bool locked;
> + uint16_t bitwidth; /**< bitwidth value */
> +};
> +
> /**
> * internal configuration
> */
> @@ -85,6 +91,8 @@ struct internal_config {
> volatile unsigned int init_complete;
> /**< indicates whether EAL has completed initialization */
> unsigned int no_telemetry; /**< true to disable Telemetry */
> + /** max simd bitwidth path to use */
> + struct simd_bitwidth max_simd_bitwidth;
> };
>
> void eal_reset_internal_config(struct internal_config *internal_cfg);
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index 89769d48b4..ef33979664 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -85,6 +85,8 @@ enum {
> OPT_TELEMETRY_NUM,
> #define OPT_NO_TELEMETRY "no-telemetry"
> OPT_NO_TELEMETRY_NUM,
> +#define OPT_FORCE_MAX_SIMD_BITWIDTH "force-max-simd-bitwidth"
> + OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
> OPT_LONG_MAX_NUM
> };
>
> diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> index ddcf6a2e7a..fb739f3474 100644
> --- a/lib/librte_eal/include/rte_eal.h
> +++ b/lib/librte_eal/include/rte_eal.h
> @@ -43,6 +43,14 @@ enum rte_proc_type_t {
> RTE_PROC_INVALID
> };
>
> +enum rte_max_simd_t {
> + RTE_NO_SIMD = 64,
While I do understand the idea of having that value from consistency point of view,
I wonder do we really need to allow user to specify values smaller then 128.
At least on x86 we always have 128 bit SIMD enabled, even for -Dmachine=default.
So seems no much point to forbid libraries using SSE code-path when compiler
is free to insert SSE instructions on its own will.
> + RTE_MAX_128_SIMD = 128,
> + RTE_MAX_256_SIMD = 256,
> + RTE_MAX_512_SIMD = 512,
> + RTE_MAX_SIMD_DISABLE = UINT16_MAX,
As a nit, I think it is safe enough to have this last value
(RTE_MAX_SIMD_DISABLE or RTE_MAX_SIMD_MAX) equal to (INT16_MAX + 1).
That would be big enough to probably never hit actual HW limit,
while it still remains power of two, as other values.
> +};
> +
> /**
> * Get the process type in a multi-process setup
> *
> @@ -51,6 +59,31 @@ enum rte_proc_type_t {
> */
> enum rte_proc_type_t rte_eal_process_type(void);
>
> +/**
> + * Get the supported SIMD bitwidth.
> + *
> + * @return
> + * uint16_t bitwidth.
> + */
> +__rte_experimental
> +uint16_t rte_get_max_simd_bitwidth(void);
> +
> +/**
> + * Set the supported SIMD bitwidth.
> + * This API should only be called once at initialization, before EAL init.
> + *
> + * @param bitwidth
> + * uint16_t bitwidth.
> + * @return
> + * 0 on success.
> + * @return
> + * -EINVAL on invalid bitwidth parameter.
> + * @return
> + * -EPERM if bitwidth is locked.
> + */
> +__rte_experimental
> +int rte_set_max_simd_bitwidth(uint16_t bitwidth);
> +
> /**
> * Request iopl privilege for all RPL.
> *
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index c32461c663..17a7195a3d 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -397,6 +397,10 @@ EXPERIMENTAL {
> rte_service_lcore_may_be_active;
> rte_thread_register;
> rte_thread_unregister;
> +
> + # added in 20.11
> + rte_get_max_simd_bitwidth;
> + rte_set_max_simd_bitwidth;
> };
>
> INTERNAL {
> --
> 2.17.1
On Thu, Oct 08, 2020 at 01:07:26PM +0000, Ananyev, Konstantin wrote:
>
> > This patch adds a max SIMD bitwidth EAL configuration. The API allows
> > for an app to set this value. It can also be set using EAL argument
> > --force-max-simd-bitwidth, which will lock the value and override any
> > modifications made by the app.
> >
> > Signed-off-by: Ciara Power <ciara.power@intel.com>
> >
> > ---
> > v3:
> > - Added enum value to essentially disable using max SIMD to choose
> > paths, intended for use by ARM SVE.
> > - Fixed parsing bitwidth argument to return an error for values
> > greater than uint16_t.
> > v2: Added to Doxygen comment for API.
> > ---
> > lib/librte_eal/common/eal_common_options.c | 64 ++++++++++++++++++++++
> > lib/librte_eal/common/eal_internal_cfg.h | 8 +++
> > lib/librte_eal/common/eal_options.h | 2 +
> > lib/librte_eal/include/rte_eal.h | 33 +++++++++++
> > lib/librte_eal/rte_eal_version.map | 4 ++
> > 5 files changed, 111 insertions(+)
> >
> > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> > index a5426e1234..e9117a96af 100644
> > --- a/lib/librte_eal/common/eal_common_options.c
> > +++ b/lib/librte_eal/common/eal_common_options.c
> > @@ -102,6 +102,7 @@ eal_long_options[] = {
> > {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
> > {OPT_TELEMETRY, 0, NULL, OPT_TELEMETRY_NUM },
> > {OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
> > + {OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
> > {0, 0, NULL, 0 }
> > };
> >
> > @@ -1309,6 +1310,34 @@ eal_parse_iova_mode(const char *name)
> > return 0;
> > }
> >
> > +static int
> > +eal_parse_simd_bitwidth(const char *arg, bool locked)
> > +{
> > + char *end;
> > + unsigned long bitwidth;
> > + int ret;
> > + struct internal_config *internal_conf =
> > + eal_get_internal_configuration();
> > +
> > + if (arg == NULL || arg[0] == '\0')
> > + return -1;
> > +
> > + errno = 0;
> > + bitwidth = strtoul(arg, &end, 0);
> > +
> > + /* check for errors */
> > + if (bitwidth > UINT16_MAX || errno != 0 || end == NULL || *end != '\0')
> > + return -1;
> > +
> > + if (bitwidth == 0)
> > + bitwidth = UINT16_MAX;
> > + ret = rte_set_max_simd_bitwidth(bitwidth);
> > + if (ret < 0)
> > + return -1;
> > + internal_conf->max_simd_bitwidth.locked = locked;
> > + return 0;
> > +}
> > +
> > static int
> > eal_parse_base_virtaddr(const char *arg)
> > {
> > @@ -1707,6 +1736,13 @@ eal_parse_common_option(int opt, const char *optarg,
> > case OPT_NO_TELEMETRY_NUM:
> > conf->no_telemetry = 1;
> > break;
> > + case OPT_FORCE_MAX_SIMD_BITWIDTH_NUM:
> > + if (eal_parse_simd_bitwidth(optarg, 1) < 0) {
> > + RTE_LOG(ERR, EAL, "invalid parameter for --"
> > + OPT_FORCE_MAX_SIMD_BITWIDTH "\n");
> > + return -1;
> > + }
> > + break;
> >
> > /* don't know what to do, leave this to caller */
> > default:
> > @@ -1903,6 +1939,33 @@ eal_check_common_options(struct internal_config *internal_cfg)
> > return 0;
> > }
> >
> > +uint16_t
> > +rte_get_max_simd_bitwidth(void)
> > +{
> > + const struct internal_config *internal_conf =
> > + eal_get_internal_configuration();
> > + return internal_conf->max_simd_bitwidth.bitwidth;
> > +}
> > +
> > +int
> > +rte_set_max_simd_bitwidth(uint16_t bitwidth)
> > +{
> > + struct internal_config *internal_conf =
> > + eal_get_internal_configuration();
> > + if (internal_conf->max_simd_bitwidth.locked) {
> > + RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user runtime override enabled");
> > + return -EPERM;
> > + }
> > +
> > + if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth < RTE_NO_SIMD ||
> > + !rte_is_power_of_2(bitwidth))) {
> > + RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
> > + return -EINVAL;
> > + }
> > + internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
> > + return 0;
> > +}
> > +
> > void
> > eal_common_usage(void)
> > {
> > @@ -1981,6 +2044,7 @@ eal_common_usage(void)
> > " --"OPT_BASE_VIRTADDR" Base virtual address\n"
> > " --"OPT_TELEMETRY" Enable telemetry support (on by default)\n"
> > " --"OPT_NO_TELEMETRY" Disable telemetry support\n"
> > + " --"OPT_FORCE_MAX_SIMD_BITWIDTH" Force the max SIMD bitwidth\n"
> > "\nEAL options for DEBUG use only:\n"
> > " --"OPT_HUGE_UNLINK" Unlink hugepage files after init\n"
> > " --"OPT_NO_HUGE" Use malloc instead of hugetlbfs\n"
> > diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> > index 13f93388a7..367e0cc19e 100644
> > --- a/lib/librte_eal/common/eal_internal_cfg.h
> > +++ b/lib/librte_eal/common/eal_internal_cfg.h
> > @@ -33,6 +33,12 @@ struct hugepage_info {
> > int lock_descriptor; /**< file descriptor for hugepage dir */
> > };
> >
> > +struct simd_bitwidth {
> > + /**< flag indicating if bitwidth is locked from further modification */
> > + bool locked;
> > + uint16_t bitwidth; /**< bitwidth value */
> > +};
> > +
> > /**
> > * internal configuration
> > */
> > @@ -85,6 +91,8 @@ struct internal_config {
> > volatile unsigned int init_complete;
> > /**< indicates whether EAL has completed initialization */
> > unsigned int no_telemetry; /**< true to disable Telemetry */
> > + /** max simd bitwidth path to use */
> > + struct simd_bitwidth max_simd_bitwidth;
> > };
> >
> > void eal_reset_internal_config(struct internal_config *internal_cfg);
> > diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> > index 89769d48b4..ef33979664 100644
> > --- a/lib/librte_eal/common/eal_options.h
> > +++ b/lib/librte_eal/common/eal_options.h
> > @@ -85,6 +85,8 @@ enum {
> > OPT_TELEMETRY_NUM,
> > #define OPT_NO_TELEMETRY "no-telemetry"
> > OPT_NO_TELEMETRY_NUM,
> > +#define OPT_FORCE_MAX_SIMD_BITWIDTH "force-max-simd-bitwidth"
> > + OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
> > OPT_LONG_MAX_NUM
> > };
> >
> > diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> > index ddcf6a2e7a..fb739f3474 100644
> > --- a/lib/librte_eal/include/rte_eal.h
> > +++ b/lib/librte_eal/include/rte_eal.h
> > @@ -43,6 +43,14 @@ enum rte_proc_type_t {
> > RTE_PROC_INVALID
> > };
> >
> > +enum rte_max_simd_t {
> > + RTE_NO_SIMD = 64,
>
> While I do understand the idea of having that value from consistency point of view,
> I wonder do we really need to allow user to specify values smaller then 128.
> At least on x86 we always have 128 bit SIMD enabled, even for -Dmachine=default.
> So seems no much point to forbid libraries using SSE code-path when compiler
> is free to insert SSE instructions on its own will.
>
The reason to support this is for testing purposes, as it allows an easy
way for a tester to check out any scalar code paths - which are often
common across architectures.
> > + RTE_MAX_128_SIMD = 128,
> > + RTE_MAX_256_SIMD = 256,
> > + RTE_MAX_512_SIMD = 512,
> > + RTE_MAX_SIMD_DISABLE = UINT16_MAX,
>
> As a nit, I think it is safe enough to have this last value
> (RTE_MAX_SIMD_DISABLE or RTE_MAX_SIMD_MAX) equal to (INT16_MAX + 1).
> That would be big enough to probably never hit actual HW limit,
> while it still remains power of two, as other values.
>
I actually think it's probably clearer as-is, because the fact of the rest
being powers of 2 is irrelevant since we just check greater than or less
than. If we did change it, then we need to put in a comment explaining why
the plus-one, while as it is now it's clearly a placeholder $BIGNUM.
/Bruce
>
> diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> index ddcf6a2e7a..fb739f3474 100644
> --- a/lib/librte_eal/include/rte_eal.h
> +++ b/lib/librte_eal/include/rte_eal.h
> @@ -43,6 +43,14 @@ enum rte_proc_type_t {
> RTE_PROC_INVALID
> };
>
> +enum rte_max_simd_t {
Just one more nit, why do we need '_t' suffix here?
Usually we '_t' is reserved for typedefs only.
> + RTE_NO_SIMD = 64,
> + RTE_MAX_128_SIMD = 128,
> + RTE_MAX_256_SIMD = 256,
> + RTE_MAX_512_SIMD = 512,
> + RTE_MAX_SIMD_DISABLE = UINT16_MAX,
> +};
> +
> On Thu, Oct 08, 2020 at 01:07:26PM +0000, Ananyev, Konstantin wrote:
> >
> > > This patch adds a max SIMD bitwidth EAL configuration. The API allows
> > > for an app to set this value. It can also be set using EAL argument
> > > --force-max-simd-bitwidth, which will lock the value and override any
> > > modifications made by the app.
> > >
> > > Signed-off-by: Ciara Power <ciara.power@intel.com>
> > >
> > > ---
> > > v3:
> > > - Added enum value to essentially disable using max SIMD to choose
> > > paths, intended for use by ARM SVE.
> > > - Fixed parsing bitwidth argument to return an error for values
> > > greater than uint16_t.
> > > v2: Added to Doxygen comment for API.
> > > ---
> > > lib/librte_eal/common/eal_common_options.c | 64 ++++++++++++++++++++++
> > > lib/librte_eal/common/eal_internal_cfg.h | 8 +++
> > > lib/librte_eal/common/eal_options.h | 2 +
> > > lib/librte_eal/include/rte_eal.h | 33 +++++++++++
> > > lib/librte_eal/rte_eal_version.map | 4 ++
> > > 5 files changed, 111 insertions(+)
> > >
> > > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> > > index a5426e1234..e9117a96af 100644
> > > --- a/lib/librte_eal/common/eal_common_options.c
> > > +++ b/lib/librte_eal/common/eal_common_options.c
> > > @@ -102,6 +102,7 @@ eal_long_options[] = {
> > > {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
> > > {OPT_TELEMETRY, 0, NULL, OPT_TELEMETRY_NUM },
> > > {OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
> > > + {OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
> > > {0, 0, NULL, 0 }
> > > };
> > >
> > > @@ -1309,6 +1310,34 @@ eal_parse_iova_mode(const char *name)
> > > return 0;
> > > }
> > >
> > > +static int
> > > +eal_parse_simd_bitwidth(const char *arg, bool locked)
> > > +{
> > > + char *end;
> > > + unsigned long bitwidth;
> > > + int ret;
> > > + struct internal_config *internal_conf =
> > > + eal_get_internal_configuration();
> > > +
> > > + if (arg == NULL || arg[0] == '\0')
> > > + return -1;
> > > +
> > > + errno = 0;
> > > + bitwidth = strtoul(arg, &end, 0);
> > > +
> > > + /* check for errors */
> > > + if (bitwidth > UINT16_MAX || errno != 0 || end == NULL || *end != '\0')
> > > + return -1;
> > > +
> > > + if (bitwidth == 0)
> > > + bitwidth = UINT16_MAX;
> > > + ret = rte_set_max_simd_bitwidth(bitwidth);
> > > + if (ret < 0)
> > > + return -1;
> > > + internal_conf->max_simd_bitwidth.locked = locked;
> > > + return 0;
> > > +}
> > > +
> > > static int
> > > eal_parse_base_virtaddr(const char *arg)
> > > {
> > > @@ -1707,6 +1736,13 @@ eal_parse_common_option(int opt, const char *optarg,
> > > case OPT_NO_TELEMETRY_NUM:
> > > conf->no_telemetry = 1;
> > > break;
> > > + case OPT_FORCE_MAX_SIMD_BITWIDTH_NUM:
> > > + if (eal_parse_simd_bitwidth(optarg, 1) < 0) {
> > > + RTE_LOG(ERR, EAL, "invalid parameter for --"
> > > + OPT_FORCE_MAX_SIMD_BITWIDTH "\n");
> > > + return -1;
> > > + }
> > > + break;
> > >
> > > /* don't know what to do, leave this to caller */
> > > default:
> > > @@ -1903,6 +1939,33 @@ eal_check_common_options(struct internal_config *internal_cfg)
> > > return 0;
> > > }
> > >
> > > +uint16_t
> > > +rte_get_max_simd_bitwidth(void)
> > > +{
> > > + const struct internal_config *internal_conf =
> > > + eal_get_internal_configuration();
> > > + return internal_conf->max_simd_bitwidth.bitwidth;
> > > +}
> > > +
> > > +int
> > > +rte_set_max_simd_bitwidth(uint16_t bitwidth)
> > > +{
> > > + struct internal_config *internal_conf =
> > > + eal_get_internal_configuration();
> > > + if (internal_conf->max_simd_bitwidth.locked) {
> > > + RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user runtime override enabled");
> > > + return -EPERM;
> > > + }
> > > +
> > > + if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth < RTE_NO_SIMD ||
> > > + !rte_is_power_of_2(bitwidth))) {
> > > + RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
> > > + return -EINVAL;
> > > + }
> > > + internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
> > > + return 0;
> > > +}
> > > +
> > > void
> > > eal_common_usage(void)
> > > {
> > > @@ -1981,6 +2044,7 @@ eal_common_usage(void)
> > > " --"OPT_BASE_VIRTADDR" Base virtual address\n"
> > > " --"OPT_TELEMETRY" Enable telemetry support (on by default)\n"
> > > " --"OPT_NO_TELEMETRY" Disable telemetry support\n"
> > > + " --"OPT_FORCE_MAX_SIMD_BITWIDTH" Force the max SIMD bitwidth\n"
> > > "\nEAL options for DEBUG use only:\n"
> > > " --"OPT_HUGE_UNLINK" Unlink hugepage files after init\n"
> > > " --"OPT_NO_HUGE" Use malloc instead of hugetlbfs\n"
> > > diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> > > index 13f93388a7..367e0cc19e 100644
> > > --- a/lib/librte_eal/common/eal_internal_cfg.h
> > > +++ b/lib/librte_eal/common/eal_internal_cfg.h
> > > @@ -33,6 +33,12 @@ struct hugepage_info {
> > > int lock_descriptor; /**< file descriptor for hugepage dir */
> > > };
> > >
> > > +struct simd_bitwidth {
> > > + /**< flag indicating if bitwidth is locked from further modification */
> > > + bool locked;
> > > + uint16_t bitwidth; /**< bitwidth value */
> > > +};
> > > +
> > > /**
> > > * internal configuration
> > > */
> > > @@ -85,6 +91,8 @@ struct internal_config {
> > > volatile unsigned int init_complete;
> > > /**< indicates whether EAL has completed initialization */
> > > unsigned int no_telemetry; /**< true to disable Telemetry */
> > > + /** max simd bitwidth path to use */
> > > + struct simd_bitwidth max_simd_bitwidth;
> > > };
> > >
> > > void eal_reset_internal_config(struct internal_config *internal_cfg);
> > > diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> > > index 89769d48b4..ef33979664 100644
> > > --- a/lib/librte_eal/common/eal_options.h
> > > +++ b/lib/librte_eal/common/eal_options.h
> > > @@ -85,6 +85,8 @@ enum {
> > > OPT_TELEMETRY_NUM,
> > > #define OPT_NO_TELEMETRY "no-telemetry"
> > > OPT_NO_TELEMETRY_NUM,
> > > +#define OPT_FORCE_MAX_SIMD_BITWIDTH "force-max-simd-bitwidth"
> > > + OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
> > > OPT_LONG_MAX_NUM
> > > };
> > >
> > > diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> > > index ddcf6a2e7a..fb739f3474 100644
> > > --- a/lib/librte_eal/include/rte_eal.h
> > > +++ b/lib/librte_eal/include/rte_eal.h
> > > @@ -43,6 +43,14 @@ enum rte_proc_type_t {
> > > RTE_PROC_INVALID
> > > };
> > >
> > > +enum rte_max_simd_t {
> > > + RTE_NO_SIMD = 64,
> >
> > While I do understand the idea of having that value from consistency point of view,
> > I wonder do we really need to allow user to specify values smaller then 128.
> > At least on x86 we always have 128 bit SIMD enabled, even for -Dmachine=default.
> > So seems no much point to forbid libraries using SSE code-path when compiler
> > is free to insert SSE instructions on its own will.
> >
>
> The reason to support this is for testing purposes, as it allows an easy
> way for a tester to check out any scalar code paths - which are often
> common across architectures.
If it is just for testing things in a consistent way, then it is probably ok.
The thing that worries me - later in this series there are patches
that insert extra checks into inline functions that use SSE instincts:
https://patches.dpdk.org/patch/79355/ (lpm: choose vector path at runtime).
Which seems like a total overkill for me.
>
> > > + RTE_MAX_128_SIMD = 128,
> > > + RTE_MAX_256_SIMD = 256,
> > > + RTE_MAX_512_SIMD = 512,
> > > + RTE_MAX_SIMD_DISABLE = UINT16_MAX,
> >
> > As a nit, I think it is safe enough to have this last value
> > (RTE_MAX_SIMD_DISABLE or RTE_MAX_SIMD_MAX) equal to (INT16_MAX + 1).
> > That would be big enough to probably never hit actual HW limit,
> > while it still remains power of two, as other values.
> >
>
> I actually think it's probably clearer as-is, because the fact of the rest
> being powers of 2 is irrelevant since we just check greater than or less
> than.
Well, rte_set_max_simd_bitwidth() does accept only power of two values
_AND_ this special one (UINT16_MAX).
By changing it to 2^15, we can remove that special value test.
> If we did change it, then we need to put in a comment explaining why
> the plus-one,
I don't think it is that big deal to put a comment,
plus for UINT16_MAX we do need some explanation too, right?
>while as it is now it's clearly a placeholder $BIGNUM.
>
> /Bruce
On Thu, Oct 08, 2020 at 03:07:54PM +0100, Ananyev, Konstantin wrote:
>
> > On Thu, Oct 08, 2020 at 01:07:26PM +0000, Ananyev, Konstantin wrote:
> > >
> > > > This patch adds a max SIMD bitwidth EAL configuration. The API allows
> > > > for an app to set this value. It can also be set using EAL argument
> > > > --force-max-simd-bitwidth, which will lock the value and override any
> > > > modifications made by the app.
> > > >
> > > > Signed-off-by: Ciara Power <ciara.power@intel.com>
> > > >
> > > > ---
> > > > v3:
> > > > - Added enum value to essentially disable using max SIMD to choose
> > > > paths, intended for use by ARM SVE.
> > > > - Fixed parsing bitwidth argument to return an error for values
> > > > greater than uint16_t.
> > > > v2: Added to Doxygen comment for API.
> > > > ---
> > > > lib/librte_eal/common/eal_common_options.c | 64 ++++++++++++++++++++++
> > > > lib/librte_eal/common/eal_internal_cfg.h | 8 +++
> > > > lib/librte_eal/common/eal_options.h | 2 +
> > > > lib/librte_eal/include/rte_eal.h | 33 +++++++++++
> > > > lib/librte_eal/rte_eal_version.map | 4 ++
> > > > 5 files changed, 111 insertions(+)
> > > >
> > > > diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> > > > index a5426e1234..e9117a96af 100644
> > > > --- a/lib/librte_eal/common/eal_common_options.c
> > > > +++ b/lib/librte_eal/common/eal_common_options.c
> > > > @@ -102,6 +102,7 @@ eal_long_options[] = {
> > > > {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
> > > > {OPT_TELEMETRY, 0, NULL, OPT_TELEMETRY_NUM },
> > > > {OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
> > > > +{OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
> > > > {0, 0, NULL, 0 }
> > > > };
> > > >
> > > > @@ -1309,6 +1310,34 @@ eal_parse_iova_mode(const char *name)
> > > > return 0;
> > > > }
> > > >
> > > > +static int
> > > > +eal_parse_simd_bitwidth(const char *arg, bool locked)
> > > > +{
> > > > +char *end;
> > > > +unsigned long bitwidth;
> > > > +int ret;
> > > > +struct internal_config *internal_conf =
> > > > +eal_get_internal_configuration();
> > > > +
> > > > +if (arg == NULL || arg[0] == '\0')
> > > > +return -1;
> > > > +
> > > > +errno = 0;
> > > > +bitwidth = strtoul(arg, &end, 0);
> > > > +
> > > > +/* check for errors */
> > > > +if (bitwidth > UINT16_MAX || errno != 0 || end == NULL || *end != '\0')
> > > > +return -1;
> > > > +
> > > > +if (bitwidth == 0)
> > > > +bitwidth = UINT16_MAX;
> > > > +ret = rte_set_max_simd_bitwidth(bitwidth);
> > > > +if (ret < 0)
> > > > +return -1;
> > > > +internal_conf->max_simd_bitwidth.locked = locked;
> > > > +return 0;
> > > > +}
> > > > +
> > > > static int
> > > > eal_parse_base_virtaddr(const char *arg)
> > > > {
> > > > @@ -1707,6 +1736,13 @@ eal_parse_common_option(int opt, const char *optarg,
> > > > case OPT_NO_TELEMETRY_NUM:
> > > > conf->no_telemetry = 1;
> > > > break;
> > > > +case OPT_FORCE_MAX_SIMD_BITWIDTH_NUM:
> > > > +if (eal_parse_simd_bitwidth(optarg, 1) < 0) {
> > > > +RTE_LOG(ERR, EAL, "invalid parameter for --"
> > > > +OPT_FORCE_MAX_SIMD_BITWIDTH "\n");
> > > > +return -1;
> > > > +}
> > > > +break;
> > > >
> > > > /* don't know what to do, leave this to caller */
> > > > default:
> > > > @@ -1903,6 +1939,33 @@ eal_check_common_options(struct internal_config *internal_cfg)
> > > > return 0;
> > > > }
> > > >
> > > > +uint16_t
> > > > +rte_get_max_simd_bitwidth(void)
> > > > +{
> > > > +const struct internal_config *internal_conf =
> > > > +eal_get_internal_configuration();
> > > > +return internal_conf->max_simd_bitwidth.bitwidth;
> > > > +}
> > > > +
> > > > +int
> > > > +rte_set_max_simd_bitwidth(uint16_t bitwidth)
> > > > +{
> > > > +struct internal_config *internal_conf =
> > > > +eal_get_internal_configuration();
> > > > +if (internal_conf->max_simd_bitwidth.locked) {
> > > > +RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user runtime override enabled");
> > > > +return -EPERM;
> > > > +}
> > > > +
> > > > +if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth < RTE_NO_SIMD ||
> > > > +!rte_is_power_of_2(bitwidth))) {
> > > > +RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
> > > > +return -EINVAL;
> > > > +}
> > > > +internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
> > > > +return 0;
> > > > +}
> > > > +
> > > > void
> > > > eal_common_usage(void)
> > > > {
> > > > @@ -1981,6 +2044,7 @@ eal_common_usage(void)
> > > > " --"OPT_BASE_VIRTADDR" Base virtual address\n"
> > > > " --"OPT_TELEMETRY" Enable telemetry support (on by default)\n"
> > > > " --"OPT_NO_TELEMETRY" Disable telemetry support\n"
> > > > + " --"OPT_FORCE_MAX_SIMD_BITWIDTH" Force the max SIMD bitwidth\n"
> > > > "\nEAL options for DEBUG use only:\n"
> > > > " --"OPT_HUGE_UNLINK" Unlink hugepage files after init\n"
> > > > " --"OPT_NO_HUGE" Use malloc instead of hugetlbfs\n"
> > > > diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> > > > index 13f93388a7..367e0cc19e 100644
> > > > --- a/lib/librte_eal/common/eal_internal_cfg.h
> > > > +++ b/lib/librte_eal/common/eal_internal_cfg.h
> > > > @@ -33,6 +33,12 @@ struct hugepage_info {
> > > > int lock_descriptor; /**< file descriptor for hugepage dir */
> > > > };
> > > >
> > > > +struct simd_bitwidth {
> > > > +/**< flag indicating if bitwidth is locked from further modification */
> > > > +bool locked;
> > > > +uint16_t bitwidth; /**< bitwidth value */
> > > > +};
> > > > +
> > > > /**
> > > > * internal configuration
> > > > */
> > > > @@ -85,6 +91,8 @@ struct internal_config {
> > > > volatile unsigned int init_complete;
> > > > /**< indicates whether EAL has completed initialization */
> > > > unsigned int no_telemetry; /**< true to disable Telemetry */
> > > > +/** max simd bitwidth path to use */
> > > > +struct simd_bitwidth max_simd_bitwidth;
> > > > };
> > > >
> > > > void eal_reset_internal_config(struct internal_config *internal_cfg);
> > > > diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> > > > index 89769d48b4..ef33979664 100644
> > > > --- a/lib/librte_eal/common/eal_options.h
> > > > +++ b/lib/librte_eal/common/eal_options.h
> > > > @@ -85,6 +85,8 @@ enum {
> > > > OPT_TELEMETRY_NUM,
> > > > #define OPT_NO_TELEMETRY "no-telemetry"
> > > > OPT_NO_TELEMETRY_NUM,
> > > > +#define OPT_FORCE_MAX_SIMD_BITWIDTH "force-max-simd-bitwidth"
> > > > +OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
> > > > OPT_LONG_MAX_NUM
> > > > };
> > > >
> > > > diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> > > > index ddcf6a2e7a..fb739f3474 100644
> > > > --- a/lib/librte_eal/include/rte_eal.h
> > > > +++ b/lib/librte_eal/include/rte_eal.h
> > > > @@ -43,6 +43,14 @@ enum rte_proc_type_t {
> > > > RTE_PROC_INVALID
> > > > };
> > > >
> > > > +enum rte_max_simd_t {
> > > > +RTE_NO_SIMD = 64,
> > >
> > > While I do understand the idea of having that value from consistency point of view,
> > > I wonder do we really need to allow user to specify values smaller then 128.
> > > At least on x86 we always have 128 bit SIMD enabled, even for -Dmachine=default.
> > > So seems no much point to forbid libraries using SSE code-path when compiler
> > > is free to insert SSE instructions on its own will.
> > >
> >
> > The reason to support this is for testing purposes, as it allows an easy
> > way for a tester to check out any scalar code paths - which are often
> > common across architectures.
>
> If it is just for testing things in a consistent way, then it is probably ok.
> The thing that worries me - later in this series there are patches
> that insert extra checks into inline functions that use SSE instincts:
> https://patches.dpdk.org/patch/79355/ (lpm: choose vector path at runtime).
> Which seems like a total overkill for me.
>
> >
> > > > +RTE_MAX_128_SIMD = 128,
> > > > +RTE_MAX_256_SIMD = 256,
> > > > +RTE_MAX_512_SIMD = 512,
> > > > +RTE_MAX_SIMD_DISABLE = UINT16_MAX,
> > >
> > > As a nit, I think it is safe enough to have this last value
> > > (RTE_MAX_SIMD_DISABLE or RTE_MAX_SIMD_MAX) equal to (INT16_MAX + 1).
> > > That would be big enough to probably never hit actual HW limit,
> > > while it still remains power of two, as other values.
> > >
> >
> > I actually think it's probably clearer as-is, because the fact of the rest
> > being powers of 2 is irrelevant since we just check greater than or less
> > than.
>
> Well, rte_set_max_simd_bitwidth() does accept only power of two values
> _AND_ this special one (UINT16_MAX).
> By changing it to 2^15, we can remove that special value test.
>
> > If we did change it, then we need to put in a comment explaining why
> > the plus-one,
>
> I don't think it is that big deal to put a comment,
> plus for UINT16_MAX we do need some explanation too, right?
>
I'm ok either way. Ciara, what do you think?
Hi Bruce, Konstantin,
>-----Original Message-----
>From: Bruce Richardson <bruce.richardson@intel.com>
>Sent: Thursday 8 October 2020 15:18
>To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
>Cc: Power, Ciara <ciara.power@intel.com>; dev@dpdk.org; Ray Kinsella
><mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
>Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>
>On Thu, Oct 08, 2020 at 03:07:54PM +0100, Ananyev, Konstantin wrote:
>>
>> > On Thu, Oct 08, 2020 at 01:07:26PM +0000, Ananyev, Konstantin wrote:
>> > >
>> > > > This patch adds a max SIMD bitwidth EAL configuration. The API
>> > > > allows for an app to set this value. It can also be set using
>> > > > EAL argument --force-max-simd-bitwidth, which will lock the
>> > > > value and override any modifications made by the app.
>> > > >
>> > > > Signed-off-by: Ciara Power <ciara.power@intel.com>
>> > > >
>> > > > ---
>> > > > v3:
>> > > > - Added enum value to essentially disable using max SIMD to choose
>> > > > paths, intended for use by ARM SVE.
>> > > > - Fixed parsing bitwidth argument to return an error for values
>> > > > greater than uint16_t.
>> > > > v2: Added to Doxygen comment for API.
>> > > > ---
>> > > > lib/librte_eal/common/eal_common_options.c | 64
>++++++++++++++++++++++
>> > > > lib/librte_eal/common/eal_internal_cfg.h | 8 +++
>> > > > lib/librte_eal/common/eal_options.h | 2 +
>> > > > lib/librte_eal/include/rte_eal.h | 33 +++++++++++
>> > > > lib/librte_eal/rte_eal_version.map | 4 ++
>> > > > 5 files changed, 111 insertions(+)
>> > > >
>> > > > diff --git a/lib/librte_eal/common/eal_common_options.c
>> > > > b/lib/librte_eal/common/eal_common_options.c
>> > > > index a5426e1234..e9117a96af 100644
>> > > > --- a/lib/librte_eal/common/eal_common_options.c
>> > > > +++ b/lib/librte_eal/common/eal_common_options.c
>> > > > @@ -102,6 +102,7 @@ eal_long_options[] = {
>> > > > {OPT_MATCH_ALLOCATIONS, 0, NULL,
>OPT_MATCH_ALLOCATIONS_NUM},
>> > > > {OPT_TELEMETRY, 0, NULL, OPT_TELEMETRY_NUM },
>> > > > {OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
>> > > > +{OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL,
>> > > > +OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
>> > > > {0, 0, NULL, 0 }
>> > > > };
>> > > >
>> > > > @@ -1309,6 +1310,34 @@ eal_parse_iova_mode(const char *name)
>> > > > return 0; }
>> > > >
>> > > > +static int
>> > > > +eal_parse_simd_bitwidth(const char *arg, bool locked) { char
>> > > > +*end; unsigned long bitwidth; int ret; struct internal_config
>> > > > +*internal_conf = eal_get_internal_configuration();
>> > > > +
>> > > > +if (arg == NULL || arg[0] == '\0') return -1;
>> > > > +
>> > > > +errno = 0;
>> > > > +bitwidth = strtoul(arg, &end, 0);
>> > > > +
>> > > > +/* check for errors */
>> > > > +if (bitwidth > UINT16_MAX || errno != 0 || end == NULL || *end
>> > > > +!= '\0') return -1;
>> > > > +
>> > > > +if (bitwidth == 0)
>> > > > +bitwidth = UINT16_MAX;
>> > > > +ret = rte_set_max_simd_bitwidth(bitwidth);
>> > > > +if (ret < 0)
>> > > > +return -1;
>> > > > +internal_conf->max_simd_bitwidth.locked = locked; return 0; }
>> > > > +
>> > > > static int
>> > > > eal_parse_base_virtaddr(const char *arg) { @@ -1707,6 +1736,13
>> > > > @@ eal_parse_common_option(int opt, const char *optarg, case
>> > > > OPT_NO_TELEMETRY_NUM:
>> > > > conf->no_telemetry = 1;
>> > > > break;
>> > > > +case OPT_FORCE_MAX_SIMD_BITWIDTH_NUM:
>> > > > +if (eal_parse_simd_bitwidth(optarg, 1) < 0) { RTE_LOG(ERR, EAL,
>> > > > +"invalid parameter for --"
>> > > > +OPT_FORCE_MAX_SIMD_BITWIDTH "\n"); return -1; } break;
>> > > >
>> > > > /* don't know what to do, leave this to caller */
>> > > > default:
>> > > > @@ -1903,6 +1939,33 @@ eal_check_common_options(struct
>> > > > internal_config *internal_cfg) return 0; }
>> > > >
>> > > > +uint16_t
>> > > > +rte_get_max_simd_bitwidth(void) { const struct internal_config
>> > > > +*internal_conf = eal_get_internal_configuration(); return
>> > > > +internal_conf->max_simd_bitwidth.bitwidth;
>> > > > +}
>> > > > +
>> > > > +int
>> > > > +rte_set_max_simd_bitwidth(uint16_t bitwidth) { struct
>> > > > +internal_config *internal_conf =
>> > > > +eal_get_internal_configuration();
>> > > > +if (internal_conf->max_simd_bitwidth.locked) { RTE_LOG(NOTICE,
>> > > > +EAL, "Cannot set max SIMD bitwidth - user runtime override
>> > > > +enabled"); return -EPERM; }
>> > > > +
>> > > > +if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth <
>RTE_NO_SIMD
>> > > > +||
>> > > > +!rte_is_power_of_2(bitwidth))) { RTE_LOG(ERR, EAL, "Invalid
>> > > > +bitwidth value!\n"); return -EINVAL; }
>> > > > +internal_conf->max_simd_bitwidth.bitwidth = bitwidth; return 0;
>> > > > +}
>> > > > +
>> > > > void
>> > > > eal_common_usage(void)
>> > > > {
>> > > > @@ -1981,6 +2044,7 @@ eal_common_usage(void)
>> > > > " --"OPT_BASE_VIRTADDR" Base virtual address\n"
>> > > > " --"OPT_TELEMETRY" Enable telemetry support (on by
>default)\n"
>> > > > " --"OPT_NO_TELEMETRY" Disable telemetry support\n"
>> > > > + " --"OPT_FORCE_MAX_SIMD_BITWIDTH" Force the max SIMD
>bitwidth\n"
>> > > > "\nEAL options for DEBUG use only:\n"
>> > > > " --"OPT_HUGE_UNLINK" Unlink hugepage files after init\n"
>> > > > " --"OPT_NO_HUGE" Use malloc instead of hugetlbfs\n"
>> > > > diff --git a/lib/librte_eal/common/eal_internal_cfg.h
>b/lib/librte_eal/common/eal_internal_cfg.h
>> > > > index 13f93388a7..367e0cc19e 100644
>> > > > --- a/lib/librte_eal/common/eal_internal_cfg.h
>> > > > +++ b/lib/librte_eal/common/eal_internal_cfg.h
>> > > > @@ -33,6 +33,12 @@ struct hugepage_info {
>> > > > int lock_descriptor; /**< file descriptor for hugepage dir */
>> > > > };
>> > > >
>> > > > +struct simd_bitwidth {
>> > > > +/**< flag indicating if bitwidth is locked from further modification */
>> > > > +bool locked;
>> > > > +uint16_t bitwidth; /**< bitwidth value */
>> > > > +};
>> > > > +
>> > > > /**
>> > > > * internal configuration
>> > > > */
>> > > > @@ -85,6 +91,8 @@ struct internal_config {
>> > > > volatile unsigned int init_complete;
>> > > > /**< indicates whether EAL has completed initialization */
>> > > > unsigned int no_telemetry; /**< true to disable Telemetry */
>> > > > +/** max simd bitwidth path to use */
>> > > > +struct simd_bitwidth max_simd_bitwidth;
>> > > > };
>> > > >
>> > > > void eal_reset_internal_config(struct internal_config *internal_cfg);
>> > > > diff --git a/lib/librte_eal/common/eal_options.h
>b/lib/librte_eal/common/eal_options.h
>> > > > index 89769d48b4..ef33979664 100644
>> > > > --- a/lib/librte_eal/common/eal_options.h
>> > > > +++ b/lib/librte_eal/common/eal_options.h
>> > > > @@ -85,6 +85,8 @@ enum {
>> > > > OPT_TELEMETRY_NUM,
>> > > > #define OPT_NO_TELEMETRY "no-telemetry"
>> > > > OPT_NO_TELEMETRY_NUM,
>> > > > +#define OPT_FORCE_MAX_SIMD_BITWIDTH "force-max-simd-
>bitwidth"
>> > > > +OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
>> > > > OPT_LONG_MAX_NUM
>> > > > };
>> > > >
>> > > > diff --git a/lib/librte_eal/include/rte_eal.h
>b/lib/librte_eal/include/rte_eal.h
>> > > > index ddcf6a2e7a..fb739f3474 100644
>> > > > --- a/lib/librte_eal/include/rte_eal.h
>> > > > +++ b/lib/librte_eal/include/rte_eal.h
>> > > > @@ -43,6 +43,14 @@ enum rte_proc_type_t {
>> > > > RTE_PROC_INVALID
>> > > > };
>> > > >
>> > > > +enum rte_max_simd_t {
>> > > > +RTE_NO_SIMD = 64,
>> > >
>> > > While I do understand the idea of having that value from consistency
>point of view,
>> > > I wonder do we really need to allow user to specify values smaller then
>128.
>> > > At least on x86 we always have 128 bit SIMD enabled, even for -
>Dmachine=default.
>> > > So seems no much point to forbid libraries using SSE code-path when
>compiler
>> > > is free to insert SSE instructions on its own will.
>> > >
>> >
>> > The reason to support this is for testing purposes, as it allows an easy
>> > way for a tester to check out any scalar code paths - which are often
>> > common across architectures.
>>
>> If it is just for testing things in a consistent way, then it is probably ok.
>> The thing that worries me - later in this series there are patches
>> that insert extra checks into inline functions that use SSE instincts:
>> https://patches.dpdk.org/patch/79355/ (lpm: choose vector path at
>runtime).
>> Which seems like a total overkill for me.
>>
>> >
>> > > > +RTE_MAX_128_SIMD = 128,
>> > > > +RTE_MAX_256_SIMD = 256,
>> > > > +RTE_MAX_512_SIMD = 512,
>> > > > +RTE_MAX_SIMD_DISABLE = UINT16_MAX,
>> > >
>> > > As a nit, I think it is safe enough to have this last value
>> > > (RTE_MAX_SIMD_DISABLE or RTE_MAX_SIMD_MAX) equal to
>(INT16_MAX + 1).
>> > > That would be big enough to probably never hit actual HW limit,
>> > > while it still remains power of two, as other values.
>> > >
>> >
>> > I actually think it's probably clearer as-is, because the fact of the rest
>> > being powers of 2 is irrelevant since we just check greater than or less
>> > than.
>>
>> Well, rte_set_max_simd_bitwidth() does accept only power of two values
>> _AND_ this special one (UINT16_MAX).
>> By changing it to 2^15, we can remove that special value test.
>>
>> > If we did change it, then we need to put in a comment explaining why
>> > the plus-one,
>>
>> I don't think it is that big deal to put a comment,
>> plus for UINT16_MAX we do need some explanation too, right?
>>
>I'm ok either way. Ciara, what do you think?
Either is fine with me, I can change it to (INT16_MAX + 1) if that is preferred, and remove the extra special case check in the rte_set_max_simd_bitwidth()
Thanks,
Ciara
On Wed, Sep 30, 2020 at 3:08 PM Ciara Power <ciara.power@intel.com> wrote:
>
> This patch adds a max SIMD bitwidth EAL configuration. The API allows
> for an app to set this value. It can also be set using EAL argument
> --force-max-simd-bitwidth, which will lock the value and override any
> modifications made by the app.
>
> Signed-off-by: Ciara Power <ciara.power@intel.com>
>
> ---
> v3:
> - Added enum value to essentially disable using max SIMD to choose
> paths, intended for use by ARM SVE.
> - Fixed parsing bitwidth argument to return an error for values
> greater than uint16_t.
> v2: Added to Doxygen comment for API.
> ---
> lib/librte_eal/common/eal_common_options.c | 64 ++++++++++++++++++++++
> lib/librte_eal/common/eal_internal_cfg.h | 8 +++
> lib/librte_eal/common/eal_options.h | 2 +
> lib/librte_eal/include/rte_eal.h | 33 +++++++++++
> lib/librte_eal/rte_eal_version.map | 4 ++
> 5 files changed, 111 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
> index a5426e1234..e9117a96af 100644
> --- a/lib/librte_eal/common/eal_common_options.c
> +++ b/lib/librte_eal/common/eal_common_options.c
> @@ -102,6 +102,7 @@ eal_long_options[] = {
> {OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
> {OPT_TELEMETRY, 0, NULL, OPT_TELEMETRY_NUM },
> {OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
> + {OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
> {0, 0, NULL, 0 }
> };
>
> @@ -1309,6 +1310,34 @@ eal_parse_iova_mode(const char *name)
> return 0;
> }
>
> +static int
> +eal_parse_simd_bitwidth(const char *arg, bool locked)
No need to pass a "locked" bool, we only care about forced value in
this function.
> +{
> + char *end;
> + unsigned long bitwidth;
> + int ret;
> + struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> +
> + if (arg == NULL || arg[0] == '\0')
> + return -1;
> +
> + errno = 0;
> + bitwidth = strtoul(arg, &end, 0);
> +
> + /* check for errors */
> + if (bitwidth > UINT16_MAX || errno != 0 || end == NULL || *end != '\0')
Nit: look at bitwidth after checking errno and consorts.
> + return -1;
> +
> + if (bitwidth == 0)
> + bitwidth = UINT16_MAX;
> + ret = rte_set_max_simd_bitwidth(bitwidth);
> + if (ret < 0)
> + return -1;
> + internal_conf->max_simd_bitwidth.locked = locked;
Please align eal option and internal config field name.
--force-max-simd-bitwidth => .forced ?
And then %s/locked/forced/g
> + return 0;
> +}
> +
> static int
> eal_parse_base_virtaddr(const char *arg)
> {
> @@ -1707,6 +1736,13 @@ eal_parse_common_option(int opt, const char *optarg,
> case OPT_NO_TELEMETRY_NUM:
> conf->no_telemetry = 1;
> break;
> + case OPT_FORCE_MAX_SIMD_BITWIDTH_NUM:
> + if (eal_parse_simd_bitwidth(optarg, 1) < 0) {
> + RTE_LOG(ERR, EAL, "invalid parameter for --"
> + OPT_FORCE_MAX_SIMD_BITWIDTH "\n");
> + return -1;
> + }
> + break;
>
> /* don't know what to do, leave this to caller */
> default:
> @@ -1903,6 +1939,33 @@ eal_check_common_options(struct internal_config *internal_cfg)
> return 0;
> }
>
> +uint16_t
> +rte_get_max_simd_bitwidth(void)
> +{
> + const struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> + return internal_conf->max_simd_bitwidth.bitwidth;
> +}
> +
> +int
> +rte_set_max_simd_bitwidth(uint16_t bitwidth)
> +{
> + struct internal_config *internal_conf =
> + eal_get_internal_configuration();
> + if (internal_conf->max_simd_bitwidth.locked) {
> + RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user runtime override enabled");
> + return -EPERM;
> + }
> +
> + if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth < RTE_NO_SIMD ||
> + !rte_is_power_of_2(bitwidth))) {
> + RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
> + return -EINVAL;
> + }
> + internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
> + return 0;
> +}
> +
> void
> eal_common_usage(void)
> {
> @@ -1981,6 +2044,7 @@ eal_common_usage(void)
> " --"OPT_BASE_VIRTADDR" Base virtual address\n"
> " --"OPT_TELEMETRY" Enable telemetry support (on by default)\n"
> " --"OPT_NO_TELEMETRY" Disable telemetry support\n"
> + " --"OPT_FORCE_MAX_SIMD_BITWIDTH" Force the max SIMD bitwidth\n"
> "\nEAL options for DEBUG use only:\n"
> " --"OPT_HUGE_UNLINK" Unlink hugepage files after init\n"
> " --"OPT_NO_HUGE" Use malloc instead of hugetlbfs\n"
> diff --git a/lib/librte_eal/common/eal_internal_cfg.h b/lib/librte_eal/common/eal_internal_cfg.h
> index 13f93388a7..367e0cc19e 100644
> --- a/lib/librte_eal/common/eal_internal_cfg.h
> +++ b/lib/librte_eal/common/eal_internal_cfg.h
> @@ -33,6 +33,12 @@ struct hugepage_info {
> int lock_descriptor; /**< file descriptor for hugepage dir */
> };
>
> +struct simd_bitwidth {
> + /**< flag indicating if bitwidth is locked from further modification */
> + bool locked;
> + uint16_t bitwidth; /**< bitwidth value */
> +};
> +
> /**
> * internal configuration
> */
> @@ -85,6 +91,8 @@ struct internal_config {
> volatile unsigned int init_complete;
> /**< indicates whether EAL has completed initialization */
> unsigned int no_telemetry; /**< true to disable Telemetry */
> + /** max simd bitwidth path to use */
/**<
> + struct simd_bitwidth max_simd_bitwidth;
> };
>
> void eal_reset_internal_config(struct internal_config *internal_cfg);
> diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
> index 89769d48b4..ef33979664 100644
> --- a/lib/librte_eal/common/eal_options.h
> +++ b/lib/librte_eal/common/eal_options.h
> @@ -85,6 +85,8 @@ enum {
> OPT_TELEMETRY_NUM,
> #define OPT_NO_TELEMETRY "no-telemetry"
> OPT_NO_TELEMETRY_NUM,
> +#define OPT_FORCE_MAX_SIMD_BITWIDTH "force-max-simd-bitwidth"
> + OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
> OPT_LONG_MAX_NUM
> };
>
> diff --git a/lib/librte_eal/include/rte_eal.h b/lib/librte_eal/include/rte_eal.h
> index ddcf6a2e7a..fb739f3474 100644
> --- a/lib/librte_eal/include/rte_eal.h
> +++ b/lib/librte_eal/include/rte_eal.h
> @@ -43,6 +43,14 @@ enum rte_proc_type_t {
> RTE_PROC_INVALID
> };
>
> +enum rte_max_simd_t {
> + RTE_NO_SIMD = 64,
> + RTE_MAX_128_SIMD = 128,
> + RTE_MAX_256_SIMD = 256,
> + RTE_MAX_512_SIMD = 512,
> + RTE_MAX_SIMD_DISABLE = UINT16_MAX,
> +};
> +
> /**
> * Get the process type in a multi-process setup
> *
> @@ -51,6 +59,31 @@ enum rte_proc_type_t {
> */
> enum rte_proc_type_t rte_eal_process_type(void);
>
> +/**
> + * Get the supported SIMD bitwidth.
> + *
> + * @return
> + * uint16_t bitwidth.
> + */
> +__rte_experimental
> +uint16_t rte_get_max_simd_bitwidth(void);
> +
> +/**
> + * Set the supported SIMD bitwidth.
> + * This API should only be called once at initialization, before EAL init.
> + *
> + * @param bitwidth
> + * uint16_t bitwidth.
> + * @return
> + * 0 on success.
> + * @return
> + * -EINVAL on invalid bitwidth parameter.
> + * @return
> + * -EPERM if bitwidth is locked.
A single @return with a bullet list is preferred (note to self: fix
rte_eal_cleanup).
@return
- 0 on success.
- -EINVAL on invalid bitwidth parameter.
- -EPERM if bitwidth is forced.
> + */
> +__rte_experimental
> +int rte_set_max_simd_bitwidth(uint16_t bitwidth);
> +
> /**
> * Request iopl privilege for all RPL.
> *
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index c32461c663..17a7195a3d 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -397,6 +397,10 @@ EXPERIMENTAL {
> rte_service_lcore_may_be_active;
> rte_thread_register;
> rte_thread_unregister;
> +
> + # added in 20.11
> + rte_get_max_simd_bitwidth;
> + rte_set_max_simd_bitwidth;
> };
>
> INTERNAL {
> --
> 2.17.1
>
@@ -102,6 +102,7 @@ eal_long_options[] = {
{OPT_MATCH_ALLOCATIONS, 0, NULL, OPT_MATCH_ALLOCATIONS_NUM},
{OPT_TELEMETRY, 0, NULL, OPT_TELEMETRY_NUM },
{OPT_NO_TELEMETRY, 0, NULL, OPT_NO_TELEMETRY_NUM },
+ {OPT_FORCE_MAX_SIMD_BITWIDTH, 1, NULL, OPT_FORCE_MAX_SIMD_BITWIDTH_NUM},
{0, 0, NULL, 0 }
};
@@ -1309,6 +1310,34 @@ eal_parse_iova_mode(const char *name)
return 0;
}
+static int
+eal_parse_simd_bitwidth(const char *arg, bool locked)
+{
+ char *end;
+ unsigned long bitwidth;
+ int ret;
+ struct internal_config *internal_conf =
+ eal_get_internal_configuration();
+
+ if (arg == NULL || arg[0] == '\0')
+ return -1;
+
+ errno = 0;
+ bitwidth = strtoul(arg, &end, 0);
+
+ /* check for errors */
+ if (bitwidth > UINT16_MAX || errno != 0 || end == NULL || *end != '\0')
+ return -1;
+
+ if (bitwidth == 0)
+ bitwidth = UINT16_MAX;
+ ret = rte_set_max_simd_bitwidth(bitwidth);
+ if (ret < 0)
+ return -1;
+ internal_conf->max_simd_bitwidth.locked = locked;
+ return 0;
+}
+
static int
eal_parse_base_virtaddr(const char *arg)
{
@@ -1707,6 +1736,13 @@ eal_parse_common_option(int opt, const char *optarg,
case OPT_NO_TELEMETRY_NUM:
conf->no_telemetry = 1;
break;
+ case OPT_FORCE_MAX_SIMD_BITWIDTH_NUM:
+ if (eal_parse_simd_bitwidth(optarg, 1) < 0) {
+ RTE_LOG(ERR, EAL, "invalid parameter for --"
+ OPT_FORCE_MAX_SIMD_BITWIDTH "\n");
+ return -1;
+ }
+ break;
/* don't know what to do, leave this to caller */
default:
@@ -1903,6 +1939,33 @@ eal_check_common_options(struct internal_config *internal_cfg)
return 0;
}
+uint16_t
+rte_get_max_simd_bitwidth(void)
+{
+ const struct internal_config *internal_conf =
+ eal_get_internal_configuration();
+ return internal_conf->max_simd_bitwidth.bitwidth;
+}
+
+int
+rte_set_max_simd_bitwidth(uint16_t bitwidth)
+{
+ struct internal_config *internal_conf =
+ eal_get_internal_configuration();
+ if (internal_conf->max_simd_bitwidth.locked) {
+ RTE_LOG(NOTICE, EAL, "Cannot set max SIMD bitwidth - user runtime override enabled");
+ return -EPERM;
+ }
+
+ if (bitwidth != RTE_MAX_SIMD_DISABLE && (bitwidth < RTE_NO_SIMD ||
+ !rte_is_power_of_2(bitwidth))) {
+ RTE_LOG(ERR, EAL, "Invalid bitwidth value!\n");
+ return -EINVAL;
+ }
+ internal_conf->max_simd_bitwidth.bitwidth = bitwidth;
+ return 0;
+}
+
void
eal_common_usage(void)
{
@@ -1981,6 +2044,7 @@ eal_common_usage(void)
" --"OPT_BASE_VIRTADDR" Base virtual address\n"
" --"OPT_TELEMETRY" Enable telemetry support (on by default)\n"
" --"OPT_NO_TELEMETRY" Disable telemetry support\n"
+ " --"OPT_FORCE_MAX_SIMD_BITWIDTH" Force the max SIMD bitwidth\n"
"\nEAL options for DEBUG use only:\n"
" --"OPT_HUGE_UNLINK" Unlink hugepage files after init\n"
" --"OPT_NO_HUGE" Use malloc instead of hugetlbfs\n"
@@ -33,6 +33,12 @@ struct hugepage_info {
int lock_descriptor; /**< file descriptor for hugepage dir */
};
+struct simd_bitwidth {
+ /**< flag indicating if bitwidth is locked from further modification */
+ bool locked;
+ uint16_t bitwidth; /**< bitwidth value */
+};
+
/**
* internal configuration
*/
@@ -85,6 +91,8 @@ struct internal_config {
volatile unsigned int init_complete;
/**< indicates whether EAL has completed initialization */
unsigned int no_telemetry; /**< true to disable Telemetry */
+ /** max simd bitwidth path to use */
+ struct simd_bitwidth max_simd_bitwidth;
};
void eal_reset_internal_config(struct internal_config *internal_cfg);
@@ -85,6 +85,8 @@ enum {
OPT_TELEMETRY_NUM,
#define OPT_NO_TELEMETRY "no-telemetry"
OPT_NO_TELEMETRY_NUM,
+#define OPT_FORCE_MAX_SIMD_BITWIDTH "force-max-simd-bitwidth"
+ OPT_FORCE_MAX_SIMD_BITWIDTH_NUM,
OPT_LONG_MAX_NUM
};
@@ -43,6 +43,14 @@ enum rte_proc_type_t {
RTE_PROC_INVALID
};
+enum rte_max_simd_t {
+ RTE_NO_SIMD = 64,
+ RTE_MAX_128_SIMD = 128,
+ RTE_MAX_256_SIMD = 256,
+ RTE_MAX_512_SIMD = 512,
+ RTE_MAX_SIMD_DISABLE = UINT16_MAX,
+};
+
/**
* Get the process type in a multi-process setup
*
@@ -51,6 +59,31 @@ enum rte_proc_type_t {
*/
enum rte_proc_type_t rte_eal_process_type(void);
+/**
+ * Get the supported SIMD bitwidth.
+ *
+ * @return
+ * uint16_t bitwidth.
+ */
+__rte_experimental
+uint16_t rte_get_max_simd_bitwidth(void);
+
+/**
+ * Set the supported SIMD bitwidth.
+ * This API should only be called once at initialization, before EAL init.
+ *
+ * @param bitwidth
+ * uint16_t bitwidth.
+ * @return
+ * 0 on success.
+ * @return
+ * -EINVAL on invalid bitwidth parameter.
+ * @return
+ * -EPERM if bitwidth is locked.
+ */
+__rte_experimental
+int rte_set_max_simd_bitwidth(uint16_t bitwidth);
+
/**
* Request iopl privilege for all RPL.
*
@@ -397,6 +397,10 @@ EXPERIMENTAL {
rte_service_lcore_may_be_active;
rte_thread_register;
rte_thread_unregister;
+
+ # added in 20.11
+ rte_get_max_simd_bitwidth;
+ rte_set_max_simd_bitwidth;
};
INTERNAL {