Re: [PATCH 7/8] perf/x86/intel: Drop fixed-counter PEBS constraints for baseline PEBS

["Mi, Dapeng" <dapeng1.mi@linux.intel.com>]

On 6/6/2026 4:32 AM, Chen, Zide wrote:
>
> On 6/4/2026 8:11 PM, Dapeng Mi wrote:
>> On SPR guests where pebs_baseline is not advertised, running:
>>
>> $ ./perf record -e cpu/event=0x00,umask=0x01,i\
>> 	 name=INST_RETIRED.PREC_DIST/p -c 10000 sleep 1
>>
>> can trigger:
>>
>> unchecked MSR access error: WRMSR to 0x3f1 ... in\
>> 	 intel_pmu_pebs_enable_all()
>>
>> Root cause:
>> SPR-specific PEBS constraints allow fixed-counter scheduling,
>> for example INST_RETIRED.PREC_DIST on fixed counter 0. In guests without
>> pebs_baseline, KVM does not support PEBS sampling on fixed counters,
>> so enabling such events reaches an invalid MSR programming path.
>>
>> Fix:
>> Drop fixed-counter entries from the PEBS constraint table. Without
>> pebs_baseline, those fixed-counter PEBS events now resolve to empty
>> constraints and are not scheduled/enabled, avoiding the warning and the
>> broken guest PEBS path.
> Seems this exposes a more general issue: constraints derived from
> host capabilities may not be applicable to a guest, since the guest may
> only has a subset of the host capabilities.
>
> For example, an event could be constrained to GP counter 7, while that
> counter is not exposed to the guest. Currently this is not gated and
> failures may only surface later during event programming.

Yes, but just for these pre-defined static constraints, it won't really
cause issue. If an event is constrained to GP counter 7 but there is no
such counter, then the event won't be really scheduled and enabled on the
counter. 


>
> Instead of dropping the constraints, should we validate counter
> availability in intel_pebs_constraints() or
> intel_get_event_constraints(), etc., and in a more generic way?

Yes, this is actually what the previous version did. In previous version,
we would leverage the dynamic constraints to limit the events additionally,
but just think twice, it's not best and simplest way to handle this issue.
It needs to allocate extra memory to store the dynamic constraints.


>
>
>> This is safe because, in pebs_baseline-capable cases, PEBS constraint
>> lookup already falls back to non-PEBS constraints when needed, and
>> fixed-counter constraints are effectively shared there.
> Can it really be removed without any consequences?
>
> If it is architecturally required that INST_RETIRED.PREC_DIST must run
> on fixed counter 0, then the constraint should be preserved. I think.

"INST_RETIRED.PREC_DIST" would still be constrained to fixed counter 0 by
the non-PEBS constraints, like the below constraints in corresponding
intel_icl_event_constraints[].

```

static struct event_constraint intel_icl_event_constraints[] = {
    FIXED_EVENT_CONSTRAINT(0x00c0, 0),    /* INST_RETIRED.ANY */
    FIXED_EVENT_CONSTRAINT(0x01c0, 0),    /* old INST_RETIRED.PREC_DIST */
    FIXED_EVENT_CONSTRAINT(0x0100, 0),    /* pseudo INST_RETIRED.ANY */

   ......

```

Currently as long as the PMU support PMU_FL_PEBS_ALL flag (adaptive PEBS
and arch-PEBS), if an event can't find the corresponding constraints for
PEBS constraints table, then it would fallback to the non-PEBS constraints,
like below code in intel_pebs_constraints() shows,

```

    /*
     * Extended PEBS support
     * Makes the PEBS code search the normal constraints.
     */
    if (x86_pmu.flags & PMU_FL_PEBS_ALL)
        return NULL;

```

If PMU doesn't support PMU_FL_PEBS_ALL flag on adaptive PEBS or arch-PEBS
hardware base, just like what currently KVM guest does, the PEBS
functionality is actually broken, and PEBS events should not be enabled.

Thanks.


