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

From: "Jan Beulich" <JBeulich@suse.com>
To: Brian Woods <brian.woods@amd.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	xen-devel@lists.xen.org, Wei Liu <wei.liu2@citrix.com>,
	Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [PATCH 1/3] mwait-idle: add support for using halt
Date: Thu, 28 Mar 2019 09:50:37 -0600	[thread overview]
Message-ID: <5C9CED4D0200007800222903@prv1-mh.provo.novell.com> (raw)
In-Reply-To: <18e167b2-b48c-7eb7-5aee-3fa63e15c116@amd.com>

>>> On 28.03.19 at 16:02, <Brian.Woods@amd.com> wrote:
> On 3/28/19 3:26 AM, Jan Beulich wrote:
>>>>> On 27.03.19 at 18:28, <Brian.Woods@amd.com> 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.

Could you be more specific what checks you mean?

> 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.

It's not a direct comparison, sure. But lack of suitable ACPI data
(known to happen in practice) would put Linux into exactly the
same position. If Linux accepted changes to the driver to use
entry methods other than MWAIT, I'd not be as opposed (but I'd
still question their reasoning then).

> Functionally, it should go in mwait-idle.

That's what I continue to question, seeing the HLT additions you're
making. Plus older families (which you didn't cover at all so far)
apparently would want HLT alone spelled out, which is even less
suitable for a driver with this name.

Jan

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