[PATCH] PCI: Update saved_config_space upon resource assignment

[PATCH] PCI: Update saved_config_space upon resource assignment

[Lukas Wunner]

Bernd reports passthrough failure of a Digital Devices Cine S2 V6 DVB
adapter plugged into an ASRock X570S PG Riptide board with BIOS version
P5.41 (09/07/2023):

  ddbridge 0000:05:00.0: detected Digital Devices Cine S2 V6 DVB adapter
  ddbridge 0000:05:00.0: cannot read registers
  ddbridge 0000:05:00.0: fail

BIOS assigns an incorrect BAR to the DVB adapter which doesn't fit into
the upstream bridge window.  The kernel corrects the BAR assignment:

  pci 0000:07:00.0: BAR 0 [mem 0xfffffffffc500000-0xfffffffffc50ffff 64bit]: can't claim; no compatible bridge window
  pci 0000:07:00.0: BAR 0 [mem 0xfc500000-0xfc50ffff 64bit]: assigned

Correction of the BAR assignment happens in an x86-specific fs_initcall,
pcibios_assign_resources(), after device enumeration in a subsys_initcall.
This order was introduced at the behest of Linus in 2004:

    https://git.kernel.org/tglx/history/c/a06a30144bbc

No other architecture performs such a late BAR correction.

Bernd bisected the issue to commit a2f1e22390ac ("PCI/ERR: Ensure error
recoverability at all times"), but it only occurs in the absence of commit
4d4c10f763d7 ("PCI: Explicitly put devices into D0 when initializing").
This combination exists in stable kernel v6.12.70, but not in mainline,
hence Bernd cannot reproduce the issue with mainline.

Since a2f1e22390ac, config space is saved on enumeration, prior to BAR
correction.  Upon passthrough, the corrected BAR is overwritten with the
incorrect saved value by:

  vfio_pci_core_register_device()
    vfio_pci_set_power_state()
      pci_restore_state()

But only if the device's current_state is PCI_UNKNOWN, as it was prior to
commit 4d4c10f763d7.  Since the commit, it is PCI_D0, which changes the
behavior of vfio_pci_set_power_state() to no longer restore the state
without saving it first.

Alexandre is reporting the same issue as Bernd, but in his case, mainline
is affected as well.  The difference is that on Alexandre's system, the
host kernel binds a driver to the device which is unbound prior to
passthrough, whereas on Bernd's system no driver gets bound by the host
kernel.

Unbinding sets current_state to PCI_UNKNOWN in pci_device_remove(), so
when vfio-pci is subsequently bound to the device, pci_restore_state() is
once again called without invoking pci_save_state() first.

To robustly fix the issue, always update saved_config_space upon resource
assignment.

Reported-by: Bernd Schumacher <bernd@bschu.de>
Tested-by: Bernd Schumacher <bernd@bschu.de>
Closes: https://lore.kernel.org/r/acfZrlP0Ua_5D3U4@eldamar.lan/
Reported-by: Alexandre N. <an.tech@mailo.com>
Tested-by: Alexandre N. <an.tech@mailo.com>
Closes: https://lore.kernel.org/r/dd3c3358-de0f-4a56-9c81-04aceaab4058@mailo.com/
Fixes: a2f1e22390ac ("PCI/ERR: Ensure error recoverability at all times")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v6.12+
---
 drivers/pci/setup-res.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index e5fcadf..1769ba9 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -102,6 +102,7 @@ static void pci_std_update_resource(struct pci_dev *dev, int resno)
 	}
 
 	pci_write_config_dword(dev, reg, new);
