From: Ferruh Yigit <ferruh.yigit@amd.com>
To: Konstantin Ananyev <konstantin.ananyev@huawei.com>,
"Tummala, Sivaprasad" <Sivaprasad.Tummala@amd.com>,
Tyler Retzlaff <roretzla@linux.microsoft.com>
Cc: "david.hunt@intel.com" <david.hunt@intel.com>,
"anatoly.burakov@intel.com" <anatoly.burakov@intel.com>,
"david.marchand@redhat.com" <david.marchand@redhat.com>,
"thomas@monjalon.net" <thomas@monjalon.net>,
"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [PATCH v5 3/3] power: amd power monitor support
Date: Fri, 18 Aug 2023 14:25:14 +0100 [thread overview]
Message-ID: <92e7bd2e-b799-9658-c90e-f50638c6fdbd@amd.com> (raw)
In-Reply-To: <ae31349bb79c4e358d4eb45ec4f66835@huawei.com>
On 8/17/2023 3:18 PM, Konstantin Ananyev wrote:
>
>>> Caution: This message originated from an External Source. Use proper caution
>>> when opening attachments, clicking links, or responding.
>>>
>>>
>>> On Wed, Aug 16, 2023 at 11:59:59AM -0700, Sivaprasad Tummala wrote:
>>>> mwaitx allows EPYC processors to enter a implementation dependent
>>>> power/performance optimized state (C1 state) for a specific period or
>>>> until a store to the monitored address range.
>>>>
>>>> Signed-off-by: Sivaprasad Tummala <sivaprasad.tummala@amd.com>
>>>> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>>> ---
>>>> lib/eal/x86/rte_power_intrinsics.c | 77
>>>> +++++++++++++++++++++++++-----
>>>> 1 file changed, 66 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/lib/eal/x86/rte_power_intrinsics.c
>>>> b/lib/eal/x86/rte_power_intrinsics.c
>>>> index 6eb9e50807..b4754e17da 100644
>>>> --- a/lib/eal/x86/rte_power_intrinsics.c
>>>> +++ b/lib/eal/x86/rte_power_intrinsics.c
>>>> @@ -17,6 +17,60 @@ static struct power_wait_status {
>>>> volatile void *monitor_addr; /**< NULL if not currently sleeping
>>>> */ } __rte_cache_aligned wait_status[RTE_MAX_LCORE];
>>>>
>>>> +/**
>>>> + * These functions uses UMONITOR/UMWAIT instructions and will enter C0.2
>>> state.
>>>> + * For more information about usage of these instructions, please
>>>> +refer to
>>>> + * Intel(R) 64 and IA-32 Architectures Software Developer's Manual.
>>>> + */
>>>> +static void intel_umonitor(volatile void *addr) {
>>>> + /* UMONITOR */
>>>> + asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf7;"
>>>> + :
>>>> + : "D"(addr));
>>>> +}
>>>> +
>>>> +static void intel_umwait(const uint64_t timeout) {
>>>> + const uint32_t tsc_l = (uint32_t)timeout;
>>>> + const uint32_t tsc_h = (uint32_t)(timeout >> 32);
>>>> + /* UMWAIT */
>>>> + asm volatile(".byte 0xf2, 0x0f, 0xae, 0xf7;"
>>>> + : /* ignore rflags */
>>>> + : "D"(0), /* enter C0.2 */
>>>> + "a"(tsc_l), "d"(tsc_h)); }
>>>
>>> question and perhaps Anatoly Burakov can chime in with expertise.
>>>
>>> gcc/clang have built-in intrinsics for umonitor and umwait i believe as per our other
>>> thread of discussion is there a benefit to also providing inline assembly over just
>>> using the intrinsics? I understand that the intrinsics may not exist for the monitorx
>>> and mwaitx below so it is probably necessary for amd.
>>>
>>> so the suggestion here is when they are available just use the intrinsics.
>>>
>>> thanks
>>>
>> The gcc built-in functions __builtin_ia32_monitorx()/__builtin_ia32_mwaitx are available only when -mmwaitx
>> is used specific for AMD platforms. On generic builds, these built-ins are not available and hence inline
>> assembly is required here.
>
> Ok... but we can probably put them into a separate .c file that will be compiled with that specific flag?
> Same thing can be probably done for Intel specific instructions.
> In general, I think it is much more preferable to use built-ins vs inline assembly
> (if possible off-course).
>
We don't compile different set of files for AMD and Intel, but there are
runtime checks, so putting into separate file is not much different.
It may be an option to always enable compiler flag (-mmwaitx), I think
it won't hurt other platforms but I am not sure about implications of
this to other platforms (what was the motivation for the compiler guys
to enable these build-ins with specific flag?).
Also this requires detecting compiler that supports 'mmwaitx' or not, etc..
> Konstantin
>
>>
>>>> +
>>>> +/**
>>>> + * These functions uses MONITORX/MWAITX instructions and will enter C1 state.
>>>> + * For more information about usage of these instructions, please
>>>> +refer to
>>>> + * AMD64 Architecture Programmer's Manual.
>>>> + */
>>>> +static void amd_monitorx(volatile void *addr) {
>>>> + /* MONITORX */
>>>> + asm volatile(".byte 0x0f, 0x01, 0xfa;"
>>>> + :
>>>> + : "a"(addr),
>>>> + "c"(0), /* no extensions */
>>>> + "d"(0)); /* no hints */ }
>>>> +
>>>> +static void amd_mwaitx(const uint64_t timeout) {
>>>> + /* MWAITX */
>>>> + asm volatile(".byte 0x0f, 0x01, 0xfb;"
>>>> + : /* ignore rflags */
>>>> + : "a"(0), /* enter C1 */
>>>> + "c"(2), /* enable timer */
>>>> + "b"(timeout));
>>>> +}
>>>> +
>>>> +static struct {
>>>> + void (*mmonitor)(volatile void *addr);
>>>> + void (*mwait)(const uint64_t timeout); } __rte_cache_aligned
>>>> +power_monitor_ops;
>>>> +
>>>> static inline void
>>>> __umwait_wakeup(volatile void *addr)
>>>> {
>>>> @@ -75,8 +129,6 @@ int
>>>> rte_power_monitor(const struct rte_power_monitor_cond *pmc,
>>>> const uint64_t tsc_timestamp) {
>>>> - const uint32_t tsc_l = (uint32_t)tsc_timestamp;
>>>> - const uint32_t tsc_h = (uint32_t)(tsc_timestamp >> 32);
>>>> const unsigned int lcore_id = rte_lcore_id();
>>>> struct power_wait_status *s;
>>>> uint64_t cur_value;
>>>> @@ -109,10 +161,8 @@ rte_power_monitor(const struct
>>> rte_power_monitor_cond *pmc,
>>>> * versions support this instruction natively.
>>>> */
>>>>
>>>> - /* set address for UMONITOR */
>>>> - asm volatile(".byte 0xf3, 0x0f, 0xae, 0xf7;"
>>>> - :
>>>> - : "D"(pmc->addr));
>>>> + /* set address for mmonitor */
>>>> + power_monitor_ops.mmonitor(pmc->addr);
>>>>
>>>> /* now that we've put this address into monitor, we can unlock */
>>>> rte_spinlock_unlock(&s->lock);
>>>> @@ -123,11 +173,8 @@ rte_power_monitor(const struct
>>> rte_power_monitor_cond *pmc,
>>>> if (pmc->fn(cur_value, pmc->opaque) != 0)
>>>> goto end;
>>>>
>>>> - /* execute UMWAIT */
>>>> - asm volatile(".byte 0xf2, 0x0f, 0xae, 0xf7;"
>>>> - : /* ignore rflags */
>>>> - : "D"(0), /* enter C0.2 */
>>>> - "a"(tsc_l), "d"(tsc_h));
>>>> + /* execute mwait */
>>>> + power_monitor_ops.mwait(tsc_timestamp);
>>>>
>>>> end:
>>>> /* erase sleep address */
>>>> @@ -173,6 +220,14 @@ RTE_INIT(rte_power_intrinsics_init) {
>>>> wait_multi_supported = 1;
>>>> if (i.power_monitor)
>>>> monitor_supported = 1;
>>>> +
>>>> + if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_MONITORX)) { /* AMD */
>>>> + power_monitor_ops.mmonitor = &amd_monitorx;
>>>> + power_monitor_ops.mwait = &amd_mwaitx;
>>>> + } else { /* Intel */
>>>> + power_monitor_ops.mmonitor = &intel_umonitor;
>>>> + power_monitor_ops.mwait = &intel_umwait;
>>>> + }
>>>> }
>>>>
>>>> int
>>>> --
>>>> 2.34.1
next prev parent reply other threads:[~2023-08-18 13:25 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-13 11:53 [PATCH v2 1/3] eal: add x86 cpuid support for monitorx Sivaprasad Tummala
2023-04-13 11:53 ` [PATCH v2 2/3] doc: announce new cpu flag added to rte_cpu_flag_t Sivaprasad Tummala
2023-04-17 4:31 ` [PATCH v3 1/4] " Sivaprasad Tummala
2023-04-18 8:14 ` [PATCH v4 0/4] power: monitor support for AMD EPYC processors Sivaprasad Tummala
2023-04-18 8:25 ` Sivaprasad Tummala
2023-04-18 8:25 ` [PATCH v4 1/4] doc: announce new cpu flag added to rte_cpu_flag_t Sivaprasad Tummala
2023-04-18 8:52 ` Ferruh Yigit
2023-04-18 9:22 ` Bruce Richardson
2023-06-01 9:23 ` David Marchand
2023-07-05 11:32 ` Konstantin Ananyev
2023-08-16 18:59 ` [PATCH v5 1/3] eal: add x86 cpuid support for monitorx Sivaprasad Tummala
2023-08-16 18:59 ` [PATCH v5 2/3] eal: removed unnecessary checks in x86 power monitor APIs Sivaprasad Tummala
2023-08-16 18:59 ` [PATCH v5 3/3] power: amd power monitor support Sivaprasad Tummala
2023-08-16 19:27 ` Tyler Retzlaff
2023-08-17 11:34 ` Tummala, Sivaprasad
2023-08-17 14:18 ` Konstantin Ananyev
2023-08-18 13:25 ` Ferruh Yigit [this message]
2023-08-18 13:48 ` Bruce Richardson
2023-08-21 15:42 ` Tyler Retzlaff
2023-08-22 22:30 ` Konstantin Ananyev
2023-08-23 9:19 ` Ferruh Yigit
2023-08-23 16:03 ` Tyler Retzlaff
2023-08-24 9:04 ` Ferruh Yigit
2023-08-25 16:00 ` Tyler Retzlaff
2023-08-30 22:45 ` Konstantin Ananyev
2023-09-27 10:38 ` Tummala, Sivaprasad
2023-09-28 10:11 ` Konstantin Ananyev
2023-10-06 8:26 ` David Marchand
2023-10-09 8:02 ` Tummala, Sivaprasad
2023-10-09 14:05 ` [PATCH v6 1/3] eal: add x86 cpuid support for monitorx Sivaprasad Tummala
2023-10-09 14:05 ` [PATCH v6 2/3] eal: removed unnecessary checks in x86 power monitor APIs Sivaprasad Tummala
2023-10-09 14:05 ` [PATCH v6 3/3] power: amd power monitor support Sivaprasad Tummala
2023-10-10 8:59 ` David Marchand
2023-10-11 9:33 ` Tummala, Sivaprasad
2023-10-10 10:14 ` Konstantin Ananyev
2023-10-09 16:23 ` [PATCH v6 1/3] eal: add x86 cpuid support for monitorx Patrick Robb
2023-10-10 8:21 ` David Marchand
2023-04-18 8:25 ` [PATCH v4 2/4] " Sivaprasad Tummala
2023-06-14 13:15 ` Burakov, Anatoly
2023-04-18 8:25 ` [PATCH v4 3/4] eal: removed unnecessary checks in x86 power monitor APIs Sivaprasad Tummala
2023-06-14 13:14 ` Burakov, Anatoly
2023-04-18 8:25 ` [PATCH v4 4/4] power: amd power monitor support Sivaprasad Tummala
2023-06-14 13:14 ` Burakov, Anatoly
2023-04-13 11:53 ` [PATCH v2 3/3] " Sivaprasad Tummala
2023-04-17 4:34 ` [PATCH v3 4/4] " Sivaprasad Tummala
2023-04-13 11:59 ` [PATCH v2 1/3] eal: add x86 cpuid support for monitorx David Marchand
2023-04-13 17:50 ` Tummala, Sivaprasad
2023-04-14 7:05 ` David Marchand
2023-04-14 8:51 ` Tummala, Sivaprasad
2023-04-14 11:48 ` Ferruh Yigit
2023-04-17 4:32 ` [PATCH v3 2/4] " Sivaprasad Tummala
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=92e7bd2e-b799-9658-c90e-f50638c6fdbd@amd.com \
--to=ferruh.yigit@amd.com \
--cc=Sivaprasad.Tummala@amd.com \
--cc=anatoly.burakov@intel.com \
--cc=david.hunt@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=konstantin.ananyev@huawei.com \
--cc=roretzla@linux.microsoft.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).