From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 884CCA057C; Fri, 27 Mar 2020 16:19:56 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 745A81C229; Fri, 27 Mar 2020 16:19:55 +0100 (CET) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by dpdk.org (Postfix) with ESMTP id 9DC881C226 for ; Fri, 27 Mar 2020 16:19:53 +0100 (CET) Received: from compute7.internal (compute7.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 45A955C0493; Fri, 27 Mar 2020 11:19:52 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute7.internal (MEProxy); Fri, 27 Mar 2020 11:19:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=jzUkDHjsqT+kwNwnimJgkO1pyH5DpjIUEhQawJeoNsw=; b=gpCI/7E44ulz 2txo5m4xiSXFf+nsueyd3gd6R6cIH128B7EKfN3DTwo4sxTTXTRBPxkIFTrtT2GH D+DvglqHO6a/4xHEnW+gSAxtcnReyIW/255dUyVKm8Vfy8qZ3qQqUPIBmmFZ6v8o txnC9fNmG5kJcqg5d+H79qIny9ZHfnc= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=jzUkDHjsqT+kwNwnimJgkO1pyH5DpjIUEhQawJeoN sw=; b=AUMvXopaeH+BkLSNVrGrl+mAyhzv9Gyzoia3dTzOZFqOQXp98qWvIF0KU F87d6yUqfSP9TSCAEs4mA/oyN50mjkXl1P/GOGDerZluITEdPmlZP3jiuQmSqP28 UOfpk2I7p9LwaOLJpCxo6LdSx8+b1C9I5NHFOBF8M+evMnyPEJia5Wy2oPGZHi4l BKyuJZw62N3Q2SmOSVQa0eMWWUqd+Q8zHgyiVRKqXFGcg9q121yQeO9PG9ksZFAB Dz8e3ITyMFXeCPXvAmBSFbiVKNrerT2uCpu4VS2y155LbHOMRk2FsHTgHRSWGSRW aoQR9KWS5EVzn2xKcCx5AZLD4rcUw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedugedrudehledgjedvucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecuff homhgrihhnpehtrhgrvhhishdqtghirdgtohhmnecukfhppeejjedrudefgedrvddtfedr udekgeenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpe hthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 686043280067; Fri, 27 Mar 2020 11:19:50 -0400 (EDT) From: Thomas Monjalon To: "Van Haaren, Harry" , Neil Horman , Ray Kinsella , David Marchand , "Laatz, Kevin" Cc: Dodji Seketeli , dev , "Richardson, Bruce" , Honnappa Nagarahalli Date: Fri, 27 Mar 2020 16:19:49 +0100 Message-ID: <2921584.TQGk6oTFT5@xps> In-Reply-To: References: <20200324114921.7184-1-kevin.laatz@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2] eal/cpuflags: add x86 based cpu flags X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 27/03/2020 15:36, Ray Kinsella: > On 27/03/2020 14:32, Van Haaren, Harry wrote: > > From: Thomas Monjalon > >> 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 > >> 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. > > > > +1 to Harrry's comments. > I can't immediately see how this might break the ABI either. So we all agree that increasing the range of valid rte_cpu_flag_t values is OK for ABI compatibility. This conclusion must be written as a libabigail exception in this patch.