mbox series

[v2,0/3] mwait support for AMD processors

Message ID 20190328150426.7295-1-brian.woods@amd.com (mailing list archive)
Headers show
Series mwait support for AMD processors | expand

Message

Woods, Brian March 28, 2019, 3:04 p.m. UTC
This patch series add support and enablement for mwait on AMD Naples
and Rome processors.  Newer AMD processors support mwait, but only for
c1, and for c2 halt is used.  The mwait-idle driver is modified to be
able to use both mwait and halt for idling.

Brian Woods (3):
  mwait-idle: add support for using halt
  mwait-idle: add support for AMD processors
  mwait-idle: add enablement for AMD Naples and Rome

 xen/arch/x86/acpi/cpu_idle.c  |  2 +-
 xen/arch/x86/cpu/mwait-idle.c | 62 ++++++++++++++++++++++++++++++++++++++-----
 xen/include/asm-x86/cpuidle.h |  1 +
 3 files changed, 57 insertions(+), 8 deletions(-)

Comments

Woods, Brian April 8, 2019, 4:26 p.m. UTC | #1
On 3/28/19 10:04 AM, Woods, Brian wrote:
> This patch series add support and enablement for mwait on AMD Naples
> and Rome processors.  Newer AMD processors support mwait, but only for
> c1, and for c2 halt is used.  The mwait-idle driver is modified to be
> able to use both mwait and halt for idling.
> 
> Brian Woods (3):
>    mwait-idle: add support for using halt
>    mwait-idle: add support for AMD processors
>    mwait-idle: add enablement for AMD Naples and Rome
> 
>   xen/arch/x86/acpi/cpu_idle.c  |  2 +-
>   xen/arch/x86/cpu/mwait-idle.c | 62 ++++++++++++++++++++++++++++++++++++++-----
>   xen/include/asm-x86/cpuidle.h |  1 +
>   3 files changed, 57 insertions(+), 8 deletions(-)
> 

Just a ping to hopefully start the discussion that mentioned in the 
community call.
Woods, Brian May 9, 2019, 10 p.m. UTC | #2
On Thu, Mar 28, 2019 at 03:04:32PM +0000, Brian Woods wrote:
> This patch series add support and enablement for mwait on AMD Naples
> and Rome processors.  Newer AMD processors support mwait, but only for
> c1, and for c2 halt is used.  The mwait-idle driver is modified to be
> able to use both mwait and halt for idling.
> 
> Brian Woods (3):
>   mwait-idle: add support for using halt
>   mwait-idle: add support for AMD processors
>   mwait-idle: add enablement for AMD Naples and Rome
> 
>  xen/arch/x86/acpi/cpu_idle.c  |  2 +-
>  xen/arch/x86/cpu/mwait-idle.c | 62 ++++++++++++++++++++++++++++++++++++++-----
>  xen/include/asm-x86/cpuidle.h |  1 +
>  3 files changed, 57 insertions(+), 8 deletions(-)
> 
> -- 
> 2.11.0
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel

Ping for Andy.
Woods, Brian May 30, 2019, 7:27 p.m. UTC | #3
On Thu, May 09, 2019 at 05:00:07PM -0500, Brian Woods wrote:
> On Thu, Mar 28, 2019 at 03:04:32PM +0000, Brian Woods wrote:
> > This patch series add support and enablement for mwait on AMD Naples
> > and Rome processors.  Newer AMD processors support mwait, but only for
> > c1, and for c2 halt is used.  The mwait-idle driver is modified to be
> > able to use both mwait and halt for idling.
> > 
> > Brian Woods (3):
> >   mwait-idle: add support for using halt
> >   mwait-idle: add support for AMD processors
> >   mwait-idle: add enablement for AMD Naples and Rome
> > 
> >  xen/arch/x86/acpi/cpu_idle.c  |  2 +-
> >  xen/arch/x86/cpu/mwait-idle.c | 62 ++++++++++++++++++++++++++++++++++++++-----
> >  xen/include/asm-x86/cpuidle.h |  1 +
> >  3 files changed, 57 insertions(+), 8 deletions(-)
> > 
> > -- 
> > 2.11.0
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xenproject.org
> > https://lists.xenproject.org/mailman/listinfo/xen-devel
> 
> Ping for Andy.
> 
> -- 
> Brian Woods

