[PATCH v2 0/2] PCI: Avoid capability searches in save/restore state

[PATCH v2 0/2] PCI: Avoid capability searches in save/restore state

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

Reduce the number of times we search config space for PCI capabilities when
saving and restoring device state.

Bjorn Helgaas (2):
  PCI: Avoid pointless capability searches
  PCI: Cache offset of Resizable BAR capability

 drivers/pci/pci.c       | 36 +++++++++++++++++++++---------------
 drivers/pci/pci.h       |  1 +
 drivers/pci/pcie/aspm.c | 15 ++++++++-------
 drivers/pci/probe.c     |  1 +
 include/linux/pci.h     |  1 +
 5 files changed, 32 insertions(+), 22 deletions(-)

v2 changes:
  - Drop VC change that broke saving state (thanks, Ilpo)

v1: https://lore.kernel.org/r/20250208050329.1092214-1-helgaas@kernel.org

-- 
2.34.1

[PATCH v2 1/2] PCI: Avoid pointless capability searches

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

Many of the save/restore functions in the pci_save_state() and
pci_restore_state() paths depend on both a PCI capability of the device and
a pci_cap_saved_state structure to hold the configuration data, and they
skip the operation if either is missing.

Look for the pci_cap_saved_state first so if we don't have one, we can skip
searching for the device capability, which requires several slow config
space accesses.

Remove some error messages if the pci_cap_saved_state is not found so we
don't complain about having no saved state for a capability the device
doesn't have.  We have already warned in pci_allocate_cap_save_buffers() if
the capability is present but we were unable to allocate a buffer.

Other than the message change, no functional change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci.c       | 27 ++++++++++++++-------------
 drivers/pci/pcie/aspm.c | 15 ++++++++-------
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 869d204a70a3..503376bf7e75 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1686,10 +1686,8 @@ static int pci_save_pcie_state(struct pci_dev *dev)
 		return 0;
 
 	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
-	if (!save_state) {
-		pci_err(dev, "buffer not found in %s\n", __func__);
+	if (!save_state)
 		return -ENOMEM;
-	}
 
 	cap = (u16 *)&save_state->cap.data[0];
 	pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &cap[i++]);
