On Fri, Mar 1, 2024 at 11:02 AM He Rongguang <herongguang@xxxxxxxxxxxxxxxxx> wrote: > > 在 2024/3/1 1:22, Rafael J. Wysocki 写道: > > On Wed, Feb 28, 2024 at 8:28 AM He Rongguang > > <herongguang@xxxxxxxxxxxxxxxxx> 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.