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 8A5A5A04BC; Fri, 9 Oct 2020 07:43:14 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 903171BF33; Fri, 9 Oct 2020 07:43:12 +0200 (CEST) Received: from mail-il1-f196.google.com (mail-il1-f196.google.com [209.85.166.196]) by dpdk.org (Postfix) with ESMTP id 288A91BF1C for ; Fri, 9 Oct 2020 07:43:10 +0200 (CEST) Received: by mail-il1-f196.google.com with SMTP id t18so8111898ilo.12 for ; Thu, 08 Oct 2020 22:43:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=s4sgJ1MpAJTgAzZI5ZLMnXLRXG4Z/GsxWD2ehhis6Pk=; b=gZ7EmndELj6GSlP6CLLHmsV3MWNNrsKigZzUROv1YWlUIUx9iiWUVxGtjmwlO7ez17 4k3AHeJeWfJD7FrvFAi9esT4na11WAdnD44fvpLpYtFRJ/zgHI1b3zF7svTqyEXoINE6 NeDaldRFnDGfiNuslCA5ZSNr6V1uIuetGnRlIRIauHzpcX5KppS/5/+OfDTspdW2qesS mSw+4DasBqEXKkxzEkkIStJEUH+pTGHiV6c7c8adYcXdUpmg23pQt07cCP+lXEbmWZuZ 2wDhJGuSFqeTvFECSLCjhimPllj8a0PDaUE9R1+Y2PLjQb2m5cEGOTyonobpgLtczmlO P7dw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=s4sgJ1MpAJTgAzZI5ZLMnXLRXG4Z/GsxWD2ehhis6Pk=; b=o7/ZM7oxUQxQNUlzR2im6OUxUDbcRO/eSGXzNw8UioILxxvZ6HUvLydLHf0u3NeXE+ T2PaPReXMTQbna5beqGSxiqqj8Drd+XUmxsEY50M9fee1Aj9w1g4/yP1ndE1nceYpeV1 tBrLTgROgHOhAfNj4CMeKPbgr42ftkyyWFe4fCGgHLYURe7+Ik2kOx7xCJX+0bBvdP4E 3bG0e2mplypokZe/fVCOfd8U9owDz9TEihLANnholndoTKnwYQri3M2mVgfwHNoLI7Tw 4Wf9hdXGqqUHu8ASENZ/mJuzL+zJbrjGD6HE/C7MM5bfuyga6gP4Vwk3AonAOyvDDJaJ 5AJg== X-Gm-Message-State: AOAM532sn3+YVcHwcBdw2Z/9NzFhw2Wizbv1RifvSyLM7zOcKaaQL8rC zKz7KcSY1PblefDEjh+F3YJKEWbRsC5snn5DL6Y= X-Google-Smtp-Source: ABdhPJxVPd05iGo63zhQVRbLfR/mrYlmBsztg0RJKYcDJ9754ye5O/ak0oLURl8XPr84gyUcZc1icFV7abp23FW9nqk= X-Received: by 2002:a92:1e02:: with SMTP id e2mr7728129ile.294.1602222188412; Thu, 08 Oct 2020 22:43:08 -0700 (PDT) MIME-Version: 1.0 References: <1599214740-3927-1-git-send-email-liang.j.ma@intel.com> <1601647919-25312-1-git-send-email-liang.j.ma@intel.com> <1601647919-25312-2-git-send-email-liang.j.ma@intel.com> <16022545.g3EcnA0i2J@thomas> <1615f7c8-d0bf-bdac-141e-422497a593e2@intel.com> In-Reply-To: From: Jerin Jacob Date: Fri, 9 Oct 2020 11:12:52 +0530 Message-ID: To: "Ananyev, Konstantin" , David Marchand Cc: "Burakov, Anatoly" , Thomas Monjalon , "Ma, Liang J" , dpdk-dev , "Hunt, David" , Stephen Hemminger , Honnappa Nagarahalli , "Ruifeng Wang (Arm Technology China)" , David Christensen , Jerin Jacob Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v4 02/10] eal: add power management intrinsics 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" On Thu, Oct 8, 2020 at 10:38 PM Ananyev, Konstantin wrote: > > > > > On Thu, Oct 8, 2020 at 6:57 PM Burakov, Anatoly > > wrote: > > > > > > On 08-Oct-20 9:44 AM, Jerin Jacob wrote: > > > > On Thu, Oct 8, 2020 at 2:04 PM Thomas Monjalon wrote: > > > >> > > > >>> Add two new power management intrinsics, and provide an implementation > > > >>> in eal/x86 based on UMONITOR/UMWAIT instructions. The instructions > > > >>> are implemented as raw byte opcodes because there is not yet widespread > > > >>> compiler support for these instructions. > > > >>> > > > >>> The power management instructions provide an architecture-specific > > > >>> function to either wait until a specified TSC timestamp is reached, or > > > >>> optionally wait until either a TSC timestamp is reached or a memory > > > >>> location is written to. The monitor function also provides an optional > > > >>> comparison, to avoid sleeping when the expected write has already > > > >>> happened, and no more writes are expected. > > > >>> > > > >>> For more details, Please reference Intel SDM Volume 2. > > > >> > > > >> I really would like to see feedbacks from other arch maintainers. > > > >> Unfortunately they were not Cc'ed. > > > > > > > > Shared the feedback from the arm64 perspective here. Yet to get a reply on this. > > > > http://mails.dpdk.org/archives/dev/2020-September/181646.html > > > > > > > >> Also please mark the new functions as experimental. > > > >> > > > >> > > > > > > Hi Jerin, > > > > Hi Anatoly, > > > > > > > > > IMO, We must introduce some arch feature-capability _get_ scheme to tell > > > > the consumer of this API is only supported on x86. Probably as > > > functions[1] > > > > or macro flags scheme and have a stub for the other architectures as the > > > > API marked as generic ie rte_power_* not rte_x86_.. > > > > > > > > This will help the consumer to create workers based on the > > > instruction features > > > > which can NOT be abstracted as a generic feature across the > > > architectures. > > > > > > I'm not entirely sure what you mean by that. > > > > > > I mean, yes, we should have added stubs for other architectures, and we > > > will add those in future revisions, but what does your proposed runtime > > > check accomplish that cannot currently be done with CPUID flags? > > > > > > RTE_CPUFLAG_WAITPKG flag definition is not available in other architectures. > > i.e RTE_CPUFLAG_WAITPKG defined in lib/librte_eal/x86/include/rte_cpuflags.h > > and it is used in http://patches.dpdk.org/patch/79540/ as generic API. > > I doubt http://patches.dpdk.org/patch/79540/ would compile on non-x86. > > > I am agree with Jerin, that we need some generic way to > figure-out does platform supports power_monitor() or not. > Though not sure do we need to create a new feature-get framework here... That's works too. Some means of generic probing is fine. Following schemed needs more documentation on that usage, as, it is not straight forward compare to feature-get framework. Also, on the other thread, we are adding the new instructions like demote cacheline etc, maybe if the user wants to KNOW if the arch supports it then the feature-get framework is good. If we think, there is no other usecase for generic arch feature-get framework then we can keep the below scheme else generic arch feature is better for more forward looking use cases. > Might be just something like: > rte_power_monitor(...) == -ENOTSUP > be enough indication for that? > So user can just do: > if (rte_power_monitor(NULL, 0, 0, 0, 0) == -ENOTSUP) { > /* not supported path */ > } > > To check is that feature supported or not. > > > > > > > If you look at patch 1 [1], we added CPUID flags that the user can > > > check, and in fact this is precisely what we do in patch 4 [2] before > > > enabling the UMWAIT path. We could perhaps document this better and > > > outline the dependency on the WAITPKG CPUID flag more explicitly, but > > > otherwise i don't see how what you're proposing isn't already possible > > > to do. > > > > > > [1] http://patches.dpdk.org/patch/79539/ > > > [2] http://patches.dpdk.org/patch/79540/ , function > > > rte_power_pmd_mgmt_queue_enable() > > > > > > -- > > > Thanks, > > > Anatoly