On Mon, Mar 4, 2024 at 7:14 AM He Rongguang
<herongguang@xxxxxxxxxxxxxxxxx> wrote:
>
> According to x86 spec ([1] and [2]), mwait hint_address[7:4] adds 1 is
> the corresponding cstate, and 0xF means C0.
>
> ACPI cstate table usually 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 instead of C16, according to spec.
>
> Since ACPI firmware is out of Linux kernel scope, fix kernel handling of
> 0xF ->(to) C0 in this situation. This is found when tweak ACPI cstate
> table qemu presenting to VM.
>
> Also fix intel_idle case by the way for kernel code consistency.
>
> [1]. Intel SDM Vol 2, Table 4-11. MWAIT Hints
> Register (EAX): "Value of 0 means C1; 1 means C2 and so on
> Value of 01111B means C0".
>
> [2]. AMD manual Vol 3, MWAIT: "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].".
>
> Signed-off-by: He Rongguang <herongguang@xxxxxxxxxxxxxxxxx>
> ---
> V1 -> V2: Amend commit message according to Rafael.
>
> arch/x86/kernel/acpi/cstate.c | 4 ++--
> drivers/idle/intel_idle.c | 3 ++-
> 2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c
> index 401808b47af3..f3ffd0a3a012 100644
> --- a/arch/x86/kernel/acpi/cstate.c
> +++ b/arch/x86/kernel/acpi/cstate.c
> @@ -131,8 +131,8 @@ static long acpi_processor_ffh_cstate_probe_cpu(void
> *_cx)
> cpuid(CPUID_MWAIT_LEAF, &eax, &ebx, &ecx, &edx);
>
> /* Check whether this particular cx_type (in CST) is supported
> or not */
> - cstate_type = ((cx->address >> MWAIT_SUBSTATE_SIZE) &
> - MWAIT_CSTATE_MASK) + 1;
> + cstate_type = (((cx->address >> MWAIT_SUBSTATE_SIZE) &
> + MWAIT_CSTATE_MASK) + 1) & MWAIT_CSTATE_MASK;
> edx_part = edx >> (cstate_type * MWAIT_SUBSTATE_SIZE);
> num_cstate_subtype = edx_part & MWAIT_SUBSTATE_MASK;
>
> diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
> index bcf1198e8991..e486027f8b07 100644
> --- a/drivers/idle/intel_idle.c
> +++ b/drivers/idle/intel_idle.c
> @@ -1934,7 +1934,8 @@ static void __init spr_idle_state_table_update(void)
>
> static bool __init intel_idle_verify_cstate(unsigned int mwait_hint)
> {
> - unsigned int mwait_cstate = MWAIT_HINT2CSTATE(mwait_hint) + 1;
> + unsigned int mwait_cstate = (MWAIT_HINT2CSTATE(mwait_hint) + 1) &
> + MWAIT_CSTATE_MASK;
> unsigned int num_substates = (mwait_substates >> mwait_cstate *
> 4) &
> MWAIT_SUBSTATE_MASK;
>
> --
Applied as 6.9 material with some edits in the subject and changelog.
Also, your email client mangles white space, so please consider using
a different one for sending patches.
Thanks!
