From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 58769A0C45; Thu, 28 Oct 2021 09:16:18 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3BEE24067B; Thu, 28 Oct 2021 09:16:18 +0200 (CEST) Received: from mail-io1-f42.google.com (mail-io1-f42.google.com [209.85.166.42]) by mails.dpdk.org (Postfix) with ESMTP id BAECB4003F for ; Thu, 28 Oct 2021 09:16:16 +0200 (CEST) Received: by mail-io1-f42.google.com with SMTP id p8so328823iod.8 for ; Thu, 28 Oct 2021 00:16:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=v/cGOvVXmvXGzWwLcrrArimVpM+TgaMrexqttWTbV8Y=; b=Vn13dyKFpW0quORfwIoWw1BcTtdj5sHCTlXTJhXsopDTMFhrQtwV/R1Bxid66Bl2kT hv1Nolq8Rno9jqDRtJXyspD2EY9XhXUHDsB4xxQlECU0gbdJareGh5Gv9GPOpSD5rmL2 qOVjDLi0R2t50431SLn7AL2bqpITS180kQGsZZvi+ILDq5JFZcHXupqWLSV/fS5x9wQW frcPU/1u0rUiRhyAi4Cx1BU9Qo9JPd/uUKo7aOx9dH5yKj36E8TeQ1pth1g5LkJKKpKM fb2NNd5KXfEVOk7R+ywmPXJduX9yi0w5r+8XSgklNN85LsR4MuOgHGtQCehrvNoh1adv yCzw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=v/cGOvVXmvXGzWwLcrrArimVpM+TgaMrexqttWTbV8Y=; b=oMbBLGy2yvR7kxQT3SoG6GqDSpDp7FvnkMnhHfSVe/NswBE+uSBK/FCjCiL6EQbaFm EdeoGNC6CYQN/d+4Ky9EFB3TaPxUnVLwcIxbts92NrR79aSHKr9oH+3LtrJpWa5sfhYg DcFkPU0yazMggeMQLeZRkDvMjH/v99ClEJatJGuV67JRe5F1YwfckJr+CE4Tw12FM2dj S++zRqnFZfZCYO9zwheyw6A7r9Jt4DMfQPFZj+jccKe6FnUbjeZYmY624dwffTeWq6fz 3ev4PTzIkQd8cqJcI0f71xzUxgMpq6yIqHcy0Ya7JNndrtHPLvKnqx9+bV1Z2V3AjUDe CerQ== X-Gm-Message-State: AOAM531CFjfMiY2Z82FcyeP/psP1opMtLCBrr/FEjcANxp0FD4KBb9Ce PIeW28ioWC/e2im5SU09Hho4kRDhWo2TLDOgc8E= X-Google-Smtp-Source: ABdhPJzThAUQb7H7ZmWoC2X6X1HPwTZyJ3nDvDmWNSRFLn93yql4LTCisSvdYMZ28uVdcSTaQuwd4hK6Fse53fXsGDM= X-Received: by 2002:a05:6602:14d2:: with SMTP id b18mr1753118iow.123.1635405375998; Thu, 28 Oct 2021 00:16:15 -0700 (PDT) MIME-Version: 1.0 References: <20210902053253.3017858-1-feifei.wang2@arm.com> <20211028065640.139655-1-feifei.wang2@arm.com> <20211028065640.139655-2-feifei.wang2@arm.com> In-Reply-To: <20211028065640.139655-2-feifei.wang2@arm.com> From: Jerin Jacob Date: Thu, 28 Oct 2021 12:45:49 +0530 Message-ID: To: Feifei Wang Cc: Ruifeng Wang , dpdk-dev , nd , "Ananyev, Konstantin" , Stephen Hemminger , David Marchand , Thomas Monjalon , =?UTF-8?Q?Mattias_R=C3=B6nnblom?= Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v7 1/5] eal: add new definitions for wait scheme X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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" On Thu, Oct 28, 2021 at 12:26 PM Feifei Wang wrote: > > Introduce macros as generic interface for address monitoring. > For different size, encapsulate '__LOAD_EXC_16', '__LOAD_EXC_32' > and '__LOAD_EXC_64' into a new macro '__LOAD_EXC'. > > Furthermore, to prevent compilation warning in arm: > ---------------------------------------------- > 'warning: implicit declaration of function ...' > ---------------------------------------------- > Delete 'undef' constructions for '__LOAD_EXC_xx', '__SEVL' and '__WFE'. > And add =E2=80=98__RTE_ARM=E2=80=99 for these macros to fix the namespace= . > > This is because original macros are undefine at the end of the file. > If new macro 'rte_wait_event' calls them in other files, they will be > seen as 'not defined'. > > Signed-off-by: Feifei Wang > Reviewed-by: Ruifeng Wang > --- > +static __rte_always_inline void > +rte_wait_until_equal_16(volatile uint16_t *addr, uint16_t expected, > + int memorder) > +{ > + uint16_t value; > + > + assert(memorder =3D=3D __ATOMIC_ACQUIRE || memorder =3D=3D __ATOM= IC_RELAXED); Assert is not good in the library, Why not RTE_BUILD_BUG_ON here > + > + __RTE_ARM_LOAD_EXC_16(addr, value, memorder) > if (value !=3D expected) { > - __SEVL() > + __RTE_ARM_SEVL() > do { > - __WFE() > - __LOAD_EXC_16(addr, value, memorder) > + __RTE_ARM_WFE() > + __RTE_ARM_LOAD_EXC_16(addr, value, memorder) > } while (value !=3D expected); > } > -#undef __LOAD_EXC_16 > } > > static __rte_always_inline void > @@ -77,34 +124,14 @@ rte_wait_until_equal_32(volatile uint32_t *addr, uin= t32_t expected, > > assert(memorder =3D=3D __ATOMIC_ACQUIRE || memorder =3D=3D __ATOM= IC_RELAXED); > > - /* > - * Atomic exclusive load from addr, it returns the 32-bit content= of > - * *addr while making it 'monitored',when it is written by someon= e > - * else, the 'monitored' state is cleared and a event is generate= d > - * implicitly to exit WFE. > - */ > -#define __LOAD_EXC_32(src, dst, memorder) { \ > - if (memorder =3D=3D __ATOMIC_RELAXED) { \ > - asm volatile("ldxr %w[tmp], [%x[addr]]" \ > - : [tmp] "=3D&r" (dst) \ > - : [addr] "r"(src) \ > - : "memory"); \ > - } else { \ > - asm volatile("ldaxr %w[tmp], [%x[addr]]" \ > - : [tmp] "=3D&r" (dst) \ > - : [addr] "r"(src) \ > - : "memory"); \ > - } } > - > - __LOAD_EXC_32(addr, value, memorder) > + __RTE_ARM_LOAD_EXC_32(addr, value, memorder) > if (value !=3D expected) { > - __SEVL() > + __RTE_ARM_SEVL() > do { > - __WFE() > - __LOAD_EXC_32(addr, value, memorder) > + __RTE_ARM_WFE() > + __RTE_ARM_LOAD_EXC_32(addr, value, memorder) > } while (value !=3D expected); > } > -#undef __LOAD_EXC_32 > } > > static __rte_always_inline void > @@ -115,38 +142,33 @@ rte_wait_until_equal_64(volatile uint64_t *addr, ui= nt64_t expected, > > assert(memorder =3D=3D __ATOMIC_ACQUIRE || memorder =3D=3D __ATOM= IC_RELAXED); remove assert and change to BUILD_BUG_ON > > - /* > - * Atomic exclusive load from addr, it returns the 64-bit content= of > - * *addr while making it 'monitored',when it is written by someon= e > - * else, the 'monitored' state is cleared and a event is generate= d > - * implicitly to exit WFE. > - */ > -#define __LOAD_EXC_64(src, dst, memorder) { \ > - if (memorder =3D=3D __ATOMIC_RELAXED) { \ > - asm volatile("ldxr %x[tmp], [%x[addr]]" \ > - : [tmp] "=3D&r" (dst) \ > - : [addr] "r"(src) \ > - : "memory"); \ > - } else { \ > - asm volatile("ldaxr %x[tmp], [%x[addr]]" \ > - : [tmp] "=3D&r" (dst) \ > - : [addr] "r"(src) \ > - : "memory"); \ > - } } > - > - __LOAD_EXC_64(addr, value, memorder) > + __RTE_ARM_LOAD_EXC_64(addr, value, memorder) > if (value !=3D expected) { > - __SEVL() > + __RTE_ARM_SEVL() > do { > - __WFE() > - __LOAD_EXC_64(addr, value, memorder) > + __RTE_ARM_WFE() > + __RTE_ARM_LOAD_EXC_64(addr, value, memorder) > } while (value !=3D expected); > } > } > -#undef __LOAD_EXC_64 > > -#undef __SEVL > -#undef __WFE > +#define rte_wait_event(addr, mask, cond, expected, memorder) = \ > +do { = \ > + RTE_BUILD_BUG_ON(!__builtin_constant_p(memorder)); = \ > + RTE_BUILD_BUG_ON(memorder !=3D __ATOMIC_ACQUIRE && = \ > + memorder !=3D __ATOMIC_RELAXED); = \ > + uint32_t size =3D sizeof(*(addr)) << 3; Add const > + typeof(*(addr)) expected_value =3D (expected); = \ > + typeof(*(addr)) value =3D 0; Why zero assignment \ > + __RTE_ARM_LOAD_EXC((addr), value, memorder, size) = \ Assert is not good in the library, Why not RTE_BUILD_BUG_ON here > + if ((value & (mask)) cond expected_value) { = \ > + __RTE_ARM_SEVL() = \ > + do { = \ > + __RTE_ARM_WFE() = \ > + __RTE_ARM_LOAD_EXC((addr), value, memorder, size)= \ if the address is the type of __int128_t. This logic will fail? Could you add 128bit support too and remove the assert from __RTE_ARM_LOAD_EXC > + } while ((value & (mask)) cond expected_value); = \ > + } = \ > +} while (0) > > #endif > > diff --git a/lib/eal/include/generic/rte_pause.h b/lib/eal/include/generi= c/rte_pause.h > index 668ee4a184..d0c5b5a415 100644 > --- a/lib/eal/include/generic/rte_pause.h > +++ b/lib/eal/include/generic/rte_pause.h > @@ -111,6 +111,34 @@ rte_wait_until_equal_64(volatile uint64_t *addr, uin= t64_t expected, > while (__atomic_load_n(addr, memorder) !=3D expected) > rte_pause(); > } > + > +/* > + * Wait until *addr breaks the condition, with a relaxed memory > + * ordering model meaning the loads around this API can be reordered. > + * > + * @param addr > + * A pointer to the memory location. > + * @param mask > + * A mask of value bits in interest. > + * @param cond > + * A symbol representing the condition. > + * @param expected > + * An expected value to be in the memory location. > + * @param memorder > + * Two different memory orders that can be specified: > + * __ATOMIC_ACQUIRE and __ATOMIC_RELAXED. These map to > + * C++11 memory orders with the same names, see the C++11 standard or > + * the GCC wiki on atomic synchronization for detailed definition. > + */ > +#define rte_wait_event(addr, mask, cond, expected, memorder) = \ > +do { = \ > + RTE_BUILD_BUG_ON(!__builtin_constant_p(memorder)); = \ > + RTE_BUILD_BUG_ON(memorder !=3D __ATOMIC_ACQUIRE && = \ > + memorder !=3D __ATOMIC_RELAXED); = \ > + typeof(*(addr)) expected_value =3D (expected); = \ > + while ((__atomic_load_n((addr), (memorder)) & (mask)) cond expect= ed_value) \ > + rte_pause(); = \ > +} while (0) > #endif > > #endif /* _RTE_PAUSE_H_ */ > -- > 2.25.1 >