Re: [PATCH] perf/x86/amd: check event before enable to avoid GPF

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

>>> Events can be deleted and the entry can be NULL.
>> Can you please also explain "how".
> It looks like x86_pmu_stop() is clearing the bit in active_mask and setting the events entry to NULL (and doing it in the correct order) for the same events index that amd_pmu_enable_all() is trying to enable.

Right, so how can it disable the exact same event which it's about to
enable?

If it's happening because of some race condition, we need to find and fix
it. For ex: https://git.kernel.org/torvalds/c/baa014b9543c8

>>> Check event for NULL in amd_pmu_enable_all() before enable to avoid a GPF.
>>> This appears to be an AMD only issue.
>>>
>>> Syzkaller reported a GPF in amd_pmu_enable_all.
>> Can you please provide a bug report link? Also, any reproducer?
> The Syzkaller reproducer can be found in this link:
> https://lore.kernel.org/netdev/CAMt6jhyec7-TSFpr3F+_ikjpu39WV3jnCBBGwpzpBrPx55w20g@mail.gmail.com/T/#u
>>
>>> @@ -760,7 +760,8 @@ static void amd_pmu_enable_all(int added)
>>>           if (!test_bit(idx, cpuc->active_mask))
>>>               continue;
>>>   -        amd_pmu_enable_event(cpuc->events[idx]);
>>> +        if (cpuc->events[idx])
>>> +            amd_pmu_enable_event(cpuc->events[idx]);
>> What if cpuc->events[idx] becomes NULL after if (cpuc->events[idx]) but
>> before amd_pmu_enable_event(cpuc->events[idx])?
> Good question, but the crash has not reproduced with the proposed fix in hours of testing. It usually reproduces within minutes without the fix.

It does not reproduce with the patch because that if() condition makes the
race window smaller?

Thanks,
Ravi