Re: [RFC/PATCH 4/4] perf/x86: Relax privilege filter restriction on AMD IBS

[Ravi Bangoria <ravi.bangoria@amd.com>]

Hi Namhyung,

On 03-Sep-24 10:04 PM, Namhyung Kim wrote:
> On Tue, Sep 3, 2024 at 1:55 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>
>> On Mon, Sep 02, 2024 at 10:30:19AM -0700, Namhyung Kim wrote:
>>> On Mon, Sep 2, 2024 at 2:12 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>>>
>>>> On Fri, Aug 30, 2024 at 04:29:10PM -0700, Namhyung Kim wrote:
>>>>> While IBS is available for per-thread profiling, still regular users
>>>>> cannot open an event due to the default paranoid setting (2) which
>>>>> doesn't allow unprivileged users to get kernel samples.  That means
>>>>> it needs to set exclude_kernel bit in the attribute but IBS driver
>>>>> would reject it since it has PERF_PMU_CAP_NO_EXCLUDE.  This is not what
>>>>> we want and I've been getting requests to fix this issue.
>>>>>
>>>>> This should be done in the hardware, but until we get the HW fix we may
>>>>> allow exclude_{kernel,user} in the attribute and silently drop the
>>>>> samples in the PMU IRQ handler.  It won't guarantee the sampling
>>>>> frequency or even it'd miss some with fixed period too.  Not ideal,
>>>>> but that'd still be helpful to regular users.
>>>>
>>>> Urgh.... this is really rather bad. And I'm sure a bunch of people are
>>>> going to be spending a lot of time trying to figure out why their
>>>> results don't make sense.
>>>
>>> I agree it can be confusing but there are use cases where regular users
>>> want IBS information like memory data source, data address and so on.
>>
>> Sure, but I'm a bit worried about users not being aware of this
>> trickery. This makes IBS events that have exclude_kernel=1 behave
>> significantly different from those that don't have it.
>>
>> At the very least you should kill the IBS forward in amd_pmu_hw_config()
>> when this is active. But perhaps we should also emit a one time
>> KERN_INFO message when such an event gets created?
> 
> What about adding a pseudo config or format for a special event that
> allows exclude_kernel=1 and drops samples?  Maybe we could use
> attr.config = 0xffffffff or attr.config=1 or something.

attr.config is directly mapped to bitmaps of IBS_OP_CTL. We can't abuse
reserved bits also since they might become valid control bits in future.

Maybe use config2 or config3 which are ignored by IBS driver currently.

Thanks,
Ravi