Re: [RESEND PATCH] x86/ACPI: invalidate all cache lines on ACPI C-state transitions

[Rafael J. Wysocki]

On Fri, May 30, 2025 at 7:54 PM Khalid Ali <khaliidcaliy@xxxxxxxxx> wrote:
>
> From: Khalid Ali <khaliidcaliy@xxxxxxxxx>
>
> According to ACPI spec 6.4 and 6.5, upon C-state

Which section?

> transitions (specifically C2 and C3) it is required and explicitly
> mentioned to invalidate and writeback all modified cache line using
> WBINVD.
>
> However the current ACPI C-state entry using monitor/mwait instructions
> it have been used CLFLUSH by flushing the cache line associated by
> monitored address. That what all about this patch addresses,
> invalidating all cache lines instead of single cache line.
>
> Let me know if there any reason and decisions behind the current
> implementation.

I think that Peter has answered this already.

Anyway: Is there any practical reason to make this change?  For
instance, any system that doesn't work before it and works after it?

> Signed-off-by: Khalid Ali <khaliidcaliy@xxxxxxxxx>
> ---
>  arch/x86/kernel/acpi/cstate.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/acpi/cstate.c b/arch/x86/kernel/acpi/cstate.c
> index d5ac34186555..eb3d435e08ad 100644
> --- a/arch/x86/kernel/acpi/cstate.c
> +++ b/arch/x86/kernel/acpi/cstate.c
> @@ -222,6 +222,9 @@ void __cpuidle acpi_processor_ffh_cstate_enter(struct acpi_processor_cx *cx)
>         struct cstate_entry *percpu_entry;
>
>         percpu_entry = per_cpu_ptr(cpu_cstate_entry, cpu);
> +       /* flush and invalidate all modified cache line on C3 and C2 state entry*/
> +       if (cx->type == ACPI_STATE_C3 || cx->type == ACPI_STATE_C2)
> +               wbinvd();
>         mwait_idle_with_hints(percpu_entry->states[cx->index].eax,
>                               percpu_entry->states[cx->index].ecx);
>  }
> --