PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted to
send upstream messages before they have been assigned a bus number and
such messages have a Requester ID with Bus number set to 00h.
If the Downstrem port has ACS Source Validation enabled, these messages
will be detected as ACS violation error.
Hence, disable ACS Source Validation in the bridge device during
hot-remove operation and re-enable it after enumeration of the
downstream hierarchy but before binding the respective device drivers.
Signed-off-by: Vidya Sagar <[email protected]>
---
drivers/pci/hotplug/pciehp_pci.c | 13 ++++++++++++-
drivers/pci/pci.c | 20 ++++++++++++++++++++
include/linux/pci.h | 6 ++++++
3 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index d17f3bf36f70..ad90bcf3f621 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -63,6 +63,7 @@ int pciehp_configure_device(struct controller *ctrl)
pci_assign_unassigned_bridge_resources(bridge);
pcie_bus_configure_settings(parent);
+ pci_configure_acs_sv(bridge, true);
pci_bus_add_devices(parent);
out:
@@ -117,6 +118,16 @@ void pciehp_unconfigure_device(struct controller *ctrl, bool presence)
}
pci_dev_put(dev);
}
-
+ /*
+ * PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted
+ * to send upstream messages before they have been assigned a bus
+ * number and such messages have a Requester ID with Bus number
+ * set to 00h. If the Downstrem port has ACS Source Validation enabled,
+ * these messages will be detected as ACS violation error.
+ * Hence, disable ACS Source Validation here and re-enable it after
+ * enumeration of the downstream hierarchy and before binding the
+ * respective device drivers in pciehp_configure_device().
+ */
+ pci_configure_acs_sv(ctrl->pcie->port, false);
pci_unlock_rescan_remove();
}
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 95bc329e74c0..d3d0246fddd9 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -991,6 +991,26 @@ static void pci_enable_acs(struct pci_dev *dev)
pci_disable_acs_redir(dev);
}
+void pci_configure_acs_sv(struct pci_dev *dev, bool flag)
+{
+ u16 cap;
+ u16 ctrl;
+
+ if (!pci_acs_enable || !dev->acs_cap)
+ return;
+
+ pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CAP, &cap);
+ pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, &ctrl);
+
+ if (flag)
+ ctrl |= (cap & PCI_ACS_SV);
+ else
+ ctrl &= ~(cap & PCI_ACS_SV);
+
+ pci_write_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, ctrl);
+}
+EXPORT_SYMBOL_GPL(pci_configure_acs_sv);
+
/**
* pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
* @dev: PCI device to have its BARs restored
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 060af91bafcd..c36ac3eb2843 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2239,6 +2239,12 @@ void pci_hp_create_module_link(struct pci_slot *pci_slot);
void pci_hp_remove_module_link(struct pci_slot *pci_slot);
#endif
+#if defined(CONFIG_HOTPLUG_PCI_PCIE)
+void pci_configure_acs_sv(struct pci_dev *dev, bool flag);
+#else
+void pci_configure_acs_sv(struct pci_dev *dev, bool flag) {}
+#endif
+
/**
* pci_pcie_cap - get the saved PCIe capability offset
* @dev: PCI device
--
2.17.1
Hi Vidya,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on helgaas-pci/next]
[also build test ERROR on helgaas-pci/for-linus linus/master v6.2-rc3 next-20230111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Vidya-Sagar/PCI-pciehp-Disable-ACS-Source-Validation-during-hot-remove/20230111-225412
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link: https://lore.kernel.org/r/20230111145206.23919-1-vidyas%40nvidia.com
patch subject: [PATCH V1] PCI: pciehp: Disable ACS Source Validation during hot-remove
config: arc-defconfig
compiler: arc-elf-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/52cbe5f597ff5e9134cf1f8f7aba6502ab0396d2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Vidya-Sagar/PCI-pciehp-Disable-ACS-Source-Validation-during-hot-remove/20230111-225412
git checkout 52cbe5f597ff5e9134cf1f8f7aba6502ab0396d2
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
arc-elf-ld: lib/devres.o: in function `pci_configure_acs_sv':
>> devres.c:(.text+0x43c): multiple definition of `pci_configure_acs_sv'; lib/pci_iomap.o:pci_iomap.c:(.text+0x0): first defined here
arc-elf-ld: drivers/pci/of.o: in function `pci_configure_acs_sv':
of.c:(.text+0x140): multiple definition of `pci_configure_acs_sv'; lib/pci_iomap.o:pci_iomap.c:(.text+0x0): first defined here
arc-elf-ld: drivers/of/address.o: in function `pci_configure_acs_sv':
address.c:(.text+0xcfc): multiple definition of `pci_configure_acs_sv'; lib/pci_iomap.o:pci_iomap.c:(.text+0x0): first defined here
arc-elf-ld: net/core/rtnetlink.o: in function `pci_configure_acs_sv':
rtnetlink.c:(.text+0x6f80): multiple definition of `pci_configure_acs_sv'; lib/pci_iomap.o:pci_iomap.c:(.text+0x0): first defined here
arc-elf-ld: net/ethernet/eth.o: in function `pci_configure_acs_sv':
eth.c:(.text+0x6b8): multiple definition of `pci_configure_acs_sv'; lib/pci_iomap.o:pci_iomap.c:(.text+0x0): first defined here
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi Vidya,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on helgaas-pci/next]
[also build test ERROR on helgaas-pci/for-linus linus/master v6.2-rc3 next-20230111]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Vidya-Sagar/PCI-pciehp-Disable-ACS-Source-Validation-during-hot-remove/20230111-225412
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
patch link: https://lore.kernel.org/r/20230111145206.23919-1-vidyas%40nvidia.com
patch subject: [PATCH V1] PCI: pciehp: Disable ACS Source Validation during hot-remove
config: alpha-defconfig
compiler: alpha-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/52cbe5f597ff5e9134cf1f8f7aba6502ab0396d2
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Vidya-Sagar/PCI-pciehp-Disable-ACS-Source-Validation-during-hot-remove/20230111-225412
git checkout 52cbe5f597ff5e9134cf1f8f7aba6502ab0396d2
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=alpha SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
In file included from drivers/pci/pci.c:18:
include/linux/pci.h:2281:6: warning: no previous prototype for 'pci_configure_acs_sv' [-Wmissing-prototypes]
2281 | void pci_configure_acs_sv(struct pci_dev *dev, bool flag) {}
| ^~~~~~~~~~~~~~~~~~~~
>> drivers/pci/pci.c:996:6: error: redefinition of 'pci_configure_acs_sv'
996 | void pci_configure_acs_sv(struct pci_dev *dev, bool flag)
| ^~~~~~~~~~~~~~~~~~~~
include/linux/pci.h:2281:6: note: previous definition of 'pci_configure_acs_sv' with type 'void(struct pci_dev *, bool)' {aka 'void(struct pci_dev *, _Bool)'}
2281 | void pci_configure_acs_sv(struct pci_dev *dev, bool flag) {}
| ^~~~~~~~~~~~~~~~~~~~
--
alpha-linux-ld: drivers/scsi/aic7xxx/aic7xxx_93cx6.o: in function `pci_configure_acs_sv':
>> include/linux/pci.h:2281: multiple definition of `pci_configure_acs_sv'; drivers/scsi/aic7xxx/aic7xxx_core.o:include/linux/pci.h:2281: first defined here
alpha-linux-ld: drivers/scsi/aic7xxx/aic7xxx_pci.o: in function `pci_configure_acs_sv':
>> include/linux/pci.h:2281: multiple definition of `pci_configure_acs_sv'; drivers/scsi/aic7xxx/aic7xxx_core.o:include/linux/pci.h:2281: first defined here
alpha-linux-ld: drivers/scsi/aic7xxx/aic7xxx_reg_print.o: in function `pci_configure_acs_sv':
>> include/linux/pci.h:2281: multiple definition of `pci_configure_acs_sv'; drivers/scsi/aic7xxx/aic7xxx_core.o:include/linux/pci.h:2281: first defined here
alpha-linux-ld: drivers/scsi/aic7xxx/aic7xxx_osm.o: in function `pci_configure_acs_sv':
>> include/linux/pci.h:2281: multiple definition of `pci_configure_acs_sv'; drivers/scsi/aic7xxx/aic7xxx_core.o:include/linux/pci.h:2281: first defined here
alpha-linux-ld: drivers/scsi/aic7xxx/aic7xxx_proc.o: in function `pci_configure_acs_sv':
>> include/linux/pci.h:2281: multiple definition of `pci_configure_acs_sv'; drivers/scsi/aic7xxx/aic7xxx_core.o:include/linux/pci.h:2281: first defined here
alpha-linux-ld: drivers/scsi/aic7xxx/aic7xxx_osm_pci.o: in function `pci_configure_acs_sv':
>> include/linux/pci.h:2281: multiple definition of `pci_configure_acs_sv'; drivers/scsi/aic7xxx/aic7xxx_core.o:include/linux/pci.h:2281: first defined here
vim +/pci_configure_acs_sv +996 drivers/pci/pci.c
995
> 996 void pci_configure_acs_sv(struct pci_dev *dev, bool flag)
997 {
998 u16 cap;
999 u16 ctrl;
1000
1001 if (!pci_acs_enable || !dev->acs_cap)
1002 return;
1003
1004 pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CAP, &cap);
1005 pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, &ctrl);
1006
1007 if (flag)
1008 ctrl |= (cap & PCI_ACS_SV);
1009 else
1010 ctrl &= ~(cap & PCI_ACS_SV);
1011
1012 pci_write_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, ctrl);
1013 }
1014 EXPORT_SYMBOL_GPL(pci_configure_acs_sv);
1015
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted to
send upstream messages before they have been assigned a bus number and
such messages have a Requester ID with Bus number set to 00h.
If the Downstrem port has ACS Source Validation enabled, these messages
will be detected as ACS violation error.
Hence, disable ACS Source Validation in the bridge device during
hot-remove operation and re-enable it after enumeration of the
downstream hierarchy but before binding the respective device drivers.
Signed-off-by: Vidya Sagar <[email protected]>
---
v2:
* Fixed build issues
drivers/pci/hotplug/pciehp_pci.c | 13 ++++++++++++-
drivers/pci/pci.c | 22 ++++++++++++++++++++++
include/linux/pci.h | 6 ++++++
3 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index d17f3bf36f70..ad90bcf3f621 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -63,6 +63,7 @@ int pciehp_configure_device(struct controller *ctrl)
pci_assign_unassigned_bridge_resources(bridge);
pcie_bus_configure_settings(parent);
+ pci_configure_acs_sv(bridge, true);
pci_bus_add_devices(parent);
out:
@@ -117,6 +118,16 @@ void pciehp_unconfigure_device(struct controller *ctrl, bool presence)
}
pci_dev_put(dev);
}
-
+ /*
+ * PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted
+ * to send upstream messages before they have been assigned a bus
+ * number and such messages have a Requester ID with Bus number
+ * set to 00h. If the Downstrem port has ACS Source Validation enabled,
+ * these messages will be detected as ACS violation error.
+ * Hence, disable ACS Source Validation here and re-enable it after
+ * enumeration of the downstream hierarchy and before binding the
+ * respective device drivers in pciehp_configure_device().
+ */
+ pci_configure_acs_sv(ctrl->pcie->port, false);
pci_unlock_rescan_remove();
}
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 95bc329e74c0..9cefaf814f49 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -991,6 +991,28 @@ static void pci_enable_acs(struct pci_dev *dev)
pci_disable_acs_redir(dev);
}
+#ifdef CONFIG_HOTPLUG_PCI_PCIE
+void pci_configure_acs_sv(struct pci_dev *dev, bool flag)
+{
+ u16 cap;
+ u16 ctrl;
+
+ if (!pci_acs_enable || !dev->acs_cap)
+ return;
+
+ pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CAP, &cap);
+ pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, &ctrl);
+
+ if (flag)
+ ctrl |= (cap & PCI_ACS_SV);
+ else
+ ctrl &= ~(cap & PCI_ACS_SV);
+
+ pci_write_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, ctrl);
+}
+EXPORT_SYMBOL_GPL(pci_configure_acs_sv);
+#endif
+
/**
* pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
* @dev: PCI device to have its BARs restored
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 060af91bafcd..edf516e39764 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2239,6 +2239,12 @@ void pci_hp_create_module_link(struct pci_slot *pci_slot);
void pci_hp_remove_module_link(struct pci_slot *pci_slot);
#endif
+#ifdef CONFIG_HOTPLUG_PCI_PCIE
+void pci_configure_acs_sv(struct pci_dev *dev, bool flag);
+#else
+static inline void pci_configure_acs_sv(struct pci_dev *dev, bool flag) { }
+#endif
+
/**
* pci_pcie_cap - get the saved PCIe capability offset
* @dev: PCI device
--
2.17.1
[+cc Lukas, Alex for pciehp and ACS comments]
On Thu, Jan 12, 2023 at 12:35:33AM +0530, Vidya Sagar wrote:
> PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted to
> send upstream messages before they have been assigned a bus number and
> such messages have a Requester ID with Bus number set to 00h.
> If the Downstrem port has ACS Source Validation enabled, these messages
> will be detected as ACS violation error.
> Hence, disable ACS Source Validation in the bridge device during
> hot-remove operation and re-enable it after enumeration of the
> downstream hierarchy but before binding the respective device drivers.
s/Downstrem/Downstream/
Format as paragraphs (blank line between).
> Signed-off-by: Vidya Sagar <[email protected]>
> ---
> v2:
> * Fixed build issues
>
> drivers/pci/hotplug/pciehp_pci.c | 13 ++++++++++++-
> drivers/pci/pci.c | 22 ++++++++++++++++++++++
> include/linux/pci.h | 6 ++++++
> 3 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
> index d17f3bf36f70..ad90bcf3f621 100644
> --- a/drivers/pci/hotplug/pciehp_pci.c
> +++ b/drivers/pci/hotplug/pciehp_pci.c
> @@ -63,6 +63,7 @@ int pciehp_configure_device(struct controller *ctrl)
>
> pci_assign_unassigned_bridge_resources(bridge);
> pcie_bus_configure_settings(parent);
> + pci_configure_acs_sv(bridge, true);
> pci_bus_add_devices(parent);
>
> out:
> @@ -117,6 +118,16 @@ void pciehp_unconfigure_device(struct controller *ctrl, bool presence)
> }
> pci_dev_put(dev);
> }
> -
> + /*
> + * PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted
> + * to send upstream messages before they have been assigned a bus
> + * number and such messages have a Requester ID with Bus number
> + * set to 00h. If the Downstrem port has ACS Source Validation enabled,
s/Downstrem/Downstream/
> + * these messages will be detected as ACS violation error.
> + * Hence, disable ACS Source Validation here and re-enable it after
> + * enumeration of the downstream hierarchy and before binding the
> + * respective device drivers in pciehp_configure_device().
> + */
> + pci_configure_acs_sv(ctrl->pcie->port, false);
What if we have a slot that's empty at boot and we add a device later?
It looks like we still might see ACS errors there because the add
happens before a remove?
> pci_unlock_rescan_remove();
> }
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 95bc329e74c0..9cefaf814f49 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -991,6 +991,28 @@ static void pci_enable_acs(struct pci_dev *dev)
> pci_disable_acs_redir(dev);
> }
>
> +#ifdef CONFIG_HOTPLUG_PCI_PCIE
> +void pci_configure_acs_sv(struct pci_dev *dev, bool flag)
> +{
> + u16 cap;
> + u16 ctrl;
> +
> + if (!pci_acs_enable || !dev->acs_cap)
> + return;
> +
> + pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CAP, &cap);
> + pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, &ctrl);
> +
> + if (flag)
> + ctrl |= (cap & PCI_ACS_SV);
> + else
> + ctrl &= ~(cap & PCI_ACS_SV);
> +
> + pci_write_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, ctrl);
I guess we don't have a way to do this for the non-standard ACS-like
devices, i.e., pci_dev_specific_enable_acs(). Not the end of the
world, just unfortunate that we'll have different behavior there.
> +}
> +EXPORT_SYMBOL_GPL(pci_configure_acs_sv);
Doesn't seem like this needs to be exported or exposed via
linux/pci.h. pciehp cannot be built as a module.
> +#endif
> +
> /**
> * pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
> * @dev: PCI device to have its BARs restored
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 060af91bafcd..edf516e39764 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2239,6 +2239,12 @@ void pci_hp_create_module_link(struct pci_slot *pci_slot);
> void pci_hp_remove_module_link(struct pci_slot *pci_slot);
> #endif
>
> +#ifdef CONFIG_HOTPLUG_PCI_PCIE
> +void pci_configure_acs_sv(struct pci_dev *dev, bool flag);
> +#else
> +static inline void pci_configure_acs_sv(struct pci_dev *dev, bool flag) { }
> +#endif
> +
> /**
> * pci_pcie_cap - get the saved PCIe capability offset
> * @dev: PCI device
> --
> 2.17.1
>
On 2/15/2023 5:00 AM, Bjorn Helgaas wrote:
> External email: Use caution opening links or attachments
>
>
> [+cc Lukas, Alex for pciehp and ACS comments]
>
> On Thu, Jan 12, 2023 at 12:35:33AM +0530, Vidya Sagar wrote:
>> PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted to
>> send upstream messages before they have been assigned a bus number and
>> such messages have a Requester ID with Bus number set to 00h.
>> If the Downstrem port has ACS Source Validation enabled, these messages
>> will be detected as ACS violation error.
>> Hence, disable ACS Source Validation in the bridge device during
>> hot-remove operation and re-enable it after enumeration of the
>> downstream hierarchy but before binding the respective device drivers.
>
> s/Downstrem/Downstream/
>
> Format as paragraphs (blank line between).
>
>> Signed-off-by: Vidya Sagar <[email protected]>
>> ---
>> v2:
>> * Fixed build issues
>>
>> drivers/pci/hotplug/pciehp_pci.c | 13 ++++++++++++-
>> drivers/pci/pci.c | 22 ++++++++++++++++++++++
>> include/linux/pci.h | 6 ++++++
>> 3 files changed, 40 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
>> index d17f3bf36f70..ad90bcf3f621 100644
>> --- a/drivers/pci/hotplug/pciehp_pci.c
>> +++ b/drivers/pci/hotplug/pciehp_pci.c
>> @@ -63,6 +63,7 @@ int pciehp_configure_device(struct controller *ctrl)
>>
>> pci_assign_unassigned_bridge_resources(bridge);
>> pcie_bus_configure_settings(parent);
>> + pci_configure_acs_sv(bridge, true);
>> pci_bus_add_devices(parent);
>>
>> out:
>> @@ -117,6 +118,16 @@ void pciehp_unconfigure_device(struct controller *ctrl, bool presence)
>> }
>> pci_dev_put(dev);
>> }
>> -
>> + /*
>> + * PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted
>> + * to send upstream messages before they have been assigned a bus
>> + * number and such messages have a Requester ID with Bus number
>> + * set to 00h. If the Downstrem port has ACS Source Validation enabled,
>
> s/Downstrem/Downstream/
>
>> + * these messages will be detected as ACS violation error.
>> + * Hence, disable ACS Source Validation here and re-enable it after
>> + * enumeration of the downstream hierarchy and before binding the
>> + * respective device drivers in pciehp_configure_device().
>> + */
>> + pci_configure_acs_sv(ctrl->pcie->port, false);
>
> What if we have a slot that's empty at boot and we add a device later?
> It looks like we still might see ACS errors there because the add
> happens before a remove?
Yes I'm afraid. But, looking at the implementation note in 6.22.1 and
6.12.1.1, I think it is expected and spec thought through this scenario.
So, does it all come down to how the ASIC designers treat different
messages (case-by-case) and see which of them need not have to be
validated for source? For example, a DRS (Device Readiness Status) from
an endpoint during enumeration (i.e. before the endpoint is being
assigned with a valid bus number) can be ignored for ACS source validation?
>
>> pci_unlock_rescan_remove();
>> }
>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index 95bc329e74c0..9cefaf814f49 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -991,6 +991,28 @@ static void pci_enable_acs(struct pci_dev *dev)
>> pci_disable_acs_redir(dev);
>> }
>>
>> +#ifdef CONFIG_HOTPLUG_PCI_PCIE
>> +void pci_configure_acs_sv(struct pci_dev *dev, bool flag)
>> +{
>> + u16 cap;
>> + u16 ctrl;
>> +
>> + if (!pci_acs_enable || !dev->acs_cap)
>> + return;
>> +
>> + pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CAP, &cap);
>> + pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, &ctrl);
>> +
>> + if (flag)
>> + ctrl |= (cap & PCI_ACS_SV);
>> + else
>> + ctrl &= ~(cap & PCI_ACS_SV);
>> +
>> + pci_write_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, ctrl);
>
> I guess we don't have a way to do this for the non-standard ACS-like
> devices, i.e., pci_dev_specific_enable_acs(). Not the end of the
> world, just unfortunate that we'll have different behavior there.
I'll remove the exporting part in the next patch. As far as exposing it
in the pci.h is concerned, since the actual usage of this function is in
pciehp_pci.c file, I thought the best place to keep the declaration is
pci.h file. Let me know if you have any other suggestion.
>
>> +}
>> +EXPORT_SYMBOL_GPL(pci_configure_acs_sv);
>
> Doesn't seem like this needs to be exported or exposed via
> linux/pci.h. pciehp cannot be built as a module.
>
>> +#endif
>> +
>> /**
>> * pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
>> * @dev: PCI device to have its BARs restored
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 060af91bafcd..edf516e39764 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -2239,6 +2239,12 @@ void pci_hp_create_module_link(struct pci_slot *pci_slot);
>> void pci_hp_remove_module_link(struct pci_slot *pci_slot);
>> #endif
>>
>> +#ifdef CONFIG_HOTPLUG_PCI_PCIE
>> +void pci_configure_acs_sv(struct pci_dev *dev, bool flag);
>> +#else
>> +static inline void pci_configure_acs_sv(struct pci_dev *dev, bool flag) { }
>> +#endif
>> +
>> /**
>> * pci_pcie_cap - get the saved PCIe capability offset
>> * @dev: PCI device
>> --
>> 2.17.1
>>
PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted to
send upstream messages before they have been assigned a bus number and
such messages have a Requester ID with Bus number set to 00h.
If the Downstream port has ACS Source Validation enabled, these messages
will be detected as ACS violation error.
Hence, disable ACS Source Validation in the bridge device during
hot-remove operation and re-enable it after enumeration of the
downstream hierarchy but before binding the respective device drivers.
Signed-off-by: Vidya Sagar <[email protected]>
---
v3:
* Addressed review comments from Bjon
v2:
* Fixed build issues
drivers/pci/hotplug/pciehp_pci.c | 13 ++++++++++++-
drivers/pci/pci.c | 21 +++++++++++++++++++++
include/linux/pci.h | 6 ++++++
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index ad12515a4a12..42d4328f2a9b 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -63,6 +63,7 @@ int pciehp_configure_device(struct controller *ctrl)
pci_assign_unassigned_bridge_resources(bridge);
pcie_bus_configure_settings(parent);
+ pci_configure_acs_sv(bridge, true);
/*
* Release reset_lock during driver binding
@@ -132,6 +133,16 @@ void pciehp_unconfigure_device(struct controller *ctrl, bool presence)
}
pci_dev_put(dev);
}
-
+ /*
+ * PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted
+ * to send upstream messages before they have been assigned a bus
+ * number and such messages have a Requester ID with Bus number
+ * set to 00h. If the Downstream port has ACS Source Validation enabled,
+ * these messages will be detected as ACS violation error.
+ * Hence, disable ACS Source Validation here and re-enable it after
+ * enumeration of the downstream hierarchy and before binding the
+ * respective device drivers in pciehp_configure_device().
+ */
+ pci_configure_acs_sv(ctrl->pcie->port, false);
pci_unlock_rescan_remove();
}
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 60230da957e0..5a21640de355 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1004,6 +1004,27 @@ static void pci_enable_acs(struct pci_dev *dev)
pci_disable_acs_redir(dev);
}
+#ifdef CONFIG_HOTPLUG_PCI_PCIE
+void pci_configure_acs_sv(struct pci_dev *dev, bool flag)
+{
+ u16 cap;
+ u16 ctrl;
+
+ if (!pci_acs_enable || !dev->acs_cap)
+ return;
+
+ pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CAP, &cap);
+ pci_read_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, &ctrl);
+
+ if (flag)
+ ctrl |= (cap & PCI_ACS_SV);
+ else
+ ctrl &= ~(cap & PCI_ACS_SV);
+
+ pci_write_config_word(dev, dev->acs_cap + PCI_ACS_CTRL, ctrl);
+}
+#endif
+
/**
* pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
* @dev: PCI device to have its BARs restored
diff --git a/include/linux/pci.h b/include/linux/pci.h
index eeb2e6f6130f..b6d53fe28371 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2350,6 +2350,12 @@ void pci_hp_create_module_link(struct pci_slot *pci_slot);
void pci_hp_remove_module_link(struct pci_slot *pci_slot);
#endif
+#ifdef CONFIG_HOTPLUG_PCI_PCIE
+void pci_configure_acs_sv(struct pci_dev *dev, bool flag);
+#else
+static inline void pci_configure_acs_sv(struct pci_dev *dev, bool flag) { }
+#endif
+
/**
* pci_pcie_cap - get the saved PCIe capability offset
* @dev: PCI device
--
2.25.1