On 18.07.2023 15:46, Simon Gaiser wrote:
> Jan Beulich:
>> On 18.07.2023 15:23, Simon Gaiser wrote:
>>> ---
>>> xen/arch/x86/acpi/cpu_idle.c | 9 ++++++---
>>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> This lacks both S-o-b and a proper description. The latter in
>> particular because you ...
>
> Yeah, I also noticed in the meantime, sorry. Will be fixed in v2.
>
>>> --- a/xen/arch/x86/acpi/cpu_idle.c
>>> +++ b/xen/arch/x86/acpi/cpu_idle.c
>>> @@ -155,6 +155,12 @@ static void cf_check do_get_hw_residencies(void *arg)
>>>
>>> switch ( c->x86_model )
>>> {
>>> + /* Tiger Lake */
>>> + case 0x8C:
>>> + case 0x8D:
>>> + /* Alder Lake */
>>> + case 0x97:
>>> + case 0x9A:
>>> /* 4th generation Intel Core (Haswell) */
>>> case 0x45:
>>> GET_PC8_RES(hw_res->pc8);
>>> @@ -185,9 +191,6 @@ static void cf_check do_get_hw_residencies(void *arg)
>>> case 0x6C:
>>> case 0x7D:
>>> case 0x7E:
>>> - /* Tiger Lake */
>>> - case 0x8C:
>>> - case 0x8D:
>>> /* Kaby Lake */
>>> case 0x8E:
>>> case 0x9E:
>>
>> ... don't just add new case labels, but you actually move two. It
>> wants explaining whether this was outright wrong, or what else
>> causes the movement.
>
> Yes, as the commit message says it get those PC{8..10} counters for
> Tiger and Alder Lake.
But that's the problem - there was no commit message.
> The former already had a label, therefore the
> move. I assume that when Tiger Lake was added it was an oversight to not
> also read those package C-state counters.
Or the SDM wasn't clear, and we needed to err on the safe side.
Jan
