[{"id":3685377,"web_url":"http://patchwork.ozlabs.org/comment/3685377/","msgid":"<20260503135109.46193C2BCB4@smtp.kernel.org>","list_archive_url":null,"date":"2026-05-03T13:51:08","subject":"Re: [PATCH] PCI: Drop unnecessary retries when restoring BARs","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 1 potential issue(s) to consider:\n- [High] Removing the read-back and retry loop for BAR restoration in `pci_restore_state()` introduces a risk of silent regressions for hardware resuming from non-FLR resets (such as D3hot to D0 transitions or custom driver resets). The commit incorrectly assumes the 60s delay from `pci_dev_wait()` covers all usages, but standard PM resume paths only delay for 10ms (`PCI_PM_D3_WAIT`) before calling `pci_restore_state()`. Historically, hardware that needed slightly longer to accept configuration writes relied on the 10x 1ms retry loop to successfully restore BARs. By removing both the retry and the read-back verification, BAR writes to slow devices will be silently dropped, leaving hardware unconfigured and causing MMIO accesses to result in IOMMU faults or kernel crashes.\n--\n\ncommit 28d12557c0e570f2b2d21da1dcbb7889df2f4672\nAuthor: Lukas Wunner \n\nPCI: Drop unnecessary retries when restoring BARs\n\nThis commit removes the retry loop when restoring PCI configuration space.\nIt relies on the lengthened FLR delays now present in the kernel, concluding\nthat retries for BAR restoration are no longer needed.\n\nCloses: https://lore.kernel.org/r/20260416225745.GA41850@bhelgaas/\n\n> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c\n> index 8f7cfcc00090..1e386dfaa669 100644\n> --- a/drivers/pci/pci.c\n> +++ b/drivers/pci/pci.c\n> @@ -1764,7 +1764,7 @@ int pci_save_state(struct pci_dev *dev)\n\n[ ... ]\n\n> static void pci_restore_config_dword(struct pci_dev *pdev, int offset,\n> -\t\t\t\t u32 saved_val, int retry, bool force)\n> +\t\t\t\t u32 saved_val, bool force)\n> {\n> \tu32 val;\n> \n> @@ -1772,52 +1772,42 @@ static void pci_restore_config_dword(struct pci_dev *pdev, int offset,\n> \tif (!force && val == saved_val)\n> \t\treturn;\n> \n> -\tfor (;;) {\n> -\t\tpci_dbg(pdev, \"restore config %#04x: %#010x -> %#010x\\n\",\n> -\t\t\toffset, val, saved_val);\n> -\t\tpci_write_config_dword(pdev, offset, saved_val);\n> -\t\tif (retry-- <= 0)\n> -\t\t\treturn;\n> +\tpci_dbg(pdev, \"restore config %#04x: %#010x -> %#010x\\n\", offset, val,\n> +\t\tsaved_val);\n> \n> -\t\tpci_read_config_dword(pdev, offset, &val);\n> -\t\tif (val == saved_val)\n> -\t\t\treturn;\n> -\n> -\t\tmdelay(1);\n> -\t}\n> +\tpci_write_config_dword(pdev, offset, saved_val);\n> }\n\nDoes removing the read-back and retry loop introduce a risk for hardware\nresuming from non-FLR resets?\n\nWhile FLR paths might now have a 60 second delay, pci_restore_state() is\nused globally across the kernel. For example, during a PM resume from\nD3hot to D0, the kernel typically only delays for 10ms (PCI_PM_D3_WAIT)\nbefore restoring state.\n\nIf a device is slow to exit reset, will these configuration writes be\nsilently dropped without the fallback retry loop, potentially leaving BARs\nunconfigured and causing MMIO accesses to fault?","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=ILJ/iEp0;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c09:e001:a7::12fc:5321; helo=sto.lore.kernel.org;\n envelope-from=linux-pci+bounces-53640-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=\"ILJ/iEp0\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sto.lore.kernel.org (sto.lore.kernel.org\n [IPv6:2600:3c09:e001:a7::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 4g7mQX0cgLz1yJV\n\tfor ; Sun, 03 May 2026 23:51:16 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sto.lore.kernel.org (Postfix) with ESMTP id D86EE300130C\n\tfor ; Sun, 3 May 2026 13:51:12 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id CC09A23E32B;\n\tSun, 3 May 2026 13:51:09 +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 A66FE2236F0\n\tfor ; Sun, 3 May 2026 13:51:09 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 46193C2BCB4;\n\tSun, 3 May 2026 13:51:09 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777816269; cv=none;\n b=puK3dGmU00TNUWoFLW6yhFUoMTvXlsqzNyT+F7fMUxH0t41Pdte2yDsGGS0XLdxpZHNgvE8ip6SY5jKh2bcZnHyFu5ii5x9ByanHlpa+Zc/dQlJVHoHDqJA3QcgqgTJN4bC6O8HdXm2XDy1FVhHI6RJJqQsG+Pcrtxjc6VCLivI=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777816269; c=relaxed/simple;\n\tbh=DgfVWxrYBRgguAFdrL7fywNOdpoYYzD63n1bUhrK4yw=;\n\th=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date:\n\t Message-Id;\n b=CY0XzCtR7Te1KH7+L5M8JGgQG0KZsYoYl9Zgxqs7ny+dMKIRAfljXYRKzowCdlH66ERkZB/4/KE+wxYiaP2AbEyA7bUnaaiZ8GOGQbuuS/aDe92Fb4PW0qdsZcfQrzoP/p3NYQVszEeaWddHg1us3tfX4VdYTq8PV7bZpllCNt4=","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=ILJ/iEp0; 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=1777816269;\n\tbh=DgfVWxrYBRgguAFdrL7fywNOdpoYYzD63n1bUhrK4yw=;\n\th=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date:From;\n\tb=ILJ/iEp0UTcZQXo6eRh+LNRnheOFsrE29s8ueNCy/OKwcTEVZaIaRIgmKPeIU57Et\n\t x+EUppoSRsa6+eDYv5OwwBgHSsDyMl1Q//4+aA55TkGIGxY6GGnpRVWxMO3zv7wJXf\n\t DcDr8H+PlmN4okdEljIDRpWjlMBJplqvt86j7+SEwHnjMhD7qxnAkvxYsJAvpp4tOF\n\t 37/eERiWs4GK2IgTavXWJQZcOGrFUg60PAelm6eMWfwygWTUyRbMtfmN4SzRk+AmOY\n\t eFAgeLKFaLBuj/to0NP/Nf6lW9NA22it0WKtxPOmWTiBAZW1kFuZ7fLHjCUKLKfiBw\n\t B3Ngp0QHYLAzw==","From":"sashiko-bot@kernel.org","Subject":"Re: [PATCH] PCI: Drop unnecessary retries when restoring BARs","Reply-To":"sashiko@lists.linux.dev","To":"\"Lukas Wunner\" ","Cc":"linux-pci@vger.kernel.org","In-Reply-To":"\n <785c98b50a7a00d0698848c75d51b8f5669ad18f.1777814679.git.lukas@wunner.de>","References":"\n <785c98b50a7a00d0698848c75d51b8f5669ad18f.1777814679.git.lukas@wunner.de>","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","Date":"Sun, 03 May 2026 13:51:08 +0000","Message-Id":"<20260503135109.46193C2BCB4@smtp.kernel.org>","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":""}},{"id":3685548,"web_url":"http://patchwork.ozlabs.org/comment/3685548/","msgid":"","list_archive_url":null,"date":"2026-05-04T07:49:36","subject":"Re: [PATCH] PCI: Drop unnecessary retries when restoring BARs","submitter":{"id":68499,"url":"http://patchwork.ozlabs.org/api/people/68499/","name":"Lukas Wunner","email":"lukas@wunner.de"},"content":"On Sun, May 03, 2026 at 01:51:08PM +0000, sashiko-bot@kernel.org wrote:\n> Thank you for your contribution! Sashiko AI review found 1 potential\n> issue(s) to consider:\n> - [High] Removing the read-back and retry loop for BAR restoration in\n> `pci_restore_state()` introduces a risk of silent regressions for\n> hardware resuming from non-FLR resets (such as D3hot to D0 transitions\n> or custom driver resets). The commit incorrectly assumes the 60s delay\n> from `pci_dev_wait()` covers all usages, but standard PM resume paths\n> only delay for 10ms (`PCI_PM_D3_WAIT`) before calling `pci_restore_state()`.\n> Historically, hardware that needed slightly longer to accept\n> configuration writes relied on the 10x 1ms retry loop to successfully\n> restore BARs. By removing both the retry and the read-back verification,\n> BAR writes to slow devices will be silently dropped, leaving hardware\n> unconfigured and causing MMIO accesses to result in IOMMU faults or\n> kernel crashes.\n\nHallucination alert:\n\nPCI_PM_D3_WAIT does not exist, it was renamed to PCI_PM_D3HOT_WAIT\nsix years ago by commit 3789af9a13e5, which went into v5.10.\n\nThe macro is used in:\n\npci_pm_resume_noirq()\n pci_pm_default_resume_early()\n pci_pm_power_up_and_verify_state()\n pci_power_up()\n pci_dev_d3_sleep()\n\nHowever before pci_power_up() calls pci_dev_d3_sleep(), it reads\nthe PMCSR register and errors out if config space is inaccessible.\n\nHence when pci_restore_state() is invoked a bit later, config space\ncan be assumed to be accessible.\n\nsashiko reviews are a mixed bag, I think this one doesn't even reach\njunior developer level. :(\n\nThanks,\n\nLukas","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 spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.105.105.114; helo=tor.lore.kernel.org;\n envelope-from=linux-pci+bounces-53662-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=83.223.78.233","smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=wunner.de","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=wunner.de"],"Received":["from tor.lore.kernel.org (tor.lore.kernel.org [172.105.105.114])\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 4g8DMk5xJ9z1y04\n\tfor ; Mon, 04 May 2026 17:50:26 +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 7B5B430021C2\n\tfor ; Mon, 4 May 2026 07:49:43 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 8EC252E040D;\n\tMon, 4 May 2026 07:49:42 +0000 (UTC)","from mailout2.hostsharing.net (mailout2.hostsharing.net\n [83.223.78.233])\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 8D0DE30F7FB\n\tfor ; Mon, 4 May 2026 07:49:39 +0000 (UTC)","from h08.hostsharing.net (h08.hostsharing.net\n [IPv6:2a01:37:1000::53df:5f1c:0])\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 client-signature ECDSA (secp384r1) client-digest SHA384)\n\t(Client CN \"*.hostsharing.net\",\n Issuer \"GlobalSign GCC R6 AlphaSSL CA 2025\" (verified OK))\n\tby mailout2.hostsharing.net (Postfix) with ESMTPS id D6BC910DDB;\n\tMon, 04 May 2026 09:49:36 +0200 (CEST)","by h08.hostsharing.net (Postfix, from userid 100393)\n\tid B74046019647; Mon, 4 May 2026 09:49:36 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777880982; cv=none;\n b=EC3PubSrXJBKyzdn4FN1nEdvr79NdLV8wc9TAcivqMsjFaVHoAsZMOLu4w80InGYxrVTo/bhZ6wRwpeM//C2yQnUHieygE2k8TkCyFGIXsFTHWq0gCosl2nn6mvZ8pGV2Hb97fSbhkz/xafAC/zGcmccaouOj5LHW79+VsPHzRw=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777880982; c=relaxed/simple;\n\tbh=iyd9zb129Pnm+iwV0/zcRqQV5H+vblJWszUxZPv1rlQ=;\n\th=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:\n\t Content-Type:Content-Disposition:In-Reply-To;\n b=SyPzyPJDIdk4ua8LLRGPU4rpEYjl5L2PqD9YF3tPlfvC23bP2Bg7xtDpIu5aXg5wHq80v4SLorM00wWo/2wPCcErfFnDHFQUNreIK1Ku+r4m2Ym+y4Vt+qWC6y9I6e64SCEFpISybyo4cJzI/HLXVFoFkYfRwPCHtRat2h3ez4M=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=none (p=none dis=none) header.from=wunner.de;\n spf=pass smtp.mailfrom=wunner.de; arc=none smtp.client-ip=83.223.78.233","Date":"Mon, 4 May 2026 09:49:36 +0200","From":"Lukas Wunner ","To":"sashiko-bot@kernel.org","Cc":"linux-pci@vger.kernel.org, sashiko@lists.linux.dev,\n\tBjorn Helgaas ,\n\tMarco Nenciarini ,\n\tMichal Winiarski ,\n\tIlpo Jarvinen ,\n\t\"Rafael J. Wysocki\" ,\n\tEric Chanudet ,\n\tJean Guyader ,\n\tAlex Williamson , Sinan Kaya ","Subject":"Re: [PATCH] PCI: Drop unnecessary retries when restoring BARs","Message-ID":"","References":"\n <785c98b50a7a00d0698848c75d51b8f5669ad18f.1777814679.git.lukas@wunner.de>\n <20260503135109.46193C2BCB4@smtp.kernel.org>","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-Disposition":"inline","In-Reply-To":"<20260503135109.46193C2BCB4@smtp.kernel.org>"}}]