[1/3] perf/x86/amd/lbr: Use freeze based on availability

Message ID 1d9106579c7781746ca39860bda8061c56d6dc48.1702833179.git.sandipan.das@amd.com
State New
Headers
Series perf/x86/amd: Miscellaneous fixes |

Commit Message

Sandipan Das Dec. 17, 2023, 5:25 p.m. UTC
  Currently, it is assumed that LBR Freeze is supported on all processors
which have CPUID leaf 0x80000022[EAX] bit 1 set. This is incorrect as
the feature availability is additionally dependent on CPUID leaf
0x80000022[EAX] bit 2 being set which may not be set for all Zen 4
processors. Define a new feature bit for LBR and PMC freeze and set the
freeze enable bit (FLBRI) in DebugCtl (MSR 0x1d9) conditionally.

It should still be possible to use LBR without freeze for profile-guided
optimization of user programs by using an user-only branch filter during
profiling. When the user-only filter is enabled, branches are no longer
recorded after the transition to CPL 0 upon PMI arrival. When branch
entries are read in the PMI handler, the branch stack does not change.

E.g.

  $ perf record -j any,u -e ex_ret_brn_tkn ./workload

Fixes: ca5b7c0d9621 ("perf/x86/amd/lbr: Add LbrExtV2 branch record support")
Signed-off-by: Sandipan Das <sandipan.das@amd.com>
Cc: stable@vger.kernel.org
---
 arch/x86/events/amd/core.c         |  4 ++--
 arch/x86/events/amd/lbr.c          | 16 ++++++++++------
 arch/x86/include/asm/cpufeatures.h |  2 +-
 arch/x86/kernel/cpu/scattered.c    |  1 +
 4 files changed, 14 insertions(+), 9 deletions(-)
  

Comments

Stephane Eranian Dec. 18, 2023, 8:43 p.m. UTC | #1
On Sun, Dec 17, 2023 at 9:26 AM Sandipan Das <sandipan.das@amd.com> wrote:
>
> Currently, it is assumed that LBR Freeze is supported on all processors
> which have CPUID leaf 0x80000022[EAX] bit 1 set. This is incorrect as
> the feature availability is additionally dependent on CPUID leaf
> 0x80000022[EAX] bit 2 being set which may not be set for all Zen 4
> processors. Define a new feature bit for LBR and PMC freeze and set the
> freeze enable bit (FLBRI) in DebugCtl (MSR 0x1d9) conditionally.


Is this new feature bit visible to users?
I think it is useful to know whether or not LBR freeze is supported.
Imagine I want to do kernel FDO, then the user-only LBR trick to freeze LBR
does not work and I need actual LBR freeze support.

Thanks.

