Received-SPF: pass (google.com: domain of linux-kernel+bounces-162453-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <7b68f364-a324-4e2c-87be-19cdef4e3ad2@amd.com>
Date: Mon, 29 Apr 2024 10:36:57 -0400
User-Agent: Mozilla Thunderbird
Cc: yazen.ghannam@amd.com, linux-edac@vger.kernel.org,
 linux-kernel@vger.kernel.org, tony.luck@intel.com, x86@kernel.org,
 Avadhut.Naik@amd.com, John.Allen@amd.com
Subject: Re: [PATCH v2 09/16] x86/mce: Unify AMD THR handler with MCA Polling
Content-Language: en-US
To: Borislav Petkov <bp@alien8.de>
References: <20240404151359.47970-1-yazen.ghannam@amd.com>
 <20240404151359.47970-10-yazen.ghannam@amd.com>
 <20240429134043.GPZi-jWzoVe3bJkyYX@fat_crate.local>
From: Yazen Ghannam <yazen.ghannam@amd.com>
In-Reply-To: <20240429134043.GPZi-jWzoVe3bJkyYX@fat_crate.local>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?MGdPZ2J0TWRLUkNCUS96TldidzEzUkNTMnFFMGhEOWgwUE82RExuTjlXem1L?=
 =?utf-8?B?N2lSa1JIcjM0VC9SQUJLZERmQ1psdVMxSnE2SVI3emVLRHBIODZsSDhLTVRq?=
 =?utf-8?B?RC9kMzdoN1Z6MGZuMUZQTDk3UkxLRWR3SkgvbzlpZGdKNnA2SXNTWVBjK01x?=
 =?utf-8?B?ZFJKdDBZa3g0dDNvUE9WOG9WUWVUaHQrcnRvOUlpdlVERjIvMnlJS1JBMUNW?=
 =?utf-8?B?dU5sV2Z4WkNUQUtmeFpoeHc2Y3I2UmRHQU1PaWRIWkhJYnZJeUxDN0g2V3pK?=
 =?utf-8?B?N2Z3YU10emtVR1ZaWGUyN1hIdFFwNkh2N0w2MUhKSllOemM1NjRaSVd5T1N1?=
 =?utf-8?B?UHhBSVpFUXlsMFVseGxLblBxcG5xMmtLVlMvc3dkVG9YbGxkR280ZWdjdTdL?=
 =?utf-8?B?OWZDa1Z5MFMxWkZrVG9aS1p5T1dvNU9qMVp3SHhta01TeGRycVpVcm96T3Rs?=
 =?utf-8?B?dWJLZTBQbzNqVExUcDZnUXByc2x1U1FjbTJTYzBoQnNUdnJ0d1phRk85ai9Z?=
 =?utf-8?B?dGJtQUxCT29kRmg0eEYwWWV3M0JxL0FpRFRrcDBCd1BBOXNwTUdvQVhMTTBW?=
 =?utf-8?B?OFNUa2UzY25KUm9uRVZSclY1K2hmR0pVUm9iLzhZYWVnT1JzVzdGT2JMUlJE?=
 =?utf-8?B?V0ZSc0VhMkF5bk8xVzJUd2dXeWhVYTRUV0d1bGF1MDhWWk8zekxUT1dpdVpJ?=
 =?utf-8?B?T1loYitvTXo5NTdvWDU0bnVVUDRJRFMvMG03bVlmSDVMaUwwYXlvNjdMNFpi?=
 =?utf-8?B?MWlIcUpNdEJheW83ZUp3WTVLZGoydU1ua1NpT05hTmg3TENkSXkvaWFTWjl3?=
 =?utf-8?B?VUJobktLR21sSU43eDhuRjZWdDdoS2hHVDcvMjArd3RZaTBSWXRmVzJTSGNR?=
 =?utf-8?B?T1EzWlVHRENTc3BwSVdmZHlpdzVJODVteHhpN0JzWTUwMWVKYXdrQ0JZdUJX?=
 =?utf-8?B?eTdMeTJMcStqQlhDaDhaQUZJcE1kdnhzeXpqMmYvUVJtTW5sSFE5aVpTajQw?=
 =?utf-8?B?QnZNQko4RVloZ3R4M3FuMXhYcldNUkZGWk5IdGZrRmVPdHlRZVdacGF6YVg3?=
 =?utf-8?B?OWw1UEJqSHEvM2Z4aW80cXJnVFJhRXVhMEM5Q3MzblN1WjB5MFQ3ZTZRekRs?=
 =?utf-8?B?TGhZOHJ6Z3VJaW5QNjZRc2dzajc4Z2d5cXJGZ1lSenM3Znc2d2hOL3pkeW44?=
 =?utf-8?B?NTlxNDV4a2NBRWd6c3F6SExmZHQxTzJKaDk2bWxHUWFQVHBYVVJhZFlmc21G?=
 =?utf-8?B?Lzk3UXJhMC81WFVuUUx5RTc3Z0QyZmVsZWZCb3NkbTA4SGd1SnRLSUYwdkdZ?=
 =?utf-8?B?YlJSR1c0YXg0MTRoejNTbW9QbmlqTHUzN0xVK3FmaTNiQUVxK2d6RG1oRHJK?=
 =?utf-8?B?VWI1K3k3Zml4NC9wTzBjZ0FJczN6WWtTUFpTaERvSmpaUGgwbU8yWDQ1WFJu?=
 =?utf-8?B?UDFVdHlwU1k0bitxbmZHQ28rKzJQSzM1cDJYMEVvbTB2d2JUQTZROHdaZnBl?=
 =?utf-8?B?cGRDVitYYUFDRkxSOEdFQzhTVGtDdGZaL3pYRDBmN0duaGFLaHZvdGgybUtO?=
 =?utf-8?B?YTMwdDQxUVFuK2NVMnIreWc5Wmc4N3BFYTQ4TStQTmhZb3RtOW1WYkdGOXV0?=
 =?utf-8?B?VERFOUlHSExNblN4WUFGQnZmWFJ1ekhWVWxVcjhWVS9FaTg4ZkJrSFQzWXYz?=
 =?utf-8?B?KzN5aVNpTEJPWmxIU2hiVUx3NHUxY1p1RGtkZzgweE1GZDRsTjBENmRNYWhl?=
 =?utf-8?B?WlJQd3lUeVRlYTdvNnl6V3dTaGpYK2FmK1BEYTRCd3FVQ29nZkFXUjAzbkNF?=
 =?utf-8?B?UFg5T2hXMTlkdW9NNnJxMVJZTkI1NzJ2NWJuZmxsLy9XWStEekgzdzZybFZV?=
 =?utf-8?B?VE9sSUZRNVBBV3VoVzZ4cEpkZ2ptY0JBQlZKUnZEM2s2Y1htejVGZVNURGQw?=
 =?utf-8?B?NDEzSktCN3dOQ1pid21WZ0pjYzgrRFg4ekFlUXJaekp5UVdQQk1YTTd2dUJP?=
 =?utf-8?B?dTRSczdXN21BOUtHQXVnOXhZZFVSNk9xU2lhY3NpQ1FyL0hKbW80YWFkSWFR?=
 =?utf-8?B?bFlldnYvZUVrblBlMHp2ZGcyNi93bmZPTWhPd1ZXdEMyU3k4NmlZak8yaFRl?=
 =?utf-8?Q?m6HD81gb3YsRfwCEmIRdqVs57?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bbea8871-1764-4fe7-1ec8-08dc6859d35e
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3108.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Apr 2024 14:36:58.8252
 (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: 3wc26LCM4mpz5gIRT1xJehCNpOe1a1bmTXd7TGofH0pgizGSh1zGBiSsDl1m7Cjj1A0/SAD1YUoHzy6g4pSQdQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB5794

On 4/29/2024 9:40 AM, Borislav Petkov wrote:
> On Thu, Apr 04, 2024 at 10:13:52AM -0500, Yazen Ghannam wrote:
>> @@ -787,6 +793,8 @@ bool machine_check_poll(enum mcp_flags flags, mce_banks_t *b)
>>  			mce_log(&m);
>>  
>>  clear_it:
>> +		vendor_handle_error(&m);
> 
> Wait, whaaat?
> 
> The normal polling happens periodically (each 5 mins) and you want to
> reset the thresholding blocks each 5 mins?
> 
> And the code has there now:
> 
> static void reset_block(struct threshold_block *block)
> {
> 
> ...
> 
>         /* Reset threshold block after logging error. */
>         memset(&tr, 0, sizeof(tr));
>         tr.b = block;
>         threshold_restart_bank(&tr);
> }
> 
> but no error has been logged.
> 
> Frankly, I don't see the point for this part: polling all banks on
> a thresholding interrupt makes sense. But this resetting from within the
> polling doesn't make any sense.
> 
> Especially if that polling interval is user-controllable.
> 
> Thx.
> 

The reset only happens on a threshold overflow event. There's a check above.

	if (!(high & MASK_OVERFLOW_HI))
                return;

Basically, all the cases in vendor_handle_error() would be conditional.

Related to this, I've been thinking that banks with thresholding enabled
should be removed from the list of polling banks. This is done on Intel but
not on AMD.

I wanted to give it more thought, because I think folks have come to expect
polling and thresholding to be independent on AMD.

Thanks,
Yazen