Received-SPF: pass (google.com: domain of linux-kernel+bounces-36083-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: <c86815fe-21dc-4dd7-bf10-74e3cc754413@amd.com>
Date: Tue, 23 Jan 2024 15:39:49 -0600
User-Agent: Mozilla Thunderbird
Subject: [PATCH v3] ACPI: APEI: Skip initialization of GHES_ASSIST structures
 for Machine Check Architecture
Content-Language: en-US
To: linux-acpi@vger.kernel.org
Cc: rafael@kernel.org, lenb@kernel.org, james.morse@arm.com,
 tony.luck@intel.com, bp@alien8.de, linux-kernel@vger.kernel.org,
 yazen.ghannam@amd.com, Avadhut Naik <avadhut.naik@amd.com>
References: <20231204192549.1953029-1-avadhut.naik@amd.com>
 <b519a7e0-77a7-44b4-809b-caf96536fe1c@amd.com>
From: "Naik, Avadhut" <avadnaik@amd.com>
In-Reply-To: <b519a7e0-77a7-44b4-809b-caf96536fe1c@amd.com>
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?ZFpRdEF5UEhxNHNDbWlTTnF5ZTJ3Tk9YS3lMOGZjVWk2Ry9QMEg4cHp4VnY3?=
 =?utf-8?B?N0hDam54TFJBNGxFWTFHN2tZQTVObndGdGROMVBjMjJpRm50MmNEc1ZjNnhY?=
 =?utf-8?B?WHBTamwxWEZJODZUQkIvbFVTd3pmQWljRDV6MkRsVkZ1MFZLU1c3RDVCK2VV?=
 =?utf-8?B?VUJXY2N2czNxK2NrbzcxKzgxdFYrZm4xbCtSTHBOQk41UFp1RVZCZm1JYUhh?=
 =?utf-8?B?Z1JhVWdwSUN4Y3JwY0tMTUFtWmZQRlpwU3dKTkF5OFQyZm1LS0ZmNG0yMkw5?=
 =?utf-8?B?M0RiVCtRc29OeEs3aGQ5K1lZTklCYzc0YTdlT0QyWCs0eHYrQ2s2eHVFUnc2?=
 =?utf-8?B?VEZZTDFjaUNmVVRtdDVEYlk3SVJQandzOEJUUjlWdkMwSGNVV1I1WXYrbUt4?=
 =?utf-8?B?T3l1eW81N3JpNlZZbFVZakxhSUVzTWx4NTRHMEFTSVNqMlF6eG5oRXdkM0pB?=
 =?utf-8?B?dkJud3NrczNJa3JNMDZtWVhkcGc0dDdvNjk1V1JNdWdvaysvMG80SU1Cb2Jh?=
 =?utf-8?B?NE1aV294Y2czWnVnZFc3dFhNWERZZlk2OEdtZ21CQS9IaC9XenVjMW9Hb3NV?=
 =?utf-8?B?UGQ2UU1LU05hRW1LMm9OSW9MTU94d0N2ekVkNFRQSnc3WTk0a3l0bUpzallI?=
 =?utf-8?B?VXFENjl3L2ZTbHdrREZQSmdhR2N1MjhQaUxWKytZVnhLTndCOTN0VFRKdmRx?=
 =?utf-8?B?Qk40ckNOMUZqeXIxWmE0OElBcWluSXQwdkZrYU50MmsrcHdVVnQvS1RCM2wz?=
 =?utf-8?B?OFp6b1ZIcXVXZHViNXk3dk9LbGRibkJjT3dkTDFhcjFwdE1xZGpqTVkyS1V6?=
 =?utf-8?B?dGhHZk5xTHhKMk5oL2JDVEkyellWOVVFTkxSZk5xMXB5ZzRkWTRmUnJtNmZH?=
 =?utf-8?B?anM4ZDcxMUJLMEZpRlFTMFpsOEdFZ3dZb0dKZWF4NDBrNGxCRFRrTWpuY21i?=
 =?utf-8?B?V2JKNGlGeTFLK3dVTGQ1Z1IzTlNXd3JGTlkxUHdMOEdjeXA0L3JNc1ErRnBz?=
 =?utf-8?B?YlNwcVZUM0F2OWplZ1pKdFhUeUFBa0UyYTljTXVuSHRaWk52YXZuTWVkYUtD?=
 =?utf-8?B?VVVORExrZjlkVmZhVlp5dktucm04aU1sMW5DOWtOcHNiNFhGbzFETTFGMmNT?=
 =?utf-8?B?SFUwU2JjZ0UyTFR4bU9sKzhMQWFVN3pJZFhacHk1djI5M3VNaFhJTnBrSUVV?=
 =?utf-8?B?WEFiZDBIdjdFcWYzTXduY1hoc1dzalZ4V2ErOHR6ZXcwbjN5ZWJSMktZeWdP?=
 =?utf-8?B?aWpWcThlTXQ3L0wyUHNwRGhwVmZTYjFhMEZtYU90MEUvaDI2b3htUlEyVXRz?=
 =?utf-8?B?cVBVclkyUTFNa2xDNHlPSzZXWS9zbjd6LzdUUHpzdzNaVUtxZlBERXpNVUto?=
 =?utf-8?B?UEVxdmdnb3JWNzB2RlRSZDZPNXdBNW5tYm44eWVSTlBsUFdWZ2t2SytyalNa?=
 =?utf-8?B?cWVxMnhxajVLL011anVZMCtWUktCQjJrVjZaNWE3Z1ovRjBUTEc1VlNpSnF2?=
 =?utf-8?B?NDRpTk4zc01vSFpEWTRmb2IvSnphQVFuN0g0bTRHY3VVeGZxTG1NUDBaZmx5?=
 =?utf-8?B?bktNVWFtRmh6bjdaRUthWk5rZ1F5VDd4UXVpaklscjg1c2JBMkNyL3hjSCtp?=
 =?utf-8?B?SVJNSHArSmlYbFhrZ2hHL001UDRiditYaWFOY1pRTVk2cHhUR0E4MzdSaVFR?=
 =?utf-8?B?cnpLZnZtZWg1bUlCN0VJS1lqQ0ZkNmZ6TS83aFJBOEUxNHFFVXVwWmFzUG9L?=
 =?utf-8?B?NEJ1R3JtVmM5T1dELy92OU5McXZDR0ZRWDdHWUpUdXUzRFlTOU8rUzNydzUv?=
 =?utf-8?B?cXhwU2NaYUVEQ3NHL01MYkZ6c1pXK28vMEgzbHQ5dkdZQkFHeXhFRVgzcVU0?=
 =?utf-8?B?aXIxNlN3amFrRGVlR24waEhWQ0NmZ1MxWU1qQzU1eVVIeDVZajI2dHVGUnV2?=
 =?utf-8?B?Ny9mc0pEOWtDODlJZGlTdjJPdllqQUtUYUVhVGI3VUNTNkdaTUZhcitwQlpS?=
 =?utf-8?B?bzd5eHQ2TGtpbWV1RXl3WHJSV3RRb3MvaDZMTnRzR1U2Z0ljd0FkVU44MkxB?=
 =?utf-8?B?Q3hZY1RZL3BvNDhVVEhqOHNyUWRHczYxOFJ3cVN0cmxLN0hBbjIvQ21DNlFu?=
 =?utf-8?Q?UMOXr751pRGLf/vBWaVnIsBAa?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7856f821-186e-4161-e711-08dc1c5bd4fc
X-MS-Exchange-CrossTenant-AuthSource: CH3PR12MB8403.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jan 2024 21:39:52.4653
 (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: q2+tjr6A5cNSVTcfjx/gWe+ERZ2a9ng4yaEq8kX1+pwmXuIi0eH1f4dFvvBnPEcmF9vjOZDcF945576+I+8FZw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB8142

Hi,

Any further comments on this patch?

On 12/18/2023 11:13 AM, Avadhut Naik wrote:
> Hi,
> 
> Any further feedback on this patch?
> 
> On 12/4/2023 13:25, Avadhut Naik wrote:
>> To support GHES_ASSIST on Machine Check Architecture (MCA) error sources,
>> a set of GHES structures is provided by the system firmware for each MCA
>> error source. Each of these sets consists of a GHES structure for each MCA
>> bank on each logical CPU, with all structures of a set sharing a common
>> Related Source ID, equal to the Source ID of one of the MCA error source
>> structures.[1] On SOCs with large core counts, this typically equates to
>> tens of thousands of GHES_ASSIST structures for MCA under
>> "/sys/bus/platform/drivers/GHES".
>>
>> Support for GHES_ASSIST however, hasn't been implemented in the kernel. As
>> such, the information provided through these structures is not consumed by
>> Linux. Moreover, these GHES_ASSIST structures for MCA, which are supposed
>> to provide supplemental information in context of an error reported by
>> hardware, are setup as independent error sources by the kernel during HEST
>> initialization.
>>
>> Additionally, if the Type field of the Notification structure, associated
>> with these GHES_ASSIST structures for MCA, is set to Polled, the kernel
>> sets up a timer for each individual structure. The duration of the timer
>> is derived from the Poll Interval field of the Notification structure. On
>> SOCs with high core counts, this will result in tens of thousands of
>> timers expiring periodically causing unnecessary preemptions and wastage
>> of CPU cycles. The problem will particularly intensify if Poll Interval
>> duration is not sufficiently high.
>>
>> Since GHES_ASSIST support is not present in kernel, skip initialization
>> of GHES_ASSIST structures for MCA to eliminate their performance impact.
>>
>> [1] ACPI specification 6.5, section 18.7
>>
>> Signed-off-by: Avadhut Naik <avadhut.naik@amd.com>
>> Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
>> ---
>> Changes in v2:
>> 1.	Since is_ghes_assist_struct() returns if any of the conditions is hit
>> if-else-if chain is redundant. Replace it with just if statements.
>> 2.	Fix formatting errors.
>> 3.	Add Reviewed-by: Yazen Ghannam <yazen.ghannam@amd.com>
>>
>> Changes in v3:
>> 1. Modify structure (mces) comment, per Tony's recommendation, to better
>> reflect the structure's usage.
>> ---
>>  drivers/acpi/apei/hest.c | 51 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 51 insertions(+)
>>
>> diff --git a/drivers/acpi/apei/hest.c b/drivers/acpi/apei/hest.c
>> index 6aef1ee5e1bd..20d757687e3d 100644
>> --- a/drivers/acpi/apei/hest.c
>> +++ b/drivers/acpi/apei/hest.c
>> @@ -37,6 +37,20 @@ EXPORT_SYMBOL_GPL(hest_disable);
>>  
>>  static struct acpi_table_hest *__read_mostly hest_tab;
>>  
>> +/*
>> + * Since GHES_ASSIST is not supported, skip initialization of GHES_ASSIST
>> + * structures for MCA.
>> + * During HEST parsing, detected MCA error sources are cached from early
>> + * table entries so that the Flags and Source Id fields from these cached
>> + * values are then referred to in later table entries to determine if the
>> + * encountered GHES_ASSIST structure should be initialized.
>> + */
>> +static struct {
>> +	struct acpi_hest_ia_corrected *cmc;
>> +	struct acpi_hest_ia_machine_check *mc;
>> +	struct acpi_hest_ia_deferred_check *dmc;
>> +} mces;
>> +
>>  static const int hest_esrc_len_tab[ACPI_HEST_TYPE_RESERVED] = {
>>  	[ACPI_HEST_TYPE_IA32_CHECK] = -1,	/* need further calculation */
>>  	[ACPI_HEST_TYPE_IA32_CORRECTED_CHECK] = -1,
>> @@ -70,22 +84,54 @@ static int hest_esrc_len(struct acpi_hest_header *hest_hdr)
>>  		cmc = (struct acpi_hest_ia_corrected *)hest_hdr;
>>  		len = sizeof(*cmc) + cmc->num_hardware_banks *
>>  			sizeof(struct acpi_hest_ia_error_bank);
>> +		mces.cmc = cmc;
>>  	} else if (hest_type == ACPI_HEST_TYPE_IA32_CHECK) {
>>  		struct acpi_hest_ia_machine_check *mc;
>>  		mc = (struct acpi_hest_ia_machine_check *)hest_hdr;
>>  		len = sizeof(*mc) + mc->num_hardware_banks *
>>  			sizeof(struct acpi_hest_ia_error_bank);
>> +		mces.mc = mc;
>>  	} else if (hest_type == ACPI_HEST_TYPE_IA32_DEFERRED_CHECK) {
>>  		struct acpi_hest_ia_deferred_check *mc;
>>  		mc = (struct acpi_hest_ia_deferred_check *)hest_hdr;
>>  		len = sizeof(*mc) + mc->num_hardware_banks *
>>  			sizeof(struct acpi_hest_ia_error_bank);
>> +		mces.dmc = mc;
>>  	}
>>  	BUG_ON(len == -1);
>>  
>>  	return len;
>>  };
>>  
>> +/*
>> + * GHES and GHESv2 structures share the same format, starting from
>> + * Source Id and ending in Error Status Block Length (inclusive).
>> + */
>> +static bool is_ghes_assist_struct(struct acpi_hest_header *hest_hdr)
>> +{
>> +	struct acpi_hest_generic *ghes;
>> +	u16 related_source_id;
>> +
>> +	if (hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR &&
>> +	    hest_hdr->type != ACPI_HEST_TYPE_GENERIC_ERROR_V2)
>> +		return false;
>> +
>> +	ghes = (struct acpi_hest_generic *)hest_hdr;
>> +	related_source_id = ghes->related_source_id;
>> +
>> +	if (mces.cmc && mces.cmc->flags & ACPI_HEST_GHES_ASSIST &&
>> +	    related_source_id == mces.cmc->header.source_id)
>> +		return true;
>> +	if (mces.mc && mces.mc->flags & ACPI_HEST_GHES_ASSIST &&
>> +	    related_source_id == mces.mc->header.source_id)
>> +		return true;
>> +	if (mces.dmc && mces.dmc->flags & ACPI_HEST_GHES_ASSIST &&
>> +	    related_source_id == mces.dmc->header.source_id)
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>>  typedef int (*apei_hest_func_t)(struct acpi_hest_header *hest_hdr, void *data);
>>  
>>  static int apei_hest_parse(apei_hest_func_t func, void *data)
>> @@ -114,6 +160,11 @@ static int apei_hest_parse(apei_hest_func_t func, void *data)
>>  			return -EINVAL;
>>  		}
>>  
>> +		if (is_ghes_assist_struct(hest_hdr)) {
>> +			hest_hdr = (void *)hest_hdr + len;
>> +			continue;
>> +		}
>> +
>>  		rc = func(hest_hdr, data);
>>  		if (rc)
>>  			return rc;
>>
>> base-commit: 629a3b49f3f957e975253c54846090b8d5ed2e9b
> 

-- 
Thanks,
Avadhut Naik