Received-SPF: pass (google.com: domain of linux-kernel+bounces-200729-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <17bfa7c1-43b7-4b25-a8cf-eb6d3797df5a@amd.com>
Date: Tue, 4 Jun 2024 19:10:54 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] perf/x86/amd: check event before enable to avoid GPF
To: George Kennedy <george.kennedy@oracle.com>
Cc: harshit.m.mogalapalli@oracle.com, peterz@infradead.org, mingo@redhat.com,
 acme@kernel.org, namhyung@kernel.org, mark.rutland@arm.com,
 alexander.shishkin@linux.intel.com, jolsa@kernel.org, irogers@google.com,
 adrian.hunter@intel.com, kan.liang@linux.intel.com, tglx@linutronix.de,
 bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org, hpa@zytor.com,
 linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
 ravi.bangoria@amd.com
References: <1716990659-2427-1-git-send-email-george.kennedy@oracle.com>
 <75f6aba1-8ed6-4ef8-8811-de40ae40be90@amd.com>
 <1e14ca4b-6e3e-4d57-acec-bc3ee2bed6ed@oracle.com>
 <0494aedf-9759-4427-a7f3-39a91bd5771d@oracle.com>
 <3ec1cb26-c889-45ab-b8cc-92f933a2b5d1@amd.com>
 <7b27a039-ddb9-4beb-a1b4-ba2b74a3043e@oracle.com>