@@ -1742,19 +1740,17 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
 
 static int pci_save_pcix_state(struct pci_dev *dev)
 {
-	int pos;
 	struct pci_cap_saved_state *save_state;
+	u8 pos;
+
+	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
+	if (!save_state)
+		return -ENOMEM;
 
 	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
 	if (!pos)
 		return 0;
 
-	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
-	if (!save_state) {
-		pci_err(dev, "buffer not found in %s\n", __func__);
-		return -ENOMEM;
-	}
-
 	pci_read_config_word(dev, pos + PCI_X_CMD,
 			     (u16 *)save_state->cap.data);
 
@@ -1763,14 +1759,19 @@ static int pci_save_pcix_state(struct pci_dev *dev)
 
 static void pci_restore_pcix_state(struct pci_dev *dev)
 {
-	int i = 0, pos;
 	struct pci_cap_saved_state *save_state;
+	u8 pos;
+	int i = 0;
 	u16 *cap;
 
 	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
-	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
-	if (!save_state || !pos)
+	if (!save_state)
 		return;
+
+	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
+	if (!pos)
+		return;
+
 	cap = (u16 *)&save_state->cap.data[0];
 
 	pci_write_config_word(dev, pos + PCI_X_CMD, cap[i++]);
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index e0bc90597dca..007e4a082e6f 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -35,16 +35,14 @@ void pci_save_ltr_state(struct pci_dev *dev)
 	if (!pci_is_pcie(dev))
 		return;
 
+	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
+	if (!save_state)
+		return;
+
 	ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
 	if (!ltr)
 		return;
 
-	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
-	if (!save_state) {
-		pci_err(dev, "no suspend buffer for LTR; ASPM issues possible after resume\n");
-		return;
-	}
-
 	/* Some broken devices only support dword access to LTR */
 	cap = &save_state->cap.data[0];
 	pci_read_config_dword(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, cap);
@@ -57,8 +55,11 @@ void pci_restore_ltr_state(struct pci_dev *dev)
 	u32 *cap;
 
 	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
+	if (!save_state)
+		return;
+
 	ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
-	if (!save_state || !ltr)
+	if (!ltr)
 		return;
 
 	/* Some broken devices only support dword access to LTR */
-- 
2.34.1

Re: [PATCH v2 1/2] PCI: Avoid pointless capability searches

[Ilpo Järvinen]

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

On Fri, 14 Feb 2025, Bjorn Helgaas wrote:

> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> Many of the save/restore functions in the pci_save_state() and
> pci_restore_state() paths depend on both a PCI capability of the device and
> a pci_cap_saved_state structure to hold the configuration data, and they
> skip the operation if either is missing.
> 
> Look for the pci_cap_saved_state first so if we don't have one, we can skip
> searching for the device capability, which requires several slow config
> space accesses.
> 
> Remove some error messages if the pci_cap_saved_state is not found so we
> don't complain about having no saved state for a capability the device
> doesn't have.  We have already warned in pci_allocate_cap_save_buffers() if
> the capability is present but we were unable to allocate a buffer.
> 
> Other than the message change, no functional change intended.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/pci.c       | 27 ++++++++++++++-------------
>  drivers/pci/pcie/aspm.c | 15 ++++++++-------
>  2 files changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 869d204a70a3..503376bf7e75 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1686,10 +1686,8 @@ static int pci_save_pcie_state(struct pci_dev *dev)
>  		return 0;
>  
>  	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
> -	if (!save_state) {
> -		pci_err(dev, "buffer not found in %s\n", __func__);
> +	if (!save_state)
>  		return -ENOMEM;
> -	}
>  
>  	cap = (u16 *)&save_state->cap.data[0];
>  	pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &cap[i++]);
> @@ -1742,19 +1740,17 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
>  
>  static int pci_save_pcix_state(struct pci_dev *dev)
>  {
> -	int pos;
>  	struct pci_cap_saved_state *save_state;
> +	u8 pos;
> +
> +	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
> +	if (!save_state)
> +		return -ENOMEM;
>  
>  	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
>  	if (!pos)
>  		return 0;
>  
> -	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
> -	if (!save_state) {
> -		pci_err(dev, "buffer not found in %s\n", __func__);
> -		return -ENOMEM;
> -	}
> -
>  	pci_read_config_word(dev, pos + PCI_X_CMD,
>  			     (u16 *)save_state->cap.data);
>  
> @@ -1763,14 +1759,19 @@ static int pci_save_pcix_state(struct pci_dev *dev)
>  
>  static void pci_restore_pcix_state(struct pci_dev *dev)
>  {
> -	int i = 0, pos;
>  	struct pci_cap_saved_state *save_state;
> +	u8 pos;
> +	int i = 0;
>  	u16 *cap;
>  
>  	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
> -	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
> -	if (!save_state || !pos)
> +	if (!save_state)
>  		return;
> +
> +	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
> +	if (!pos)
> +		return;
> +
>  	cap = (u16 *)&save_state->cap.data[0];
>  
>  	pci_write_config_word(dev, pos + PCI_X_CMD, cap[i++]);
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index e0bc90597dca..007e4a082e6f 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -35,16 +35,14 @@ void pci_save_ltr_state(struct pci_dev *dev)
>  	if (!pci_is_pcie(dev))
>  		return;
>  
> +	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
> +	if (!save_state)
> +		return;
> +
>  	ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
>  	if (!ltr)
>  		return;
>  
> -	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
> -	if (!save_state) {
> -		pci_err(dev, "no suspend buffer for LTR; ASPM issues possible after resume\n");
> -		return;
> -	}
> -
>  	/* Some broken devices only support dword access to LTR */
>  	cap = &save_state->cap.data[0];
>  	pci_read_config_dword(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, cap);
> @@ -57,8 +55,11 @@ void pci_restore_ltr_state(struct pci_dev *dev)
>  	u32 *cap;
>  
>  	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
> +	if (!save_state)
> +		return;
> +
>  	ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
> -	if (!save_state || !ltr)
> +	if (!ltr)
>  		return;
>  
>  	/* Some broken devices only support dword access to LTR */
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH v2 1/2] PCI: Avoid pointless capability searches

[Kohei Enju]

> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> Many of the save/restore functions in the pci_save_state() and
> pci_restore_state() paths depend on both a PCI capability of the device and
> a pci_cap_saved_state structure to hold the configuration data, and they
> skip the operation if either is missing.
> 
> Look for the pci_cap_saved_state first so if we don't have one, we can skip
> searching for the device capability, which requires several slow config
> space accesses.
> 
> Remove some error messages if the pci_cap_saved_state is not found so we
> don't complain about having no saved state for a capability the device
> doesn't have.  We have already warned in pci_allocate_cap_save_buffers() if
> the capability is present but we were unable to allocate a buffer.
> 
> Other than the message change, no functional change intended.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>  drivers/pci/pci.c       | 27 ++++++++++++++-------------
>  drivers/pci/pcie/aspm.c | 15 ++++++++-------
>  2 files changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 869d204a70a3..503376bf7e75 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1686,10 +1686,8 @@ static int pci_save_pcie_state(struct pci_dev *dev)
>  		return 0;
>  
>  	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_EXP);
> -	if (!save_state) {
> -		pci_err(dev, "buffer not found in %s\n", __func__);
> +	if (!save_state)
>  		return -ENOMEM;
> -	}
>  
>  	cap = (u16 *)&save_state->cap.data[0];
>  	pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &cap[i++]);
> @@ -1742,19 +1740,17 @@ static void pci_restore_pcie_state(struct pci_dev *dev)
>  
>  static int pci_save_pcix_state(struct pci_dev *dev)
>  {
> -	int pos;
>  	struct pci_cap_saved_state *save_state;
> +	u8 pos;
> +
> +	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
> +	if (!save_state)
> +		return -ENOMEM;
>  
>  	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
>  	if (!pos)
>  		return 0;
>  
> -	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
> -	if (!save_state) {
> -		pci_err(dev, "buffer not found in %s\n", __func__);
> -		return -ENOMEM;
> -	}

When devices don't have PCI_CAP_ID_PCIX, this change in order appears to 
cause a functional change.
Since probe functions of some drivers (e.g. Intel e1000) rely on the return 
value, I think they fail after that change in this situation.

Actually in my QEMU VM, e1000 driver failed to probe the device due to 
-ENOMEM from pci_save_pcix_state().
```
[root@localhost ~]# dmesg | grep e1000
[    0.400303] [      T1] e1000: Intel(R) PRO/1000 Network Driver
[    0.400805] [      T1] e1000: Copyright (c) 1999-2006 Intel Corporation.
[    0.710970] [      T1] e1000 0000:00:03.0: probe with driver e1000 failed with error -12

[root@localhost ~]# lspci -nnvs 00:03.0
00:03.0 Ethernet controller [0200]: Intel Corporation 82540EM Gigabit Ethernet Controller [8086:100e] (rev 03)
        Subsystem: Red Hat, Inc. QEMU Virtual Machine [1af4:1100]
        Flags: fast devsel, IRQ 11
        Memory at febc0000 (32-bit, non-prefetchable) [size=128K]
        I/O ports at c000 [size=64]
        Expansion ROM at feb80000 [disabled] [size=256K]
lspci: Unable to load libkmod resources: error -2
```

Regarding pci_save_vc_state(), I found that similar comments were provided in 
this context:
https://lore.kernel.org/all/7dbb0d8b-3708-60ba-ee9e-78aa48bee160@linux.intel.com/

However, the same type of order change is still left in 
pci_save_pcix_state().

> -
>  	pci_read_config_word(dev, pos + PCI_X_CMD,
>  			     (u16 *)save_state->cap.data);
>  
> @@ -1763,14 +1759,19 @@ static int pci_save_pcix_state(struct pci_dev *dev)
>  
>  static void pci_restore_pcix_state(struct pci_dev *dev)
>  {
> -	int i = 0, pos;
>  	struct pci_cap_saved_state *save_state;
> +	u8 pos;
> +	int i = 0;
>  	u16 *cap;
>  
>  	save_state = pci_find_saved_cap(dev, PCI_CAP_ID_PCIX);
> -	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
> -	if (!save_state || !pos)
> +	if (!save_state)
>  		return;
> +
> +	pos = pci_find_capability(dev, PCI_CAP_ID_PCIX);
> +	if (!pos)
> +		return;
> +
>  	cap = (u16 *)&save_state->cap.data[0];
>  
>  	pci_write_config_word(dev, pos + PCI_X_CMD, cap[i++]);
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index e0bc90597dca..007e4a082e6f 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -35,16 +35,14 @@ void pci_save_ltr_state(struct pci_dev *dev)
>  	if (!pci_is_pcie(dev))
>  		return;
>  
> +	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
> +	if (!save_state)
> +		return;
> +
>  	ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
>  	if (!ltr)
>  		return;
>  
> -	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
> -	if (!save_state) {
> -		pci_err(dev, "no suspend buffer for LTR; ASPM issues possible after resume\n");
> -		return;
> -	}
> -
>  	/* Some broken devices only support dword access to LTR */
>  	cap = &save_state->cap.data[0];
>  	pci_read_config_dword(dev, ltr + PCI_LTR_MAX_SNOOP_LAT, cap);
> @@ -57,8 +55,11 @@ void pci_restore_ltr_state(struct pci_dev *dev)
>  	u32 *cap;
>  
>  	save_state = pci_find_saved_ext_cap(dev, PCI_EXT_CAP_ID_LTR);
> +	if (!save_state)
> +		return;
> +
>  	ltr = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_LTR);
> -	if (!save_state || !ltr)
> +	if (!ltr)
>  		return;
>  
>  	/* Some broken devices only support dword access to LTR */
> -- 
> 2.34.1

Regards,
Kohei

Re: [PATCH v2 1/2] PCI: Avoid pointless capability searches

[kernel test robot]

Hello,

kernel test robot noticed "last_state.booting" on:

commit: c8a9382e172ac80bc96820b3ec758e35cdc05c06 ("[PATCH v2 1/2] PCI: Avoid pointless capability searches")
url: https://github.com/intel-lab-lkp/linux/commits/Bjorn-Helgaas/PCI-Avoid-pointless-capability-searches/20250215-080525
base: https://git.kernel.org/cgit/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/all/20250215000301.175097-2-helgaas@kernel.org/
patch subject: [PATCH v2 1/2] PCI: Avoid pointless capability searches

in testcase: xfstests
version: xfstests-x86_64-8467552f-1_20241215
with following parameters:

	disk: 4HDD
	fs: ext2
	test: generic-holetest



config: x86_64-rhel-9.4-func
compiler: gcc-12
test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (Skylake) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202503042124.7627f722-lkp@intel.com


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250304/202503042124.7627f722-lkp@intel.com



[   62.784123][  T284] LKP: waiting for network...
[   62.784128][  T284] 
[   63.218408][  T284] ls /sys/class/net
[   63.218417][  T284] 
[   63.224484][  T284] lo
[   63.224490][  T284] 
[   64.076175][    T1] watchdog: watchdog0: watchdog did not stop!
[   64.157917][    T1] watchdog: watchdog0: watchdog did not stop!
[   64.196710][    T1] sd 1:0:0:0: [sdb] Synchronizing SCSI cache
[   64.202659][    T1] sd 0:0:0:0: [sda] Synchronizing SCSI cache
[   64.969888][    T1] pcieport 0000:00:01.0: VC0 negotiation stuck pending
[   64.977342][    T1] ACPI: PM: Preparing to enter system sleep state S5
[   64.986446][    T1] kvm: exiting hardware virtualization
reboot: Restarting system


there is no more useful information, seems our test machine just reboot here.
this is not observed on the parent commit, c71f7bbc5d794, which chosen as the
base by bot.

* a234e07a63859 (linux-review/Bjorn-Helgaas/PCI-Avoid-pointless-capability-searches/20250215-080525) PCI: Cache offset of Resizable BAR capability
* c8a9382e172ac PCI: Avoid pointless capability searches
*   c71f7bbc5d794 Merge branch 'pci/endpoint'

c71f7bbc5d794984 c8a9382e172ac80bc96820b3ec7
---------------- ---------------------------
       fail:runs  %reproduction    fail:runs
           |             |             |
           :6          100%           6:6     last_state.booting


-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/2] PCI: Avoid pointless capability searches

