[{"id":3686749,"web_url":"http://patchwork.ozlabs.org/comment/3686749/","msgid":"<20260505222853.00913C2BCB4@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-05T22:28:52","subject":"Re: [PATCH v17 06/11] PCI: Establish common CXL Port protocol error\n flow","submitter":{"id":93311,"url":"http://patchwork.ozlabs.org/api/people/93311/","name":null,"email":"sashiko-bot@kernel.org"},"content":"Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:\n- [High] Mixed AER uncorrectable errors and Fatal errors bypass standard PCIe error recovery.\n- [High] `cxl_forward_error()` unconditionally writes to `pdev->aer_cap + PCI_ERR_COR_STATUS`, which can corrupt BAR0 if `aer_cap` is 0.\n- [Medium] AER Uncorrectable Error status is left uncleared if the CXL port driver is unbound.\n--\n\ncommit 40776581efad3966047d440ec936219af21ff33c\nAuthor: Terry Bowman \n\nPCI: Establish common CXL Port protocol error flow\n\nThis commit adds CXL Port protocol error handling callbacks to unify\ndetection, logging, and recovery across CXL Ports and Endpoints. It also\nroutes CXL protocol errors through the CXL RAS helpers and wires the AER\ncore to a kfifo for error processing.\n\n> The correctable AER status is cleared by the producer in\n> cxl_forward_error().\n\nThe patch 'PCI/CXL: Add RCH support to CXL handlers' updates\ncxl_rch_handle_error_iter() to call cxl_forward_error() directly for\nRCD Endpoints (RCiEPs).\n\nIf cxl_forward_error() unconditionally writes to\npdev->aer_cap + PCI_ERR_COR_STATUS to clear the correctable AER status,\nwhat happens if an RCD Endpoint does not expose an AER capability to the\nOS and dev->aer_cap is 0?\n\nWould writing to 0 + PCI_ERR_COR_STATUS (offset 0x10) inadvertently\noverwrite the PCI Base Address Register 0 (BAR0) and corrupt the device's\nmemory mapping? \n\nThe original pci_aer_handle_error() explicitly guarded this write by\nchecking if the aer offset is non-zero.\n\n> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c\n> --- a/drivers/pci/pcie/aer.c\n> +++ b/drivers/pci/pcie/aer.c\n\n[ ... ]\n\n> @@ -1186,7 +1187,10 @@ static void pci_aer_handle_error(struct pci_dev *dev, struct aer_err_info *info)\n> static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)\n> {\n> \tcxl_rch_handle_error(dev, info);\n> -\tpci_aer_handle_error(dev, info);\n> +\tif (is_cxl_error(dev, info))\n> +\t\tcxl_forward_error(dev, info);\n> +\telse\n> +\t\tpci_aer_handle_error(dev, info);\n> \tpci_dev_put(dev);\n> }\n\nSince is_cxl_error() returns true when the CXL Internal Error bit is set,\ndoes this bypass standard PCIe error recovery if a device experiences a\nconcurrent CXL Internal Error and a standard PCIe Uncorrectable Error\n(such as a Data Link Protocol Error)?\n\nAdditionally, if the error is fatal, the PCIe link is frozen. Standard\nhandling in pci_aer_handle_error() would normally call pcie_do_recovery()\nto perform a link reset.\n\nBecause cxl_do_recovery() only clears the AER status and returns without\nresetting the link, could the device be left permanently frozen?\n\n> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c\n> --- a/drivers/cxl/core/ras.c\n> +++ b/drivers/cxl/core/ras.c\n\n[ ... ]\n\n> +static int __cxl_proto_err_work_fn(struct cxl_proto_err_work_data *wd)\n> +{\n> +\tstruct cxl_dport *dport;\n> +\tstruct cxl_port *port __free(put_cxl_port) =\n> +\t\tfind_cxl_port_by_dev(&wd->pdev->dev, &dport);\n> +\n> +\tif (!port) {\n> +\t\tdev_err_ratelimited(&wd->pdev->dev,\n> +\t\t\t\t \"Failed to find parent port device in CXL topology\\n\");\n> +\t\treturn 0;\n> +\t}\n> +\n> +\t/*\n> +\t * Hold the port device lock and verify a driver is bound before\n> +\t * handling errors. Protects against NULL deref if an error is\n> +\t * dispatched before probe completion or after driver removal.\n> +\t */\n> +\tguard(device)(&port->dev);\n> +\tif (!port->dev.driver) {\n> +\t\tdev_err_ratelimited(&port->dev,\n> +\t\t\t\t \"Port device is unbound, abort error handling\\n\");\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tcxl_handle_proto_error(wd->pdev, port, dport, wd->severity);\n> +\n> +\treturn 0;\n> +}\n\nIf the port is not found or the driver is unbound, this function aborts\nand returns early before calling cxl_handle_proto_error().\n\nSince handle_error_source() already bypassed the standard PCIe error\nrecovery, does returning early here leave the AER Uncorrectable Error\nstatus uncleared on the device?\n\nStandard PCIe handling normally ensures these registers are cleared even\nif no driver is bound. Could leaving them uncleared block future error\nreporting?","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=eUsz642U;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-pci+bounces-53808-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"eUsz642U\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g9Cpw24kQz1yJx\n\tfor ; Wed, 06 May 2026 08:28:56 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 717D3304481B\n\tfor ; Tue, 5 May 2026 22:28:54 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 93B1D4A2E2E;\n\tTue, 5 May 2026 22:28:53 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 70A5B37B014\n\tfor ; Tue, 5 May 2026 22:28:53 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 00913C2BCB4;\n\tTue, 5 May 2026 22:28:52 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778020133; cv=none;\n b=b9dgej61ph520zcr20gwiWberg1/EClYGKaT5F7sushskb30CB6Eh2FGu65kZnQ0rw8a2V9k+87gO4skNc+xggOUtwvI0uHEnZ6ojVGvmyqasWulwl2wiAvHIl9jHh3mIMgcl977xuhOI8dSBkGmmRYVOzZRmaCbGR1sZkHrsWA=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778020133; c=relaxed/simple;\n\tbh=NcJrR+AoDc1OAKzaL766DaGMVHu0D+hJUS0ew21RXaA=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=qRIDVhDEd0IhYB6KXZlJb65XCQWggclWXmKGhICD79QyFp989k/IoOM9Wt0eXtBTPb252eYHsQdhi70707LwmjTz3LqayFagYJqQdxv2oqJC+lBD1dsRhEPo7+d5jdAxRH1TUVJo38kScce2xQFvz7zlKgSKxrhRDcdcY0jve/k=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=eUsz642U; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1778020133;\n\tbh=NcJrR+AoDc1OAKzaL766DaGMVHu0D+hJUS0ew21RXaA=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=eUsz642UQWOAq3jJsXYvu6qi1QBEwd287p38VeCVt9YG/lz9UgjslD6H40wBjYlmx\n\t Df337SfIzUbYFu38v+Ff1S7v1rTzrDl5gJ6TVeXr64/TQpHPjcu60oPJL3NDYUQZZa\n\t ohQQUj501BgLVt8iq/4kiuIfOD0AgYu8WHSrHektWnmTGNeV0lxknsFXHgjkHsOesp\n\t RjMsX+qLMdjjLtNIOB4fft2DgXqkLlf+Somql1673eL3b/cszvfsNm3jeH4Zw/uDoh\n\t fJtHr/etf+JwAHEKLjMkBQoHMeFCdcHVZvO+JwutG2F6UTACM28kyYzaBlRLf9H3eD\n\t 9hv0tQg2z6S4g==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH v17 06/11] PCI: Establish common CXL Port protocol error\n flow","Reply-To":"sashiko@lists.linux.dev","To":"\"Terry Bowman\" ","Cc":"linux-pci@vger.kernel.org","In-Reply-To":"<20260505173029.2718246-7-terry.bowman@amd.com>","References":"<20260505173029.2718246-7-terry.bowman@amd.com>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Tue, 05 May 2026 22:28:52 +0000","Message-Id":"<20260505222853.00913C2BCB4@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}},{"id":3687984,"web_url":"http://patchwork.ozlabs.org/comment/3687984/","msgid":"<20260507192210.766d54fd@jic23-huawei>","list_archive_url":null,"date":"2026-05-07T18:22:10","subject":"Re: [PATCH v17 06/11] PCI: Establish common CXL Port protocol error\n flow","submitter":{"id":10151,"url":"http://patchwork.ozlabs.org/api/people/10151/","name":"Jonathan Cameron","email":"jic23@kernel.org"},"content":"On Tue, 5 May 2026 12:30:24 -0500\nTerry Bowman wrote:\n\n> Add CXL Port protocol error handling callbacks to unify detection,\n> logging, and recovery across CXL Ports and Endpoints. Establish a\n> common flow for correctable and uncorrectable CXL protocol errors.\n> RCH Downstream Port error handling is added in a following patch.\n> \n> Add cxl_handle_proto_error() to dispatch correctable and uncorrectable\n> errors through the CXL RAS helpers. Add cxl_do_recovery() to coordinate\n> uncorrectable recovery. Panic via panic() on any uncorrectable CXL RAS\n> error. CXL.cachemem traffic cannot be safely recovered from an\n> uncorrectable protocol error in software, so panic regardless of the\n> AER severity reported. Gate error handling on the port driver being\n> bound to avoid processing errors on disabled devices.\n> \n> Panic explicitly on pci_dev_is_disconnected() before accessing the RAS\n> registers. A CXL device disconnecting during an uncorrectable error event\n> is itself unrecoverable, particularly for devices in interleaved HDM\n> regions. Relying on the status readl() returning ~0u to trip the existing\n> panic path leaves the cause ambiguous.\n> \n> The panic policy applies to the RAS register block of the device whose\n> error triggered the recovery: Root/Downstream Port RAS for VH Ports,\n> Endpoint Port RAS for VH Endpoints and RCDs. Upstream RCH Downstream\n> Port RAS UEs handled via cxl_handle_rdport_errors() are logged only, as\n> before this series. Only the RCD Endpoint's own RAS UE drives the panic.\n> \n> Add to_ras_base() to centralize the RAS base lookup. It selects\n> dport->regs.ras for Root/Downstream Ports and port->regs.ras for\n> Upstream Ports and Endpoints.\n> \n> Export pcie_clear_device_status() and pci_aer_clear_fatal_status() so\n> cxl_core can clear PCIe/AER state during recovery.\n> \n> Wire the AER core to the kfifo in this commit by adding the\n> is_cxl_error() switch in handle_error_source() alongside the consumer\n> registration. This way the producer and consumer go live in the same\n> commit, so CXL errors are not silently dropped during bisect.\n> \n> The correctable AER status is cleared by the producer in\n> cxl_forward_error().\n> \n> Co-developed-by: Dan Williams \n> Signed-off-by: Dan Williams \n> Signed-off-by: Terry Bowman \n> \nA few trivial things inline. With those tidied up\nReviewed-by: Jonathan Cameron \n\n> + * find_cxl_port_by_dev - Use @dev as hint to do a _by_dport or _by_uport lookup\n> + * @dev: generic device that may either be a companion of port or target dport\n> + * @dport: output parameter; set to the matched dport for dport-class\n> + * lookups (Root Port, Downstream Port), NULL otherwise.\n> + *\n> + * Return a 'struct cxl_port' with an elevated reference if found. Use\n> + * __free(put_cxl_port) to release.\n> + */\n> +static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dport **dport)\n> +{\n> +\tstruct pci_dev *pdev;\n> +\n> +\t*dport = NULL;\n> +\tif (!dev_is_pci(dev))\n> +\t\treturn NULL;\n> +\n> +\tpdev = to_pci_dev(dev);\n\nOnly used once. So little point in this step...\n\n> +\n> +\tswitch (pci_pcie_type(pdev)) {\n\tswitch (pci_pcie_type(to_pci_dev(dev))) {\n\nlooks readable enough to me.\n\n> +\tcase PCI_EXP_TYPE_ROOT_PORT:\n> +\tcase PCI_EXP_TYPE_DOWNSTREAM:\n> +\t\treturn find_cxl_port_by_dport(dev, dport);\n> +\tcase PCI_EXP_TYPE_UPSTREAM:\n> +\tcase PCI_EXP_TYPE_ENDPOINT:\n> +\tcase PCI_EXP_TYPE_RC_END:\n> +\t\treturn find_cxl_port_by_uport(dev);\n> +\t}\n> +\n> +\treturn NULL;\n> +}\n\n> +\n> +static void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port, struct cxl_dport *dport)\n> +{\n> +\tstruct device *dev = &pdev->dev;\n> +\tbool ue;\n> +\n> +\tif (pci_dev_is_disconnected(pdev))\n> +\t\tpanic(\"CXL cachemem error: device disconnected during UE recovery\");\n> +\n> +\tue = cxl_handle_ras(dev, pci_get_dsn(pdev),\n> +\t\t\t to_ras_base(port, dport));\n\nMy lazy (or maybe busy) nature means I haven't checked, but if this remains\nthe same for rest of series it fits on one line of around 78 chars.\n\n> +\tif (ue)\n> +\t\tpanic(\"CXL cachemem error.\");\n> +\n> +\tpcie_clear_device_status(pdev);\n> +\tpci_aer_clear_nonfatal_status(pdev);\n> +\tpci_aer_clear_fatal_status(pdev);\n> +}\n\n> +int cxl_ras_init(void)\n> +{\n> +\tcxl_cper_register_prot_err_work(&cxl_cper_prot_err_work);\n> +\tcxl_register_proto_err_work(&cxl_proto_err_work);\n> +\n> +\treturn 0;\n\nvoid cxl_ras_init() as per earlier suggestion still looks good ;)\n\n> +}\n> +\n> +void cxl_ras_exit(void)\n> +{\n> +\tcxl_cper_unregister_prot_err_work();\n> +\tcxl_unregister_proto_err_work();\n> +}","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=aZHZ5b+s;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c04:e001:36c::12fc:5321; helo=tor.lore.kernel.org;\n envelope-from=linux-pci+bounces-54114-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"aZHZ5b+s\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from tor.lore.kernel.org (tor.lore.kernel.org\n [IPv6:2600:3c04:e001:36c::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4gBLFg5Rh2z1y04\n\tfor ; Fri, 08 May 2026 04:22:31 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id 363E5300F179\n\tfor ; Thu, 7 May 2026 18:22:29 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 21568451054;\n\tThu, 7 May 2026 18:22:26 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id EFEBB2DECBF;\n\tThu, 7 May 2026 18:22:25 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 81146C2BCB2;\n\tThu, 7 May 2026 18:22:16 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1778178146; cv=none;\n b=a1DZGNHwtOYYco7kAjNaLyMIV5lIqAx5JcKU/xJDjtTAoTHoaMfTMMeAV20pgJpZddd5jWnoTGsGjurrnKJDC5tUIMiwoa7owbLE6k1jeW+k/3eb9tRRS/rj6k3G69Wsb99kHRLeS8I9AkmGO0gTlbeBC29+UlRoMi4zu3xnzJ8=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1778178146; c=relaxed/simple;\n\tbh=0W+ZsekoTxcgfyklGmZMS7rKNnmw2pmUheslWTYG0n8=;\n\th=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References:\n\t MIME-Version:Content-Type;\n b=nml0+j53jQMbBhJZn6CTl809c+8mfbSaToPYCRUoABSiQKkYJG1ukX3xH9Qb37OVy0wazLLR6keyb/xEZr5Fv26l+ImTRe8Bp+GwRRC1fazfelh869wfyQB7BivVZaoBvzukDfosCzkeOpQ8dJg8Sl2fUWgr0Ce5VDJlGBdFcVE=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=aZHZ5b+s; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1778178145;\n\tbh=0W+ZsekoTxcgfyklGmZMS7rKNnmw2pmUheslWTYG0n8=;\n\th=Date:From:To:Cc:Subject:In-Reply-To:References:From;\n\tb=aZHZ5b+sPLaTMdnR0f+scK9jeHcb+ZD307ThWgBoiq3AkPS74Ogf0bZzgcpxhGVE+\n\t vZTkGp66zmMPksDOzz2UXUrbihomKmfTgHcXDuzCFhFGsBE7yvxdOGb6jMWG3MuAAI\n\t kpNMGsfMqOViUTbj9wJNKbb2PdciNdXvaVZw1JYYblG4RV/AmRp31yTQawufETVIh8\n\t Jc2sudlVtI/cHG4cGQYfQXsFkxAs15ADa5bbG5f0gR0eUQC40xfSUWMjM+0BxD+6AI\n\t Dm9u2SupA0MQwcenMyahiTvQePUpfS1a9yh4w31XJyTRkfYu9DiLUOUND2MhIXMFCL\n\t oAhBZ7qUHPR2A==","Date":"Thu, 7 May 2026 19:22:10 +0100","From":"Jonathan Cameron ","To":"Terry Bowman ","Cc":", ,\n , , ,\n , ,\n , ,\n , ,\n , ,\n , ,\n , , ,\n , ,\n , ,\n , ,\n ","Subject":"Re: [PATCH v17 06/11] PCI: Establish common CXL Port protocol error\n flow","Message-ID":"<20260507192210.766d54fd@jic23-huawei>","In-Reply-To":"<20260505173029.2718246-7-terry.bowman@amd.com>","References":"<20260505173029.2718246-1-terry.bowman@amd.com>\n\t<20260505173029.2718246-7-terry.bowman@amd.com>","X-Mailer":"Claws Mail 4.4.0 (GTK 3.24.52; x86_64-pc-linux-gnu)","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit"}}]