Re: [RFC PATCH 06/13] pci: pcsc: handle device resets

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Fri, 3 Oct 2025 09:00:42 +0000
Evangelos Petrongonas <epetron@amazon.de> wrote:

> The PCI Configuration Space Cache (PCSC) maintains cached values of
> configuration space registers for performance optimization. When a PCI
> device is reset or bus operations are dynamically changed, cached values
> become stale and can cause incorrect behavior. This patch ensures cache
> coherency by invalidating the PCSC cache in all scenarios where the
> underlying configuration space values may have changed.
> 
> Device Reset Handling:
> ----------------------
> When PCI devices are reset, their configuration space registers return
> to default values. Add pcsc_device_reset() calls after all device reset
> operations to invalidate stale cached values:
> 
> - Function Level Resets (FLR) in `pcie_flr()`
> - Advanced Features FLR in `pci_af_flr()`
> - Power Management resets (D3hot->D0 transition) in `pci_pm_reset()`
> - Device-specific resets in `pci_dev_specific_reset()`
> - D3cold power state transitions in `__pci_set_power_state()`
> - ACPI-based resets in `pci_dev_acpi_reset()`
> - Bus restore operations in `pci_bus_restore_locked()`
> - Slot restore operations in `pci_slot_restore_locked()`
> - Secondary bus resets in `pci_bridge_secondary_bus_reset()`

cxl bus reset? 

> 
> For secondary bus resets, `pcsc_reset_bus_recursively()` invalidates the
> cache for all devices on the secondary bus and subordinate buses. This
> also covers hotplug slot reset operations since `pciehp_reset_slot()`
> calls `pci_bridge_secondary_bus_reset()`.
> 
> In addition, functions like `pci_dev_wait` are configured to bypass the
> cahce and reads the actual HW values.

cache

> 
> Dynamic Ops Changes:
> --------------------
> The patch also addresses cache consistency issues when bus operations
> are dynamically changed via `pci_bus_set_ops()``. Different ops
> implementations may return different values for the same registers, and
> hardware state may have changed while using the different ops. This
> commit resets the cache for all devices on the affected bus
> 
> Implementation Details:
> -----------------------
> The cache invalidation clears the cached_bitmask while preserving the
> cacheable_bitmask, as the configuration space layout remains unchanged
> after a reset. This allows the cache to be repopulated with fresh values
> on subsequent configuration space accesses.
> 
> Known Limitations:
> ------------------
> - There is currently a gap in handling PowerPC secondary bus resets, as
> the architecture-specific `pcibios_reset_secondary_bus()` can bypass the
> generic `pci_reset_secondary_bus()` where our cache invalidation occurs.
> 
> Signed-off-by: Evangelos Petrongonas <epetron@amazon.de>
> Signed-off-by: Stanislav Spassov <stanspas@amazon.de>


> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index f518cfa266b5..db940f8fd408 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -26,6 +26,7 @@
>  #include <linux/device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/pci_hotplug.h>
> +#include <linux/pcsc.h>
>  #include <linux/vmalloc.h>
>  #include <asm/dma.h>
>  #include <linux/aer.h>
> @@ -1248,11 +1249,19 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
>  		}
>  
>  		if (root && root->config_rrs_sv) {
> +#ifdef CONFIG_PCSC
> +			pcsc_hw_config_read(dev->bus, dev->devfn, PCI_VENDOR_ID, 4, &id);
> +#else
>  			pci_read_config_dword(dev, PCI_VENDOR_ID, &id);
> +#endif
>  			if (!pci_bus_rrs_vendor_id(id))
>  				break;
>  		} else {
> +#ifdef CONFIG_PCSC
> +			pcsc_hw_config_read(dev->bus, dev->devfn, PCI_COMMAND, 4, &id);
In the !CONFIG case define this to be pci_read_config_dword()

> +#else
>  			pci_read_config_dword(dev, PCI_COMMAND, &id);
> +#endif
>  			if (!PCI_POSSIBLE_ERROR(id))
>  				break;
>  		}

>  void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
> @@ -5542,6 +5594,9 @@ static void pci_bus_restore_locked(struct pci_bus *bus)
>  
>  	list_for_each_entry(dev, &bus->devices, bus_list) {
>  		pci_dev_restore(dev);
> +#ifdef CONFIG_PCSC
> +		pcsc_device_reset(dev);
> +#endif
>  		if (dev->subordinate) {
>  			pci_bridge_wait_for_secondary_bus(dev, "bus reset");
>  			pci_bus_restore_locked(dev->subordinate);
> @@ -5579,6 +5634,9 @@ static void pci_slot_restore_locked(struct pci_slot *slot)
>  		if (!dev->slot || dev->slot != slot)
>  			continue;
>  		pci_dev_restore(dev);
> +#ifdef CONFIG_PCSC
> +		pcsc_device_reset(dev);

Definitely use a stub for these.

> +#endif
>  		if (dev->subordinate) {
>  			pci_bridge_wait_for_secondary_bus(dev, "slot reset");
>  			pci_bus_restore_locked(dev->subordinate);