Content-Language: en-US
From: Ravi Bangoria <ravi.bangoria@amd.com>
In-Reply-To: <7b27a039-ddb9-4beb-a1b4-ba2b74a3043e@oracle.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?azJEZXhxUGNhRnovaGZzck5FL28ydEYrVkg2VGJ6N0JKVUlXL0FLbHM5TFpr?=
 =?utf-8?B?QVpHa1VEYXJTRktRUW9CZmdtK2N4c1YxK3dWY1FJczJaZlRmTm1ZUlNwYUpm?=
 =?utf-8?B?QmIrWmNMejdJUEhidjd5cU94SEpHK2lTR0FWa1dkRFl3bGZKaXo3RXgxOVAz?=
 =?utf-8?B?Nno3S0hZcU9JYytrRVVCS0U4Q3BzaU1jYjRNNzlac0ZCSUZrL0RqSUxvaldK?=
 =?utf-8?B?RUR4V3h6Q1pYWG81c0N4QjRyYTl3bGNjR2s0cWNVRFp1bVlIb2kzaExzZ2FO?=
 =?utf-8?B?N1RzRXd5dHFEU2pJZllpMCtkeXZoSEhUc3FSOGRxRkdiVjludzhJZjlmS3k4?=
 =?utf-8?B?VFV1amdaM1VsYXk2T24zbGdtRHZjTDdOTGMwL0N4cDV5eldacGFzdEVLeUFV?=
 =?utf-8?B?MXArem13WDhCd3pXcHErV1V1N1M2a3U0OVhFbmhqVE14d1Q5blh0anB2N1Rh?=
 =?utf-8?B?TXhnL0pZanBWeXRsVVErU3docHU5eWVaOTVPeG5ib0laZHRBYjFVSVpISU5G?=
 =?utf-8?B?OUZDeHdKQXF3c3VWWVoxK0tHdFg2RDFqWFg5Q2dzYld4UExtd0xRYkRFd1RE?=
 =?utf-8?B?N25jSFp1UXErVnQyanV4ZWhmRmlIcVZ3d1hlOERlTER5eTc2N25jTWxJeEVh?=
 =?utf-8?B?R2RiNkFINE1qV0NRV000VGtSaWI0a1BIbVd4TS8yVTN2Nldma1RwenBGNmdN?=
 =?utf-8?B?NG1SYUphUFFrZlBqcXUxWlAzK3h1OHZRZXgyeEowazFFVEdERzQxVVRxVXVI?=
 =?utf-8?B?UWxXNXlSZHBPMGtXRkRKUXVtczRadEtTTU55cVZUS3pmRjV5U1RoT0R5d1Jy?=
 =?utf-8?B?bUZoNkIrTDZMVUJRZ3VSL2RmbVo1a3dRKyttTXpQS2tVUTBiR3JmbFZJWmxh?=
 =?utf-8?B?ZUFWeThYdnNVeTJ1QzExa3JiOVRGYnJEVlE2VzBHNVc0aVJXZDN2amZiQVNM?=
 =?utf-8?B?ekpqWE5wRjg3amw1OGNzWU5oRlE3YmM1MG16eUhveDhBZGR5VEdkSTR0dWUx?=
 =?utf-8?B?VXRibWMvdzZDWVM5a1VaNVdPL05TY0VuTi9Yd1lTQkRZYkl0L0hrZDVmcERa?=
 =?utf-8?B?MWxGdVpkMHNYZmUrRmJUVTc5Vml2RzZVbUZURzVPTGJQaW96QTQ3VDBoOTJt?=
 =?utf-8?B?MmVXQnFoa2ZWNmZQZklWUWdkVVlEVWR4TDZPd0VQRXJyWVd6bEdaZjVpczgy?=
 =?utf-8?B?dHEvOFl3dW11Y3MvUUdjTWxPc2Q5UUhFaDZsVDZYM1U0MHAwaW1qbnJoSnVr?=
 =?utf-8?B?eisrK28vK2xmRy9yY0lORWFXV0lLbEZsOW5PM1lsTzl0MjJUUEV3NTByN0dW?=
 =?utf-8?B?VjgwSG15ZFl1R29GVGxsaTViR1FNdHBPUkJDc2ZLU0tJN2NpUmxZdXJtanUz?=
 =?utf-8?B?L2pBTEhvN1BOMU1DRHRKR2VWcHBHMDJKcG5wS3p0UlVFU2RDUXpWUUZ2TUUw?=
 =?utf-8?B?bGY1VDZxbDdBcUlXZVhNcEpzM0NCcHp1djFHQVN4allhN2NnV1NVbXdoUHBo?=
 =?utf-8?B?Q0ttVjVXV25lcGxTcXJXRkxJZlhUSFYxc0JockNLTnorUHFFNzMzTUtWYllU?=
 =?utf-8?B?cEZlbzYxRE1PeVk5ckNSUWhtV25BcUNaU2tBYkdZNWlybEs2VDNWMGdRdncy?=
 =?utf-8?B?TWVMWjlZM0ZMblB4Wi9yVzEzQVRQRUc0eTFtRXg4c0RkNXk4MnE3bit3N2hR?=
 =?utf-8?B?ZnZHVkV4alh6WlR2eGRuMjFoaWZibFZSN0hSa1VIS1hXRTJWYWVaaGVZK0ow?=
 =?utf-8?B?VEFvV1BRU3Q3WTUwcVpScFRWdlM4ZkN3dnE2aUJZWVlwcXVRUWxvMVJ0VjFE?=
 =?utf-8?B?dkdoRHpZVzUvdEdXR3k1NVBzYjdZT0R4KzVZRFZOMGdiU1RyUnc3aTJhS3Jk?=
 =?utf-8?B?TG1Vd1IxT2VYa01IZVVXQmRhQSs2ajFCYlV0SVpuRFNlZStQUlkyVG1LaW4w?=
 =?utf-8?B?VllMSVJDSVZOdXhEOWJZUTAvN1JMTDJ2cmtacGFCbGFnOVYrZFplT1g2ZzlT?=
 =?utf-8?B?MU5pVVcyRTgwTExadGRCT1ZWODFrU3JGYnNPUXFCRmppZFVQZTRVbXhubCtk?=
 =?utf-8?B?V2FmR2NIallBZlZ5YWFsdkQ2RFZYUFF3ZUdKVWxRK2U3U3hGbDNXVnRJd2xq?=
 =?utf-8?Q?wVA86J8mp6G2jRw/G9pwMpDQ6?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 12321ab3-3a44-4360-5ffd-08dc849bfefd
X-MS-Exchange-CrossTenant-AuthSource: DS0PR12MB6584.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jun 2024 13:41:12.7676
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9P+6RuVBFq6yupOWYrUsAQkRtQigTJ/7LeaeYdFdj8M8pXrP/6Ap8T3Tb4mjkC52Q/CYl1wwVZydW+RyDJhbAg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR12MB5681

> On 6/4/2024 9:16 AM, Ravi Bangoria wrote:
>>>>>> 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.
>>>>>> 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.
>>> Also, a similar fix is done in __intel_pmu_enable_all() in arch/x86/events/intel/core.c except that a WARN_ON_ONCE is done as well.
>>> See: https://elixir.bootlin.com/linux/v6.10-rc1/source/arch/x86/events/intel/core.c#L2256
>> There are subtle differences between Intel and AMD pmu implementation.
>> __intel_pmu_enable_all() enables all event with single WRMSR whereas
>> amd_pmu_enable_all() loops over each PMC and enables it individually.
>>
>> The WARN_ON_ONCE() is important because it will warn about potential
>> sw bug somewhere else.
> We could add a similar WARN_ON_ONCE() to the proposed patch.

Sure, that would help in future. But for current splat, can you please
try to rootcause the underlying race condition?

Thanks,
Ravi