On Sun, Oct 11, 2020 at 1:13 AM Alexander Monakov <amona...@ispras.ru> wrote: > > Intel SDM does not explicitly say that entering a C-state via MWAIT will > implicitly flush CPU caches as appropriate for that C-state. However, > documentation for individual Intel CPU generations does mention this > behavior. > > Since intel_idle binds to any Intel CPU with MWAIT, mention this > assumption on MWAIT behavior. In passing, reword opening comment > to make it clear that driver can load on any future Intel CPU with MWAIT. > > Signed-off-by: Alexander Monakov <amona...@ispras.ru> > Cc: Rafael J. Wysocki <rafael.j.wyso...@intel.com> > --- > Hi, > > I noticed that one of significant optimizations of intel_idle over > acpi_idle is elision of explicit wbinvd: ACPI requires the OS to flush > caches when entering C3, and Linux issues an explicit wbinvd to do that, > but intel_idle simply issues mwait with the expectation that the CPU > will automatically flush caches if needed. > > To me this is a fairly subtle point that became even more subtle > following the update to intel_idle that made it capable to bind to old > and future Intel CPUs with MWAIT (by the way, thanks for that!) > > Can you take this patch to spell out the assumption? > > drivers/idle/intel_idle.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c > index f4495841bf68..1e5666cf8763 100644 > --- a/drivers/idle/intel_idle.c > +++ b/drivers/idle/intel_idle.c > @@ -8,7 +8,7 @@ > */ > > /* > - * intel_idle is a cpuidle driver that loads on specific Intel processors > + * intel_idle is a cpuidle driver that loads on all Intel CPUs with MWAIT > * in lieu of the legacy ACPI processor_idle driver. The intent is to > * make Linux more efficient on these processors, as intel_idle knows > * more than ACPI, as well as make Linux more immune to ACPI BIOS bugs. > @@ -20,7 +20,11 @@ > * All CPUs have same idle states as boot CPU > * > * Chipset BM_STS (bus master status) bit is a NOP > - * for preventing entry into deep C-stats > + * for preventing entry into deep C-states > + * > + * CPU will flush caches as needed when entering a C-state via MWAIT
I would rephrase this to mention that the above actually is an assumption. > + * (in contrast to entering ACPI C3, where acpi_idle driver is And mentioning acpi_idle here is not needed; it would be sufficient to say something like "in which case the WBINVD instruction needs to be executed to flush the caches". > + * itself responsible for flushing, via WBINVD) > */ > > /* > -- Thanks!
