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.m...@6wind.com>
> >Sent: Tuesday 6 October 2020 10:32
> >To: Power, Ciara <ciara.po...@intel.com>
> >Cc: dev@dpdk.org; Ray Kinsella <m...@ashroe.eu>; Neil Horman
> ><nhor...@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.po...@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?
