Re: [PATCH] x86/perf: Default freeze_on_smi on for Comet Lake and later.

[Andrew Cooper <Andrew.Cooper3@citrix.com>]

On 27/01/2022 11:31, Peter Zijlstra wrote:
> On Thu, Jan 27, 2022 at 02:22:23AM +0000, Andrew Cooper wrote:
>
>> Frankly, it is an error that FREEZE_WHILE_SMM is under the kernels
>> control, and not SMM's control.  After all, it's SMM handling all the
>> UEFI secrets/etc.
>>
>> Linux ought to set FREEZE_WHILE_SMM unilaterally, because most kernel
>> profiling probably won't want interference from SMM.  Root can always
>> disable FREEZE_WHILE_SMM if profiling is really wanted.
>>
>> I'm not sure if anything can be done on pre-FREEZE_WHILE_SMM CPUs.  Nor
>> AMD CPUs which are also gaining CPL3 SMM logic, and don't appear to have
>> any equivalent functionality.
> Which suggests something like this?
>
> ---
> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> index c91434056c29..5874fa088630 100644
> --- a/arch/x86/events/intel/core.c
> +++ b/arch/x86/events/intel/core.c
> @@ -4703,6 +4703,19 @@ static __initconst const struct x86_pmu intel_pmu = {
>  	.lbr_read		= intel_pmu_lbr_read_64,
>  	.lbr_save		= intel_pmu_lbr_save,
>  	.lbr_restore		= intel_pmu_lbr_restore,
> +
> +	/*
> +	 * SMM has access to all 4 rings and while traditionally SMM code only
> +	 * ran in CPL0, newer firmware is starting to make use of CPL3 in SMM.

"2021-era firmware" ?

> +	 *
> +	 * Since the EVENTSEL.{USR,OS} CPL filtering makes no distinction
> +	 * between SMM or not, this results in what should be pure userspace
> +	 * counters including SMM data.
> +	 *
> +	 * This is a clear privilege issue, therefore globally disable
> +	 * counting SMM by default.
> +	 */
> +	.attr_freeze_on_smi	= 1,
>  };
>  
>  static __init void intel_clovertown_quirk(void)

I'd say so, yes.

Elsewhere on this thread, there has been claims of "everyone like seeing
SMM samples" as an argument for the current default, but I don't buy this.

NMIs are blocked in SMM, so PMIs will be attributed to an arbitrary
wrong instruction boundary, and the entire SMM Handler's worth of
counter changes will disappear into thin air, appearing as over-counting
as far as the profiling target is concerned.

When it comes to SMIs, the two interesting cases are:
1) Am I deterministically triggering SMIs when I oughtn't to be, and
2) If I did race with an SMI, should I discard everything and start again.

Both require paying attention to MSR_SMI_COUNT, even if the answer to 2)
is probably not when freeze_on_smi is active.

~Andrew