> It should still be possible to use LBR without freeze for profile-guided
> optimization of user programs by using an user-only branch filter during
> profiling. When the user-only filter is enabled, branches are no longer
> recorded after the transition to CPL 0 upon PMI arrival. When branch
> entries are read in the PMI handler, the branch stack does not change.
>
> E.g.
>
>   $ perf record -j any,u -e ex_ret_brn_tkn ./workload
>
> Fixes: ca5b7c0d9621 ("perf/x86/amd/lbr: Add LbrExtV2 branch record support")
> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
> Cc: stable@vger.kernel.org
> ---
>  arch/x86/events/amd/core.c         |  4 ++--
>  arch/x86/events/amd/lbr.c          | 16 ++++++++++------
>  arch/x86/include/asm/cpufeatures.h |  2 +-
>  arch/x86/kernel/cpu/scattered.c    |  1 +
>  4 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
> index 4ee6390b45c9..ffdfaee08b08 100644
> --- a/arch/x86/events/amd/core.c
> +++ b/arch/x86/events/amd/core.c
> @@ -905,8 +905,8 @@ static int amd_pmu_v2_handle_irq(struct pt_regs *regs)
>         if (!status)
>                 goto done;
>
> -       /* Read branch records before unfreezing */
> -       if (status & GLOBAL_STATUS_LBRS_FROZEN) {
> +       /* Read branch records */
> +       if (x86_pmu.lbr_nr) {
>                 amd_pmu_lbr_read();
>                 status &= ~GLOBAL_STATUS_LBRS_FROZEN;
>         }
> diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
> index eb31f850841a..110e34c59643 100644
> --- a/arch/x86/events/amd/lbr.c
> +++ b/arch/x86/events/amd/lbr.c
> @@ -400,10 +400,12 @@ void amd_pmu_lbr_enable_all(void)
>                 wrmsrl(MSR_AMD64_LBR_SELECT, lbr_select);
>         }
>
> -       rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
> -       rdmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg);
> +       if (cpu_feature_enabled(X86_FEATURE_AMD_LBR_PMC_FREEZE)) {
> +               rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
> +               wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl | DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
> +       }
>
> -       wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl | DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
> +       rdmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg);
>         wrmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg | DBG_EXTN_CFG_LBRV2EN);
>  }
>
> @@ -416,10 +418,12 @@ void amd_pmu_lbr_disable_all(void)
>                 return;
>
>         rdmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg);
> -       rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
> -
>         wrmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg & ~DBG_EXTN_CFG_LBRV2EN);
> -       wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl & ~DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
> +
> +       if (cpu_feature_enabled(X86_FEATURE_AMD_LBR_PMC_FREEZE)) {
> +               rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
> +               wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl & ~DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
> +       }
>  }
>
>  __init int amd_pmu_lbr_init(void)
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 4af140cf5719..9790e906d5e5 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -97,7 +97,7 @@
>  #define X86_FEATURE_SYSENTER32         ( 3*32+15) /* "" sysenter in IA32 userspace */
>  #define X86_FEATURE_REP_GOOD           ( 3*32+16) /* REP microcode works well */
>  #define X86_FEATURE_AMD_LBR_V2         ( 3*32+17) /* AMD Last Branch Record Extension Version 2 */
> -/* FREE, was #define X86_FEATURE_LFENCE_RDTSC          ( 3*32+18) "" LFENCE synchronizes RDTSC */
> +#define X86_FEATURE_AMD_LBR_PMC_FREEZE ( 3*32+18) /* "" AMD LBR and PMC Freeze */
>  #define X86_FEATURE_ACC_POWER          ( 3*32+19) /* AMD Accumulated Power Mechanism */
>  #define X86_FEATURE_NOPL               ( 3*32+20) /* The NOPL (0F 1F) instructions */
>  #define X86_FEATURE_ALWAYS             ( 3*32+21) /* "" Always-present feature */
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
> index 0dad49a09b7a..a515328d9d7d 100644
> --- a/arch/x86/kernel/cpu/scattered.c
> +++ b/arch/x86/kernel/cpu/scattered.c
> @@ -49,6 +49,7 @@ static const struct cpuid_bit cpuid_bits[] = {
>         { X86_FEATURE_BMEC,             CPUID_EBX,  3, 0x80000020, 0 },
>         { X86_FEATURE_PERFMON_V2,       CPUID_EAX,  0, 0x80000022, 0 },
>         { X86_FEATURE_AMD_LBR_V2,       CPUID_EAX,  1, 0x80000022, 0 },
> +       { X86_FEATURE_AMD_LBR_PMC_FREEZE,       CPUID_EAX,  2, 0x80000022, 0 },
>         { 0, 0, 0, 0, 0 }
>  };
>
> --
> 2.34.1
>
  
Sandipan Das Dec. 19, 2023, 8:08 a.m. UTC | #2
On 12/19/2023 2:13 AM, Stephane Eranian wrote:
> On Sun, Dec 17, 2023 at 9:26 AM Sandipan Das <sandipan.das@amd.com> wrote:
>>
>> Currently, it is assumed that LBR Freeze is supported on all processors
>> which have CPUID leaf 0x80000022[EAX] bit 1 set. This is incorrect as
>> the feature availability is additionally dependent on CPUID leaf
>> 0x80000022[EAX] bit 2 being set which may not be set for all Zen 4
>> processors. Define a new feature bit for LBR and PMC freeze and set the
>> freeze enable bit (FLBRI) in DebugCtl (MSR 0x1d9) conditionally.
> 
> 
> Is this new feature bit visible to users?
> I think it is useful to know whether or not LBR freeze is supported.
> Imagine I want to do kernel FDO, then the user-only LBR trick to freeze LBR
> does not work and I need actual LBR freeze support.
> 
> Thanks.
> 

Agreed. Will make it a visible flag in /proc/cpuinfo.

- Sandipan