[Bjorn Helgaas]

On Tue, Mar 04, 2025 at 10:12:48PM +0800, kernel test robot wrote:
> kernel test robot noticed "last_state.booting" on:
> 
> commit: c8a9382e172ac80bc96820b3ec758e35cdc05c06 ("[PATCH v2 1/2] PCI: Avoid pointless capability searches")
> url: https://github.com/intel-lab-lkp/linux/commits/Bjorn-Helgaas/PCI-Avoid-pointless-capability-searches/20250215-080525
> base: https://git.kernel.org/cgit/linux/kernel/git/pci/pci.git next
> patch link: https://lore.kernel.org/all/20250215000301.175097-2-helgaas@kernel.org/
> patch subject: [PATCH v2 1/2] PCI: Avoid pointless capability searches

This patch was dropped in next-20250224.

> in testcase: xfstests
> version: xfstests-x86_64-8467552f-1_20241215
> with following parameters:
> 
> 	disk: 4HDD
> 	fs: ext2
> 	test: generic-holetest
> 
> 
> 
> config: x86_64-rhel-9.4-func
> compiler: gcc-12
> test machine: 4 threads Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (Skylake) with 32G memory
> 
> (please refer to attached dmesg/kmsg for entire log/backtrace)
> 
> 
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202503042124.7627f722-lkp@intel.com
> 
> 
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20250304/202503042124.7627f722-lkp@intel.com
> 
> 
> 
> [   62.784123][  T284] LKP: waiting for network...
> [   62.784128][  T284] 
> [   63.218408][  T284] ls /sys/class/net
> [   63.218417][  T284] 
> [   63.224484][  T284] lo
> [   63.224490][  T284] 
> [   64.076175][    T1] watchdog: watchdog0: watchdog did not stop!
> [   64.157917][    T1] watchdog: watchdog0: watchdog did not stop!
> [   64.196710][    T1] sd 1:0:0:0: [sdb] Synchronizing SCSI cache
> [   64.202659][    T1] sd 0:0:0:0: [sda] Synchronizing SCSI cache
> [   64.969888][    T1] pcieport 0000:00:01.0: VC0 negotiation stuck pending
> [   64.977342][    T1] ACPI: PM: Preparing to enter system sleep state S5
> [   64.986446][    T1] kvm: exiting hardware virtualization
> reboot: Restarting system
> 
> 
> there is no more useful information, seems our test machine just reboot here.
> this is not observed on the parent commit, c71f7bbc5d794, which chosen as the
> base by bot.
> 
> * a234e07a63859 (linux-review/Bjorn-Helgaas/PCI-Avoid-pointless-capability-searches/20250215-080525) PCI: Cache offset of Resizable BAR capability
> * c8a9382e172ac PCI: Avoid pointless capability searches
> *   c71f7bbc5d794 Merge branch 'pci/endpoint'
> 
> c71f7bbc5d794984 c8a9382e172ac80bc96820b3ec7
> ---------------- ---------------------------
>        fail:runs  %reproduction    fail:runs
>            |             |             |
>            :6          100%           6:6     last_state.booting
> 
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
> 

