| Message ID | 20241021180252.3531-11-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add support for AMD hardware feedback interface | expand |
Hello Mario, On Mon, Oct 21, 2024 at 01:02:49PM -0500, Mario Limonciello wrote: > From: Perry Yuan <perry.yuan@amd.com> > > Incorporate a mechanism within the context switching code to reset > the hardware history for AMD processors. Specifically, when a task > is switched in, the class ID was read and reset the hardware workload > classification history of CPU firmware and then it start to trigger > workload classification for the next running thread. > > Signed-off-by: Perry Yuan <perry.yuan@amd.com> > Co-developed-by: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > arch/x86/include/asm/hreset.h | 6 ++++++ > arch/x86/kernel/cpu/common.c | 15 +++++++++++++++ > arch/x86/kernel/process_32.c | 3 +++ > arch/x86/kernel/process_64.c | 3 +++ > 4 files changed, 27 insertions(+) > create mode 100644 arch/x86/include/asm/hreset.h > > diff --git a/arch/x86/include/asm/hreset.h b/arch/x86/include/asm/hreset.h > new file mode 100644 > index 0000000000000..ae1f72602bbd0 > --- /dev/null > +++ b/arch/x86/include/asm/hreset.h > @@ -0,0 +1,6 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_X86_HRESET_H > + > +void reset_hardware_history_hetero(void); > + > +#endif /* _ASM_X86_HRESET_H */ > diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c > index 07a34d7235057..658c8fb4e25df 100644 > --- a/arch/x86/kernel/cpu/common.c > +++ b/arch/x86/kernel/cpu/common.c > @@ -57,6 +57,7 @@ > #include <asm/mce.h> > #include <asm/msr.h> > #include <asm/cacheinfo.h> > +#include <asm/hreset.h> > #include <asm/memtype.h> > #include <asm/microcode.h> > #include <asm/intel-family.h> > @@ -403,6 +404,7 @@ static const unsigned long cr4_pinned_mask = X86_CR4_SMEP | X86_CR4_SMAP | X86_C > X86_CR4_FSGSBASE | X86_CR4_CET | X86_CR4_FRED; > static DEFINE_STATIC_KEY_FALSE_RO(cr_pinning); > static unsigned long cr4_pinned_bits __ro_after_init; > +static DEFINE_STATIC_KEY_FALSE_RO(hardware_history_features); > > void native_write_cr0(unsigned long val) > { > @@ -481,6 +483,12 @@ void cr4_init(void) > this_cpu_write(cpu_tlbstate.cr4, cr4); > } > > +static void __init setup_hreset(struct cpuinfo_x86 *c) > +{ > + if (cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS)) > + static_key_enable_cpuslocked(&hardware_history_features.key); > +} > + > /* > * Once CPU feature detection is finished (and boot params have been > * parsed), record any of the sensitive CR bits that are set, and > @@ -1844,6 +1852,7 @@ static void identify_cpu(struct cpuinfo_x86 *c) > setup_smep(c); > setup_smap(c); > setup_umip(c); > + setup_hreset(c); Since setup_hreset() just enables the static key once when the AMD_WORKLOAD_CLASS feature is enabled, does it make sense to call setup_hreset() in identify_boot_cpu() instead of in identify_cpu() ? -- Thanks and Regards gautham. > > /* Enable FSGSBASE instructions if available. */ > if (cpu_has(c, X86_FEATURE_FSGSBASE)) { > @@ -2410,3 +2419,9 @@ void __init arch_cpu_finalize_init(void) > */ > mem_encrypt_init(); > } > + > +__always_inline void reset_hardware_history_hetero(void) > +{ > + if (static_branch_unlikely(&hardware_history_features)) > + wrmsrl(AMD_WORKLOAD_HRST, 0x1); > +} > diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c > index 0917c7f25720b..6a3a1339f7a77 100644 > --- a/arch/x86/kernel/process_32.c > +++ b/arch/x86/kernel/process_32.c > @@ -52,6 +52,7 @@ > #include <asm/switch_to.h> > #include <asm/vm86.h> > #include <asm/resctrl.h> > +#include <asm/hreset.h> > #include <asm/proto.h> > > #include "process.h" > @@ -213,6 +214,8 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p) > /* Load the Intel cache allocation PQR MSR. */ > resctrl_sched_in(next_p); > > + reset_hardware_history_hetero(); > + > return prev_p; > } > > diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c > index 226472332a70d..ea7f765c6262a 100644 > --- a/arch/x86/kernel/process_64.c > +++ b/arch/x86/kernel/process_64.c > @@ -54,6 +54,7 @@ > #include <asm/xen/hypervisor.h> > #include <asm/vdso.h> > #include <asm/resctrl.h> > +#include <asm/hreset.h> > #include <asm/unistd.h> > #include <asm/fsgsbase.h> > #include <asm/fred.h> > @@ -709,6 +710,8 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p) > /* Load the Intel cache allocation PQR MSR. */ > resctrl_sched_in(next_p); > > + reset_hardware_history_hetero(); > + > return prev_p; > } > > -- > 2.43.0 >
On 10/22/2024 01:28, Gautham R. Shenoy wrote: > Hello Mario, > > On Mon, Oct 21, 2024 at 01:02:49PM -0500, Mario Limonciello wrote: >> From: Perry Yuan <perry.yuan@amd.com> >> >> Incorporate a mechanism within the context switching code to reset >> the hardware history for AMD processors. Specifically, when a task >> is switched in, the class ID was read and reset the hardware workload >> classification history of CPU firmware and then it start to trigger >> workload classification for the next running thread. >> >> Signed-off-by: Perry Yuan <perry.yuan@amd.com> >> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> arch/x86/include/asm/hreset.h | 6 ++++++ >> arch/x86/kernel/cpu/common.c | 15 +++++++++++++++ >> arch/x86/kernel/process_32.c | 3 +++ >> arch/x86/kernel/process_64.c | 3 +++ >> 4 files changed, 27 insertions(+) >> create mode 100644 arch/x86/include/asm/hreset.h >> >> diff --git a/arch/x86/include/asm/hreset.h b/arch/x86/include/asm/hreset.h >> new file mode 100644 >> index 0000000000000..ae1f72602bbd0 >> --- /dev/null >> +++ b/arch/x86/include/asm/hreset.h >> @@ -0,0 +1,6 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +#ifndef _ASM_X86_HRESET_H >> + >> +void reset_hardware_history_hetero(void); >> + >> +#endif /* _ASM_X86_HRESET_H */ >> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c >> index 07a34d7235057..658c8fb4e25df 100644 >> --- a/arch/x86/kernel/cpu/common.c >> +++ b/arch/x86/kernel/cpu/common.c >> @@ -57,6 +57,7 @@ >> #include <asm/mce.h> >> #include <asm/msr.h> >> #include <asm/cacheinfo.h> >> +#include <asm/hreset.h> >> #include <asm/memtype.h> >> #include <asm/microcode.h> >> #include <asm/intel-family.h> >> @@ -403,6 +404,7 @@ static const unsigned long cr4_pinned_mask = X86_CR4_SMEP | X86_CR4_SMAP | X86_C >> X86_CR4_FSGSBASE | X86_CR4_CET | X86_CR4_FRED; >> static DEFINE_STATIC_KEY_FALSE_RO(cr_pinning); >> static unsigned long cr4_pinned_bits __ro_after_init; >> +static DEFINE_STATIC_KEY_FALSE_RO(hardware_history_features); >> >> void native_write_cr0(unsigned long val) >> { >> @@ -481,6 +483,12 @@ void cr4_init(void) >> this_cpu_write(cpu_tlbstate.cr4, cr4); >> } >> >> +static void __init setup_hreset(struct cpuinfo_x86 *c) >> +{ >> + if (cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS)) >> + static_key_enable_cpuslocked(&hardware_history_features.key); >> +} >> + >> /* >> * Once CPU feature detection is finished (and boot params have been >> * parsed), record any of the sensitive CR bits that are set, and >> @@ -1844,6 +1852,7 @@ static void identify_cpu(struct cpuinfo_x86 *c) >> setup_smep(c); >> setup_smap(c); >> setup_umip(c); >> + setup_hreset(c); > > Since setup_hreset() just enables the static key once when the > AMD_WORKLOAD_CLASS feature is enabled, does it make sense to call > setup_hreset() in identify_boot_cpu() instead of in identify_cpu() ? > Good suggestion. I'll change this for the next revision. > -- > Thanks and Regards > gautham. > >> >> /* Enable FSGSBASE instructions if available. */ >> if (cpu_has(c, X86_FEATURE_FSGSBASE)) { >> @@ -2410,3 +2419,9 @@ void __init arch_cpu_finalize_init(void) >> */ >> mem_encrypt_init(); >> } >> + >> +__always_inline void reset_hardware_history_hetero(void) >> +{ >> + if (static_branch_unlikely(&hardware_history_features)) >> + wrmsrl(AMD_WORKLOAD_HRST, 0x1); >> +} >> diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c >> index 0917c7f25720b..6a3a1339f7a77 100644 >> --- a/arch/x86/kernel/process_32.c >> +++ b/arch/x86/kernel/process_32.c >> @@ -52,6 +52,7 @@ >> #include <asm/switch_to.h> >> #include <asm/vm86.h> >> #include <asm/resctrl.h> >> +#include <asm/hreset.h> >> #include <asm/proto.h> >> >> #include "process.h" >> @@ -213,6 +214,8 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p) >> /* Load the Intel cache allocation PQR MSR. */ >> resctrl_sched_in(next_p); >> >> + reset_hardware_history_hetero(); >> + >> return prev_p; >> } >> >> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c >> index 226472332a70d..ea7f765c6262a 100644 >> --- a/arch/x86/kernel/process_64.c >> +++ b/arch/x86/kernel/process_64.c >> @@ -54,6 +54,7 @@ >> #include <asm/xen/hypervisor.h> >> #include <asm/vdso.h> >> #include <asm/resctrl.h> >> +#include <asm/hreset.h> >> #include <asm/unistd.h> >> #include <asm/fsgsbase.h> >> #include <asm/fred.h> >> @@ -709,6 +710,8 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p) >> /* Load the Intel cache allocation PQR MSR. */ >> resctrl_sched_in(next_p); >> >> + reset_hardware_history_hetero(); >> + >> return prev_p; >> } >> >> -- >> 2.43.0 >>
diff --git a/arch/x86/include/asm/hreset.h b/arch/x86/include/asm/hreset.h new file mode 100644 index 0000000000000..ae1f72602bbd0 --- /dev/null +++ b/arch/x86/include/asm/hreset.h @@ -0,0 +1,6 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_X86_HRESET_H + +void reset_hardware_history_hetero(void); + +#endif /* _ASM_X86_HRESET_H */ diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 07a34d7235057..658c8fb4e25df 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -57,6 +57,7 @@ #include <asm/mce.h> #include <asm/msr.h> #include <asm/cacheinfo.h> +#include <asm/hreset.h> #include <asm/memtype.h> #include <asm/microcode.h> #include <asm/intel-family.h> @@ -403,6 +404,7 @@ static const unsigned long cr4_pinned_mask = X86_CR4_SMEP | X86_CR4_SMAP | X86_C X86_CR4_FSGSBASE | X86_CR4_CET | X86_CR4_FRED; static DEFINE_STATIC_KEY_FALSE_RO(cr_pinning); static unsigned long cr4_pinned_bits __ro_after_init; +static DEFINE_STATIC_KEY_FALSE_RO(hardware_history_features); void native_write_cr0(unsigned long val) { @@ -481,6 +483,12 @@ void cr4_init(void) this_cpu_write(cpu_tlbstate.cr4, cr4); } +static void __init setup_hreset(struct cpuinfo_x86 *c) +{ + if (cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS)) + static_key_enable_cpuslocked(&hardware_history_features.key); +} + /* * Once CPU feature detection is finished (and boot params have been * parsed), record any of the sensitive CR bits that are set, and @@ -1844,6 +1852,7 @@ static void identify_cpu(struct cpuinfo_x86 *c) setup_smep(c); setup_smap(c); setup_umip(c); + setup_hreset(c); /* Enable FSGSBASE instructions if available. */ if (cpu_has(c, X86_FEATURE_FSGSBASE)) { @@ -2410,3 +2419,9 @@ void __init arch_cpu_finalize_init(void) */ mem_encrypt_init(); } + +__always_inline void reset_hardware_history_hetero(void) +{ + if (static_branch_unlikely(&hardware_history_features)) + wrmsrl(AMD_WORKLOAD_HRST, 0x1); +} diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c index 0917c7f25720b..6a3a1339f7a77 100644 --- a/arch/x86/kernel/process_32.c +++ b/arch/x86/kernel/process_32.c @@ -52,6 +52,7 @@ #include <asm/switch_to.h> #include <asm/vm86.h> #include <asm/resctrl.h> +#include <asm/hreset.h> #include <asm/proto.h> #include "process.h" @@ -213,6 +214,8 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p) /* Load the Intel cache allocation PQR MSR. */ resctrl_sched_in(next_p); + reset_hardware_history_hetero(); + return prev_p; } diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index 226472332a70d..ea7f765c6262a 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -54,6 +54,7 @@ #include <asm/xen/hypervisor.h> #include <asm/vdso.h> #include <asm/resctrl.h> +#include <asm/hreset.h> #include <asm/unistd.h> #include <asm/fsgsbase.h> #include <asm/fred.h> @@ -709,6 +710,8 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p) /* Load the Intel cache allocation PQR MSR. */ resctrl_sched_in(next_p); + reset_hardware_history_hetero(); + return prev_p; }