Ping in general...
Rich Persaud May 31, 2019, 2:08 a.m. UTC | #4
> On Mar 28, 2019, at 11:04, Woods, Brian <Brian.Woods@amd.com> wrote:
> 
> This patch series add support and enablement for mwait on AMD Naples
> and Rome processors.  Newer AMD processors support mwait, but only for
> c1, and for c2 halt is used.  The mwait-idle driver is modified to be
> able to use both mwait and halt for idling.

Would you mind if I create a Xen Project JIRA ticket, or a wiki page, to track requirements and implementations related to this patch series?

From the initial thread [1]:
>>> On certain AMD families that support mwait, only c1 can be reached by
>>> + * mwait and to reach c2, halt has to be used.
>>> + */
>>> +#define CPUIDLE_FLAG_USE_HALT        0x2
>> 
>> Could you point us at where in the manuals this behavior is described?
>> While PM Vol 2 has a chapter talking about P-states, I can't seem to
>> find any mention of C-states there.
> 
> IIRC it's in the NDA PPR and internally it's in some other documents. 
> We don't have support to use mwait while in CC6 due to caches being 
> turned off etc.  If we did have mwait suport for CC6, we'd use that here 
> (basically mirroring Intel).  Sadly I don't think we have any public 
> information directly detailing this information.  

Can this be documented in the patch comment, or an AMD-specific page on wiki.xenproject.org?  It's a requirement/input to all possible implementations.  

From a comment in the April 2018 Linux patch by Yazen [2]:
> x86/smpboot: Don't use mwait_play_dead() on AMD systems
> Recent AMD systems support using MWAIT for C1 state. However, MWAIT will
> not allow deeper cstates than C1 on current systems.
> 
> play_dead() expects to use the deepest state available.  The deepest state
> available on AMD systems is reached through SystemIO or HALT. If MWAIT is
> available, it is preferred over the other methods, so the CPU never reaches
> the deepest possible state.
> 
> Don't try to use MWAIT to play_dead() on AMD systems. Instead, use CPUIDLE
> to enter the deepest state advertised by firmware. If CPUIDLE is not
> available then fallback to HALT.

For the ticket/wiki: what are the expected benefits of the proposed Xen change?  Would it reduce idle power consumption on Ryzen 1000/2000/3000? Epyc 3000/7000? Any sample data available for idle power before/after the v2 patch?

From a thread [3] posted by Jan this week, "x86/AMD: make C-state handling independent of Dom0":
> The 3rd patch is my counterproposal to Brian's intended abuse (as I would call it) of the mwait-idle driver. 

Do we need a new, patch-independent, thread for convergence of candidate implementations which address the requirements (documented in ticket/wiki)?  Should discussion move from the initial thread [1] to the counter-proposal thread [3]?  Or this thread?

Rich

[1] https://lists.gt.net/xen/devel/545688

[2] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86-urgent-for-linus&id=da6fa7ef67f07108a1b0cb9fd9e7fcaabd39c051

