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

[Chen, Zide]

On 6/7/2026 9:46 PM, Mi, Dapeng wrote:
> 
> 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. 

In this hypothetical case, currently the guest perf driver assumes GP7
is available, schedules and programs the event, until the hypervisor
yells, right?


>>
>> 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.

I vote for dynamic constraints to resolve this issue in a generic way.
Can we make use of the existing cpuc->constraint_list to do this?
>>
>>
>>> 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@xxxxxxxxx>
>>> Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
>>> ---
>>>  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 */