From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0CDBF4A04; Thu, 11 Sep 2025 19:48:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.7 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757620121; cv=none; b=HZRKDt0en4+O4PO9MF9ljEblb4+m1OpGSDmF+5Jd5qOFYkqBpAJgysAjPVD3mR8hYmMqejO2JVUSbYOGQyardxP8Rj2EsI42eFBNRtWmVICmi25kWEILsoScYIyg5Gp8vqSryf6u8pchV7hCYtb9d3nFGyqMlaKAVt4e+AQPVu0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757620121; c=relaxed/simple; bh=uQOsad0nz/KYJh5/hj8ZWUnDMLnxZP/+FjfS1WQCY4c=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=paPCKBMc0PyZBbXQxBi+Kh1DiMIx0xj6RYKOMp3bof8f3iRLN34dnVIa6TZHY5pJTp+DF9Ikkw37sVB4MJZvTH4KZpICYex4xkzG9lEBtLdvNFpxxbNRQF4FJzr8AknTzwL2v/sG23/bvecFPU4kLtZIitKBeaGpyvYc994X9GI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=Fl2JtG+k; arc=none smtp.client-ip=192.198.163.7 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="Fl2JtG+k" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1757620119; x=1789156119; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=uQOsad0nz/KYJh5/hj8ZWUnDMLnxZP/+FjfS1WQCY4c=; b=Fl2JtG+kPHCLWFNQkfIsrkb71TuKmFAUP7shPwWAK9XjXxC57/b/tmAD /7763ZsuUUTkBjVyP07yJhgoq32zPbLLVnDLrN9RsL/21ktZFzLZvAeZl YexMWnpwjXYNy5pMOAbag2ZnxzubqpoXmglWX+ZvblYPS4oaj4lyLtZSJ Ym1sJTuffVudE+TQgzAHT6rnCoZFSPmYjn+VE38IIIIOcEhvo8EIWdbvs zk52pyVtNsv1MpUzCIesmF4OudgJ3bOe4uP6MrRlvB7bzx0KwxKS04ktv TYNCXnmVUA+otfwLCbXFoXlPqGcqQCaOZGY5CJMTOEs82Ex+BMGBq44/y g==; X-CSE-ConnectionGUID: 4le8YkKISs6yAS5gNPvWZA== X-CSE-MsgGUID: Qkt8Yp4LQhiPgDQpoDjvdg== X-IronPort-AV: E=McAfee;i="6800,10657,11550"; a="85404405" X-IronPort-AV: E=Sophos;i="6.18,258,1751266800"; d="scan'208";a="85404405" Received: from orviesa006.jf.intel.com ([10.64.159.146]) by fmvoesa101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Sep 2025 12:48:38 -0700 X-CSE-ConnectionGUID: 4PHrH3kaQJacKju9XG3LPA== X-CSE-MsgGUID: 5vrI3QVzTNqG5Vpf5yhzQA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.18,258,1751266800"; d="scan'208";a="172958126" Received: from msatwood-mobl.amr.corp.intel.com (HELO [10.125.111.21]) ([10.125.111.21]) by orviesa006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Sep 2025 12:48:37 -0700 Message-ID: Date: Thu, 11 Sep 2025 12:48:35 -0700 Precedence: bulk X-Mailing-List: linux-pci@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v11 21/23] CXL/PCI: Introduce CXL uncorrectable protocol error recovery To: "Bowman, Terry" , dave@stgolabs.net, jonathan.cameron@huawei.com, alison.schofield@intel.com, dan.j.williams@intel.com, bhelgaas@google.com, shiju.jose@huawei.com, ming.li@zohomail.com, Smita.KoralahalliChannabasappa@amd.com, rrichter@amd.com, dan.carpenter@linaro.org, PradeepVineshReddy.Kodamati@amd.com, lukas@wunner.de, Benjamin.Cheatham@amd.com, sathyanarayanan.kuppuswamy@linux.intel.com, linux-cxl@vger.kernel.org, alucerop@amd.com, ira.weiny@intel.com Cc: linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org References: <20250827013539.903682-1-terry.bowman@amd.com> <20250827013539.903682-22-terry.bowman@amd.com> <7a59101b-4ccd-4d86-b97b-21602ebcd1a5@intel.com> <67d11134-8916-4ef8-ab78-06bcbb87d9cb@amd.com> Content-Language: en-US From: Dave Jiang In-Reply-To: <67d11134-8916-4ef8-ab78-06bcbb87d9cb@amd.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 9/11/25 12:19 PM, Bowman, Terry wrote: > > > On 9/3/2025 5:30 PM, Dave Jiang wrote: >> >> On 8/26/25 6:35 PM, Terry Bowman wrote: >>> Populate the cxl_do_recovery() function with uncorrectable protocol error (UCE) >>> handling. Follow similar design as found in PCIe error driver, >>> pcie_do_recovery(). One difference is cxl_do_recovery() will treat all UCEs >>> as fatal with a kernel panic. This is to prevent corruption on CXL memory. >>> >>> Introduce cxl_walk_port(). Make this analogous to pci_walk_bridge() but walking >>> CXL ports instead. This will iterate through the CXL topology from the >>> erroring device through the downstream CXL Ports and Endpoints. >>> >>> Export pci_aer_clear_fatal_status() for CXL to use if a UCE is not found. >>> >>> Signed-off-by: Terry Bowman >>> >>> --- >>> Changes in v10->v11: >>> - pci_ers_merge_results() - Move to earlier patch >>> --- >>> drivers/cxl/core/port.c | 1 + >>> drivers/cxl/core/ras.c | 94 +++++++++++++++++++++++++++++++++++++++++ >>> drivers/pci/pci.h | 2 - >>> include/linux/aer.h | 2 + >>> 4 files changed, 97 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c >>> index 758fb73374c1..085c8620a797 100644 >>> --- a/drivers/cxl/core/port.c >>> +++ b/drivers/cxl/core/port.c >>> @@ -1347,6 +1347,7 @@ struct cxl_port *find_cxl_port(struct device *dport_dev, >>> port = __find_cxl_port(&ctx); >>> return port; >>> } >>> +EXPORT_SYMBOL_NS_GPL(find_cxl_port, "CXL"); >>> >>> static struct cxl_port *find_cxl_port_at(struct cxl_port *parent_port, >>> struct device *dport_dev, >>> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c >>> index 536ca9c815ce..3da675f72616 100644 >>> --- a/drivers/cxl/core/ras.c >>> +++ b/drivers/cxl/core/ras.c >>> @@ -6,6 +6,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> #include "trace.h" >>> >>> static void cxl_cper_trace_corr_port_prot_err(struct pci_dev *pdev, >>> @@ -468,8 +469,101 @@ void cxl_endpoint_port_init_ras(struct cxl_port *ep) >>> } >>> EXPORT_SYMBOL_NS_GPL(cxl_endpoint_port_init_ras, "CXL"); >>> >>> +static int cxl_report_error_detected(struct device *dev, void *data) >>> +{ >>> + struct pci_dev *pdev = to_pci_dev(dev); >>> + pci_ers_result_t vote, *result = data; >>> + >>> + guard(device)(dev); >>> + >>> + if (pci_pcie_type(pdev) == PCI_EXP_TYPE_ENDPOINT) >>> + vote = cxl_error_detected(dev); >>> + else >>> + vote = cxl_port_error_detected(dev); >>> + >>> + vote = cxl_error_detected(dev); >>> + *result = pci_ers_merge_result(*result, vote); >>> + >>> + return 0; >>> +} >>> + >>> +static int match_port_by_parent_dport(struct device *dev, const void *dport_dev) >>> +{ >>> + struct cxl_port *port; >>> + >>> + if (!is_cxl_port(dev)) >>> + return 0; >>> + >>> + port = to_cxl_port(dev); >>> + >>> + return port->parent_dport->dport_dev == dport_dev; >>> +} >>> + >>> +static void cxl_walk_port(struct device *port_dev, >>> + int (*cb)(struct device *, void *), >>> + void *userdata) >>> +{ >>> + struct cxl_dport *dport = NULL; >>> + struct cxl_port *port; >>> + unsigned long index; >>> + >>> + if (!port_dev) >>> + return; >>> + >>> + port = to_cxl_port(port_dev); >>> + if (port->uport_dev && dev_is_pci(port->uport_dev)) >>> + cb(port->uport_dev, userdata); >>> + >>> + xa_for_each(&port->dports, index, dport) >>> + { >>> + struct device *child_port_dev __free(put_device) = >>> + bus_find_device(&cxl_bus_type, &port->dev, dport, >>> + match_port_by_parent_dport); >>> + >>> + cb(dport->dport_dev, userdata); >>> + >>> + cxl_walk_port(child_port_dev, cxl_report_error_detected, userdata); >>> + } >>> + >>> + if (is_cxl_endpoint(port)) >>> + cb(port->uport_dev->parent, userdata); >>> +} >>> + >>> static void cxl_do_recovery(struct device *dev) >>> { >>> + pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER; >>> + struct pci_dev *pdev = to_pci_dev(dev); >>> + struct cxl_dport *dport; >>> + struct cxl_port *port; >>> + >>> + if ((pci_pcie_type(pdev) == PCI_EXP_TYPE_ROOT_PORT) || >>> + (pci_pcie_type(pdev) == PCI_EXP_TYPE_DOWNSTREAM)) { >>> + port = find_cxl_port(&pdev->dev, &dport); >>> + } else if (pci_pcie_type(pdev) == PCI_EXP_TYPE_UPSTREAM) { >>> + struct device *port_dev = bus_find_device(&cxl_bus_type, NULL, >>> + &pdev->dev, match_uport); >>> + port = to_cxl_port(port_dev); >>> + } >> Do we not attempt recovery if the device is an endpoint? Is it because it is handled directly by AER callback of the cxl_pci driver? Should endpoint error just not be forwarded from the AER kfifo producer instead of being checked on the consumer end after going through the kfifo mechanism? >> >> DJ > > The UCE fatal case is handled in the PCIe AER handling callback which is  > what I used for testing. I need to add EP support here for use in the EP  > UCE non-fatal case.  I think that's what's missing. My point is why forward the information to just be dropped. Maybe a comment to make note of that if the support is not there yet. DJ > > I don't think bypassing the kfifo for EPs is ideal. The only headache with  > the EPs is EP fatal UCE is handled in PCIe AER callbacks. We may be able to  > improve with future series by adding logic to detect the link health and then  > access if ok in the UCE fatal case.  > > Terry > >>> + >>> + if (!port) >>> + return; >>> + >>> + cxl_walk_port(&port->dev, cxl_report_error_detected, &status); >>> + if (status == PCI_ERS_RESULT_PANIC) >>> + panic("CXL cachemem error."); >>> + >>> + /* >>> + * If we have native control of AER, clear error status in the device >>> + * that detected the error. If the platform retained control of AER, >>> + * it is responsible for clearing this status. In that case, the >>> + * signaling device may not even be visible to the OS. >>> + */ >>> + if (cxl_error_is_native(pdev)) { >>> + pcie_clear_device_status(pdev); >>> + pci_aer_clear_nonfatal_status(pdev); >>> + pci_aer_clear_fatal_status(pdev); >>> + } >>> + put_device(&port->dev); >>> } >>> >>> static void cxl_handle_cor_ras(struct device *dev, u64 serial, void __iomem *ras_base) >>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h >>> index 69ff7c2d214f..0c4f73dd645f 100644 >>> --- a/drivers/pci/pci.h >>> +++ b/drivers/pci/pci.h >>> @@ -1170,13 +1170,11 @@ static inline void cxl_rch_enable_rcec(struct pci_dev *rcec) { } >>> >>> #ifdef CONFIG_CXL_RAS >>> void pci_aer_unmask_internal_errors(struct pci_dev *dev); >>> -bool cxl_error_is_native(struct pci_dev *dev); >>> bool is_internal_error(struct aer_err_info *info); >>> bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info); >>> void cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info); >>> #else >>> static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { } >>> -static inline bool cxl_error_is_native(struct pci_dev *dev) { return false; } >>> static inline bool is_internal_error(struct aer_err_info *info) { return false; } >>> static inline bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info) { return false; } >>> static inline void cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info) { } >>> diff --git a/include/linux/aer.h b/include/linux/aer.h >>> index 1f79f0be4bf7..751a026fea73 100644 >>> --- a/include/linux/aer.h >>> +++ b/include/linux/aer.h >>> @@ -81,10 +81,12 @@ static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; } >>> int cxl_proto_err_kfifo_get(struct cxl_proto_err_work_data *wd); >>> void cxl_register_proto_err_work(struct work_struct *work); >>> void cxl_unregister_proto_err_work(void); >>> +bool cxl_error_is_native(struct pci_dev *dev); >>> #else >>> static inline int cxl_proto_err_kfifo_get(struct cxl_proto_err_work_data *wd) { return 0; } >>> static inline void cxl_register_proto_err_work(struct work_struct *work) { } >>> static inline void cxl_unregister_proto_err_work(void) { } >>> +static inline bool cxl_error_is_native(struct pci_dev *dev) { return false; } >>> #endif >>> >>> void pci_print_aer(struct pci_dev *dev, int aer_severity, >