| Message ID | 20241130140703.557-10-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for AMD hardware feedback interface | expand |
On Sat, Nov 30, 2024 at 08:07:00AM -0600, 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/kernel/process_32.c | 4 ++++ > arch/x86/kernel/process_64.c | 4 ++++ > 2 files changed, 8 insertions(+) > > diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c > index 0917c7f25720b..0bb6391b9089b 100644 > --- a/arch/x86/kernel/process_32.c > +++ b/arch/x86/kernel/process_32.c > @@ -213,6 +213,10 @@ __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 hw history on AMD CPUs */ > + if (cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS)) > + wrmsrl(AMD_WORKLOAD_HRST, 0x1); > + > return prev_p; > } Are you really going to support all this jazz on 32bit builds?
On Sat, Nov 30, 2024 at 08:07:00AM -0600, 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> > --- > diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c > index 226472332a70d..371e0e8f987fa 100644 > --- a/arch/x86/kernel/process_64.c > +++ b/arch/x86/kernel/process_64.c > @@ -709,6 +709,10 @@ __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 hw history on AMD CPUs */ > + if (cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS)) > + wrmsrl(AMD_WORKLOAD_HRST, 0x1); > + > return prev_p; > } BTW, how many cycles for this WRMSR ? And, as already stated, I'm failing to find the actual classification code, but assuming that's RDMSR(s?), how much for them?
On 11/30/24 06:07, Mario Limonciello wrote: > --- a/arch/x86/kernel/process_64.c > +++ b/arch/x86/kernel/process_64.c > @@ -709,6 +709,10 @@ __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 hw history on AMD CPUs */ > + if (cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS)) > + wrmsrl(AMD_WORKLOAD_HRST, 0x1); > + > return prev_p; Could we do a little refactoring here, please? This, plus the resctrl_sched_in() is sure starting to look like a pattern. It would be nice to have a single, common function that 32-bit and 64-bit call at the end of __switch_to(). The X86_BUG_SYSRET_SS_ATTRS hunk can probably go in there too.
On 12/2/2024 09:59, Peter Zijlstra wrote: > On Sat, Nov 30, 2024 at 08:07:00AM -0600, 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> >> --- > >> diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c >> index 226472332a70d..371e0e8f987fa 100644 >> --- a/arch/x86/kernel/process_64.c >> +++ b/arch/x86/kernel/process_64.c >> @@ -709,6 +709,10 @@ __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 hw history on AMD CPUs */ >> + if (cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS)) >> + wrmsrl(AMD_WORKLOAD_HRST, 0x1); >> + >> return prev_p; >> } > > BTW, how many cycles for this WRMSR ? When I measured it the average delay was ~119ns with a wider range of delays (standard deviation was 25% greater). > > And, as already stated, I'm failing to find the actual classification > code, but assuming that's RDMSR(s?), how much for them? > As mentioned on patch 1 comments the current patch series doesn't use any classification data produced by the hardware.
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c index 0917c7f25720b..0bb6391b9089b 100644 --- a/arch/x86/kernel/process_32.c +++ b/arch/x86/kernel/process_32.c @@ -213,6 +213,10 @@ __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 hw history on AMD CPUs */ + if (cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS)) + wrmsrl(AMD_WORKLOAD_HRST, 0x1); + return prev_p; } diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c index 226472332a70d..371e0e8f987fa 100644 --- a/arch/x86/kernel/process_64.c +++ b/arch/x86/kernel/process_64.c @@ -709,6 +709,10 @@ __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 hw history on AMD CPUs */ + if (cpu_feature_enabled(X86_FEATURE_AMD_WORKLOAD_CLASS)) + wrmsrl(AMD_WORKLOAD_HRST, 0x1); + return prev_p; }