[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <b50d9b7b-dccd-451e-91bc-fde0c467977c@amd.com>
Date: Tue, 22 Oct 2024 15:02:07 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: "Gautham R. Shenoy" <gautham.shenoy@....com>
Cc: Borislav Petkov <bp@...en8.de>, Hans de Goede <hdegoede@...hat.com>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
x86@...nel.org, Perry Yuan <perry.yuan@....com>,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
linux-pm@...r.kernel.org, platform-driver-x86@...r.kernel.org,
Shyam Sundar S K <Shyam-sundar.S-k@....com>
Subject: Re: [PATCH v4 10/13] x86/process: Clear hardware feedback history for
AMD processors
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@....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@....com>
>> Co-developed-by: Mario Limonciello <mario.limonciello@....com>
>> Signed-off-by: Mario Limonciello <mario.limonciello@....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
>>
Powered by blists - more mailing lists