Re: [Xen-devel] [PATCH v2 0/3] mwait support for AMD processors

["Woods, Brian" <Brian.Woods@xxxxxxx>]

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@xxxxxxx<mailto:Brian.Woods@xxxxxxx>> 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
> 

-- 
Brian Woods

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel