[PATCH v2] perf/amd/ibs: add phy_addr_only software filter for ibs_op

[PATCH v2] perf/amd/ibs: add phy_addr_only software filter for ibs_op

[Huang Shijie]

We use the following command to track a process's memory footprint:
  #perf record -e ibs_fetch// -e ibs_op// -p xxx --phys-data -- sleep 10

In the mysql tests, I caught over 1,804,973 records for ibs_op event.
I found that over 53% records(970,554) are like this:
        ibs_op//:  ffffffffb6017739               0

The physical address is 0 for these records. These records are not needed
by us, and they wasted the CPU cycles.

This patch adds an ibs_op/phy_addr_only term that discards samples
without a valid physical address (dc_phy_addr_valid == 0) in the interrupt
handler. And it avoids the phys_addr=0 noise when used with --phys-data.
The term is only meaningful together with PERF_SAMPLE_PHYS_ADDR.

Test result:
   On 384-CPU Hygon machine running MySQL 8 + sysbench oltp_read_only with
400 threads. Use perf-record in a 3s window after 15s of warmup.

Compared with BPF filter:
  "perf record --phys-data --filter 'phys_addr != 0'"

    "-c = 50000":
                   samples
    filter        captured
    ----------------------
    phy_addr_only 25158085
    bpf           24628937

    "-c = 40000":
                   samples
    filter        captured
    ----------------------
    phy_addr_only 28215473
    bpf             FAILED

At -c 40000 the BPF rounds stop working.
The phy_addr_only rounds keep capturing at that density,
and every one of them still reports zero phys_addr==0 samples.

Signed-off-by: Huang Shijie <huangsj@hygon.cn>
---
v1 -->v2:
   1.) filter the zero-address
   2.) returen invalid if PERF_SAMPLE_PHYS_ADDR is not set.

  v1:  
   https://lore.kernel.org/all/20260902085251.4046240-1-huangsj@hygon.cn/#t
---
 arch/x86/events/amd/ibs.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index e5376ad5b2ec..f9949853e991 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -32,6 +32,7 @@ static u32 ibs_caps;
 
 /* attr.config2 */
 #define IBS_SW_FILTER_MASK	1
+#define IBS_PHY_ADDR_ONLY_MASK	2
 
 /* attr.config1 */
 #define IBS_OP_CONFIG1_LDLAT_MASK		(0xFFFULL <<  0)
@@ -303,6 +304,13 @@ static bool perf_ibs_strmst_event(struct perf_ibs *perf_ibs,
 	       (event->attr.config1 & IBS_OP_CONFIG1_STRMST_MASK);
 }
 
+static bool perf_ibs_phy_addr_only_event(struct perf_ibs *perf_ibs,
+					 struct perf_event *event)
+{
+	return perf_ibs == &perf_ibs_op &&
+	       (event->attr.config2 & IBS_PHY_ADDR_ONLY_MASK);
+}
+
 static int perf_ibs_init(struct perf_event *event)
 {
 	struct hw_perf_event *hwc = &event->hw;
@@ -439,6 +447,10 @@ static int perf_ibs_init(struct perf_event *event)
 		hwc->extra_reg.config |= strmst << IBS_OP_2_STRM_ST_FILTER_SHIFT;
 	}
 