[PATCH v2 2/2] PCI: Cache offset of Resizable BAR capability

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

Previously most resizable BAR interfaces (pci_rebar_get_possible_sizes(),
pci_rebar_set_size(), etc) as well as pci_restore_state() searched config
space for a Resizable BAR capability.  Most devices don't have such a
capability, so this is wasted effort, especially for pci_restore_state().

Search for a Resizable BAR capability once at enumeration-time and cache
the offset so we don't have to search every time we need it.  No functional
change intended.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/pci.c   | 9 +++++++--
 drivers/pci/pci.h   | 1 +
 drivers/pci/probe.c | 1 +
 include/linux/pci.h | 1 +
 4 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 503376bf7e75..cf2632080a94 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1872,7 +1872,7 @@ static void pci_restore_rebar_state(struct pci_dev *pdev)
 	unsigned int pos, nbars, i;
 	u32 ctrl;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+	pos = pdev->rebar_cap;
 	if (!pos)
 		return;
 
@@ -3719,6 +3719,11 @@ void pci_acs_init(struct pci_dev *dev)
 	pci_enable_acs(dev);
 }
 
+void pci_rebar_init(struct pci_dev *pdev)
+{
+	pdev->rebar_cap = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+}
+
 /**
  * pci_rebar_find_pos - find position of resize ctrl reg for BAR
  * @pdev: PCI device
@@ -3733,7 +3738,7 @@ static int pci_rebar_find_pos(struct pci_dev *pdev, int bar)
 	unsigned int pos, nbars, i;
 	u32 ctrl;
 
-	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
+	pos = pdev->rebar_cap;
 	if (!pos)
 		return -ENOTSUPP;
 
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 01e51db8d285..d7b46ddfd6d2 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -799,6 +799,7 @@ static inline int acpi_get_rc_resources(struct device *dev, const char *hid,
 }
 #endif
 
+void pci_rebar_init(struct pci_dev *pdev);
 int pci_rebar_get_current_size(struct pci_dev *pdev, int bar);
 int pci_rebar_set_size(struct pci_dev *pdev, int bar, int size);
 static inline u64 pci_rebar_size_to_bytes(int size)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b6536ed599c3..24dd3dcfd223 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2564,6 +2564,7 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_rcec_init(dev);		/* Root Complex Event Collector */
 	pci_doe_init(dev);		/* Data Object Exchange */
 	pci_tph_init(dev);		/* TLP Processing Hints */
