Message ID | 20190328150426.7295-1-brian.woods@amd.com (mailing list archive) |
---|---|
Headers | show |
Series | mwait support for AMD processors | expand |
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.
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.
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...
> 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 <<a href="mailto:Brian.Woods@amd.com">Brian.Woods@amd.com</a>> 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. Newer AMD processors support mwait, but only for</span><br><span>c1, and for c2 halt is used. 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 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. <br>We don't have support to use mwait while in CC6 due to caches being <br>turned off etc. If we did have mwait suport for CC6, we'd use that here <br>(basically mirroring Intel). Sadly I don't think we have any public <br>information directly detailing this information. </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>? It's a requirement/input to all possible implementations. </div><div><br></div><div>From a comment in the April 2018 Linux patch by Yazen [2]:</div><div><div>> x86/smpboot: Don't use mwait_play_dead() on AMD systems</div><div>> Recent AMD systems support using MWAIT for C1 state. However, MWAIT will</div><div>> not allow deeper cstates than C1 on current systems.</div><div>> </div><div>> play_dead() expects to use the deepest state available. The deepest state</div><div>> available on AMD systems is reached through SystemIO or HALT. If MWAIT is</div><div>> available, it is preferred over the other methods, so the CPU never reaches</div><div>> the deepest possible state.</div><div>> </div><div>> Don't try to use MWAIT to play_dead() on AMD systems. Instead, use CPUIDLE</div><div>> to enter the deepest state advertised by firmware. If CPUIDLE is not</div><div>> 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? 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>> The 3rd patch is my counterproposal to Brian's intended abuse (as I would call it) of the mwait-idle driver. </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)? Should discussion move from the initial thread [1] to the counter-proposal thread [3]? Or this thread?</div><div><br></div><div>Rich</div><div><br></div><div>[1]<span style="background-color: rgba(255, 255, 255, 0);"> <a href="https://lists.gt.net/xen/devel/545688">https://lists.gt.net/xen/devel/545688</a></span></div><div><br></div><div>[2] <a href="https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86-urgent-for-linus&id=da6fa7ef67f07108a1b0cb9fd9e7fcaabd39c051&utm_source=anz">https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=x86-urgent-for-linus&id=da6fa7ef67f07108a1b0cb9fd9e7fcaabd39c051</a></div><div><br></div><div>[3] <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>
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 >