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 378F745547; Tue, 2 Jul 2024 10:29:18 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 281A540A8B; Tue, 2 Jul 2024 10:29:18 +0200 (CEST) Received: from fout6-smtp.messagingengine.com (fout6-smtp.messagingengine.com [103.168.172.149]) by mails.dpdk.org (Postfix) with ESMTP id 944F4402AD for ; Tue, 2 Jul 2024 10:29:16 +0200 (CEST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailfout.nyi.internal (Postfix) with ESMTP id 059391380491; Tue, 2 Jul 2024 04:29:16 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Tue, 02 Jul 2024 04:29:16 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1719908956; x=1719995356; bh=zYD26Ra6ls+pt1vZk94vGYj+XecPHEC9b82KGl8lQEc=; b= YUPnxjKawMKX6Sb6h9AXEgE8XBEeCvn4+fVplJNaNOhyBP97lPGxxnDYqgZtS+DC noe1c0vv/G4hrqDM33v2AVW3rJHch1+krv6d+CemAjZiwR7lHwRwp8fkTIUragEg otNptJgEIeeHv2DLRcIiKPaAs8DOOjSzIvv6uVjX/QrBGZEyBTJzMAiqMSpAFp7Q +troojcZdMvGrwM8kt/ycyvlpunmAjlCIcb2GpRKzwtRPqZAQ5lGj9lYYZR17PAn Pr5AegC/cBaWOPnvd0QZB5Lp6B5Uj81KHZeOhNmJwI5pWHjeiHdTCTjObdzJ/CkW OHct90atCxEFFasKd1aQug== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1719908956; x= 1719995356; bh=zYD26Ra6ls+pt1vZk94vGYj+XecPHEC9b82KGl8lQEc=; b=c GCKuyyC19T++wkJle8EKv9aneIWKhOPaj1xpf28xaJ6y90gNw0YS/qTnYU5i2nUg jRUDz0oAk/Yak8t7HIl0qXZP1mRCl8P3Zoenywe5edhBo4yGkptOuyD21vrTm6sh LzGnMx3NvzfNSSI+x/pjoqV9UwHocbvESQOva/1xiLxgrWvP+1eLRZE5QqzA3NNj 5KwsYY+WXYQUmb5z/lnMjxeo0FhC4koanJURgzua5OpflwZZDCcEOzANgKQ6QLtM tZ/B9HUZR23/gqCQvgyPLiLghsVPBjo2uDDu3VlZSzJObQrK1Gp42/vML4qH9BrV 0bjGyxOsMo1cL6+dWPLaA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrudehgddtvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvfevufffkfgjfhgggfgtsehtufertddttdejnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepjeduveehieevuddutdevfffgtdegkeeuveejffejgedtgeegkefg vdeugfefkeejnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 2 Jul 2024 04:29:13 -0400 (EDT) From: Thomas Monjalon To: Wathsala Wathawana Vithanage Cc: Tyler Retzlaff , Ruifeng Wang , "dev@dpdk.org" , nd , Dhruv Tripathi , Honnappa Nagarahalli , Jack Bond-Preston , Nick Connolly , Vinod Krishna , "david.marchand@redhat.com" , nd Subject: Re: [PATCH v2 2/2] eal: add Arm WFET in power management intrinsics Date: Tue, 02 Jul 2024 10:29:09 +0200 Message-ID: <859956985.pAmPZ1aGmH@thomas> In-Reply-To: References: <20240604044401.3577707-2-wathsala.vithanage@arm.com> <6442594.6TB3vktIsb@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="utf-8" 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 01/07/2024 23:34, Wathsala Wathawana Vithanage: > > 19/06/2024 08:45, Wathsala Vithanage: > > > rte_cpu_get_intrinsics_support(struct rte_cpu_intrinsics *intrinsics) > > > { > > > memset(intrinsics, 0, sizeof(*intrinsics)); -#ifdef RTE_ARM_USE_WFE > > > intrinsics->power_monitor = 1; > > > -#endif > > > > Why removing this #ifdef? > > WFE is available in all the Arm platforms DPDK currently supports. Therefore, an explicit flag is not > required at build time. > > The purpose of RTE_ARM_USE_WFE seems to be controlling the use of WFE in rte_wait_until_equal > functions by defining RTE_WAIT_UNTIL_EQUAL_ARCH_DEFINED macro only when RTE_ARM_USE_WFE > is defined. (eal/arm/include/rte_pause_64.h:15) > From what I'm hearing this was done due to a performance issue rte_wait_until_equal_X functions had > when using WFE. > However, that design is flawed because it made other users of WFE dependent on the choice of > the use of WFE in rte_wait_until_equal functions as __RTE_ARM_WFE was wrapped in an > RTE_WAIT_UNTIL_EQUAL_ARCH_DEFINED #ifdef block. > This patch fixes this issue by moving __RTE_ARM_WFE out of RTE_WAIT_UNTIL_EQUAL_ARCH_DEFINED > block. > > Perhaps we should change RTE_ARM_USE_WFE to something like > RTE_ARM_USE_WFE_IN_WAIT_UNTIL_EQUAL ? Yes perhaps. And more importantly, you should do such change in a separate patch. Don't hide it in WFET patch. > > > +uint8_t wfet_en; > > > > It should be made static probably. > > This variable will be unused in some cases, needs #ifdef. > > > > This variable is used in all cases. It's 1 when WFET is available, 0 when it's not. It would be 0 if you make it static yes. > > > + > > > +RTE_INIT(rte_power_intrinsics_init) > > > +{ > > > +#ifdef RTE_ARCH_64 > > > + if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_WFXT)) > > > + wfet_en = 1; > > > +#endif > > > +} > > > + > > > +/** > > > + * This function uses WFE/WFET instruction to make lcore suspend > > > * execution on ARM. > > > - * Note that timestamp based timeout is not supported yet. > > > */ > > > int > > > rte_power_monitor(const struct rte_power_monitor_cond *pmc, > > > const uint64_t tsc_timestamp) > > > { > > > - RTE_SET_USED(tsc_timestamp); > > > - > > > -#ifdef RTE_ARM_USE_WFE > > > +#ifdef RTE_ARCH_64 > > > > It looks wrong. > > If RTE_ARM_USE_WFE is disabled, you should not call __RTE_ARM_WFE(). > > > > RTE_ARM_USE_WFE should be renamed to reflect its actual use. It's safe to assume that > WFE is available universally in Arm DPDK context.