[PATCH v2 0/2] x86/cpuidle: Cannon Lake adjustments

Jan Beulich posted 2 patches 4 years ago
Only 0 patches received!
[PATCH v2 0/2] x86/cpuidle: Cannon Lake adjustments
Posted by Jan Beulich 4 years ago
As requested in reply to v1, this is now a pair of patches with
the expectation that only patch 1 would be acked and go in.

1: drop Cannon Lake support
2: support Cannon Lake (again)

Jan

Re: [PATCH v2 0/2] x86/cpuidle: Cannon Lake adjustments
Posted by Andrew Cooper 4 years ago
On 02/04/2020 09:22, Jan Beulich wrote:
> As requested in reply to v1, this is now a pair of patches with
> the expectation that only patch 1 would be acked and go in.
>
> 1: drop Cannon Lake support
> 2: support Cannon Lake (again)

Dropping Cannon Lake support is only of any incremental benefit if we
drop it from everywhere, and I didn't mean to block this single patch on it.

Consider either A-by.

However, it would be helpful to consider what we could do for better
KCONFIG-able CPU support.

Some downstreams have a separate build for each platform, and some go as
far as cramming Xen into the boot SPI ROM, so space saving is a very
important aspect.

On a different front, being able to compile out support for CPUs without
VMX unrestricted mode, or without CMPXCHG16B would be helpful.

I would certainly like to get CONFIG_{INTEL,AMD} usable even if only so
randconfig can help keep our interfaces clean.

~Andrew

Re: [PATCH v2 0/2] x86/cpuidle: Cannon Lake adjustments
Posted by Jan Beulich 4 years ago
On 02.04.2020 16:58, Andrew Cooper wrote:
> On 02/04/2020 09:22, Jan Beulich wrote:
>> As requested in reply to v1, this is now a pair of patches with
>> the expectation that only patch 1 would be acked and go in.
>>
>> 1: drop Cannon Lake support
>> 2: support Cannon Lake (again)
> 
> Dropping Cannon Lake support is only of any incremental benefit if we
> drop it from everywhere, and I didn't mean to block this single patch on it.

How would dropping it from everywhere in one go be any better?
I would see a benefit then only if we added code to refuse
booting there.

> Consider either A-by.

I'm sorry to ask, but "either" here is unclear to me: Do you
mean both of the above, or "the first one here or the original
v1 one"? I don't see a point committing this in two pieces, if
the combination of both is fine by you as well.

> However, it would be helpful to consider what we could do for better
> KCONFIG-able CPU support.

Yes, sure. Future work.

> Some downstreams have a separate build for each platform, and some go as
> far as cramming Xen into the boot SPI ROM, so space saving is a very
> important aspect.

Yes.

> On a different front, being able to compile out support for CPUs without
> VMX unrestricted mode, or without CMPXCHG16B would be helpful.

Yes.

> I would certainly like to get CONFIG_{INTEL,AMD} usable even if only so
> randconfig can help keep our interfaces clean.

And yes again.

Jan

Re: [PATCH v2 0/2] x86/cpuidle: Cannon Lake adjustments
Posted by Andrew Cooper 4 years ago
On 03/04/2020 09:06, Jan Beulich wrote:
> On 02.04.2020 16:58, Andrew Cooper wrote:
>> On 02/04/2020 09:22, Jan Beulich wrote:
>>> As requested in reply to v1, this is now a pair of patches with
>>> the expectation that only patch 1 would be acked and go in.
>>>
>>> 1: drop Cannon Lake support
>>> 2: support Cannon Lake (again)
>> Dropping Cannon Lake support is only of any incremental benefit if we
>> drop it from everywhere, and I didn't mean to block this single patch on it.
> How would dropping it from everywhere in one go be any better?
> I would see a benefit then only if we added code to refuse
> booting there.
>
>> Consider either A-by.
> I'm sorry to ask, but "either" here is unclear to me: Do you
> mean both of the above, or "the first one here or the original
> v1 one"? I don't see a point committing this in two pieces, if
> the combination of both is fine by you as well.

Pick whichever patch you prefer.

Looking at Linux recently, it appears that Ice Lake inherited some of
the Cannon Lake uarch designs, so while we don't necessarily care about
Cannon Lake CPUs themselves, the same details might be applicable in
later CPUs as well.

~Andrew

[PATCH v2 1/2] x86/cpuidle: drop Cannon Lake support
Posted by Jan Beulich 4 years ago
As per SDM rev 071 Cannon Lake has
- no CC3 residency MSR at 3FC,
- a CC1 residency MSR ar 660 (like various Atoms),
- a useless (always zero) CC3 residency MSR at 662.
However, this CPU model has been discontinued and isn't a primary target
of Xen anyway. Hence (at least for now) rather than correcting things,
simply drop the case label altogether.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Split from larger patch.

--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -180,8 +180,6 @@ static void do_get_hw_residencies(void *
     case 0x4E:
     case 0x55:
     case 0x5E:
-    /* Cannon Lake */
-    case 0x66:
     /* Kaby Lake */
     case 0x8E:
     case 0x9E:


[PATCH v2 2/2] x86/cpuidle: support Cannon Lake (again)
Posted by Jan Beulich 4 years ago
As per SDM rev 071 Cannon Lake has
- no CC3 residency MSR at 3FC,
- a CC1 residency MSR ar 660 (like various Atoms),
- a useless (always zero) CC3 residency MSR at 662.
Hence it needs a separate case label.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Split. This patch is beign submitted more for completeness than with
    the expectation that it would get acked, as per the v1 feedback.
---
Using the MSR cross reference in the same SDM revision one might even
get the impression that further MSRs are unavailable, but newer CPUs
don't appear to be consistently listed there at all, so may rather be a
doc shortcoming. I've pointed this out to Intel, but I'm not expecting
swift feedback.

--- a/xen/arch/x86/acpi/cpu_idle.c
+++ b/xen/arch/x86/acpi/cpu_idle.c
@@ -69,7 +69,7 @@
 #define GET_PC8_RES(val)  GET_HW_RES_IN_NS(0x630, val) /* some Haswells only */
 #define GET_PC9_RES(val)  GET_HW_RES_IN_NS(0x631, val) /* some Haswells only */
 #define GET_PC10_RES(val) GET_HW_RES_IN_NS(0x632, val) /* some Haswells only */
-#define GET_CC1_RES(val)  GET_HW_RES_IN_NS(0x660, val) /* Silvermont only */
+#define GET_CC1_RES(val)  GET_HW_RES_IN_NS(0x660, val)
 #define GET_CC3_RES(val)  GET_HW_RES_IN_NS(0x3FC, val)
 #define GET_CC6_RES(val)  GET_HW_RES_IN_NS(0x3FD, val)
 #define GET_CC7_RES(val)  GET_HW_RES_IN_NS(0x3FE, val) /* SNB onwards */
@@ -201,6 +201,16 @@ static void do_get_hw_residencies(void *
         GET_CC3_RES(hw_res->cc3);
         GET_CC6_RES(hw_res->cc6);
         break;
+    /* Cannon Lake */
+    case 0x66:
+        GET_PC2_RES(hw_res->pc2);
+        GET_PC3_RES(hw_res->pc3);
+        GET_PC6_RES(hw_res->pc6);
+        GET_PC7_RES(hw_res->pc7);
+        GET_CC1_RES(hw_res->cc1);
+        GET_CC6_RES(hw_res->cc6);
+        GET_CC7_RES(hw_res->cc7);
+        break;
     /* Xeon Phi Knights Landing */
     case 0x57:
     /* Xeon Phi Knights Mill */