[PATCH 1/2] arch/powerpc/perf: Check the instruction type before creating sample with perf_mem_data_src

[PATCH 1/2] arch/powerpc/perf: Check the instruction type before creating sample with perf_mem_data_src

[Athira Rajeev]

perf mem report aborts as below sometimes (during some corner
case) in powerpc:

   # ./perf mem report 1>out
   *** stack smashing detected ***: terminated
   Aborted (core dumped)

The backtrace is as below:
   __pthread_kill_implementation ()
   raise ()
   abort ()
   __libc_message
   __fortify_fail
   __stack_chk_fail
   hist_entry.lvl_snprintf
   __sort__hpp_entry
   __hist_entry__snprintf
   hists.fprintf
   cmd_report
   cmd_mem

Snippet of code which triggers the issue
from tools/perf/util/sort.c

   static int hist_entry__lvl_snprintf(struct hist_entry *he, char *bf,
                                    size_t size, unsigned int width)
   {
        char out[64];

        perf_mem__lvl_scnprintf(out, sizeof(out), he->mem_info);
        return repsep_snprintf(bf, size, "%-*s", width, out);
   }

The value of "out" is filled from perf_mem_data_src value.
Debugging this further showed that for some corner cases, the
value of "data_src" was pointing to wrong value. This resulted
in bigger size of string and causing stack check fail.

The perf mem data source values are captured in the sample via
isa207_get_mem_data_src function. The initial check is to fetch
the type of sampled instruction. If the type of instruction is
not valid (not a load/store instruction), the function returns.

Since 'commit e16fd7f2cb1a ("perf: Use sample_flags for data_src")',
data_src field is not initialized by the perf_sample_data_init()
function. If the PMU driver doesn't set the data_src value to zero if
type is not valid, this will result in uninitailised value for data_src.
The uninitailised value of data_src resulted in stack check fail
followed by abort for "perf mem report".

When requesting for data source information in the sample, the
instruction type is expected to be load or store instruction.
In ISA v3.0, due to hardware limitation, there are corner cases
where the instruction type other than load or store is observed.
In ISA v3.0 and before values "0" and "7" are considered reserved.
In ISA v3.1, value "7" has been used to indicate "larx/stcx".
Drop the sample if instruction type has reserved values for this
field with a ISA version check. Initialize data_src to zero in
isa207_get_mem_data_src if the instruction type is not load/store.

Reported-by: Disha Goel <disgoel@linux.vnet.ibm.com>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 arch/powerpc/perf/core-book3s.c   | 17 +++++++++++++++++
 arch/powerpc/perf/isa207-common.c |  4 +++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 2b79171ee185..ee206fe718a4 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -22,6 +22,7 @@
 
 #ifdef CONFIG_PPC64
 #include "internal.h"
+#include "isa207-common.h"
 #endif
 
 #define BHRB_MAX_ENTRIES	32
@@ -2290,6 +2291,22 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
 	    is_kernel_addr(mfspr(SPRN_SIAR)))
 		record = 0;
 
+	/*
+	 * SIER[46-48] presents instruction type of the sampled instruction.
+	 * In ISA v3.0 and before values "0" and "7" are considered reserved.
+	 * In ISA v3.1, value "7" has been used to indicate "larx/stcx".
+	 * Drop the sample if "type" has reserved values for this field with a
+	 * ISA version check.
+	 */
+	if (event->attr.sample_type & PERF_SAMPLE_DATA_SRC &&
+			ppmu->get_mem_data_src) {
+		val = (regs->dar & ISA207_SIER_TYPE_MASK) >> ISA207_SIER_TYPE_SHIFT;
+		if (val == 0 || (val == 7 && !cpu_has_feature(CPU_FTR_ARCH_31))) {
+			record = 0;
+			atomic64_inc(&event->lost_samples);
+		}
+	}
+
 	/*
 	 * Finally record data if requested.
 	 */
diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
index 56301b2bc8ae..031a2b63c171 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -321,8 +321,10 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
 
 	sier = mfspr(SPRN_SIER);
 	val = (sier & ISA207_SIER_TYPE_MASK) >> ISA207_SIER_TYPE_SHIFT;
-	if (val != 1 && val != 2 && !(val == 7 && cpu_has_feature(CPU_FTR_ARCH_31)))
+	if (val != 1 && val != 2 && !(val == 7 && cpu_has_feature(CPU_FTR_ARCH_31))) {
+		dsrc->val = 0;
 		return;
+	}
 
 	idx = (sier & ISA207_SIER_LDST_MASK) >> ISA207_SIER_LDST_SHIFT;
 	sub_idx = (sier & ISA207_SIER_DATA_SRC_MASK) >> ISA207_SIER_DATA_SRC_SHIFT;
-- 
2.43.5

[PATCH 2/2] arch/powerpc/perf: Update get_mem_data_src function to use saved values of sier and mmcra regs

[Athira Rajeev]

During performance monitor interrupt handling, the regs are setup using
perf_read_regs function. Here some of the pt_regs fields is overloaded.
Samples Instruction Event Register (SIER) is loaded into pt_regs,
overloading regs->dar. And regs->dsisr to store MMCRA (Monitor Mode
Control Register A) so that we only need to read these once on each
interrupt.

Update the isa207_get_mem_data_src function to use regs->dar instead of
reading from SPRN_SIER again. Also use regs->dsisr to read the mmcra
value

Reported-by: Disha Goel <disgoel@linux.vnet.ibm.com>
Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 arch/powerpc/perf/isa207-common.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
index 031a2b63c171..2b3547fdba4a 100644
--- a/arch/powerpc/perf/isa207-common.c
+++ b/arch/powerpc/perf/isa207-common.c
@@ -319,7 +319,13 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
 		return;
 	}
 
-	sier = mfspr(SPRN_SIER);
+	/*
+	 * Use regs-dar for SPRN_SIER which is saved
+	 * during perf_read_regs at the beginning
+	 * of the PMU interrupt handler to avoid multiple
+	 * reads of SPRN_SIER
+	 */
+	sier = regs->dar;
 	val = (sier & ISA207_SIER_TYPE_MASK) >> ISA207_SIER_TYPE_SHIFT;
 	if (val != 1 && val != 2 && !(val == 7 && cpu_has_feature(CPU_FTR_ARCH_31))) {
 		dsrc->val = 0;
@@ -340,8 +346,12 @@ void isa207_get_mem_data_src(union perf_mem_data_src *dsrc, u32 flags,
 		 * to determine the exact instruction type. If the sampling
 		 * criteria is neither load or store, set the type as default
 		 * to NA.
+		 *
+		 * Use regs->dsisr for MMCRA which is saved during perf_read_regs
+		 * at the beginning of the PMU interrupt handler to avoid
+		 * multiple reads of SPRN_MMCRA
 		 */
-		mmcra = mfspr(SPRN_MMCRA);
+		mmcra = regs->dsisr;
 
 		op_type = (mmcra >> MMCRA_SAMP_ELIG_SHIFT) & MMCRA_SAMP_ELIG_MASK;
 		switch (op_type) {
-- 
2.43.5

Re: [PATCH 1/2] arch/powerpc/perf: Check the instruction type before creating sample with perf_mem_data_src

[kernel test robot]

Hi Athira,

kernel test robot noticed the following build errors:

[auto build test ERROR on powerpc/next]
[also build test ERROR on powerpc/fixes linus/master v6.13-rc7 next-20250117]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Athira-Rajeev/arch-powerpc-perf-Update-get_mem_data_src-function-to-use-saved-values-of-sier-and-mmcra-regs/20250113-143059
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
patch link:    https://lore.kernel.org/r/20250113062818.33187-1-atrajeev%40linux.vnet.ibm.com
patch subject: [PATCH 1/2] arch/powerpc/perf: Check the instruction type before creating sample with perf_mem_data_src
config: powerpc-gamecube_defconfig (https://download.01.org/0day-ci/archive/20250118/202501180825.LpRG6wYe-lkp@intel.com/config)
compiler: clang version 16.0.6 (https://github.com/llvm/llvm-project 7cbf1a2591520c2491aa35339f227775f4d3adf6)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250118/202501180825.LpRG6wYe-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501180825.LpRG6wYe-lkp@intel.com/

All errors (new ones prefixed by >>):

>> arch/powerpc/perf/core-book3s.c:2303:22: error: use of undeclared identifier 'ISA207_SIER_TYPE_MASK'
                   val = (regs->dar & ISA207_SIER_TYPE_MASK) >> ISA207_SIER_TYPE_SHIFT;
                                      ^
>> arch/powerpc/perf/core-book3s.c:2303:48: error: use of undeclared identifier 'ISA207_SIER_TYPE_SHIFT'
                   val = (regs->dar & ISA207_SIER_TYPE_MASK) >> ISA207_SIER_TYPE_SHIFT;
                                                                ^
   2 errors generated.


vim +/ISA207_SIER_TYPE_MASK +2303 arch/powerpc/perf/core-book3s.c

  2222	
  2223	#define PERF_SAMPLE_ADDR_TYPE  (PERF_SAMPLE_ADDR |		\
  2224					PERF_SAMPLE_PHYS_ADDR |		\
  2225					PERF_SAMPLE_DATA_PAGE_SIZE)
  2226	/*
  2227	 * A counter has overflowed; update its count and record
  2228	 * things if requested.  Note that interrupts are hard-disabled
  2229	 * here so there is no possibility of being interrupted.
  2230	 */
  2231	static void record_and_restart(struct perf_event *event, unsigned long val,
  2232				       struct pt_regs *regs)
  2233	{
  2234		u64 period = event->hw.sample_period;
  2235		s64 prev, delta, left;
  2236		int record = 0;
  2237	
  2238		if (event->hw.state & PERF_HES_STOPPED) {
  2239			write_pmc(event->hw.idx, 0);
  2240			return;
  2241		}
  2242	
  2243		/* we don't have to worry about interrupts here */
  2244		prev = local64_read(&event->hw.prev_count);
  2245		delta = check_and_compute_delta(prev, val);
  2246		local64_add(delta, &event->count);
  2247	
  2248		/*
  2249		 * See if the total period for this event has expired,
  2250		 * and update for the next period.
  2251		 */
  2252		val = 0;
  2253		left = local64_read(&event->hw.period_left) - delta;
  2254		if (delta == 0)
  2255			left++;
  2256		if (period) {
  2257			if (left <= 0) {
  2258				left += period;
  2259				if (left <= 0)
  2260					left = period;
  2261	
  2262				/*
  2263				 * If address is not requested in the sample via
  2264				 * PERF_SAMPLE_IP, just record that sample irrespective
  2265				 * of SIAR valid check.
  2266				 */
  2267				if (event->attr.sample_type & PERF_SAMPLE_IP)
  2268					record = siar_valid(regs);
  2269				else
  2270					record = 1;
  2271	
  2272				event->hw.last_period = event->hw.sample_period;
  2273			}
  2274			if (left < 0x80000000LL)
  2275				val = 0x80000000LL - left;
  2276		}
  2277	
  2278		write_pmc(event->hw.idx, val);
  2279		local64_set(&event->hw.prev_count, val);
  2280		local64_set(&event->hw.period_left, left);
  2281		perf_event_update_userpage(event);
  2282	
  2283		/*
  2284		 * Due to hardware limitation, sometimes SIAR could sample a kernel
  2285		 * address even when freeze on supervisor state (kernel) is set in
  2286		 * MMCR2. Check attr.exclude_kernel and address to drop the sample in
  2287		 * these cases.
  2288		 */
  2289		if (event->attr.exclude_kernel &&
  2290		    (event->attr.sample_type & PERF_SAMPLE_IP) &&
  2291		    is_kernel_addr(mfspr(SPRN_SIAR)))
  2292			record = 0;
  2293	
  2294		/*
  2295		 * SIER[46-48] presents instruction type of the sampled instruction.
  2296		 * In ISA v3.0 and before values "0" and "7" are considered reserved.
  2297		 * In ISA v3.1, value "7" has been used to indicate "larx/stcx".
  2298		 * Drop the sample if "type" has reserved values for this field with a
  2299		 * ISA version check.
  2300		 */
  2301		if (event->attr.sample_type & PERF_SAMPLE_DATA_SRC &&
  2302				ppmu->get_mem_data_src) {
> 2303			val = (regs->dar & ISA207_SIER_TYPE_MASK) >> ISA207_SIER_TYPE_SHIFT;
  2304			if (val == 0 || (val == 7 && !cpu_has_feature(CPU_FTR_ARCH_31))) {
  2305				record = 0;
  2306				atomic64_inc(&event->lost_samples);
  2307			}
  2308		}
  2309	
  2310		/*
  2311		 * Finally record data if requested.
  2312		 */
  2313		if (record) {
  2314			struct perf_sample_data data;
  2315	
  2316			perf_sample_data_init(&data, ~0ULL, event->hw.last_period);
  2317	
  2318			if (event->attr.sample_type & PERF_SAMPLE_ADDR_TYPE)
  2319				perf_get_data_addr(event, regs, &data.addr);
  2320	
  2321			if (event->attr.sample_type & PERF_SAMPLE_BRANCH_STACK) {
  2322				struct cpu_hw_events *cpuhw;
  2323				cpuhw = this_cpu_ptr(&cpu_hw_events);
  2324				power_pmu_bhrb_read(event, cpuhw);
  2325				perf_sample_save_brstack(&data, event, &cpuhw->bhrb_stack, NULL);
  2326			}
  2327	
  2328			if (event->attr.sample_type & PERF_SAMPLE_DATA_SRC &&
  2329							ppmu->get_mem_data_src) {
  2330				ppmu->get_mem_data_src(&data.data_src, ppmu->flags, regs);
  2331				data.sample_flags |= PERF_SAMPLE_DATA_SRC;
  2332			}
  2333	
  2334			if (event->attr.sample_type & PERF_SAMPLE_WEIGHT_TYPE &&
  2335							ppmu->get_mem_weight) {
  2336				ppmu->get_mem_weight(&data.weight.full, event->attr.sample_type);
  2337				data.sample_flags |= PERF_SAMPLE_WEIGHT_TYPE;
  2338			}
  2339			if (perf_event_overflow(event, &data, regs))
  2340				power_pmu_stop(event, 0);
  2341		} else if (period) {
  2342			/* Account for interrupt in case of invalid SIAR */
  2343			if (perf_event_account_interrupt(event))
  2344				power_pmu_stop(event, 0);
  2345		}
  2346	}
  2347	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki