Received-SPF: pass (google.com: domain of linux-kernel+bounces-4365-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Precedence: bulk
MIME-Version: 1.0
References: <cover.1702833179.git.sandipan.das@amd.com> <1d9106579c7781746ca39860bda8061c56d6dc48.1702833179.git.sandipan.das@amd.com>
In-Reply-To: <1d9106579c7781746ca39860bda8061c56d6dc48.1702833179.git.sandipan.das@amd.com>
From: Stephane Eranian <eranian@google.com>
Date: Mon, 18 Dec 2023 12:43:13 -0800
Message-ID: <CABPqkBQ0Zn_orR_9FnHA7Y1pNHAzG0E=86MkdMjOtGfKXDp29g@mail.gmail.com>
Subject: Re: [PATCH 1/3] perf/x86/amd/lbr: Use freeze based on availability
To: Sandipan Das <sandipan.das@amd.com>
Cc: linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, 
	x86@kernel.org, peterz@infradead.org, mingo@redhat.com, acme@kernel.org, 
	mark.rutland@arm.com, alexander.shishkin@linux.intel.com, jolsa@kernel.org, 
	namhyung@kernel.org, adrian.hunter@intel.com, tglx@linutronix.de, 
	bp@alien8.de, irogers@google.com, mario.limonciello@amd.com, 
	ravi.bangoria@amd.com, ananth.narayan@amd.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sun, Dec 17, 2023 at 9:26=E2=80=AFAM 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 suppor=
t")
> 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 &=3D ~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_O=
N_PMI);
> +
> +       if (cpu_feature_enabled(X86_FEATURE_AMD_LBR_PMC_FREEZE)) {
> +               rdmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl);
> +               wrmsrl(MSR_IA32_DEBUGCTLMSR, dbg_ctl & ~DEBUGCTLMSR_FREEZ=
E_LBRS_ON_PMI);
> +       }
>  }
>
>  __init int amd_pmu_lbr_init(void)
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cp=
ufeatures.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 Rec=
ord Extension Version 2 */
> -/* FREE, was #define X86_FEATURE_LFENCE_RDTSC          ( 3*32+18) "" LFE=
NCE 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 Pow=
er Mechanism */
>  #define X86_FEATURE_NOPL               ( 3*32+20) /* The NOPL (0F 1F) in=
structions */
>  #define X86_FEATURE_ALWAYS             ( 3*32+21) /* "" Always-present f=
eature */
> diff --git a/arch/x86/kernel/cpu/scattered.c b/arch/x86/kernel/cpu/scatte=
red.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[] =3D {
>         { 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
>