Re: [Xen-devel] [PATCH 1/3] mwait-idle: add support for using halt

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

On 3/28/19 3:26 AM, Jan Beulich wrote:
>>>> On 27.03.19 at 18:28, <Brian.Woods@xxxxxxx> wrote:
>> This also lacks some of the features of mwait-idle has and duplicates
>> the limited functionality.
> 
> Would you mind clarifying the lack-of-features aspect? The
> only difference to your patches that I can spot is that you set
> .target_residency in the static tables. If the value wanted
> for CC6 is really 1000 instead of the 800 the default
> calculation would produce, then this would be easy enough
> to arrange for in my variant of the patch as well.
> 
> The mwait-idle driver would not have been needed at all if all
> BIOSes surfaced complete data via ACPI. Therefore, by
> suitably populating tables, it ought to be possible in theory to
> use just that one driver. It's just that for Intel CPUs we've
> decided to follow what Linux does, hence the separate
> driver. There's no Linux precedent for AMD (afaict).

target_residency and some of the checks IIRC.

Yes, but that's Linux and this is Xen.  Linux has an AML interpreter and 
Xen does not.  That's an apple to oranges comparison.  You can't compare 
Xen to Linux for this because the features they have and how they work 
are different.

>>   There's also a lack of comments which may or
>> may not be needed.  So that would add to the line change count if you
>> care about that.
>>
>> I'm not sure why you're so adverse to the mwait-idle patches.  We're
>> hard coding values in and using mwait (just like Intel is), but the only
>> real change we need is using halt for one c-state.
> 
> But that's precisely what I dislike, as getting us further away
> from the Linux driver. And btw, if we were to go that route,
> then I think we'd better call acpi_idle_do_entry() than to
> duplicate further parts of it. But that would also remove some
> of that other part of the benefits of mwait_idle() over
> acpi_processor_idle(): The former is much more streamlined,
> due to not having to care about anything other than MWAIT.
> 
> As an aside, despite having followed the HLT approach in my
> draft patch, I continue to be unconvinced that this is what we
> actually want. There's a respective TBD remark there.
> 
> Jan
> 
> 

The changes needed are small though... most of the changes are 
non-intrusive.  It's just a couple of lines here and then and then 
something where it calls what entry_method.  Although, I think using 
acpi_idle_do_entry() is perfectly fine. With the acpi_idle_do_entry 
change, the line change count of the mwait-idle patches is 65 lines. A 
lot of that is the structures (28 lines), which isn't any _real_ code 
change.

One function call and one switch statement isn't going to change the 
performance or how streamlined it is.  The only other change is when 
it's initialized which is only at start up.

Functionally, it should go in mwait-idle.  The changes are small, the 
functionally the same, there's no duplication of functionality or code.

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