[3] https://lists.xenproject.org/archives/html/xen-devel/2019-05/msg01894.html
<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto"><div dir="ltr"><span></span></div><div dir="ltr"><div dir="ltr"><span></span></div><div dir="ltr"><div dir="ltr"><span></span></div><div dir="ltr"><meta http-equiv="content-type" content="text/html; charset=utf-8"><div dir="ltr"><span></span></div><div dir="ltr"><meta http-equiv="content-type" content="text/html; charset=utf-8"><div dir="ltr"></div><div dir="ltr">On Mar 28, 2019, at 11:04, Woods, Brian &lt;<a href="mailto:Brian.Woods@amd.com">Brian.Woods@amd.com</a>&gt; wrote:</div><div dir="ltr"><br></div><blockquote type="cite"><div dir="ltr"><span>This patch series add support and enablement for mwait on AMD Naples</span><br><span>and Rome processors. &nbsp;Newer AMD processors support mwait, but only for</span><br><span>c1, and for c2 halt is used. &nbsp;The mwait-idle driver is modified to be</span><br><span>able to use both mwait and halt for idling.</span></div></blockquote><br><div>Would you mind if I create a Xen Project JIRA ticket, or a wiki page, to track requirements and implementations related to this patch series?</div><div><br></div><div>From the initial thread [1]:</div><div><blockquote type="cite"><blockquote type="cite"><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);"></span></font></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">On certain AMD families that support mwait, only c1 can be reached by<br></span></font></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">+ * mwait and to reach c2, halt has to be used.<br></span></font></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">+ */<br></span></font></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">+#define CPUIDLE_FLAG_USE_HALT &nbsp; &nbsp; &nbsp; &nbsp;0x2<br></span></font></blockquote></blockquote><blockquote type="cite"><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);"><br></span></font></blockquote><blockquote type="cite"><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">Could you point us at where in the manuals this behavior is described?<br></span></font></blockquote><blockquote type="cite"><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">While PM Vol 2 has a chapter talking about P-states, I can't seem to<br></span></font></blockquote><blockquote type="cite"><font color="#000000"><span style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);">find any mention of C-states there.<br></span></font></blockquote><span style="background-color: rgba(255, 255, 255, 0);"><br>IIRC it's in the NDA PPR and internally it's in some other documents.&nbsp;<br>We don't have support to use mwait while in CC6 due to caches being&nbsp;<br>turned off etc. &nbsp;If we did have mwait suport for CC6, we'd use that here&nbsp;<br>(basically mirroring Intel). &nbsp;Sadly I don't think we have any public&nbsp;<br>information directly detailing this information. &nbsp;</span></blockquote><br></div><div>Can this be documented in the patch comment, or an AMD-specific page on <a href="http://wiki.xenproject.org">wiki.xenproject.org</a>? &nbsp;It's a requirement/input to all possible implementations. &nbsp;</div><div><br></div><div>From a comment in the April 2018 Linux patch by Yazen [2]:</div><div><div>&gt; x86/smpboot: Don't use mwait_play_dead() on AMD systems</div><div>&gt; Recent AMD systems support using MWAIT for C1 state. However, MWAIT will</div><div>&gt; not allow deeper cstates than C1 on current systems.</div><div>&gt;&nbsp;</div><div>&gt; play_dead() expects to use the deepest state available. &nbsp;The deepest state</div><div>&gt; available on AMD systems is reached through SystemIO or HALT. If MWAIT is</div><div>&gt; available, it is preferred over the other methods, so the CPU never reaches</div><div>&gt; the deepest possible state.</div><div>&gt;&nbsp;</div><div>&gt; Don't try to use MWAIT to play_dead() on AMD systems. Instead, use CPUIDLE</div><div>&gt; to enter the deepest state advertised by firmware. If CPUIDLE is not</div><div>&gt; available then fallback to HALT.</div></div><div><br></div><div>For the ticket/wiki: what are the expected benefits of the proposed Xen change? &nbsp;Would it reduce idle power consumption on Ryzen 1000/2000/3000? Epyc 3000/7000? Any sample data available for idle power before/after the v2 patch?</div><div><br></div><div>From a thread [3] posted by Jan this week, "x86/AMD: make C-state handling independent of Dom0":</div><div>&gt;&nbsp;The 3rd patch is my counterproposal to Brian's intended abuse (as I would call it) of the mwait-idle driver.&nbsp;</div><div><br></div><div>Do we need a new, patch-independent, thread for convergence of candidate implementations which address the requirements (documented in ticket/wiki)? &nbsp;Should discussion move from the initial thread [1] to the counter-proposal thread [3]? &nbsp;Or this thread?</div><div><br></div><div>Rich</div><div><br></div><div>[1]<span style="background-color: rgba(255, 255, 255, 0);">&nbsp;<a href="https://lists.gt.net/xen/devel/545688">https://lists.gt.net/xen/devel/545688</a></span></div><div><br></div><div>[2]&nbsp;<a href="https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86-urgent-for-linus&amp;id=da6fa7ef67f07108a1b0cb9fd9e7fcaabd39c051&amp;utm_source=anz">https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86-urgent-for-linus&amp;id=da6fa7ef67f07108a1b0cb9fd9e7fcaabd39c051</a></div><div><br></div><div>[3]&nbsp;<a href="https://lists.xenproject.org/archives/html/xen-devel/2019-05/msg01894.html" style="caret-color: rgb(0, 0, 0); background-color: rgba(255, 255, 255, 0);"><font color="#000000">https://lists.xenproject.org/archives/html/xen-devel/2019-05/msg01894.html</font></a></div><div><br></div></div></div></div></div></body></html>
Woods, Brian June 17, 2019, 9:32 p.m. UTC | #5
On Thu, May 30, 2019 at 10:08:12PM -0400, Rich Persaud wrote:
> [CAUTION: External Email]
> On Mar 28, 2019, at 11:04, Woods, Brian <Brian.Woods@amd.com<mailto:Brian.Woods@amd.com>> wrote:
> 
> This patch series add support and enablement for mwait on AMD Naples
> and Rome processors.  Newer AMD processors support mwait, but only for
> c1, and for c2 halt is used.  The mwait-idle driver is modified to be
> able to use both mwait and halt for idling.
> 
> Would you mind if I create a Xen Project JIRA ticket, or a wiki page, to track requirements and implementations related to this patch series?

You can, but I doubt this patch series will go anywhere since Jan was
completely opposed to adding this to the mwait-idle.c file since it
included halt for C2.  Since then, Jan has released some other patches
which have gotten reviews/comments so.

> From the initial thread [1]:
> On certain AMD families that support mwait, only c1 can be reached by
> + * mwait and to reach c2, halt has to be used.
> + */
> +#define CPUIDLE_FLAG_USE_HALT        0x2
> 
> Could you point us at where in the manuals this behavior is described?
> While PM Vol 2 has a chapter talking about P-states, I can't seem to
> find any mention of C-states there.

Technically I should clairfy.  You can reach C1 and C2 via sysio and
acpi as well.  But mwait only uses C1.  Halt (after a timer and a
transition state), assuming C2 is enabled, does put the CPU in C2.

Sadly this isn't documented well, (even in the NDA docs), but the
documentation you'd be looking for is the NDA PPR.  Sadly the public
PPR doesn't include it.

> IIRC it's in the NDA PPR and internally it's in some other documents.
> We don't have support to use mwait while in CC6 due to caches being
> turned off etc.  If we did have mwait suport for CC6, we'd use that here
> (basically mirroring Intel).  Sadly I don't think we have any public
> information directly detailing this information.

None that I know of.

> Can this be documented in the patch comment, or an AMD-specific page on wiki.xenproject.org<http://wiki.xenproject.org>?  It's a requirement/input to all possible implementations.
> 
> From a comment in the April 2018 Linux patch by Yazen [2]:
> > x86/smpboot: Don't use mwait_play_dead() on AMD systems
> > Recent AMD systems support using MWAIT for C1 state. However, MWAIT will
> > not allow deeper cstates than C1 on current systems.
> >
> > play_dead() expects to use the deepest state available.  The deepest state
> > available on AMD systems is reached through SystemIO or HALT. If MWAIT is
> > available, it is preferred over the other methods, so the CPU never reaches
> > the deepest possible state.
> >
> > Don't try to use MWAIT to play_dead() on AMD systems. Instead, use CPUIDLE
> > to enter the deepest state advertised by firmware. If CPUIDLE is not
> > available then fallback to HALT.
> 
> For the ticket/wiki: what are the expected benefits of the proposed Xen change?  Would it reduce idle power consumption on Ryzen 1000/2000/3000? Epyc 3000/7000? Any sample data available for idle power before/after the v2 patch?

Since Xen uses HALT be default, it would be a performance feature,
since it would use HALT/C2 for ALL idle.  mwait has a much better
response time from being woken up (at the cost power).

> From a thread [3] posted by Jan this week, "x86/AMD: make C-state handling independent of Dom0":
> > The 3rd patch is my counterproposal to Brian's intended abuse (as I would call it) of the mwait-idle driver.
> 
> Do we need a new, patch-independent, thread for convergence of candidate implementations which address the requirements (documented in ticket/wiki)?  Should discussion move from the initial thread [1] to the counter-proposal thread [3]?  Or this thread?
> 
> Rich

Yes, that's Jan's patch I was talking about before.  Glad to know the
cleanest solution with the least code duplication and a single path
for Intel and AMD was considered abuse.

> [1] https://lists.gt.net/xen/devel/545688
> 
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86-urgent-for-linus&id=da6fa7ef67f07108a1b0cb9fd9e7fcaabd39c051<https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86-urgent-for-linus&id=da6fa7ef67f07108a1b0cb9fd9e7fcaabd39c051&utm_source=anz>
> 
> [3] https://lists.xenproject.org/archives/html/xen-devel/2019-05/msg01894.html
>