>> It should still be possible to use LBR without freeze for profile-guided
>> optimization of user programs by using an user-only branch filter during
>> profiling. When the user-only filter is enabled, branches are no longer
>> recorded after the transition to CPL 0 upon PMI arrival. When branch
>> entries are read in the PMI handler, the branch stack does not change.
>>
>> E.g.
>>
>>   $ perf record -j any,u -e ex_ret_brn_tkn ./workload
>>
>> Fixes: ca5b7c0d9621 ("perf/x86/amd/lbr: Add LbrExtV2 branch record support")
>> Signed-off-by: Sandipan Das <sandipan.das@amd.com>
>> Cc: stable@vger.kernel.org
>> ---
>>  arch/x86/events/amd/core.c         |  4 ++--
>>  arch/x86/events/amd/lbr.c          | 16 ++++++++++------
>>  arch/x86/include/asm/cpufeatures.h |  2 +-
>>  arch/x86/kernel/cpu/scattered.c    |  1 +
>>  4 files changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
>> index 4ee6390b45c9..ffdfaee08b08 100644
>> --- a/arch/x86/events/amd/core.c
>> +++ b/arch/x86/events/amd/core.c
>> @@ -905,8 +905,8 @@ static int amd_pmu_v2_handle_irq(struct pt_regs *regs)
>>         if (!status)
>>                 goto done;
>>
>> -       /* Read branch records before unfreezing */
>> -       if (status & GLOBAL_STATUS_LBRS_FROZEN) {
>> +       /* Read branch records */
>> +       if (x86_pmu.lbr_nr) {
>>                 amd_pmu_lbr_read();
>>                 status &= ~GLOBAL_STATUS_LBRS_FROZEN;
>>         }
>> diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
>> index eb31f850841a..110e34c59643 100644
>> --- a/arch/x86/events/amd/lbr.c
>> +++ b/arch/x86/events/amd/lbr.c
>> @@ -400,10 +400,12 @@ void amd_pmu_lbr_enable_all(void)
>>                 wrmsrl(MSR_AMD64_LBR_SELECT, lbr_select);
>>         }
>>
>> -       rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
>> -       rdmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg);
>> +       if (cpu_feature_enabled(X86_FEATURE_AMD_LBR_PMC_FREEZE)) {
>> +               rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
>> +               wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl | DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
>> +       }
>>
>> -       wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl | DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
>> +       rdmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg);
>>         wrmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg | DBG_EXTN_CFG_LBRV2EN);
>>  }
>>
>> @@ -416,10 +418,12 @@ void amd_pmu_lbr_disable_all(void)
>>                 return;
>>
>>         rdmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg);
>> -       rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
>> -
>>         wrmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg & ~DBG_EXTN_CFG_LBRV2EN);
>> -       wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl & ~DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
>> +
>> +       if (cpu_feature_enabled(X86_FEATURE_AMD_LBR_PMC_FREEZE)) {
>> +               rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
>> +               wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl & ~DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
>> +       }
>>  }
>>
>>  __init int amd_pmu_lbr_init(void)
>> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
>> index 4af140cf5719..9790e906d5e5 100644
>> --- a/arch/x86/include/asm/cpufeatures.h
>> +++ b/arch/x86/include/asm/cpufeatures.h
>> @@ -97,7 +97,7 @@
>>  #define X86_FEATURE_SYSENTER32         ( 3*32+15) /* "" sysenter in IA32 userspace */
>>  #define X86_FEATURE_REP_GOOD           ( 3*32+16) /* REP microcode works well */
>>  #define X86_FEATURE_AMD_LBR_V2         ( 3*32+17) /* AMD Last Branch Record Extension Version 2 */
>> -/* FREE, was #define X86_FEATURE_LFENCE_RDTSC          ( 3*32+18) "" LFENCE synchronizes RDTSC */
>> +#define X86_FEATURE_AMD_LBR_PMC_FREEZE ( 3*32+18) /* "" AMD LBR and PMC Freeze */
>>  #define X86_FEATURE_ACC_POWER          ( 3*32+19) /* AMD Accumulated Power Mechanism */
>>  #define X86_FEATURE_NOPL               ( 3*32+20) /* The NOPL (0F 1F) instructions */
>>  #define X86_FEATURE_ALWAYS             ( 3*32+21) /* "" Always-present feature */
>> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
>> index 0dad49a09b7a..a515328d9d7d 100644
>> --- a/arch/x86/kernel/cpu/scattered.c
>> +++ b/arch/x86/kernel/cpu/scattered.c
>> @@ -49,6 +49,7 @@ static const struct cpuid_bit cpuid_bits[] = {
>>         { X86_FEATURE_BMEC,             CPUID_EBX,  3, 0x80000020, 0 },
>>         { X86_FEATURE_PERFMON_V2,       CPUID_EAX,  0, 0x80000022, 0 },
>>         { X86_FEATURE_AMD_LBR_V2,       CPUID_EAX,  1, 0x80000022, 0 },
>> +       { X86_FEATURE_AMD_LBR_PMC_FREEZE,       CPUID_EAX,  2, 0x80000022, 0 },
>>         { 0, 0, 0, 0, 0 }
>>  };
>>
>> --
>> 2.34.1
>>
  