+	dev->saved_config_space[reg / 4] = new;
 	pci_read_config_dword(dev, reg, &check);
 
 	if ((new ^ check) & mask) {
@@ -112,6 +113,7 @@ static void pci_std_update_resource(struct pci_dev *dev, int resno)
 	if (res->flags & IORESOURCE_MEM_64) {
 		new = region.start >> 16 >> 16;
 		pci_write_config_dword(dev, reg + 4, new);
+		dev->saved_config_space[(reg + 4) / 4] = new;
 		pci_read_config_dword(dev, reg + 4, &check);
 		if (check != new) {
 			pci_err(dev, "%s: error updating (high %#010x != %#010x)\n",
-- 
2.51.0

Re: [PATCH] PCI: Update saved_config_space upon resource assignment

[Ilpo Järvinen]

On Wed, 15 Apr 2026, Lukas Wunner wrote:

> Bernd reports passthrough failure of a Digital Devices Cine S2 V6 DVB
> adapter plugged into an ASRock X570S PG Riptide board with BIOS version
> P5.41 (09/07/2023):
> 
>   ddbridge 0000:05:00.0: detected Digital Devices Cine S2 V6 DVB adapter
>   ddbridge 0000:05:00.0: cannot read registers
>   ddbridge 0000:05:00.0: fail
> 
> BIOS assigns an incorrect BAR to the DVB adapter which doesn't fit into
> the upstream bridge window.  The kernel corrects the BAR assignment:
> 
>   pci 0000:07:00.0: BAR 0 [mem 0xfffffffffc500000-0xfffffffffc50ffff 64bit]: can't claim; no compatible bridge window
>   pci 0000:07:00.0: BAR 0 [mem 0xfc500000-0xfc50ffff 64bit]: assigned
> 
> Correction of the BAR assignment happens in an x86-specific fs_initcall,
> pcibios_assign_resources(), after device enumeration in a subsys_initcall.
> This order was introduced at the behest of Linus in 2004:
> 
>     https://git.kernel.org/tglx/history/c/a06a30144bbc
> 
> No other architecture performs such a late BAR correction.
> 
> Bernd bisected the issue to commit a2f1e22390ac ("PCI/ERR: Ensure error
> recoverability at all times"), but it only occurs in the absence of commit
> 4d4c10f763d7 ("PCI: Explicitly put devices into D0 when initializing").
> This combination exists in stable kernel v6.12.70, but not in mainline,
> hence Bernd cannot reproduce the issue with mainline.
> 
> Since a2f1e22390ac, config space is saved on enumeration, prior to BAR
> correction.  Upon passthrough, the corrected BAR is overwritten with the
> incorrect saved value by:
> 
>   vfio_pci_core_register_device()
>     vfio_pci_set_power_state()
>       pci_restore_state()
> 
> But only if the device's current_state is PCI_UNKNOWN, as it was prior to
> commit 4d4c10f763d7.  Since the commit, it is PCI_D0, which changes the
> behavior of vfio_pci_set_power_state() to no longer restore the state
> without saving it first.
> 
> Alexandre is reporting the same issue as Bernd, but in his case, mainline
> is affected as well.  The difference is that on Alexandre's system, the
> host kernel binds a driver to the device which is unbound prior to
> passthrough, whereas on Bernd's system no driver gets bound by the host
> kernel.
> 
> Unbinding sets current_state to PCI_UNKNOWN in pci_device_remove(), so
> when vfio-pci is subsequently bound to the device, pci_restore_state() is
> once again called without invoking pci_save_state() first.
> 
> To robustly fix the issue, always update saved_config_space upon resource
> assignment.
> 
> Reported-by: Bernd Schumacher <bernd@bschu.de>
> Tested-by: Bernd Schumacher <bernd@bschu.de>
> Closes: https://lore.kernel.org/r/acfZrlP0Ua_5D3U4@eldamar.lan/
> Reported-by: Alexandre N. <an.tech@mailo.com>
> Tested-by: Alexandre N. <an.tech@mailo.com>
> Closes: https://lore.kernel.org/r/dd3c3358-de0f-4a56-9c81-04aceaab4058@mailo.com/
> Fixes: a2f1e22390ac ("PCI/ERR: Ensure error recoverability at all times")
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> Cc: stable@vger.kernel.org # v6.12+
> ---
>  drivers/pci/setup-res.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index e5fcadf..1769ba9 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -102,6 +102,7 @@ static void pci_std_update_resource(struct pci_dev *dev, int resno)
>  	}
>  
>  	pci_write_config_dword(dev, reg, new);
> +	dev->saved_config_space[reg / 4] = new;
>  	pci_read_config_dword(dev, reg, &check);
>  
>  	if ((new ^ check) & mask) {
> @@ -112,6 +113,7 @@ static void pci_std_update_resource(struct pci_dev *dev, int resno)
>  	if (res->flags & IORESOURCE_MEM_64) {
>  		new = region.start >> 16 >> 16;
>  		pci_write_config_dword(dev, reg + 4, new);
> +		dev->saved_config_space[(reg + 4) / 4] = new;
>  		pci_read_config_dword(dev, reg + 4, &check);
>  		if (check != new) {
>  			pci_err(dev, "%s: error updating (high %#010x != %#010x)\n",

Hi Lukas,

I'm wondering if there's something that makes this problem specific to 
only standard BARs?

Can other resources, namely IOV resources or bridge windows, similarly be 
updated "too late" and not get correctly updated into the saved config 
space?

-- 
 i.

Re: [PATCH] PCI: Update saved_config_space upon resource assignment

[Lukas Wunner]

On Thu, Apr 16, 2026 at 02:08:03PM +0300, Ilpo Järvinen wrote:
> On Wed, 15 Apr 2026, Lukas Wunner wrote:
> > Bernd reports passthrough failure of a Digital Devices Cine S2 V6 DVB
> > adapter plugged into an ASRock X570S PG Riptide board with BIOS version
> > P5.41 (09/07/2023):
[...]
> > Since a2f1e22390ac, config space is saved on enumeration, prior to BAR
> > correction.  Upon passthrough, the corrected BAR is overwritten with the
> > incorrect saved value by:
> 
> I'm wondering if there's something that makes this problem specific to 
> only standard BARs?
> 
> Can other resources, namely IOV resources or bridge windows, similarly be 
> updated "too late" and not get correctly updated into the saved config 
> space?

IOV registers are not saved, they're reconstructed from pci_dev->resource[]:

pci_restore_iov_state()
  sriov_restore_state()
    pci_update_resource()
      pci_iov_update_resource()

Bridge windows are saved when portdrv probes (see call to pci_save_state()
in pcie_portdrv_probe()) and that happens after the fs_initcall() because
portdrv is registered from a device_initcall().  So those should be fine
as well.

(FWIW sashiko also flagged the bridge windows not getting saved as an
(alleged) problem.)

Thanks for taking a look!

Lukas

Re: [PATCH] PCI: Update saved_config_space upon resource assignment

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1976 bytes --]

On Thu, 16 Apr 2026, Lukas Wunner wrote:

> On Thu, Apr 16, 2026 at 02:08:03PM +0300, Ilpo Järvinen wrote:
> > On Wed, 15 Apr 2026, Lukas Wunner wrote:
> > > Bernd reports passthrough failure of a Digital Devices Cine S2 V6 DVB
> > > adapter plugged into an ASRock X570S PG Riptide board with BIOS version
> > > P5.41 (09/07/2023):
> [...]
> > > Since a2f1e22390ac, config space is saved on enumeration, prior to BAR
> > > correction.  Upon passthrough, the corrected BAR is overwritten with the
> > > incorrect saved value by:
> > 
> > I'm wondering if there's something that makes this problem specific to 
> > only standard BARs?
> > 
> > Can other resources, namely IOV resources or bridge windows, similarly be 
> > updated "too late" and not get correctly updated into the saved config 
> > space?
> 
> IOV registers are not saved, they're reconstructed from pci_dev->resource[]:
> 
> pci_restore_iov_state()
>   sriov_restore_state()
>     pci_update_resource()
>       pci_iov_update_resource()
>
> Bridge windows are saved when portdrv probes (see call to pci_save_state()
> in pcie_portdrv_probe()) and that happens after the fs_initcall() because
> portdrv is registered from a device_initcall().  So those should be fine
> as well.
> 
> (FWIW sashiko also flagged the bridge windows not getting saved as an
> (alleged) problem.)
> 
> Thanks for taking a look!

Okay, fair.

It feels a bit odd they're all handled differently (not a problem when it 
comes to this patch). Maybe it would make sense to eventually restore them 
all from the pci_dev's resource array. Add something like 
pci_restore_resources() that would restore all the resources that are 
relevant for the type. 

I don't know if that's doable when it comes to the order of things, 
especially with sriov, but if doable, it would prevent them ever getting 
out of sync again and would avoid the trouble of having to save them 
elsewhere.

-- 
 i.

Re: [PATCH] PCI: Update saved_config_space upon resource assignment

[Lukas Wunner]

On Thu, Apr 16, 2026 at 03:16:08PM +0300, Ilpo Järvinen wrote:
> On Thu, 16 Apr 2026, Lukas Wunner wrote:
> > IOV registers are not saved, they're reconstructed from
> > pci_dev->resource[]:
[...]
> > Bridge windows are saved when portdrv probes (see call to pci_save_state()
> > in pcie_portdrv_probe()) and that happens after the fs_initcall() because
> > portdrv is registered from a device_initcall().  So those should be fine
> > as well.
> 
> It feels a bit odd they're all handled differently (not a problem when it 
> comes to this patch). Maybe it would make sense to eventually restore them 
> all from the pci_dev's resource array. Add something like 
> pci_restore_resources() that would restore all the resources that are 
> relevant for the type. 
> 
> I don't know if that's doable when it comes to the order of things, 
> especially with sriov, but if doable, it would prevent them ever getting 
> out of sync again and would avoid the trouble of having to save them 
> elsewhere.

I did consider restoring standard BARs from pci_dev->resource[],
but the calculations and iterations involved looked expensive to me,
compared to the cheap copying from pci_dev->saved_config_space[]
that we currently do.  It didn't look like something that lends itself
well for a fix that needs to be backported.

A different approach that I'm dwelling on is to amend accessors such as
pci_write_config_dword() to update pci_dev->saved_config_space[]
implicitly if the offset in config space is < 64.  And also implicitly
update pci_dev->saved_cap_space[].  The former is easy, the latter isn't.

The end goal would be to remove pci_save_state() everywhere, except to
populate the saved state once on enumeration.  I think originally the
idea was, we go to system sleep, so we save state.  We resume, we restore
state.  But then error handling came along and that means we cannot always
save the state before reset recovery, so ideally we want to save whenever
we write to config space so that cache is never stale.

I'm thinking of having a helper in <linux/pci.h> to determine whether
a register write needs to be saved.  Basically, return true for everything
with offset < 64 and for Control registers in capabilities, return false
for everything else such as Status registers.  By having that in a header, 
the compiler could deduce at compile time whether saving is necessary
or can be skipped.

The problem is that to save registers in capabilities, the hlist needs
to be searched, which is expensive.  Maybe we can speed it up by using
an xarray.  Or cache the offset of all capabilities whose registers are
saved.  We already cache e.g. aer_cap, but other offsets are not cached.
Hmmmm...

Thanks,

Lukas