| Message ID | 20211206122952.74139-4-kirill.shutemov@linux.intel.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | ACPI/ACPICA: Only flush caches on S1/S2/S3 and C3 | expand |
On Mon, Dec 06, 2021 at 03:29:51PM +0300, Kirill A. Shutemov wrote: > According to the ACPI spec v6.4, section 8.2, cache flushing required > on entering C3 power state. > > Avoid flushing cache on entering other power states. > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > --- > drivers/acpi/processor_idle.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > index 76ef1bcc8848..01495aca850e 100644 > --- a/drivers/acpi/processor_idle.c > +++ b/drivers/acpi/processor_idle.c > @@ -567,7 +567,8 @@ static int acpi_idle_play_dead(struct cpuidle_device *dev, int index) > { > struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu); > > - ACPI_FLUSH_CPU_CACHE(); > + if (cx->type == ACPI_STATE_C3) > + ACPI_FLUSH_CPU_CACHE(); > acpi_idle_enter() already does this, acpi_idle_enter_s2idle() has it confused again, Also, I think acpi_idle_enter() does it too late; consider acpi_idle_enter_mb(). Either that or the BM crud needs more comments.
On Mon, Dec 6, 2021 at 4:03 PM Peter Zijlstra <peterz@infradead.org> wrote: > > On Mon, Dec 06, 2021 at 03:29:51PM +0300, Kirill A. Shutemov wrote: > > According to the ACPI spec v6.4, section 8.2, cache flushing required > > on entering C3 power state. > > > > Avoid flushing cache on entering other power states. > > > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > --- > > drivers/acpi/processor_idle.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > > index 76ef1bcc8848..01495aca850e 100644 > > --- a/drivers/acpi/processor_idle.c > > +++ b/drivers/acpi/processor_idle.c > > @@ -567,7 +567,8 @@ static int acpi_idle_play_dead(struct cpuidle_device *dev, int index) > > { > > struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu); > > > > - ACPI_FLUSH_CPU_CACHE(); > > + if (cx->type == ACPI_STATE_C3) > > + ACPI_FLUSH_CPU_CACHE(); > > > > acpi_idle_enter() already does this, acpi_idle_enter_s2idle() has it > confused again, No, they do the same thing: acpi_idle_enter_bm() if flags.bm_check is set. > Also, I think acpi_idle_enter() does it too late; consider > acpi_idle_enter_mb(). Either that or the BM crud needs more comments. I think the latter. Evidently, acpi_idle_play_dead(() doesn't support FFH and the BM thing, so it is only necessary to flush the cache when using ACPI_CSTATE_SYSTEMIO and when cx->type is C3.
On Wed, Dec 08, 2021 at 05:26:12PM +0100, Rafael J. Wysocki wrote: > On Mon, Dec 6, 2021 at 4:03 PM Peter Zijlstra <peterz@infradead.org> wrote: > > > > On Mon, Dec 06, 2021 at 03:29:51PM +0300, Kirill A. Shutemov wrote: > > > According to the ACPI spec v6.4, section 8.2, cache flushing required > > > on entering C3 power state. > > > > > > Avoid flushing cache on entering other power states. > > > > > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > > --- > > > drivers/acpi/processor_idle.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > > > index 76ef1bcc8848..01495aca850e 100644 > > > --- a/drivers/acpi/processor_idle.c > > > +++ b/drivers/acpi/processor_idle.c > > > @@ -567,7 +567,8 @@ static int acpi_idle_play_dead(struct cpuidle_device *dev, int index) > > > { > > > struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu); > > > > > > - ACPI_FLUSH_CPU_CACHE(); > > > + if (cx->type == ACPI_STATE_C3) > > > + ACPI_FLUSH_CPU_CACHE(); > > > > > > > acpi_idle_enter() already does this, acpi_idle_enter_s2idle() has it > > confused again, > > No, they do the same thing: acpi_idle_enter_bm() if flags.bm_check is set. > > > Also, I think acpi_idle_enter() does it too late; consider > > acpi_idle_enter_mb(). Either that or the BM crud needs more comments. > > I think the latter. > > Evidently, acpi_idle_play_dead(() doesn't support FFH and the BM > thing, so it is only necessary to flush the cache when using > ACPI_CSTATE_SYSTEMIO and when cx->type is C3. I'm new to this and not completely follow what I need to change. Does it look correct? From 3c544bc95a16d6a23dcb0aa50ee905d5e97c9ce5 Mon Sep 17 00:00:00 2001 From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> Date: Thu, 9 Dec 2021 16:24:44 +0300 Subject: [PATCH] ACPI: processor idle: Only flush cache on entering C3 According to the ACPI spec v6.4, section 8.2, cache flushing required on entering C3 power state. Avoid flushing cache on entering other power states. Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> --- drivers/acpi/processor_idle.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 76ef1bcc8848..d2a4d4446eff 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -567,7 +567,9 @@ static int acpi_idle_play_dead(struct cpuidle_device *dev, int index) { struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu); - ACPI_FLUSH_CPU_CACHE(); + if (cx->entry_method == ACPI_CSTATE_SYSTEMIO && + cx->type == ACPI_STATE_C3) + ACPI_FLUSH_CPU_CACHE(); while (1) {
On Thu, Dec 9, 2021 at 2:33 PM Kirill A. Shutemov <kirill.shutemov@linux.intel.com> wrote: > > On Wed, Dec 08, 2021 at 05:26:12PM +0100, Rafael J. Wysocki wrote: > > On Mon, Dec 6, 2021 at 4:03 PM Peter Zijlstra <peterz@infradead.org> wrote: > > > > > > On Mon, Dec 06, 2021 at 03:29:51PM +0300, Kirill A. Shutemov wrote: > > > > According to the ACPI spec v6.4, section 8.2, cache flushing required > > > > on entering C3 power state. > > > > > > > > Avoid flushing cache on entering other power states. > > > > > > > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > > > --- > > > > drivers/acpi/processor_idle.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > > > > index 76ef1bcc8848..01495aca850e 100644 > > > > --- a/drivers/acpi/processor_idle.c > > > > +++ b/drivers/acpi/processor_idle.c > > > > @@ -567,7 +567,8 @@ static int acpi_idle_play_dead(struct cpuidle_device *dev, int index) > > > > { > > > > struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu); > > > > > > > > - ACPI_FLUSH_CPU_CACHE(); > > > > + if (cx->type == ACPI_STATE_C3) > > > > + ACPI_FLUSH_CPU_CACHE(); > > > > > > > > > > acpi_idle_enter() already does this, acpi_idle_enter_s2idle() has it > > > confused again, > > > > No, they do the same thing: acpi_idle_enter_bm() if flags.bm_check is set. > > > > > Also, I think acpi_idle_enter() does it too late; consider > > > acpi_idle_enter_mb(). Either that or the BM crud needs more comments. > > > > I think the latter. > > > > Evidently, acpi_idle_play_dead(() doesn't support FFH and the BM > > thing, so it is only necessary to flush the cache when using > > ACPI_CSTATE_SYSTEMIO and when cx->type is C3. > > I'm new to this and not completely follow what I need to change. > > Does it look correct? It does, but I liked the original one more and so that one has been applied as 5.17 material (with some edits in the changelog). Thanks! > From 3c544bc95a16d6a23dcb0aa50ee905d5e97c9ce5 Mon Sep 17 00:00:00 2001 > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com> > Date: Thu, 9 Dec 2021 16:24:44 +0300 > Subject: [PATCH] ACPI: processor idle: Only flush cache on entering C3 > > According to the ACPI spec v6.4, section 8.2, cache flushing required > on entering C3 power state. > > Avoid flushing cache on entering other power states. > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > --- > drivers/acpi/processor_idle.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c > index 76ef1bcc8848..d2a4d4446eff 100644 > --- a/drivers/acpi/processor_idle.c > +++ b/drivers/acpi/processor_idle.c > @@ -567,7 +567,9 @@ static int acpi_idle_play_dead(struct cpuidle_device *dev, int index) > { > struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu); > > - ACPI_FLUSH_CPU_CACHE(); > + if (cx->entry_method == ACPI_CSTATE_SYSTEMIO && > + cx->type == ACPI_STATE_C3) > + ACPI_FLUSH_CPU_CACHE(); > > while (1) { > > -- > Kirill A. Shutemov
diff --git a/drivers/acpi/processor_idle.c b/drivers/acpi/processor_idle.c index 76ef1bcc8848..01495aca850e 100644 --- a/drivers/acpi/processor_idle.c +++ b/drivers/acpi/processor_idle.c @@ -567,7 +567,8 @@ static int acpi_idle_play_dead(struct cpuidle_device *dev, int index) { struct acpi_processor_cx *cx = per_cpu(acpi_cstate[index], dev->cpu); - ACPI_FLUSH_CPU_CACHE(); + if (cx->type == ACPI_STATE_C3) + ACPI_FLUSH_CPU_CACHE(); while (1) {
According to the ACPI spec v6.4, section 8.2, cache flushing required on entering C3 power state. Avoid flushing cache on entering other power states. Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> --- drivers/acpi/processor_idle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)