Patch

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 4ee6390b45c9..ffdfaee08b08 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -905,8 +905,8 @@  static int amd_pmu_v2_handle_irq(struct pt_regs *regs)
 	if (!status)
 		goto done;
 
-	/* Read branch records before unfreezing */
-	if (status & GLOBAL_STATUS_LBRS_FROZEN) {
+	/* Read branch records */
+	if (x86_pmu.lbr_nr) {
 		amd_pmu_lbr_read();
 		status &= ~GLOBAL_STATUS_LBRS_FROZEN;
 	}
diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
index eb31f850841a..110e34c59643 100644
--- a/arch/x86/events/amd/lbr.c
+++ b/arch/x86/events/amd/lbr.c
@@ -400,10 +400,12 @@  void amd_pmu_lbr_enable_all(void)
 		wrmsrl(MSR_AMD64_LBR_SELECT, lbr_select);
 	}
 
-	rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
-	rdmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg);
+	if (cpu_feature_enabled(X86_FEATURE_AMD_LBR_PMC_FREEZE)) {
+		rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
+		wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl | DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
+	}
 
-	wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl | DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
+	rdmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg);
 	wrmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg | DBG_EXTN_CFG_LBRV2EN);
 }
 
@@ -416,10 +418,12 @@  void amd_pmu_lbr_disable_all(void)
 		return;
 
 	rdmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg);
-	rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
-
 	wrmsrl(MSR_AMD_DBG_EXTN_CFG, dbg_extn_cfg & ~DBG_EXTN_CFG_LBRV2EN);
-	wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl & ~DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
+
+	if (cpu_feature_enabled(X86_FEATURE_AMD_LBR_PMC_FREEZE)) {
+		rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
+		wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl & ~DEBUGCTLMSR_FREEZE_LBRS_ON_PMI);
+	}
 }
 
 __init int amd_pmu_lbr_init(void)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 4af140cf5719..9790e906d5e5 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -97,7 +97,7 @@ 
 #define X86_FEATURE_SYSENTER32		( 3*32+15) /* "" sysenter in IA32 userspace */
 #define X86_FEATURE_REP_GOOD		( 3*32+16) /* REP microcode works well */
 #define X86_FEATURE_AMD_LBR_V2		( 3*32+17) /* AMD Last Branch Record Extension Version 2 */
-/* FREE, was #define X86_FEATURE_LFENCE_RDTSC		( 3*32+18) "" LFENCE synchronizes RDTSC */
+#define X86_FEATURE_AMD_LBR_PMC_FREEZE	( 3*32+18) /* "" AMD LBR and PMC Freeze */
 #define X86_FEATURE_ACC_POWER		( 3*32+19) /* AMD Accumulated Power Mechanism */
 #define X86_FEATURE_NOPL		( 3*32+20) /* The NOPL (0F 1F) instructions */
 #define X86_FEATURE_ALWAYS		( 3*32+21) /* "" Always-present feature */
diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scattered.c
index 0dad49a09b7a..a515328d9d7d 100644
--- a/arch/x86/kernel/cpu/scattered.c
+++ b/arch/x86/kernel/cpu/scattered.c
@@ -49,6 +49,7 @@  static const struct cpuid_bit cpuid_bits[] = {
 	{ X86_FEATURE_BMEC,		CPUID_EBX,  3, 0x80000020, 0 },
 	{ X86_FEATURE_PERFMON_V2,	CPUID_EAX,  0, 0x80000022, 0 },
 	{ X86_FEATURE_AMD_LBR_V2,	CPUID_EAX,  1, 0x80000022, 0 },
+	{ X86_FEATURE_AMD_LBR_PMC_FREEZE,	CPUID_EAX,  2, 0x80000022, 0 },
 	{ 0, 0, 0, 0, 0 }
 };