mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Chen, Zide" <zide.chen@intel.com>
To: "Mi, Dapeng" <dapeng1.mi@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Ian Rogers <irogers@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Andi Kleen <ak@linux.intel.com>,
	Eranian Stephane <eranian@google.com>
Cc: linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	Dapeng Mi <dapeng1.mi@intel.com>,
	Falcon Thomas <thomas.falcon@intel.com>,
	Xudong Hao <xudong.hao@intel.com>, Yi Lai <yi1.lai@intel.com>
Subject: Re: [PATCH 7/8] perf/x86/intel: Drop fixed-counter PEBS constraints for baseline PEBS
Date: Mon, 8 Jun 2026 10:46:09 -0500	[thread overview]
Message-ID: <17cc1e8c-84ed-459e-8b8b-dd1d52261105@intel.com> (raw)
In-Reply-To: <64abd498-8ce2-45fa-b41e-c95aa0bc8b40@linux.intel.com>



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@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 */


  reply	other threads:[~2026-06-08 15:46 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-05  1:11 [PATCH 0/8] perf/x86: Miscellaneous PMU bug fixes Dapeng Mi
2026-06-05  1:11 ` [PATCH 1/8] perf/x86/intel: Remove anythread_deprecated bit from perf_capabilities Dapeng Mi
2026-06-05 17:04   ` Falcon, Thomas
2026-06-08  1:37     ` Mi, Dapeng
2026-06-05  1:11 ` [PATCH 2/8] perf/x86: Introduce is_x86_pmu() helper Dapeng Mi
2026-06-05 17:08   ` Falcon, Thomas
2026-06-05  1:11 ` [PATCH 3/8] perf/x86: Update cap_user_rdpmc base on rdpmc user disable state Dapeng Mi
2026-06-05 17:15   ` Falcon, Thomas
2026-06-05  1:11 ` [PATCH 4/8] perf/x86/intel: Fix redundant branch type check in intel_pmu_lbr_filter() Dapeng Mi
2026-06-05 18:28   ` Falcon, Thomas
2026-06-08  1:56     ` Mi, Dapeng
2026-06-08  6:15       ` Mi, Dapeng
2026-06-05  1:11 ` [PATCH 5/8] perf/x86/intel: Fix kernel address leakages in LBR stack Dapeng Mi
2026-06-05  1:11 ` [PATCH 6/8] perf/x86/intel: Validate return value of intel_pmu_init_hybrid() Dapeng Mi
2026-06-05 16:17   ` Chen, Zide
2026-06-08  2:48     ` Mi, Dapeng
2026-06-05 18:47   ` Falcon, Thomas
2026-06-05  1:11 ` [PATCH 7/8] perf/x86/intel: Drop fixed-counter PEBS constraints for baseline PEBS Dapeng Mi
2026-06-05 20:32   ` Chen, Zide
2026-06-08  2:46     ` Mi, Dapeng
2026-06-08 15:46       ` Chen, Zide [this message]
2026-06-09  0:36         ` Mi, Dapeng
2026-06-05  1:11 ` [PATCH 8/8] perf/core: Fix kernel register info leak via hardware skid Dapeng Mi
2026-06-05 19:08   ` Falcon, Thomas
2026-06-08  2:47     ` Mi, Dapeng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=17cc1e8c-84ed-459e-8b8b-dd1d52261105@intel.com \
    --to=zide.chen@intel.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=dapeng1.mi@intel.com \
    --cc=dapeng1.mi@linux.intel.com \
    --cc=eranian@google.com \
    --cc=irogers@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=thomas.falcon@intel.com \
    --cc=xudong.hao@intel.com \
    --cc=yi1.lai@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome