On 2024-04-19 2:22 a.m., Ian Rogers wrote:
>>> + /* Simple modifiers copied to the evsel. */
>>> + if (mod.precise) {
>>> + u8 precise = evsel->core.attr.precise_ip + mod.precise;
>>> + /*
>>> + * precise ip:
>>> + *
>>> + * 0 - SAMPLE_IP can have arbitrary skid
>>> + * 1 - SAMPLE_IP must have constant skid
>>> + * 2 - SAMPLE_IP requested to have 0 skid
>>> + * 3 - SAMPLE_IP must have 0 skid
>>> + *
>>> + * See also PERF_RECORD_MISC_EXACT_IP
>>> + */
>>> + if (precise > 3) {
>> The pmu_max_precise() should return the max precise the current kernel
>> supports. It checks the /sys/devices/cpu/caps/max_precise.
>>
>> I think we should use that value rather than hard code it to 3.
> I'll add an extra patch to do that. I'm a bit concerned it may break
> event parsing on platforms not supporting max_precise of 3.
The kernel already rejects the precise_ip > max_precise (using the same
x86_pmu_max_precise()). It should be fine to apply the same logic in the
tool.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/events/core.c#n566
Will the extra patch be sent separately?
Thanks,
Kan
On Fri, Apr 19, 2024 at 6:20 AM Liang, Kan <[email protected]> wrote:
>
>
>
> On 2024-04-19 2:22 a.m., Ian Rogers wrote:
> >>> + /* Simple modifiers copied to the evsel. */
> >>> + if (mod.precise) {
> >>> + u8 precise = evsel->core.attr.precise_ip + mod.precise;
> >>> + /*
> >>> + * precise ip:
> >>> + *
> >>> + * 0 - SAMPLE_IP can have arbitrary skid
> >>> + * 1 - SAMPLE_IP must have constant skid
> >>> + * 2 - SAMPLE_IP requested to have 0 skid
> >>> + * 3 - SAMPLE_IP must have 0 skid
> >>> + *
> >>> + * See also PERF_RECORD_MISC_EXACT_IP
> >>> + */
> >>> + if (precise > 3) {
> >> The pmu_max_precise() should return the max precise the current kernel
> >> supports. It checks the /sys/devices/cpu/caps/max_precise.
> >>
> >> I think we should use that value rather than hard code it to 3.
> > I'll add an extra patch to do that. I'm a bit concerned it may break
> > event parsing on platforms not supporting max_precise of 3.
>
> The kernel already rejects the precise_ip > max_precise (using the same
> x86_pmu_max_precise()). It should be fine to apply the same logic in the
> tool.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/events/core.c#n566
>
> Will the extra patch be sent separately?
Let's do it separately. I'm concerned about the behavior on AMD (and
possibly similar architectures) where certain events support precision
like cycles, as they detour to the IBS PMU, but not all events support
it. The max_precise should reflect that AMD's Zen core PMU does
support precision as a consequence of detouring to IBS, but maybe
things in sysfs aren't set up correctly.
Thanks,
Ian
> Thanks,
> Kan
On 2024-04-24 11:18 a.m., Ian Rogers wrote:
> On Fri, Apr 19, 2024 at 6:20 AM Liang, Kan <[email protected]> wrote:
>>
>>
>>
>> On 2024-04-19 2:22 a.m., Ian Rogers wrote:
>>>>> + /* Simple modifiers copied to the evsel. */
>>>>> + if (mod.precise) {
>>>>> + u8 precise = evsel->core.attr.precise_ip + mod.precise;
>>>>> + /*
>>>>> + * precise ip:
>>>>> + *
>>>>> + * 0 - SAMPLE_IP can have arbitrary skid
>>>>> + * 1 - SAMPLE_IP must have constant skid
>>>>> + * 2 - SAMPLE_IP requested to have 0 skid
>>>>> + * 3 - SAMPLE_IP must have 0 skid
>>>>> + *
>>>>> + * See also PERF_RECORD_MISC_EXACT_IP
>>>>> + */
>>>>> + if (precise > 3) {
>>>> The pmu_max_precise() should return the max precise the current kernel
>>>> supports. It checks the /sys/devices/cpu/caps/max_precise.
>>>>
>>>> I think we should use that value rather than hard code it to 3.
>>> I'll add an extra patch to do that. I'm a bit concerned it may break
>>> event parsing on platforms not supporting max_precise of 3.
>>
>> The kernel already rejects the precise_ip > max_precise (using the same
>> x86_pmu_max_precise()). It should be fine to apply the same logic in the
>> tool.
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/events/core.c#n566
>>
>> Will the extra patch be sent separately?
>
> Let's do it separately. I'm concerned about the behavior on AMD (and
> possibly similar architectures) where certain events support precision
> like cycles, as they detour to the IBS PMU, but not all events support
> it. The max_precise should reflect that AMD's Zen core PMU does
> support precision as a consequence of detouring to IBS, but maybe
> things in sysfs aren't set up correctly.
>
The x86_pmu_max_precise() is a generic function for X86. It should apply
to AMD as well.
A separate patch looks good to me.
Thanks,
Kan