From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7EC63C77B77 for ; Fri, 14 Apr 2023 11:55:51 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230183AbjDNLzu (ORCPT ); Fri, 14 Apr 2023 07:55:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51250 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229752AbjDNLzs (ORCPT ); Fri, 14 Apr 2023 07:55:48 -0400 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 02FFA3A8D; Fri, 14 Apr 2023 04:55:47 -0700 (PDT) Received: from lhrpeml500005.china.huawei.com (unknown [172.18.147.201]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4PyZXj4Qc2z67Ct2; Fri, 14 Apr 2023 19:51:17 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Fri, 14 Apr 2023 12:55:44 +0100 Date: Fri, 14 Apr 2023 12:55:43 +0100 From: Jonathan Cameron To: Robert Richter CC: Ira Weiny , Bjorn Helgaas , Terry Bowman , , , , , , , , , Oliver O'Halloran , "Mahesh J Salgaonkar" , , Subject: Re: [PATCH v3 6/6] PCI/AER: Unmask RCEC internal errors to enable RCH downstream port error handling Message-ID: <20230414125543.000021f6@Huawei.com> In-Reply-To: References: <20230411180302.2678736-7-terry.bowman@amd.com> <20230412212901.GA81099@bhelgaas> <20230413180122.00007471@Huawei.com> <643887b44b2d4_3a1882949d@iweiny-mobl.notmuch> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.202.227.76] X-ClientProxiedBy: lhrpeml100004.china.huawei.com (7.191.162.219) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 14 Apr 2023 13:21:37 +0200 Robert Richter wrote: > On 13.04.23 15:52:36, Ira Weiny wrote: > > Jonathan Cameron wrote: > > > On Wed, 12 Apr 2023 16:29:01 -0500 > > > Bjorn Helgaas wrote: > > > > > > > On Tue, Apr 11, 2023 at 01:03:02PM -0500, Terry Bowman wrote: > > > > > From: Robert Richter > > > > > > > > > > > +static int __cxl_unmask_internal_errors(struct pci_dev *rcec) > > > > > +{ > > > > > + int aer, rc; > > > > > + u32 mask; > > > > > + > > > > > + /* > > > > > + * Internal errors are masked by default, unmask RCEC's here > > > > > + * PCI6.0 7.8.4.3 Uncorrectable Error Mask Register (Offset 08h) > > > > > + * PCI6.0 7.8.4.6 Correctable Error Mask Register (Offset 14h) > > > > > + */ > > > > > > > > Unmasking internal errors doesn't have anything specific to do with > > > > CXL, so I don't think it should have "cxl" in the function name. > > > > Maybe something like "pci_aer_unmask_internal_errors()". > > > > > > This reminds me. Not sure we resolved earlier discussion on changing > > > the system wide policy to turn these on > > > https://lore.kernel.org/linux-cxl/20221229172731.GA611562@bhelgaas/ > > > which needs pretty much the same thing. > > > > > > Ira, I think you were picking this one up? > > > https://lore.kernel.org/linux-cxl/63e5fb533f304_13244829412@iweiny-mobl.notmuch/ > > > > After this discussion I posted an RFC to enable those errors. > > > > https://lore.kernel.org/all/20230209-cxl-pci-aer-v1-1-f9a817fa4016@intel.com/ > > Ah. I'd forgotten that thread. Thanks! > > Unfortunately the prevailing opinion was that this was unsafe. And no one > > piped up with a reason to pursue the alternative of a pci core call to enable > > them as needed. > > > > So I abandoned the work. > > > > I think the direction things where headed was to have a call like: > > > > int pci_enable_pci_internal_errors(struct pci_dev *dev) > > { > > int pos_cap_err; > > u32 reg; > > > > if (!pcie_aer_is_native(dev)) > > return -EIO; > > > > pos_cap_err = dev->aer_cap; > > > > /* Unmask correctable and uncorrectable (non-fatal) internal errors */ > > pci_read_config_dword(dev, pos_cap_err + PCI_ERR_COR_MASK, ®); > > reg &= ~PCI_ERR_COR_INTERNAL; > > pci_write_config_dword(dev, pos_cap_err + PCI_ERR_COR_MASK, reg); > > > > pci_read_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_SEVER, ®); > > reg &= ~PCI_ERR_UNC_INTN; > > pci_write_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_SEVER, reg); > > > > pci_read_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_MASK, ®); > > reg &= ~PCI_ERR_UNC_INTN; > > pci_write_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_MASK, reg); > > > > return 0; > > } > > > > ... and call this from the cxl code where it is needed. > > The version I have ready after addressing Bjorn's comments is pretty > much the same, apart from error checking of the read/writes. > > From your patch proposed you will need it in aer.c too and we do not > need to export it. I think for the other components we'll want to call it from cxl_pci_ras_unmask() so an export needed. I also wonder if a more generic function would be better as seems likely similar code will be needed for errors other than this pair. > > This patch only enables it for (CXL) RCECs. You might want to extend > this for CXL endpoints (and ports?) then. Definitely. We have the same limitation you are seeing. No errors without turning this on. Jonathan > > > > > Is this an acceptable direction? Terry is welcome to steal the above from my > > patch and throw it into the PCI core. > > > > Looking at the current state of things I think cxl_pci_ras_unmask() may > > actually be broken now without calling something like the above. For that I > > dropped the ball. > > Thanks, > > -Robert > > > > > Ira From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.ozlabs.org (lists.ozlabs.org [112.213.38.117]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id E2385C77B72 for ; Fri, 14 Apr 2023 11:56:25 +0000 (UTC) Received: from boromir.ozlabs.org (localhost [IPv6:::1]) by lists.ozlabs.org (Postfix) with ESMTP id 4PyZfc0cBhz3fX6 for ; Fri, 14 Apr 2023 21:56:24 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=huawei.com (client-ip=185.176.79.56; helo=frasgout.his.huawei.com; envelope-from=jonathan.cameron@huawei.com; receiver=) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4PyZf335gMz3c8G for ; Fri, 14 Apr 2023 21:55:51 +1000 (AEST) Received: from lhrpeml500005.china.huawei.com (unknown [172.18.147.201]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4PyZXj4Qc2z67Ct2; Fri, 14 Apr 2023 19:51:17 +0800 (CST) Received: from localhost (10.202.227.76) by lhrpeml500005.china.huawei.com (7.191.163.240) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.23; Fri, 14 Apr 2023 12:55:44 +0100 Date: Fri, 14 Apr 2023 12:55:43 +0100 From: Jonathan Cameron To: Robert Richter Subject: Re: [PATCH v3 6/6] PCI/AER: Unmask RCEC internal errors to enable RCH downstream port error handling Message-ID: <20230414125543.000021f6@Huawei.com> In-Reply-To: References: <20230411180302.2678736-7-terry.bowman@amd.com> <20230412212901.GA81099@bhelgaas> <20230413180122.00007471@Huawei.com> <643887b44b2d4_3a1882949d@iweiny-mobl.notmuch> Organization: Huawei Technologies Research and Development (UK) Ltd. X-Mailer: Claws Mail 4.1.0 (GTK 3.24.33; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.202.227.76] X-ClientProxiedBy: lhrpeml100004.china.huawei.com (7.191.162.219) To lhrpeml500005.china.huawei.com (7.191.163.240) X-CFilter-Loop: Reflected X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alison.schofield@intel.com, dave.jiang@intel.com, Terry Bowman , vishal.l.verma@intel.com, linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org, Mahesh J Salgaonkar , bhelgaas@google.com, Bjorn Helgaas , linux-pci@vger.kernel.org, bwidawsk@kernel.org, Oliver O'Halloran , dan.j.williams@intel.com, Ira Weiny Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Fri, 14 Apr 2023 13:21:37 +0200 Robert Richter wrote: > On 13.04.23 15:52:36, Ira Weiny wrote: > > Jonathan Cameron wrote: > > > On Wed, 12 Apr 2023 16:29:01 -0500 > > > Bjorn Helgaas wrote: > > > > > > > On Tue, Apr 11, 2023 at 01:03:02PM -0500, Terry Bowman wrote: > > > > > From: Robert Richter > > > > > > > > > > > +static int __cxl_unmask_internal_errors(struct pci_dev *rcec) > > > > > +{ > > > > > + int aer, rc; > > > > > + u32 mask; > > > > > + > > > > > + /* > > > > > + * Internal errors are masked by default, unmask RCEC's here > > > > > + * PCI6.0 7.8.4.3 Uncorrectable Error Mask Register (Offset 08h) > > > > > + * PCI6.0 7.8.4.6 Correctable Error Mask Register (Offset 14h) > > > > > + */ > > > > > > > > Unmasking internal errors doesn't have anything specific to do with > > > > CXL, so I don't think it should have "cxl" in the function name. > > > > Maybe something like "pci_aer_unmask_internal_errors()". > > > > > > This reminds me. Not sure we resolved earlier discussion on changing > > > the system wide policy to turn these on > > > https://lore.kernel.org/linux-cxl/20221229172731.GA611562@bhelgaas/ > > > which needs pretty much the same thing. > > > > > > Ira, I think you were picking this one up? > > > https://lore.kernel.org/linux-cxl/63e5fb533f304_13244829412@iweiny-mobl.notmuch/ > > > > After this discussion I posted an RFC to enable those errors. > > > > https://lore.kernel.org/all/20230209-cxl-pci-aer-v1-1-f9a817fa4016@intel.com/ > > Ah. I'd forgotten that thread. Thanks! > > Unfortunately the prevailing opinion was that this was unsafe. And no one > > piped up with a reason to pursue the alternative of a pci core call to enable > > them as needed. > > > > So I abandoned the work. > > > > I think the direction things where headed was to have a call like: > > > > int pci_enable_pci_internal_errors(struct pci_dev *dev) > > { > > int pos_cap_err; > > u32 reg; > > > > if (!pcie_aer_is_native(dev)) > > return -EIO; > > > > pos_cap_err = dev->aer_cap; > > > > /* Unmask correctable and uncorrectable (non-fatal) internal errors */ > > pci_read_config_dword(dev, pos_cap_err + PCI_ERR_COR_MASK, ®); > > reg &= ~PCI_ERR_COR_INTERNAL; > > pci_write_config_dword(dev, pos_cap_err + PCI_ERR_COR_MASK, reg); > > > > pci_read_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_SEVER, ®); > > reg &= ~PCI_ERR_UNC_INTN; > > pci_write_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_SEVER, reg); > > > > pci_read_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_MASK, ®); > > reg &= ~PCI_ERR_UNC_INTN; > > pci_write_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_MASK, reg); > > > > return 0; > > } > > > > ... and call this from the cxl code where it is needed. > > The version I have ready after addressing Bjorn's comments is pretty > much the same, apart from error checking of the read/writes. > > From your patch proposed you will need it in aer.c too and we do not > need to export it. I think for the other components we'll want to call it from cxl_pci_ras_unmask() so an export needed. I also wonder if a more generic function would be better as seems likely similar code will be needed for errors other than this pair. > > This patch only enables it for (CXL) RCECs. You might want to extend > this for CXL endpoints (and ports?) then. Definitely. We have the same limitation you are seeing. No errors without turning this on. Jonathan > > > > > Is this an acceptable direction? Terry is welcome to steal the above from my > > patch and throw it into the PCI core. > > > > Looking at the current state of things I think cxl_pci_ras_unmask() may > > actually be broken now without calling something like the above. For that I > > dropped the ball. > > Thanks, > > -Robert > > > > > Ira