+	pci_rebar_init(dev);		/* Resizable BAR */
 
 	pcie_report_downtraining(dev);
 	pci_init_reset_methods(dev);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 47b31ad724fa..9e5bbd996c83 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -353,6 +353,7 @@ struct pci_dev {
 	struct pci_dev  *rcec;          /* Associated RCEC device */
 #endif
 	u32		devcap;		/* PCIe Device Capabilities */
+	u16		rebar_cap;	/* Resizable BAR capability offset */
 	u8		pcie_cap;	/* PCIe capability offset */
 	u8		msi_cap;	/* MSI capability offset */
 	u8		msix_cap;	/* MSI-X capability offset */
-- 
2.34.1

Re: [PATCH v2 0/2] PCI: Avoid capability searches in save/restore state

[Bjorn Helgaas]

On Fri, Feb 14, 2025 at 06:02:59PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> Reduce the number of times we search config space for PCI capabilities when
> saving and restoring device state.
> 
> Bjorn Helgaas (2):
>   PCI: Avoid pointless capability searches
>   PCI: Cache offset of Resizable BAR capability
> 
>  drivers/pci/pci.c       | 36 +++++++++++++++++++++---------------
>  drivers/pci/pci.h       |  1 +
>  drivers/pci/pcie/aspm.c | 15 ++++++++-------
>  drivers/pci/probe.c     |  1 +
>  include/linux/pci.h     |  1 +
>  5 files changed, 32 insertions(+), 22 deletions(-)
> 
> v2 changes:
>   - Drop VC change that broke saving state (thanks, Ilpo)
> 
> v1: https://lore.kernel.org/r/20250208050329.1092214-1-helgaas@kernel.org

Applied to pci/enumeration with Ilpo's reviewed-by for v6.15.