Re: [Patch v3 15/22] perf/x86/intel: Support SSP register capturing for arch-PEBS

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

On 4/15/2025 10:07 PM, Peter Zijlstra wrote:
> On Tue, Apr 15, 2025 at 11:44:21AM +0000, Dapeng Mi wrote:
>> Arch-PEBS supports to capture shadow stack pointer (SSP) register in GPR
>> group. This patch supports to capture and output SSP register at
>> interrupt or user space, but capturing SSP at user space requires
>> 'exclude_kernel' attribute must be set. That avoids kernel space SSP
>> register is captured unintentionally.
>>
>> Signed-off-by: Dapeng Mi <dapeng1.mi@linux.intel.com>
>> ---
>>  arch/x86/events/core.c                | 15 +++++++++++++++
>>  arch/x86/events/intel/core.c          |  3 ++-
>>  arch/x86/events/intel/ds.c            |  9 +++++++--
>>  arch/x86/events/perf_event.h          |  4 ++++
>>  arch/x86/include/asm/perf_event.h     |  1 +
>>  arch/x86/include/uapi/asm/perf_regs.h |  4 +++-
>>  arch/x86/kernel/perf_regs.c           |  7 +++++++
>>  7 files changed, 39 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
>> index 9c205a8a4fa6..0ccbe8385c7f 100644
>> --- a/arch/x86/events/core.c
>> +++ b/arch/x86/events/core.c
>> @@ -650,6 +650,21 @@ int x86_pmu_hw_config(struct perf_event *event)
>>  			return -EINVAL;
>>  	}
>>  
>> +	if (unlikely(event->attr.sample_regs_user & BIT_ULL(PERF_REG_X86_SSP))) {
>> +		/* Only arch-PEBS supports to capture SSP register. */
>> +		if (!x86_pmu.arch_pebs || !event->attr.precise_ip)
>> +			return -EINVAL;
>> +		/* Only user space is allowed to capture. */
>> +		if (!event->attr.exclude_kernel)
>> +			return -EINVAL;
>> +	}
> We should be able to support this for !PEBS samples by reading the MSR
> just fine, no?

Yes, It can be supported for !PEBS samples. I ever hesitated whether to add
support to capture SSP for !PEBS. Since there is a latency between counter
overflowing and SW reading SSP, the captured SSP may be inaccurate for
!PEBS samples. This makes it not so valuable considering we already support
it for PEBS sampling.

Anyway, we can support it in next version if it's necessary.


>
> ISTR making a similar comment last time.