Received-SPF: pass (google.com: domain of linux-kernel+bounces-188017-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Subject: Re: [PATCH 3/4] acpi/ghes, cxl/pci: Trace FW-First CXL Protocol
 Errors
To: Dave Jiang <dave.jiang@intel.com>, linux-efi@vger.kernel.org,
 linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org
Cc: Ard Biesheuvel <ardb@kernel.org>,
 Alison Schofield <alison.schofield@intel.com>,
 Vishal Verma <vishal.l.verma@intel.com>, Ira Weiny <ira.weiny@intel.com>,
 Dan Williams <dan.j.williams@intel.com>,
 Jonathan Cameron <Jonathan.Cameron@huawei.com>,
 Yazen Ghannam <yazen.ghannam@amd.com>, Bowman Terry <terry.bowman@amd.com>
References: <20240522150839.27578-1-Smita.KoralahalliChannabasappa@amd.com>
 <20240522150839.27578-4-Smita.KoralahalliChannabasappa@amd.com>
 <7dc64418-6821-49c2-b9af-ee4ab148ba9f@intel.com>
From: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
Message-ID: <c7cb3a9c-02e5-09e7-d3f3-b0a3b93f324d@amd.com>
Date: Thu, 23 May 2024 14:23:01 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.4.0
In-Reply-To: <7dc64418-6821-49c2-b9af-ee4ab148ba9f@intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
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?ZjhvcjYvdkJ0WU9kRHdsUFVpYjVhS2JzaW1iSW9iRDY1a1JzZzdyWi9wZldW?=
 =?utf-8?B?NGp4cGpFOEIrbktOZWZVbTI0QUxjZERzQnY3czNHeldOT01IT3ZRMmNpdVo1?=
 =?utf-8?B?V2x5dzhlbzh3OGtuTGpqWUhVT0FTczAyZTU3OG9NUzF2YTNVRFY5cGhxSGFt?=
 =?utf-8?B?WkRRSElVZXdGcmtvdmpoZFJuc2w4MlBCa0I3Mll1TDNsak9WS0tKSVpMMXVI?=
 =?utf-8?B?UExXd1hvUVNoNGh5TE96bWV1NGZTbjQzaUhZQTBlMW9HWVBkN0pkUDM4aWZp?=
 =?utf-8?B?Zmtqc0Q5RGZ6Z2pidFpIeDFQOGpKRzlvY3lteWw2eXBoUk14dHZ4YlJtQTFF?=
 =?utf-8?B?ZFVFSFRZSGNacW81d09PV3NnSlpxemZIYW5QZWs3bzd6YzFHVUlvMXE4Q3pZ?=
 =?utf-8?B?eEJybGQzTFNWZTFHVVRLcWhtdjVBSUNhWHg1K1FrMHk2N1NDSjBROXEzS1VS?=
 =?utf-8?B?RHdBU1U0OS9TeWRUYk0yOWxVNmxKb3BZa2htb1RpNTIwVzFSSGxCbFVMbEpm?=
 =?utf-8?B?Sm41Nmk3WXAvUS9FSzFibGNaa05kOU8vOS80QkpsbEZTMFVtdTFSTmNOMElV?=
 =?utf-8?B?ZzhlREFMaGZNcm83eFBnZTJ1SFc4NGMyMHNoUzV1UE5SdWdFSkx5QmVCMGxj?=
 =?utf-8?B?UEZobk5PME9VaEFBYmdXaHdlL2NhWkRzWDFrNXFyRXUvSDZnZmtEOGduNXkv?=
 =?utf-8?B?NmlvV00wU281K2VnRG9nREg2TmtKVzRyS1JqMXA2VFk3YkxBRUtGY0hqNk96?=
 =?utf-8?B?K3pDY2pMQUlPaTJ0ZEFvUHpXSnRuTDFxbHVudkVnVTQyTGwvZ0xDZWVQb2Qx?=
 =?utf-8?B?QXAyb1pWVDJNOFVpS1lvTVR2aFRsb1JjRW9LT1dRSE5ad0F5eForVnZoUkg3?=
 =?utf-8?B?aHcwU0JqMGlwY2ovRm8wajlDVlFWSkNuK0lpbFU4MENvNjllMVV5ZmwzU2JV?=
 =?utf-8?B?bmh0QXU3azM0Qk9YMGxzeExyYUhlYmZFb0xHdEI1Q2dNanRBbkhrUzVRQ3J4?=
 =?utf-8?B?SHVLUCtJaGdtNUFlWEtXZmVXa2RZV3BwcEdHU01FRWdUSXN0eTZUSzdSRUdR?=
 =?utf-8?B?Y0tQd2pNczJvYWl0Y0xUM1ZsYzkzbWFxWHQrKzhJa2xQdlMvMjNrYjNNR3BB?=
 =?utf-8?B?SW5DaE9XeGxSckxmcHh5bGE0UUVxU2dSTVErdjVHL2xSbzgyRCtzaG9JU1FI?=
 =?utf-8?B?M3M4aWNiaktpQmxTTW50MlJ3Z0hpS0lkVkxEckVFaWlqdWdaWnFHb243cHhR?=
 =?utf-8?B?M1BrZG1hSEdheTNFSVRsSldkZTVyVXhualN1TnlsR01TOWpjdDd0bk1WS3Vp?=
 =?utf-8?B?R2JFMkZxRS9CN3U4Y1M0bmVQZnpabVcyQW5oSWRBUmhlWFZMQXd4M1JCMHhT?=
 =?utf-8?B?aXBBekM5STJjRE0ycnRZeUpSSVBnQUhlKzVJQWdTalV0cjBUMHJPek9PUWhs?=
 =?utf-8?B?WWRaZkZrZ2g1dXNaaUlDaFVhNjJmNUQ1QnZxMEsvckNzUktXcnZuSlhGSXhZ?=
 =?utf-8?B?V2w1cENzSmlWNXRLL1ZjUEJJUDFBYW1La3VQdkZibU4veFdQSUtBQkVSTjNu?=
 =?utf-8?B?bzZNWmtRSHhSUTRjYWlKQTR6cHRzVmJKUHpYeTBLSjVBL3NsYmpJYXh5emlV?=
 =?utf-8?B?bTNabURiQW8relI4c0Fkc2hONFF2bzNDYmdxYThiNm5ZcVl0TlE0WThiUHhY?=
 =?utf-8?B?UFAzcDNDb05DWnpZdGZpNXp2dHpOVy9FL0RHN3UzSEpnMlBDOEJtb2F0SlZS?=
 =?utf-8?B?V3NHb3d2VUQ5RVJKckQ2RlhyUzROcDRBeDI2MlMzNWUveVRENXZ3NFl3c3Zn?=
 =?utf-8?B?N2lUdHVTdVJSajYwRWZPODFDZVEwWHVaYW1VSnFPa0NacmVVcXJCclphdnEz?=
 =?utf-8?B?MTQzWGF3elZodStDU0tXSFV1VFhoa01XN1gzUUJlZXZndkpWQXVLbGVSTFJz?=
 =?utf-8?B?VGNDYm9KcFNWbTV4NFdqNEpjTHk0M00xWHdCb2pTVkZuTVE0Q2V4NlJ5RzZu?=
 =?utf-8?B?dFBBRjJ6ZnVJaVFLUjV3SG5kOWdRdEs1Sm5XQUtadlNqUytSdXBSZWhkaTZC?=
 =?utf-8?B?cHQrQ3JVWjV5RVc0aFh3czlYSFFPcGJOVHhvYTRPUUNrcHdROWxJRWlZK01v?=
 =?utf-8?Q?moCbUqUoZ6o2nMaSkVn4YHER3?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d07a5cd5-5a8d-4610-e0b8-08dc7b6e873f
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB2869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2024 21:23:02.6373
 (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: 0iZIyCQWEnuzfWe2JIBsEutZ0VaVrebL64NgdAIEVrocq6m0hplZ/xKuLuzwbuKBpmgZpHeZNVk3Lu7iXcA8zQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB7329

On 5/22/2024 11:05 AM, Dave Jiang wrote:
> 
> 
> On 5/22/24 8:08 AM, Smita Koralahalli wrote:
>> When PCIe AER is in FW-First, OS should process CXL Protocol errors from
>> CPER records.
>>
>> Reuse the existing work queue cxl_cper_work registered with GHES to notify
>> the CXL subsystem on a Protocol error.
>>
>> The defined trace events cxl_aer_uncorrectable_error and
>> cxl_aer_correctable_error currently trace native CXL AER errors. Reuse
>> them to trace FW-First Protocol Errors.
>>
>> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
>> ---
>>   drivers/acpi/apei/ghes.c  | 14 ++++++++++++++
>>   drivers/cxl/core/pci.c    | 24 ++++++++++++++++++++++++
>>   drivers/cxl/cxlpci.h      |  3 +++
>>   drivers/cxl/pci.c         | 34 ++++++++++++++++++++++++++++++++--
>>   include/linux/cxl-event.h |  1 +
>>   5 files changed, 74 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 1a58032770ee..a31bd91e9475 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -723,6 +723,20 @@ static void cxl_cper_handle_prot_err(struct acpi_hest_generic_data *gdata)
>>   
>>   	if (cxl_cper_handle_prot_err_info(gdata, &wd.p_err))
>>   		return;
>> +
>> +	guard(spinlock_irqsave)(&cxl_cper_work_lock);
>> +
>> +	if (!cxl_cper_work)
>> +		return;
>> +
>> +	wd.event_type = CXL_CPER_EVENT_PROT_ERR;
>> +
>> +	if (!kfifo_put(&cxl_cper_fifo, wd)) {
>> +		pr_err_ratelimited("CXL CPER kfifo overflow\n");
>> +		return;
>> +	}
>> +
>> +	schedule_work(cxl_cper_work);
>>   }
>>   
>>   int cxl_cper_register_work(struct work_struct *work)
>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>> index 0df09bd79408..ef9438cb1dd6 100644
>> --- a/drivers/cxl/core/pci.c
>> +++ b/drivers/cxl/core/pci.c
>> @@ -686,6 +686,30 @@ void read_cdat_data(struct cxl_port *port)
>>   }
>>   EXPORT_SYMBOL_NS_GPL(read_cdat_data, CXL);
>>   
>> +void cxl_trace_prot_err(struct cxl_dev_state *cxlds,
>> +			struct cxl_cper_prot_err *p_err)
>> +{
>> +	u32 status, fe;
>> +
>> +	if (p_err->severity == CXL_AER_CORRECTABLE) {
>> +		status = p_err->cxl_ras.cor_status & ~p_err->cxl_ras.cor_mask;
>> +
>> +		trace_cxl_aer_correctable_error(cxlds->cxlmd, status);
>> +	} else {
>> +		status = p_err->cxl_ras.uncor_status & ~p_err->cxl_ras.uncor_mask;
>> +
>> +		if (hweight32(status) > 1)
>> +			fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
>> +					   p_err->cxl_ras.cap_control));
>> +		else
>> +			fe = status;
>> +
>> +		trace_cxl_aer_uncorrectable_error(cxlds->cxlmd, status, fe,
>> +						  p_err->cxl_ras.header_log);
>> +	}
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_trace_prot_err, CXL);
>> +
>>   static void __cxl_handle_cor_ras(struct cxl_dev_state *cxlds,
>>   				 void __iomem *ras_base)
>>   {
>> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
>> index 93992a1c8eec..0ba3215786e1 100644
>> --- a/drivers/cxl/cxlpci.h
>> +++ b/drivers/cxl/cxlpci.h
>> @@ -130,4 +130,7 @@ void read_cdat_data(struct cxl_port *port);
>>   void cxl_cor_error_detected(struct pci_dev *pdev);
>>   pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
>>   				    pci_channel_state_t state);
>> +struct cxl_cper_prot_err;
>> +void cxl_trace_prot_err(struct cxl_dev_state *cxlds,
>> +			struct cxl_cper_prot_err *p_err);
>>   #endif /* __CXL_PCI_H__ */
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index 74876c9835e8..3e3c36983686 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -1011,12 +1011,42 @@ static void cxl_handle_cper_event(enum cxl_event_type ev_type,
>>   			       &uuid_null, &rec->event);
>>   }
>>   
>> +static void cxl_handle_prot_err(struct cxl_cper_prot_err *p_err)
>> +{
>> +	struct pci_dev *pdev __free(pci_dev_put) = NULL;
>> +	struct cxl_dev_state *cxlds;
>> +	unsigned int devfn;
>> +
>> +	devfn = PCI_DEVFN(p_err->device, p_err->function);
>> +	pdev = pci_get_domain_bus_and_slot(p_err->segment,
>> +					   p_err->bus, devfn);
>> +	if (!pdev)
>> +		return;
>> +
>> +	guard(device)(&pdev->dev);
>> +	if (pdev->driver != &cxl_pci_driver)
>> +		return;
>> +
>> +	cxlds = pci_get_drvdata(pdev);
>> +	if (!cxlds)
>> +		return;
>> +
>> +	if (((u64)p_err->upper_dw << 32 | p_err->lower_dw) != cxlds->serial)
>> +		pr_warn("CPER-reported device serial number does not match expected value\n");
> 
> Given that we are operating on a device, perhaps dev_warn(&pdev->dev, ...) may be better served.

Will fix.

Thanks,
Smita

[snip]