Received-SPF: pass (google.com: domain of linux-kernel+bounces-206651-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <36e86320-1a43-45d4-b25e-a5d60857f4b2@oracle.com>
Date: Fri, 7 Jun 2024 15:13:53 -0400
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] perf/x86/amd: check event before enable to avoid GPF
To: Ravi Bangoria <ravi.bangoria@amd.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
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>
 <17bfa7c1-43b7-4b25-a8cf-eb6d3797df5a@amd.com>
Content-Language: en-US
From: George Kennedy <george.kennedy@oracle.com>
In-Reply-To: <17bfa7c1-43b7-4b25-a8cf-eb6d3797df5a@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
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?RkNYd29DVm9xQkNMVUhIWHZwNEkzZ1l1RFNyOWd5VFE2UVFpSWlOdC8xVUJy?=
 =?utf-8?B?R2hTaXhIWlZZNXZBbzFoWG91SWFmSko3bkswL20yd052NDRaN2ZlZWNicUVB?=
 =?utf-8?B?K0NEQjdvc3hFRFY2VXVOV0EwdmZwK2Y5U3NlY1lJZzRaWW1hTU1NVWNqL3dM?=
 =?utf-8?B?N1orc0dnQ3dyS2xHb1VWMUlDNjBvQ3N0U1J5YUhpOE8yZW85djJwK3JVaTZU?=
 =?utf-8?B?VVkxYWhsbHE5REk3LzFMd0UyTXFHNVFJMXhpbld2Um82ay95TUhaSXVWZ2s0?=
 =?utf-8?B?cUt3WjVvdUJqSHZpbWo3UzBXREVIMXlBeHlVd0duYVpvSE9yU29BZlRxY0x4?=
 =?utf-8?B?eWJoZjNoSVByR08wd3FuS2JaME1ocHRmbmVTZjVVcDkvbHNESGZQQ1BkeE4w?=
 =?utf-8?B?T2Q5VjNJOTlzMlg2LzJYT2JKWnRCQS9zRGkvdW5lTnJSNWh2dkpEb3FTYmxY?=
 =?utf-8?B?UVZyT3kySFdrRXJGQTM3TnAzQkVXNksrTGxGVkM3NTlUSHNSMEVqVmx2cUd0?=
 =?utf-8?B?TWNKMDlJYUFCbFZqOFloM0NUZTZHVGh4OEtQbDBHQmxFUUJwK29KaXBHVERm?=
 =?utf-8?B?Q2ovVU1KYjNLVzRrZVlQeFJUVkQva3RrK2FFTFFnVTZYRVlzWU9ZWm9XdGEr?=
 =?utf-8?B?enNUUEVqNTNXZVZVdWlGZ1ZEQ3ZoU2k0MlJ6N2pTRk8xbGwvOXlCNU5rRmQ2?=
 =?utf-8?B?SkhRSHpvbWlIemNXWURWYkRjTkoyL1Q4cWFUZm1zOU5mdDRDNU0rVHp1alNH?=
 =?utf-8?B?Q21Qd1h1WUNIRkg1RlU5Q01GdVBpTWYzaHEyeWs4d2x2SnNmZEhHcnpkdVY0?=
 =?utf-8?B?SVVPOFVzRFNPaFVDdXo2VXF6Y0ZYREdnREtVMGx3eVFmMUtuYmZXK3V5ZjE2?=
 =?utf-8?B?alk2SncvRFRkZGxHTGNSbC9XdE1PQVJxNUdjalU0ZW84a3dZQVl6VjQycTlm?=
 =?utf-8?B?dXByWDRLOVRNYUVOd2xFN0Y1KzBqRHloMjRubDVmMTFoKzBwUzFNVCtCdVNj?=
 =?utf-8?B?ek9NZFZCUTE3emFpMWxSVDhka0FaS0VQL0lhVWtyTnpLWjhnYzVJNUNmQmRo?=
 =?utf-8?B?ampjMVRqZzNDNFhTOWNsZVd6VGRHRFhCVmJRbzJtTWJ4Tk9TaG8zdlJJcDcr?=
 =?utf-8?B?RGFmbE9nbVNIc3cwNmZ1RTJVK1VWdWcveFpOa1Rna2c0cEJTaVdURWlnVkNo?=
 =?utf-8?B?c0tYT2VkUERibENTQTRkc1R0dUZFK1NlaEZMQ3NWazE3R1ZvOVRTOVQyTWJN?=
 =?utf-8?B?NUR4WGYwSFM5cnY3M3JFc3I1bnNrQ2thVG5CcGtGSW1nZEQ3Mkc0TCtzK1Rl?=
 =?utf-8?B?djNjc2N3K1VQcGl3YmVMT1k0bGRHbDRjK3VKZXF5SXJVSzNrZ3Nlek8rbHdB?=
 =?utf-8?B?V25id2VSNUQ1ejFuWUFhNkszNXE4WEFDZUYwQmNaN0w2YlFzQysxc0FueXlV?=
 =?utf-8?B?UUYreXlhd3BIRURnbjN1UHN2dTkzeWlCS2sxSDZkZHUvMjZsbkIxMTRsdGVm?=
 =?utf-8?B?YzR1TUoybzdKWldCVEMxVm9qR2dzQVlSNiszTGV5WTNOMGM2MDJUOGNPejJW?=
 =?utf-8?B?Zzl6a05ZYVorR1Yrd3c0VE15cmlsY0JxMnZSN09OTEtvS2FDSmVoblA5T0lk?=
 =?utf-8?B?TnVHeDFNbGgwWXd2cWNCbkdwL2xnMndoSE5PZUtRUHRIY1VVZm5FZmxsbEFj?=
 =?utf-8?B?QnI4aVdpTEdVcUxUdFVITWRiNU5GcXArM05xcXBtOENJYmtxbnM3ajR2Zmda?=
 =?utf-8?B?a1pjZXQyRUlSNElFTE5sRzJvQzJiTEJmUlQxV2FFK2VyZWhHRUFRMnFNQmU5?=
 =?utf-8?B?Q2VKamtWNVVBVlI1VUFqVm1JdDdRUG1JWXJFa2FTTUFHVWU3MHlEYVRNK2Fp?=
 =?utf-8?B?bERRMFJqTHNmMTNaZjh1MzNZMXhwVTluTXdaejYrd2Q1bmgwSU1IL2lvQzhX?=
 =?utf-8?B?WFBGRDBveWVNdHlLUUQyZ3NnWndWNmpzMUU4RjVMajJsTU9SSHhaeVN3S0Nr?=
 =?utf-8?B?bDVNS2kvV2pQcVowSDE0WkpvQjJERDdmWmh3MzlWK1kvR3RJU2gwcVZCZjA4?=
 =?utf-8?B?dW1nb0pzL2xrcGtySU9jMEw3QVh6U3dwS1YxSXVvQi9IRC9tcXo0YUJRSmFZ?=
 =?utf-8?B?U2hDdXY3NHl5SW1scmRsd1o3SjlOeVNrbmNEVjBMWVpnMkl5d2hXS21wSThh?=
 =?utf-8?B?WVE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	Xhxrqa1n9VLeiFJD/QhbXdFwxWGlchE9H57+OORHCNyaB2On8RsD2mCFgZJm+FJa31bPkdjWaydBg+wp5EtTO5xW+sAtFDXnfqXO0/oQriIJQlOsOGoN1Y7sMptMPMOUNUU8k16Yt63HmvdVxtKLVs3ek9HJuNNPw2jTPuzsyTi0O2mlMLCwn8BRW5nx33XUmJZRXTrfSAYNRT0j+89bgwhqLRlappofNv8LsIQZvwxBF8H1bCFklrD2SpxxdGwig8DWRjztCLbHjref5o+tY3c1H+cYkZiX2DeoaP/te5+bR5brRFpf3ZpDvNZceCbkJ3XgY2MZNWioj1DgAtGhwOUclflHbXSacLqmMM/fFy8LPybAKN6Vei3499NDJBCd2Bt5VZnkcOBVwjOZPmgVccIQ/Wztj7Ji3iV2qOYDHB8uPkKwMg4zO7KxVF5tZuHVUzEvvgEmmxMV4CkNKENPbqO2eI9GP/cWN1MoieGdKPA3LCisX+nuUcqQXZQYK2qrzZsvA4wAoqc3d1pxS1UlSVJuTvZdN0LiZS/LTN0d2D0SZN1fEorVtM1wAt9gBDrky/w7e1DycAxQDcMgAXFXray2Bicp2jsELg/lH6NxeZ8=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4a4ba7b0-2138-4ab2-a866-08dc8725fa9c
X-MS-Exchange-CrossTenant-AuthSource: PH0PR10MB5895.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Jun 2024 19:13:56.9245
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bsNrzQy9/ht8hGYdH3PT60JhWG1sB/ezhbSjOoU+ivlh2bw808JxAn0xASSjsLyOhmFzdGmZTkuxW7dr/DVm+5rsNV3HQJMlel5D8ydIU9I=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR10MB5751
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.28.16
 definitions=2024-06-07_12,2024-06-06_02,2024-05-17_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 bulkscore=0 spamscore=0
 adultscore=0 suspectscore=0 malwarescore=0 mlxscore=0 phishscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2405010000
 definitions=main-2406070141
X-Proofpoint-ORIG-GUID: lQhCvj_yJrYS5kvRiMakn63qXIzA4LFa
X-Proofpoint-GUID: lQhCvj_yJrYS5kvRiMakn63qXIzA4LFa

Hi Ravi,

On 6/4/2024 9:40 AM, Ravi Bangoria wrote:
>> 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?

Were you able to reproduce the crash on the AMD machine?

Thanks,
George
> Thanks,
> Ravi