>
>
>> Reported-by: Yi Lai <yi1.lai@intel.com>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
>> ---
>>  arch/x86/events/intel/ds.c | 13 -------------
>>  1 file changed, 13 deletions(-)
>>
>> diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
>> index cb72af9b61ce..5db15a92017a 100644
>> --- a/arch/x86/events/intel/ds.c
>> +++ b/arch/x86/events/intel/ds.c
>> @@ -1447,10 +1447,6 @@ struct event_constraint intel_skl_pebs_event_constraints[] = {
>>  };
>>  
>>  struct event_constraint intel_icl_pebs_event_constraints[] = {
>> -	INTEL_FLAGS_UEVENT_CONSTRAINT(0x01c0, 0x100000000ULL),	/* old INST_RETIRED.PREC_DIST */
>> -	INTEL_FLAGS_UEVENT_CONSTRAINT(0x0100, 0x100000000ULL),	/* INST_RETIRED.PREC_DIST */
>> -	INTEL_FLAGS_UEVENT_CONSTRAINT(0x0400, 0x800000000ULL),	/* SLOTS */
>> -
>>  	INTEL_PLD_CONSTRAINT(0x1cd, 0xff),			/* MEM_TRANS_RETIRED.LOAD_LATENCY */
>>  	INTEL_FLAGS_UEVENT_CONSTRAINT_DATALA_LD(0x11d0, 0xf),	/* MEM_INST_RETIRED.STLB_MISS_LOADS */
>>  	INTEL_FLAGS_UEVENT_CONSTRAINT_DATALA_ST(0x12d0, 0xf),	/* MEM_INST_RETIRED.STLB_MISS_STORES */
>> @@ -1473,9 +1469,6 @@ struct event_constraint intel_icl_pebs_event_constraints[] = {
>>  };
>>  
>>  struct event_constraint intel_glc_pebs_event_constraints[] = {
>> -	INTEL_FLAGS_UEVENT_CONSTRAINT(0x100, 0x100000000ULL),	/* INST_RETIRED.PREC_DIST */
>> -	INTEL_FLAGS_UEVENT_CONSTRAINT(0x0400, 0x800000000ULL),
>> -
>>  	INTEL_FLAGS_EVENT_CONSTRAINT(0xc0, 0xfe),
>>  	INTEL_PLD_CONSTRAINT(0x1cd, 0xfe),
>>  	INTEL_PSD_CONSTRAINT(0x2cd, 0x1),
>> @@ -1500,9 +1493,6 @@ struct event_constraint intel_glc_pebs_event_constraints[] = {
>>  };
>>  
>>  struct event_constraint intel_lnc_pebs_event_constraints[] = {
>> -	INTEL_FLAGS_UEVENT_CONSTRAINT(0x100, 0x100000000ULL),	/* INST_RETIRED.PREC_DIST */
>> -	INTEL_FLAGS_UEVENT_CONSTRAINT(0x0400, 0x800000000ULL),
>> -
>>  	INTEL_FLAGS_UEVENT_CONSTRAINT(0x012a, 0x1),		/* OCR.* events */
>>  	INTEL_FLAGS_UEVENT_CONSTRAINT(0x012b, 0x1),		/* OCR.* events */
>>  
>> @@ -1534,9 +1524,6 @@ struct event_constraint intel_lnc_pebs_event_constraints[] = {
>>  };
>>  
>>  struct event_constraint intel_pnc_pebs_event_constraints[] = {
>> -	INTEL_FLAGS_UEVENT_CONSTRAINT(0x100, 0x100000000ULL),	/* INST_RETIRED.PREC_DIST */
>> -	INTEL_FLAGS_UEVENT_CONSTRAINT(0x0400, 0x800000000ULL),
>> -
>>  	INTEL_HYBRID_LDLAT_CONSTRAINT(0x1cd, 0xfc),
>>  	INTEL_HYBRID_STLAT_CONSTRAINT(0x2cd, 0x3),
>>  	INTEL_FLAGS_UEVENT_CONSTRAINT_DATALA_LD(0x11d0, 0xf),	/* MEM_INST_RETIRED.STLB_MISS_LOADS */