+	if (perf_ibs_phy_addr_only_event(perf_ibs, event) &&
+	    !(event->attr.sample_type & PERF_SAMPLE_PHYS_ADDR))
+		return -EINVAL;
+
 	/*
 	 * If we modify hwc->sample_period, we also need to update
 	 * hwc->last_period and hwc->period_left.
@@ -721,6 +733,7 @@ static struct attribute_group empty_caps_group = {
 PMU_FORMAT_ATTR(rand_en,	"config:57");
 PMU_FORMAT_ATTR(cnt_ctl,	"config:19");
 PMU_FORMAT_ATTR(swfilt,		"config2:0");
+PMU_FORMAT_ATTR(phy_addr_only,	"config2:1");
 PMU_EVENT_ATTR_STRING(l3missonly, fetch_l3missonly, "config:59");
 PMU_EVENT_ATTR_STRING(l3missonly, op_l3missonly, "config:16");
 PMU_EVENT_ATTR_STRING(ldlat, ibs_op_ldlat_format, "config1:0-11");
@@ -890,6 +903,7 @@ cnt_ctl_is_visible(struct kobject *kobj, struct attribute *attr, int i)
 
 static struct attribute *op_attrs[] = {
 	&format_attr_swfilt.attr,
+	&format_attr_phy_addr_only.attr,
 	NULL,
 };
 
@@ -1357,7 +1371,8 @@ static int perf_ibs_get_offset_max(struct perf_ibs *perf_ibs,
 	if (event->attr.sample_type & PERF_SAMPLE_RAW ||
 	    perf_ibs_is_mem_sample_type(perf_ibs, event) ||
 	    perf_ibs_ldlat_event(perf_ibs, event) ||
-	    perf_ibs_fetch_lat_event(perf_ibs, event))
+	    perf_ibs_fetch_lat_event(perf_ibs, event) ||
+	    perf_ibs_phy_addr_only_event(perf_ibs, event))
 		return perf_ibs->offset_max;
 	else if (check_rip)
 		return 3;
@@ -1499,6 +1514,23 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 		}
 	}
 
+	if (perf_ibs_phy_addr_only_event(perf_ibs, event)) {
+		union ibs_op_data3 op_data3;
+		u64 phys_addr;
+
+		op_data3.val = ibs_data.regs[ibs_op_msr_idx(MSR_AMD64_IBSOPDATA3)];
+		if (!op_data3.dc_phy_addr_valid) {
+			throttle = perf_event_account_interrupt(event);
+			goto out;
+		}
+		/* Also skip the case: zero address in MSR_AMD64_IBSDCPHYSAD */
+		phys_addr = ibs_data.regs[ibs_op_msr_idx(MSR_AMD64_IBSDCPHYSAD)];
+		if (!phys_addr) {
+			throttle = perf_event_account_interrupt(event);
+			goto out;
+		}
+	}
+
 	if (perf_ibs_fetch_lat_event(perf_ibs, event)) {
 		union ibs_fetch_ctl fetch_ctl;
 
-- 
2.53.0

Re: [PATCH v2] perf/amd/ibs: add phy_addr_only software filter for ibs_op

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] perf/amd/ibs: min_period clamp bypassed on discard in perf_ibs_handle_irq()
- [Medium] perf/amd/ibs: missing ld_op/st_op check in perf_ibs_handle_irq() filter
--

commit 2b588b2eb9d2eeb02990e56aab4b6d6ff659932b
Author: Huang Shijie <huangsj@hygon.cn>

perf/amd/ibs: add phy_addr_only software filter for ibs_op

This commit adds a phy_addr_only software filter attribute for the AMD IBS op
PMU to discard samples without a valid non-zero physical address in the
interrupt handler when used with PERF_SAMPLE_PHYS_ADDR.

> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index 3531f9c23b8c9..12075d7d4703b 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c

[ ... ]

> @@ -1479,6 +1494,23 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
>  		}
>  	}
>  
> +	if (perf_ibs_phy_addr_only_event(perf_ibs, event)) {
> +		union ibs_op_data3 op_data3;
> +		u64 phys_addr;
> +
> +		op_data3.val = ibs_data.regs[ibs_op_msr_idx(MSR_AMD64_IBSOPDATA3)];
> +		if (!op_data3.dc_phy_addr_valid) {
> +			throttle = perf_event_account_interrupt(event);
> +			goto out;
> +		}
> +		/* Also skip the case: zero address in MSR_AMD64_IBSDCPHYSAD */
> +		phys_addr = ibs_data.regs[ibs_op_msr_idx(MSR_AMD64_IBSDCPHYSAD)];
> +		if (!phys_addr) {
> +			throttle = perf_event_account_interrupt(event);
> +			goto out;
> +		}
> +	}

[Severity: Medium]
When an ibs_op event is opened in frequency mode (event->attr.freq == 1, the
default for perf record when -c is not specified) with phy_addr_only and
PERF_SAMPLE_PHYS_ADDR, can jumping to out after perf_event_account_interrupt()
in perf_ibs_handle_irq() bypass the minimum period clamp and program a
hardware period below perf_ibs->min_period?

