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 X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D3F61C43459 for ; Fri, 10 Jul 2020 22:20:37 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AD7E62068F for ; Fri, 10 Jul 2020 22:20:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Eyf7Dqdc" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726802AbgGJWU2 (ORCPT ); Fri, 10 Jul 2020 18:20:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60188 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726790AbgGJWU1 (ORCPT ); Fri, 10 Jul 2020 18:20:27 -0400 Received: from mail-wr1-x443.google.com (mail-wr1-x443.google.com [IPv6:2a00:1450:4864:20::443]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 481BAC08C5DD; Fri, 10 Jul 2020 15:20:27 -0700 (PDT) Received: by mail-wr1-x443.google.com with SMTP id o11so7350538wrv.9; Fri, 10 Jul 2020 15:20:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=TDNee8GVeUeyBsCjyc6danC/ynEOptI+9KXBNHn5gUo=; b=Eyf7Dqdco0XD7yMgVIUmRlSHWZB3Yqq8yEWV6erE3fEu1ZVDFybSEoai1AU2qlB8nC tHOY1GTRGb0ZrmdlrlWHPoD5Q5jDdZig1hTAELuRNjP5qYKhynkh2LXjSw5gvSW0wbMI Hr1zTWrE2ZeZJiCUnsHnw5mJ1Et3f8gOoxwmhG7Q42fVi+sb4gmCFzl1cCcbPRkvMTRI CE/T9r99ELm4jgXAYzcrG7cCFNg06rWI5ZD78vepUnm4DNCzqgitB0kLfshyEnyOpkgN k57LCFynEjxTokoNDId4z1pkPe06ZoR7KjXhZ0lTkDKyDTUz4mTuGObVAisk47mhj7so ZrbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=TDNee8GVeUeyBsCjyc6danC/ynEOptI+9KXBNHn5gUo=; b=PGdhr4Ym+YCgY6GL5zawKRUf7/PdpvjAr+Gvod+a6AXMVZEFhSeyotM8Iqpap9Mrb0 QysOQnP5C86MZjq8BetqjmMiCgd4rRYjwWQzcGXVlGlL7tOldESBE10vm4gGMYriSwDK 44dr9oL/RF2n0ME3kyR3BFo99Nfr402SucJwIxaCljchOV3ZfNtGjVJyja0sv76BSrIe DtdgVCWONH8a+RUlo3RpYttV6th7qCMk2GaOzqBYRdqqOBMkk8rRt3Z88PbrhaOD6QIH jbo60cqxklrgbP7AEnHXOWMJO0f3dbXHKgcBr9YtzVpEwfPfxCQU69sM111WE2HzWjWH buGw== X-Gm-Message-State: AOAM531zOEKHA0QTbjF8CkWeKzP2uH0gtL8Wr613KG0JvEz+6dmknAFh VBpxDoL5IYQwC1BSGzafD+o= X-Google-Smtp-Source: ABdhPJxrYIsBkfnBH9S5RucmDZWvO0yCzpX9cyPyswpKtcK/fL4BpEXiODFaS1wjD/qMUUJfHrsJHw== X-Received: by 2002:adf:ef46:: with SMTP id c6mr69693527wrp.34.1594419625955; Fri, 10 Jul 2020 15:20:25 -0700 (PDT) Received: from net.saheed (54007186.dsl.pool.telekom.hu. [84.0.113.134]) by smtp.gmail.com with ESMTPSA id l18sm12170281wrm.52.2020.07.10.15.20.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jul 2020 15:20:25 -0700 (PDT) From: Saheed Olayemi Bolarinwa To: helgaas@kernel.org Cc: Bolarinwa Olayemi Saheed , bjorn@helgaas.com, skhan@linuxfoundation.org, linux-pci@vger.kernel.org, linux-kernel-mentees@lists.linuxfoundation.org, linux-kernel@vger.kernel.org, Russell Currey , Sam Bobroff , "Oliver O'Halloran" , linuxppc-dev@lists.ozlabs.org, "Rafael J. Wysocki" , Len Brown , Lukas Wunner , linux-acpi@vger.kernel.org, Mike Marciniszyn , Dennis Dalessandro , Doug Ledford , Jason Gunthorpe , linux-rdma@vger.kernel.org, Arnd Bergmann , Greg Kroah-Hartman , "David S. Miller" , Kalle Valo , Jakub Kicinski , QCA ath9k Development , linux-wireless@vger.kernel.org, netdev@vger.kernel.org, Stanislaw Gruszka Subject: [PATCH 14/14 v3] PCI: Remove '*val = 0' from pcie_capability_read_*() Date: Fri, 10 Jul 2020 23:20:26 +0200 Message-Id: <20200710212026.27136-15-refactormyself@gmail.com> X-Mailer: git-send-email 2.18.2 In-Reply-To: <20200710212026.27136-1-refactormyself@gmail.com> References: <20200710212026.27136-1-refactormyself@gmail.com> Sender: linux-wireless-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org From: Bolarinwa Olayemi Saheed There are several reasons why a PCI capability read may fail whether the device is present or not. If this happens, pcie_capability_read_*() will return -EINVAL/PCIBIOS_BAD_REGISTER_NUMBER or PCIBIOS_DEVICE_NOT_FOUND and *val is set to 0. This behaviour if further ensured by this code inside pcie_capability_read_*() ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val); /* * Reset *val to 0 if pci_read_config_dword() fails, it may * have been written as 0xFFFFFFFF if hardware error happens * during pci_read_config_dword(). */ if (ret) *val = 0; return ret; a) Since all pci_generic_config_read() does is read a register value, it may return success after reading a ~0 which *may* have been fabricated by the PCI host bridge due to a read timeout. Hence pci_read_config_*() will return success with a fabricated ~0 in *val, indicating a problem. In this case, the assumed behaviour of pcie_capability_read_*() will be wrong. To avoid error slipping through, more checks are necessary. b) pci_read_config_*() will return PCIBIOS_DEVICE_NOT_FOUND only if dev->error_state = pci_channel_io_perm_failure (i.e. pci_dev_is_disconnected()) or if pci_generic_config_read() can't find the device. In both cases *val is initially set to ~0 but as shown in the code above pcie_capability_read_*() resets it back to 0. Even with this effort, drivers still have to perform validation checks more so if 0 is a valid value. Most drivers only consider the case (b) and in some cases, there is the expectation that on timeout *val has a fabricated value of ~0, which *may* not always be true as explained in (a). In any case, checks need to be done to validate the value read and maybe confirm which error has occurred. It is better left to the drivers to do. Remove the reset of *val to 0 when pci_read_config_*() fails. Suggested-by: Bjorn Helgaas Signed-off-by: Bolarinwa Olayemi Saheed --- This patch depends on all of the preceeding patches in this series, otherwise it will introduce bugs as pointed out in the commit message of each. drivers/pci/access.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 79c4a2ef269a..ec95edbb1ac8 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -413,13 +413,6 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val) if (pcie_capability_reg_implemented(dev, pos)) { ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val); - /* - * Reset *val to 0 if pci_read_config_word() fails, it may - * have been written as 0xFFFF if hardware error happens - * during pci_read_config_word(). - */ - if (ret) - *val = 0; return ret; } @@ -448,13 +441,6 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val) if (pcie_capability_reg_implemented(dev, pos)) { ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val); - /* - * Reset *val to 0 if pci_read_config_dword() fails, it may - * have been written as 0xFFFFFFFF if hardware error happens - * during pci_read_config_dword(). - */ - if (ret) - *val = 0; return ret; } -- 2.18.2 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 X-Spam-Level: X-Spam-Status: No, score=-12.5 required=3.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED,DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4E1CDC433E0 for ; Fri, 10 Jul 2020 22:24:13 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E6CD12075D for ; Fri, 10 Jul 2020 22:24:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Eyf7Dqdc" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E6CD12075D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 4B3SJB1Yp2zDqWy for ; Sat, 11 Jul 2020 08:24:10 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gmail.com (client-ip=2a00:1450:4864:20::442; helo=mail-wr1-x442.google.com; envelope-from=refactormyself@gmail.com; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=Eyf7Dqdc; dkim-atps=neutral Received: from mail-wr1-x442.google.com (mail-wr1-x442.google.com [IPv6:2a00:1450:4864:20::442]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4B3SCx3KSxzDrPG for ; Sat, 11 Jul 2020 08:20:29 +1000 (AEST) Received: by mail-wr1-x442.google.com with SMTP id z13so7340995wrw.5 for ; Fri, 10 Jul 2020 15:20:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=TDNee8GVeUeyBsCjyc6danC/ynEOptI+9KXBNHn5gUo=; b=Eyf7Dqdco0XD7yMgVIUmRlSHWZB3Yqq8yEWV6erE3fEu1ZVDFybSEoai1AU2qlB8nC tHOY1GTRGb0ZrmdlrlWHPoD5Q5jDdZig1hTAELuRNjP5qYKhynkh2LXjSw5gvSW0wbMI Hr1zTWrE2ZeZJiCUnsHnw5mJ1Et3f8gOoxwmhG7Q42fVi+sb4gmCFzl1cCcbPRkvMTRI CE/T9r99ELm4jgXAYzcrG7cCFNg06rWI5ZD78vepUnm4DNCzqgitB0kLfshyEnyOpkgN k57LCFynEjxTokoNDId4z1pkPe06ZoR7KjXhZ0lTkDKyDTUz4mTuGObVAisk47mhj7so ZrbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=TDNee8GVeUeyBsCjyc6danC/ynEOptI+9KXBNHn5gUo=; b=Y1HwsLJ22qr9fEtLSR4HcmfrtNjjR3so0XWF3dvvF/VzaF6V0WCcaGndnb+8PwVQF2 jqlmo/OIWoNb702cVTuTP82t1+KIjGWhLLony7/5sEvCUTy+C3l8yviKQmNJ4+ywIhQK J2wwjxoZunBJutxFP/C7tTvhtoOCln7moTsfh7WfNWz34p6yhArtZe8gxYYonVuzY/3V Qz60p+DXAFjNBmaA/5vh/mYH8hgaTAvmSy8/knif5SWsWCud45a8h4oF7XNJpKIqeKGc LlNVCr2gr6mimkd2xTvA7QQa1Z8rlUP4qqSvfJ14yTKgV2R5sHUPDgz4e3ycnMt6Y5wt JrLQ== X-Gm-Message-State: AOAM531RZ4WcqxBnVV3mtDVvvHE79PEAdzaZ83pBnVzO7qCor3I/57Co 0npvH0hIeMOOnuJp+5LqLko= X-Google-Smtp-Source: ABdhPJxrYIsBkfnBH9S5RucmDZWvO0yCzpX9cyPyswpKtcK/fL4BpEXiODFaS1wjD/qMUUJfHrsJHw== X-Received: by 2002:adf:ef46:: with SMTP id c6mr69693527wrp.34.1594419625955; Fri, 10 Jul 2020 15:20:25 -0700 (PDT) Received: from net.saheed (54007186.dsl.pool.telekom.hu. [84.0.113.134]) by smtp.gmail.com with ESMTPSA id l18sm12170281wrm.52.2020.07.10.15.20.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jul 2020 15:20:25 -0700 (PDT) From: Saheed Olayemi Bolarinwa To: helgaas@kernel.org Subject: [PATCH 14/14 v3] PCI: Remove '*val = 0' from pcie_capability_read_*() Date: Fri, 10 Jul 2020 23:20:26 +0200 Message-Id: <20200710212026.27136-15-refactormyself@gmail.com> X-Mailer: git-send-email 2.18.2 In-Reply-To: <20200710212026.27136-1-refactormyself@gmail.com> References: <20200710212026.27136-1-refactormyself@gmail.com> 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: linux-wireless@vger.kernel.org, linux-pci@vger.kernel.org, QCA ath9k Development , netdev@vger.kernel.org, Oliver O'Halloran , Stanislaw Gruszka , linux-acpi@vger.kernel.org, linux-rdma@vger.kernel.org, Jason Gunthorpe , Doug Ledford , Jakub Kicinski , linux-kernel-mentees@lists.linuxfoundation.org, Len Brown , Arnd Bergmann , skhan@linuxfoundation.org, bjorn@helgaas.com, Kalle Valo , Mike Marciniszyn , Sam Bobroff , Greg Kroah-Hartman , Dennis Dalessandro , "Rafael J. Wysocki" , linux-kernel@vger.kernel.org, Lukas Wunner , Bolarinwa Olayemi Saheed , linuxppc-dev@lists.ozlabs.org, "David S. Miller" Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" From: Bolarinwa Olayemi Saheed There are several reasons why a PCI capability read may fail whether the device is present or not. If this happens, pcie_capability_read_*() will return -EINVAL/PCIBIOS_BAD_REGISTER_NUMBER or PCIBIOS_DEVICE_NOT_FOUND and *val is set to 0. This behaviour if further ensured by this code inside pcie_capability_read_*() ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val); /* * Reset *val to 0 if pci_read_config_dword() fails, it may * have been written as 0xFFFFFFFF if hardware error happens * during pci_read_config_dword(). */ if (ret) *val = 0; return ret; a) Since all pci_generic_config_read() does is read a register value, it may return success after reading a ~0 which *may* have been fabricated by the PCI host bridge due to a read timeout. Hence pci_read_config_*() will return success with a fabricated ~0 in *val, indicating a problem. In this case, the assumed behaviour of pcie_capability_read_*() will be wrong. To avoid error slipping through, more checks are necessary. b) pci_read_config_*() will return PCIBIOS_DEVICE_NOT_FOUND only if dev->error_state = pci_channel_io_perm_failure (i.e. pci_dev_is_disconnected()) or if pci_generic_config_read() can't find the device. In both cases *val is initially set to ~0 but as shown in the code above pcie_capability_read_*() resets it back to 0. Even with this effort, drivers still have to perform validation checks more so if 0 is a valid value. Most drivers only consider the case (b) and in some cases, there is the expectation that on timeout *val has a fabricated value of ~0, which *may* not always be true as explained in (a). In any case, checks need to be done to validate the value read and maybe confirm which error has occurred. It is better left to the drivers to do. Remove the reset of *val to 0 when pci_read_config_*() fails. Suggested-by: Bjorn Helgaas Signed-off-by: Bolarinwa Olayemi Saheed --- This patch depends on all of the preceeding patches in this series, otherwise it will introduce bugs as pointed out in the commit message of each. drivers/pci/access.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 79c4a2ef269a..ec95edbb1ac8 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -413,13 +413,6 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val) if (pcie_capability_reg_implemented(dev, pos)) { ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val); - /* - * Reset *val to 0 if pci_read_config_word() fails, it may - * have been written as 0xFFFF if hardware error happens - * during pci_read_config_word(). - */ - if (ret) - *val = 0; return ret; } @@ -448,13 +441,6 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val) if (pcie_capability_reg_implemented(dev, pos)) { ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val); - /* - * Reset *val to 0 if pci_read_config_dword() fails, it may - * have been written as 0xFFFFFFFF if hardware error happens - * during pci_read_config_dword(). - */ - if (ret) - *val = 0; return ret; } -- 2.18.2 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 X-Spam-Level: X-Spam-Status: No, score=-12.5 required=3.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED,DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E8306C43457 for ; Fri, 10 Jul 2020 22:20:32 +0000 (UTC) Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BCD562068F for ; Fri, 10 Jul 2020 22:20:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Eyf7Dqdc" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BCD562068F Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linux-kernel-mentees-bounces@lists.linuxfoundation.org Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 986D32E990; Fri, 10 Jul 2020 22:20:32 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8xvhYM-mnuIh; Fri, 10 Jul 2020 22:20:31 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by silver.osuosl.org (Postfix) with ESMTP id 750DE2E967; Fri, 10 Jul 2020 22:20:31 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 64A14C077B; Fri, 10 Jul 2020 22:20:31 +0000 (UTC) Received: from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id AE68BC016F for ; Fri, 10 Jul 2020 22:20:30 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by hemlock.osuosl.org (Postfix) with ESMTP id 9B1BB89FB9 for ; Fri, 10 Jul 2020 22:20:30 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from hemlock.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PtNIzssRfuEQ for ; Fri, 10 Jul 2020 22:20:27 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by hemlock.osuosl.org (Postfix) with ESMTPS id 8CCCA89FD2 for ; Fri, 10 Jul 2020 22:20:27 +0000 (UTC) Received: by mail-wr1-f65.google.com with SMTP id q5so7348986wru.6 for ; Fri, 10 Jul 2020 15:20:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=TDNee8GVeUeyBsCjyc6danC/ynEOptI+9KXBNHn5gUo=; b=Eyf7Dqdco0XD7yMgVIUmRlSHWZB3Yqq8yEWV6erE3fEu1ZVDFybSEoai1AU2qlB8nC tHOY1GTRGb0ZrmdlrlWHPoD5Q5jDdZig1hTAELuRNjP5qYKhynkh2LXjSw5gvSW0wbMI Hr1zTWrE2ZeZJiCUnsHnw5mJ1Et3f8gOoxwmhG7Q42fVi+sb4gmCFzl1cCcbPRkvMTRI CE/T9r99ELm4jgXAYzcrG7cCFNg06rWI5ZD78vepUnm4DNCzqgitB0kLfshyEnyOpkgN k57LCFynEjxTokoNDId4z1pkPe06ZoR7KjXhZ0lTkDKyDTUz4mTuGObVAisk47mhj7so ZrbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=TDNee8GVeUeyBsCjyc6danC/ynEOptI+9KXBNHn5gUo=; b=D5lamyg3p8AJTjH27Ut2Jc3sGaSsKDipy70Ty70sL2Pu6JKC7ctg4nYceiRi2bSyan s6QfiyktSe9uuoNGzUOCyHW93TshrKLd2GUHj0/Jl/5zjR/7oRuNQ7ES3eeQ9RZZalkA vkGsTD6jL5NxwAC7ieBjfanvoWk5/UXDnVxdxJKcaxYML2l+BiVRPGp+ehlCe5Fz5pxC KT2VSIGq6vWfZv9/8YOPD2Bh2y0IKuJTo9xbSisuQxuPsj+Qw9XrLplTea+VwX9fxM/6 LGE/l34oxnC+X1rqOfluuf1fGa0Po4KQ4muVLDJlMOw4X8BBshvUkXraDJV36HkMTy+9 j65w== X-Gm-Message-State: AOAM533sAZjswWC5CMrgkhtTUHoBsld6TR8TgT2AdCXyranMeyxLvDa3 CkS1a6Jib5Xh62SD8bCOuIY= X-Google-Smtp-Source: ABdhPJxrYIsBkfnBH9S5RucmDZWvO0yCzpX9cyPyswpKtcK/fL4BpEXiODFaS1wjD/qMUUJfHrsJHw== X-Received: by 2002:adf:ef46:: with SMTP id c6mr69693527wrp.34.1594419625955; Fri, 10 Jul 2020 15:20:25 -0700 (PDT) Received: from net.saheed (54007186.dsl.pool.telekom.hu. [84.0.113.134]) by smtp.gmail.com with ESMTPSA id l18sm12170281wrm.52.2020.07.10.15.20.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jul 2020 15:20:25 -0700 (PDT) From: Saheed Olayemi Bolarinwa To: helgaas@kernel.org Date: Fri, 10 Jul 2020 23:20:26 +0200 Message-Id: <20200710212026.27136-15-refactormyself@gmail.com> X-Mailer: git-send-email 2.18.2 In-Reply-To: <20200710212026.27136-1-refactormyself@gmail.com> References: <20200710212026.27136-1-refactormyself@gmail.com> Cc: linux-wireless@vger.kernel.org, linux-pci@vger.kernel.org, QCA ath9k Development , netdev@vger.kernel.org, Oliver O'Halloran , Russell Currey , Stanislaw Gruszka , linux-acpi@vger.kernel.org, linux-rdma@vger.kernel.org, Jason Gunthorpe , Doug Ledford , Jakub Kicinski , linux-kernel-mentees@lists.linuxfoundation.org, Len Brown , Arnd Bergmann , Kalle Valo , Mike Marciniszyn , Sam Bobroff , Dennis Dalessandro , "Rafael J. Wysocki" , linux-kernel@vger.kernel.org, Lukas Wunner , Bolarinwa Olayemi Saheed , linuxppc-dev@lists.ozlabs.org, "David S. Miller" Subject: [Linux-kernel-mentees] [PATCH 14/14 v3] PCI: Remove '*val = 0' from pcie_capability_read_*() X-BeenThere: linux-kernel-mentees@lists.linuxfoundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-kernel-mentees-bounces@lists.linuxfoundation.org Sender: "Linux-kernel-mentees" From: Bolarinwa Olayemi Saheed There are several reasons why a PCI capability read may fail whether the device is present or not. If this happens, pcie_capability_read_*() will return -EINVAL/PCIBIOS_BAD_REGISTER_NUMBER or PCIBIOS_DEVICE_NOT_FOUND and *val is set to 0. This behaviour if further ensured by this code inside pcie_capability_read_*() ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val); /* * Reset *val to 0 if pci_read_config_dword() fails, it may * have been written as 0xFFFFFFFF if hardware error happens * during pci_read_config_dword(). */ if (ret) *val = 0; return ret; a) Since all pci_generic_config_read() does is read a register value, it may return success after reading a ~0 which *may* have been fabricated by the PCI host bridge due to a read timeout. Hence pci_read_config_*() will return success with a fabricated ~0 in *val, indicating a problem. In this case, the assumed behaviour of pcie_capability_read_*() will be wrong. To avoid error slipping through, more checks are necessary. b) pci_read_config_*() will return PCIBIOS_DEVICE_NOT_FOUND only if dev->error_state = pci_channel_io_perm_failure (i.e. pci_dev_is_disconnected()) or if pci_generic_config_read() can't find the device. In both cases *val is initially set to ~0 but as shown in the code above pcie_capability_read_*() resets it back to 0. Even with this effort, drivers still have to perform validation checks more so if 0 is a valid value. Most drivers only consider the case (b) and in some cases, there is the expectation that on timeout *val has a fabricated value of ~0, which *may* not always be true as explained in (a). In any case, checks need to be done to validate the value read and maybe confirm which error has occurred. It is better left to the drivers to do. Remove the reset of *val to 0 when pci_read_config_*() fails. Suggested-by: Bjorn Helgaas Signed-off-by: Bolarinwa Olayemi Saheed --- This patch depends on all of the preceeding patches in this series, otherwise it will introduce bugs as pointed out in the commit message of each. drivers/pci/access.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/drivers/pci/access.c b/drivers/pci/access.c index 79c4a2ef269a..ec95edbb1ac8 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -413,13 +413,6 @@ int pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val) if (pcie_capability_reg_implemented(dev, pos)) { ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val); - /* - * Reset *val to 0 if pci_read_config_word() fails, it may - * have been written as 0xFFFF if hardware error happens - * during pci_read_config_word(). - */ - if (ret) - *val = 0; return ret; } @@ -448,13 +441,6 @@ int pcie_capability_read_dword(struct pci_dev *dev, int pos, u32 *val) if (pcie_capability_reg_implemented(dev, pos)) { ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val); - /* - * Reset *val to 0 if pci_read_config_dword() fails, it may - * have been written as 0xFFFFFFFF if hardware error happens - * during pci_read_config_dword(). - */ - if (ret) - *val = 0; return ret; } -- 2.18.2 _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees