Re: [PATCH] x86/cstate: fix mwait hint target cstate calc

[He Rongguang <herongguang@linux.alibaba.com>]

在 2024/3/1 19:04, Rafael J. Wysocki 写道:
> On Fri, Mar 1, 2024 at 11:02 AM He Rongguang
> <herongguang@linux.alibaba.com> wrote:
>>
>> 在 2024/3/1 1:22, Rafael J. Wysocki 写道:
>>> On Wed, Feb 28, 2024 at 8:28 AM He Rongguang
>>> <herongguang@linux.alibaba.com> wrote:
>>>>
>>>> According to x86 manual (Intel SDM Vol 2, Table 4-11. MWAIT Hints
>>>> Register (EAX) and AMD manual Vol 3, MWAIT), mwait hint[7:4] adds 1 is
>>>> the corresponding cstate, and 0xF means C0, so fix the handling of
>>>> 0xF -> C0.
>>>>
>>>> Intel: "Value of 0 means C1; 1 means C2 and so on
>>>> Value of 01111B means C0".
>>>>
>>>> AMD: "The processor C-state is EAX[7:4]+1, so to request C0 is to place
>>>> the value F in EAX[7:4] and to request C1 is to place the value 0 in
>>>> EAX[7:4].".
>>>
>>> Yes, 0x0F is defined to correspond to C0.  However, the value in
>>> question is never equal to 0x0F in any of the functions modified by
>>> this patch.
>>>
>>> What's the purpose of the modification, then?
>>>
>>
>> Hi, this is found when I tweak ACPI cstate table qemu presenting to VM.
>>
>> Usually, ACPI cstate table only contains C1+, but nothing prevents ACPI
>> firmware from presenting a cstate (maybe C1+) but using a mwait address
>> C0 (i.e., 0xF in ACPI FFH MWAIT hint address). And if this is the case,
>> Linux erroneously treat this cstate as C16, while actually this should
>> be legal C0 state.
>>
>> As I think ACPI firmware is out of Linux kernel scope, so I think it’s
>> better for Linux kernel to implement here referring to spec, how do you
>> think? :)
> 
> That's fair, but you need to put the above information into the patch
> changelog.  Otherwise it is unclear why you want to make this change.

Ok, will send a V2 patch.