Inside perf_event_account_interrupt() -> __perf_event_account_interrupt() ->
perf_adjust_period(), hwc->sample_period is recalculated and only clamped to a
lower bound of 1. Because goto out jumps past the minimum period check at the
end of perf_ibs_handle_irq():

arch/x86/events/amd/ibs.c:perf_ibs_handle_irq() {
	...
	if (event->attr.freq && hwc->sample_period < perf_ibs->min_period)
		hwc->sample_period = perf_ibs->min_period;

out:
	if (!throttle) {
		perf_ibs_enable_event(perf_ibs, hwc, period >> 4);
	...
}

hwc->sample_period can drop below perf_ibs->min_period (0x90 for ibs_op). On
the subsequent IBS NMI, perf_ibs_set_period() -> perf_event_set_period()
computes the next hardware period from hwc->sample_period:

arch/x86/events/amd/ibs.c:perf_event_set_period() {
	...
	if (unlikely(left < (s64)min)) {
		left += period;
		local64_set(&hwc->period_left, left);
		hwc->last_period = period;
		overflow = 1;
	}
	...
}

When left <= 0 and period < min, left += period results in a hardware period
smaller than perf_ibs->min_period (or 0 after shifting by 4) being written to
MSR_AMD64_IBSOPCTL, which commit fa5d0a824e3bbd ("perf/x86/amd/ibs: Fix
 ->min_period violation") notes has undefined hardware behavior.

Should the min_period clamp in perf_ibs_handle_irq() be moved after the out:
label (or applied before enabling the event) so that discarded samples in
frequency mode also respect perf_ibs->min_period?

[Severity: Medium]
Could samples with phys_addr == 0 still slip through this filter in
perf_ibs_handle_irq() if neither op_data3.ld_op nor op_data3.st_op is set?

In arch/x86/events/amd/ibs.c, perf_ibs_parse_ld_st_data() checks the memory
operation type before reading MSR_AMD64_IBSDCPHYSAD:

arch/x86/events/amd/ibs.c:perf_ibs_parse_ld_st_data() {
	...
	perf_ibs_get_mem_op(&op_data3, data);
	if (data->data_src.mem_op != PERF_MEM_OP_LOAD &&
	    data->data_src.mem_op != PERF_MEM_OP_STORE)
		return;
	...
	if (sample_type & PERF_SAMPLE_PHYS_ADDR && op_data3.dc_phy_addr_valid) {
		data->phys_addr = ibs_data->regs[ibs_op_msr_idx(MSR_AMD64_IBSDCPHYSAD)];
		data->sample_flags |= PERF_SAMPLE_PHYS_ADDR;
	}
}

If an op sample has op_data3.dc_phy_addr_valid == 1 and a non-zero
MSR_AMD64_IBSDCPHYSAD while both op_data3.ld_op and op_data3.st_op are 0 (for
instance, commit f04a8a7649f9b2 ("perf amd ibs: Suppress bogus TlbRefillLat
and DCPhysAd on Zen4+") notes that Zen4+ hardware can set dc_phy_addr_valid
with a bogus non-zero IbsDCPhysAd when dc_lin_addr_valid is 0), the filter in
perf_ibs_handle_irq() will let the sample pass, but perf_ibs_parse_ld_st_data()
will return early without setting PERF_SAMPLE_PHYS_ADDR in data->sample_flags.

When perf_event_overflow() then calls perf_prepare_sample() in
kernel/events/core.c:

kernel/events/core.c:perf_prepare_sample() {
	...
	if (filtered_sample_type & PERF_SAMPLE_ADDR) {
		data->addr = 0;
		data->sample_flags |= PERF_SAMPLE_ADDR;
	}
	...
	if (filtered_sample_type & PERF_SAMPLE_PHYS_ADDR) {
		data->phys_addr = perf_virt_to_phys(data->addr);
		data->sample_flags |= PERF_SAMPLE_PHYS_ADDR;
	}
}

perf_prepare_sample() will fall back to perf_virt_to_phys(0) and emit a sample
with data->phys_addr = 0.

Should the perf_ibs_phy_addr_only_event() check in perf_ibs_handle_irq() also
require (op_data3.ld_op || op_data3.st_op), matching perf_ibs_ldlat_event()
and perf_ibs_parse_ld_st_data()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102121324.371157-1-huangsj@hygon.cn?part=1