DPDK patches and discussions
 help / color / Atom feed
From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
To: Thomas Monjalon <thomas@monjalon.net>,
	Neil Horman <nhorman@tuxdriver.com>,
	Dodji Seketeli <dodji@redhat.com>,
	"mdr@ashroe.eu" <mdr@ashroe.eu>
Cc: David Marchand <david.marchand@redhat.com>,
	"Laatz, Kevin" <kevin.laatz@intel.com>, dev <dev@dpdk.org>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
Subject: Re: [dpdk-dev] [PATCH v2] eal/cpuflags: add x86 based cpu flags
Date: Fri, 27 Mar 2020 14:32:51 +0000
Message-ID: <BYAPR11MB3143EF670B2F9FDA4A738BEDD7CC0@BYAPR11MB3143.namprd11.prod.outlook.com> (raw)
In-Reply-To: <1849712.2IRrRt1zHL@xps>

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Friday, March 27, 2020 2:16 PM
> To: Neil Horman <nhorman@tuxdriver.com>; Dodji Seketeli <dodji@redhat.com>;
> mdr@ashroe.eu
> Cc: David Marchand <david.marchand@redhat.com>; Laatz, Kevin
> <kevin.laatz@intel.com>; dev <dev@dpdk.org>; Richardson, Bruce
> <bruce.richardson@intel.com>; Van Haaren, Harry <harry.van.haaren@intel.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v2] eal/cpuflags: add x86 based cpu flags
> 
> 27/03/2020 14:44, Neil Horman:
> > On Fri, Mar 27, 2020 at 01:24:12PM +0100, David Marchand wrote:
> > > On Wed, Mar 25, 2020 at 12:11 PM Kevin Laatz <kevin.laatz@intel.com>
> wrote:
> > > > --- a/lib/librte_eal/common/include/arch/x86/rte_cpuflags.h
> > > > +++ b/lib/librte_eal/common/include/arch/x86/rte_cpuflags.h
> > > > @@ -113,6 +113,24 @@ enum rte_cpu_flag_t {
> > > >         /* (EAX 80000007h) EDX features */
> > > >         RTE_CPUFLAG_INVTSC,                 /**< INVTSC */
> > > >
> > > > +       RTE_CPUFLAG_AVX512DQ,               /**< AVX512 Doubleword and
> Quadword */
> > > > +       RTE_CPUFLAG_AVX512IFMA,             /**< AVX512 Integer Fused
> Multiply-Add */
> > > > +       RTE_CPUFLAG_AVX512CD,               /**< AVX512 Conflict
> Detection*/
> > > > +       RTE_CPUFLAG_AVX512BW,               /**< AVX512 Byte and Word */
> > > > +       RTE_CPUFLAG_AVX512VL,               /**< AVX512 Vector Length */
> > > > +       RTE_CPUFLAG_AVX512VBMI,             /**< AVX512 Vector Bit
> Manipulation */
> > > > +       RTE_CPUFLAG_AVX512VBMI2,            /**< AVX512 Vector Bit
> Manipulation 2 */
> > > > +       RTE_CPUFLAG_GFNI,                   /**< Galois Field New
> Instructions */
> > > > +       RTE_CPUFLAG_VAES,                   /**< Vector AES */
> > > > +       RTE_CPUFLAG_VPCLMULQDQ,             /**< Vector Carry-less
> Multiply */
> > > > +       RTE_CPUFLAG_AVX512VNNI,             /**< AVX512 Vector Neural
> Network Instructions */
> > > > +       RTE_CPUFLAG_AVX512BITALG,           /**< AVX512 Bit Algorithms
> */
> > > > +       RTE_CPUFLAG_AVX512VPOPCNTDQ,        /**< AVX512 Vector Popcount
> */
> > > > +       RTE_CPUFLAG_CLDEMOTE,               /**< Cache Line Demote */
> > > > +       RTE_CPUFLAG_MOVDIRI,                /**< Direct Store
> Instructions */
> > > > +       RTE_CPUFLAG_MOVDIR64B,              /**< Direct Store
> Instructions 64B */
> > > > +       RTE_CPUFLAG_AVX512VP2INTERSECT,     /**< AVX512 Two Register
> Intersection */
> > > > +
> > > >         /* The last item */
> > > >         RTE_CPUFLAG_NUMFLAGS,               /**< This should always be
> the last! */
> > >
> > > This is seen as an ABI break because of the change on _NUMFLAGS:
> > > https://travis-ci.com/github/ovsrobot/dpdk/jobs/302524264#L2351
> > >
> > It shouldn't be, as the only API calls we expose that use rte_cpu_flag_t
> accept
> > it as an integer parameter to see if the flag is enabled.  Theres no use of
> the
> > enum in a public array or any struct that is sized based on the number of
> flags,
> > so you should be good to go
> 
> Indeed I cannot imagine an ABI incompatibility in this case.
> The only behaviour change is to accept new (higher) RTE_CPUFLAG values
> in functions rte_cpu_get_flag_enabled() and rte_cpu_get_flag_name().
> Is changing the range of valid values an ABI break?
> Why is it flagged by libabigail?

If this enum _MAX value was used by the application to allocate an array, that later our DPDK code would write to it could cause out-of-bounds array accesses of the application supplied array. Abigail doesn't know what applications could use the value for, so it flags it.

IMO Abigail is right to flag it to us - a manual review to understand what that _MAX enum value is used for, and then decide on a case by case basis seems the best way forward to me.

Thanks Neil/Thomas for reviewing, as reply in this thread, I also believe this is not going to break ABI.

  reply index

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-24 11:49 [dpdk-dev] [PATCH 00/17] Add CPU flags Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 01/17] eal/cpuflags: add avx512 doubleword and quadword Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 02/17] eal/cpuflags: add avx512 integer fused multiply-add Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 03/17] eal/cpuflags: add avx512 conflict detection Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 04/17] eal/cpuflags: add avx512 byte and word Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 05/17] eal/cpuflags: add avx512 vector length Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 06/17] eal/cpuflags: add avx512 vector bit manipulation Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 07/17] eal/cpuflags: add avx512 vector bit manipulation 2 Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 08/17] eal/cpuflags: add galois field new instructions Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 09/17] eal/cpuflags: add vector AES Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 10/17] eal/cpuflags: add vector carry-less multiply Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 11/17] eal/cpuflags: add avx512 vector neural network instructions Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 12/17] eal/cpuflags: add avx512 bit algorithms Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 13/17] eal/cpuflags: add avx512 vector popcount Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 14/17] eal/cpuflags: add cache line demote Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 15/17] eal/cpuflags: add direct store instructions Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 16/17] eal/cpuflags: add direct store instructions 64B Kevin Laatz
2020-03-24 11:49 ` [dpdk-dev] [PATCH 17/17] eal/cpuflags: add avx512 two register intersection Kevin Laatz
2020-03-25  8:36 ` [dpdk-dev] [PATCH 00/17] Add CPU flags David Marchand
2020-03-25 11:10 ` [dpdk-dev] [PATCH v2] eal/cpuflags: add x86 based cpu flags Kevin Laatz
2020-03-27 12:24   ` David Marchand
2020-03-27 13:18     ` Van Haaren, Harry
2020-03-27 15:04       ` David Marchand
2020-03-27 13:44     ` Neil Horman
2020-03-27 14:15       ` Thomas Monjalon
2020-03-27 14:32         ` Van Haaren, Harry [this message]
2020-03-27 14:36           ` Ray Kinsella
2020-03-27 15:19             ` Thomas Monjalon
2020-03-30 12:15   ` [dpdk-dev] [PATCH v3] " Kevin Laatz
2020-04-16 10:08     ` Van Haaren, Harry
2020-04-16 11:00     ` [dpdk-dev] [PATCH v4] " Kevin Laatz
2020-04-25 16:04       ` Thomas Monjalon
2020-04-27  9:22         ` Kinsella, Ray
2020-04-27  9:27         ` Ray Kinsella
2020-04-27  9:31           ` Laatz, Kevin
2020-04-27  9:35             ` Ray Kinsella
2020-04-27 10:08               ` Laatz, Kevin
2020-04-27 12:31           ` Thomas Monjalon
2020-04-27 13:58             ` Ray Kinsella
2020-04-29 11:22               ` Neil Horman
2020-04-30  7:59                 ` Ray Kinsella
2020-04-28 12:40       ` [dpdk-dev] [PATCH v5] " Kevin Laatz
2020-04-28 16:39         ` Ray Kinsella
2020-04-28 18:11           ` Laatz, Kevin
2020-04-28 19:55             ` Thomas Monjalon
2020-04-28 19:58           ` Stephen Hemminger
2020-04-29 11:39           ` David Marchand
2020-04-30 10:02             ` Ray Kinsella
2020-05-07 13:00               ` Thomas Monjalon

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=BYAPR11MB3143EF670B2F9FDA4A738BEDD7CC0@BYAPR11MB3143.namprd11.prod.outlook.com \
    --to=harry.van.haaren@intel.com \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=dodji@redhat.com \
    --cc=kevin.laatz@intel.com \
    --cc=mdr@ashroe.eu \
    --cc=nhorman@tuxdriver.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox