Skip to contentSkip to conversation

x86: ACPI: cstate: Optimize C3 entry on AMD CPUs

PATCH RESEND Applied Deepak Sharma 4 participants 5 messages Sep 24, 2021 – Oct 1, 2021

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH RESEND 1/1 x86: ACPI: cstate: Optimize C3 entry on AMD CPUs

All Zen or newer CPU which support C3 shares cache. Its not necessary to flush the caches in software before entering C3. This will cause drop in performance for the cores which share some caches. ARB_DIS is not used with current AMD C state implementation. So set related flags correctly.

---
 arch/x86/kernel/acpi/cstate.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
arch/x86/kernel/acpi/cstate.c +15 -0
index 7de599eba7f0..7945eae5b315 100644
@@ -79,6 +79,21 @@ void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
 		 */
 		flags->bm_control = 0;
 	}
+	if (c->x86_vendor == X86_VENDOR_AMD && c->x86 >= 0x17) {
+		/*
+		 * For all AMD Zen or newer CPUs that support C3, caches
+		 * should not be flushed by software while entering C3
+		 * type state. Set bm->check to 1 so that kernel doesn't
+		 * need to execute cache flush operation.
+		 */
+		flags->bm_check = 1;
+		/*
+		 * In current AMD C state implementation ARB_DIS is no longer
+		 * used. So set bm_control to zero to indicate ARB_DIS is not
+		 * required while entering C3 type state.
+		 */
+		flags->bm_control = 0;
+	}
 }
 EXPORT_SYMBOL(acpi_processor_power_init_bm_check);
 
Signature · 1 line
2.25.1
#1 lore raw
Quoted 7 lines · Deepak Sharma

All Zen or newer CPU which support C3 shares cache. Its not necessary to flush the caches in software before entering C3. This will cause drop in performance for the cores which share some caches. ARB_DIS is not used with current AMD C state implementation. So set related flags correctly.

I'm planning to take this one unless the x86 maintainers have concerns, thanks.

Quoted 33 lines
---
 arch/x86/kernel/acpi/cstate.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
arch/x86/kernel/acpi/cstate.c +15 -1
index 7de599eba7f0..7945eae5b315 100644
@@ -79,6 +79,21 @@ void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
                 */
                flags->bm_control = 0;
        }
+       if (c->x86_vendor == X86_VENDOR_AMD && c->x86 >= 0x17) {
+               /*
+                * For all AMD Zen or newer CPUs that support C3, caches
+                * should not be flushed by software while entering C3
+                * type state. Set bm->check to 1 so that kernel doesn't
+                * need to execute cache flush operation.
+                */
+               flags->bm_check = 1;
+               /*
+                * In current AMD C state implementation ARB_DIS is no longer
+                * used. So set bm_control to zero to indicate ARB_DIS is not
+                * required while entering C3 type state.
+                */
+               flags->bm_control = 0;
+       }
 }
 EXPORT_SYMBOL(acpi_processor_power_init_bm_check);
 
--

2.25.1

#2 lore raw
Quoted 10 lines · Rafael J. Wysocki
Quoted 7 lines · Deepak Sharma

All Zen or newer CPU which support C3 shares cache. Its not necessary to flush the caches in software before entering C3. This will cause drop in performance for the cores which share some caches. ARB_DIS is not used with current AMD C state implementation. So set related flags correctly.

I'm planning to take this one unless the x86 maintainers have concerns, thanks.

maintainer Applied
#3 lore raw
Quoted 10 lines · Thomas Gleixner
Quoted 7 lines · Rafael J. Wysocki
Quoted 6 lines · Deepak Sharma

All Zen or newer CPU which support C3 shares cache. Its not necessary to flush the caches in software before entering C3. This will cause drop in performance for the cores which share some caches. ARB_DIS is not used with current AMD C state implementation. So set related flags correctly.

Quoted 5 lines
Quoted 3 lines
Quoted 1 line

I'm planning to take this one unless the x86 maintainers have concerns, thanks.

Thanks!

Applied as 5.16 material, thanks!

#4 lore raw
In reply to Rafael J. Wysocki
Quoted 30 lines · Rafael J. Wysocki
Quoted 7 lines · Deepak Sharma

All Zen or newer CPU which support C3 shares cache. Its not necessary to flush the caches in software before entering C3. This will cause drop in performance for the cores which share some caches. ARB_DIS is not used with current AMD C state implementation. So set related flags correctly.

I'm planning to take this one unless the x86 maintainers have concerns, thanks.

Quoted 19 lines
---
 arch/x86/kernel/acpi/cstate.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
arch/x86/kernel/acpi/cstate.c +7 -0
index 7de599eba7f0..7945eae5b315 100644
@@ -79,6 +79,21 @@ void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags,
                 */
                flags->bm_control = 0;
        }
+       if (c->x86_vendor == X86_VENDOR_AMD && c->x86 >= 0x17) {
+               /*
+                * For all AMD Zen or newer CPUs that support C3, caches
+                * should not be flushed by software while entering C3
+                * type state. Set bm->check to 1 so that kernel doesn't
+                * need to execute cache flush operation.
+                */
Quoted 8 lines

arch/x86/kernel/acpi/cstate.c void acpi_processor_power_init_bm_check(struct acpi_processor_flags *flags, · line 96

Quoted 8 lines
arch/x86/kernel/acpi/cstate.c +8 -0
+               flags->bm_check = 1;
+               /*
+                * In current AMD C state implementation ARB_DIS is no longer
+                * used. So set bm_control to zero to indicate ARB_DIS is not
+                * required while entering C3 type state.
+                */
+               flags->bm_control = 0;
+       }

My only concern is the blatant code duplication between AMD and ZHAOXIN here. Other than that, this is obviously correct since the ZHAOXIN thing is basically rebranded AMD IP.