On Thu, Oct 8, 2020 at 6:57 PM Burakov, Anatoly <anatoly.bura...@intel.com> wrote: > > On 08-Oct-20 9:44 AM, Jerin Jacob wrote: > > On Thu, Oct 8, 2020 at 2:04 PM Thomas Monjalon <tho...@monjalon.net> 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. > > 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