lkml.org 
[lkml]   [2021]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/4] perf/x86/intel: Do not deploy workaround when TSX is deprecated
On 11.06.2021 09:50, Borislav Petkov wrote:
>On Wed, Jun 09, 2021 at 02:12:38PM -0700, Pawan Gupta wrote:
>> Earlier workaround added by commit 400816f60c54 ("perf/x86/intel:
>> Implement support for TSX Force Abort") for perf counter interactions
>> [1] are not required on some client systems which received a microcode
>> update that deprecates TSX.
>>
>> Bypass the perf workaround when such microcode is enumerated.
>>
>> [1] Performance Monitoring Impact of Intel® Transactional Synchronization Extension Memory
>> http://cdrdv2.intel.com/v1/dl/getContent/604224
>>
>> Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com>
>> Reviewed-by: Andi Kleen <ak@linux.intel.com>
>> Reviewed-by: Tony Luck <tony.luck@intel.com>
>> Tested-by: Neelima Krishnan <neelima.krishnan@intel.com>
>> ---
>> arch/x86/events/intel/core.c | 22 ++++++++++++++++++----
>> 1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
>> index e28892270c58..b5953e1e59a2 100644
>> --- a/arch/x86/events/intel/core.c
>> +++ b/arch/x86/events/intel/core.c
>> @@ -6016,10 +6016,24 @@ __init int intel_pmu_init(void)
>> intel_pmu_pebs_data_source_skl(pmem);
>>
>> if (boot_cpu_has(X86_FEATURE_TSX_FORCE_ABORT)) {
>> - x86_pmu.flags |= PMU_FL_TFA;
>> - x86_pmu.get_event_constraints = tfa_get_event_constraints;
>> - x86_pmu.enable_all = intel_tfa_pmu_enable_all;
>> - x86_pmu.commit_scheduling = intel_tfa_commit_scheduling;
>> + u64 msr;
>> +
>> + rdmsrl(MSR_TSX_FORCE_ABORT, msr);
>> + /* Systems that enumerate CPUID.RTM_ALWAYS_ABORT or
>> + * support MSR_TSX_FORCE_ABORT[SDV_ENABLE_RTM] bit have
>> + * TSX deprecated by default. TSX force abort hooks are
>> + * not required on these systems.
>
>So if they're not required, why aren't you simply disabling the force
>abort "workaround" by clearing the feature flag?

Feature flag also enumerates MSR_TSX_FORCE_ABORT, which is still present
after the microcode update. Patch 3/4 in this series clears the TSX
CPUID bits using MSR_TSX_FORCE_ABORT. So we do need the feature flag
X86_FEATURE_TSX_FORCE_ABORT.

>
> if (boot_cpu_has(X86_FEATURE_TSX_FORCE_ABORT)) {
> if (boot_cpu_has(X86_FEATURE_RTM_ALWAYS_ABORT))
> setup_clear_cpu_cap(X86_FEATURE_TSX_FORCE_ABORT);
> }
>
>so that it doesn't get enabled in the first place?

Feature flag is needed as explained above.

Thanks,
Pawan

\
 
 \ /
  Last update: 2021-06-11 23:35    [W:0.171 / U:0.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site