diff mbox series

[V1] PCI: pciehp: Disable ACS Source Validation during hot-remove

Message ID 20230111145206.23919-1-vidyas@nvidia.com (mailing list archive)
State Superseded
Headers show
Series [V1] PCI: pciehp: Disable ACS Source Validation during hot-remove | expand

Commit Message

Vidya Sagar Jan. 11, 2023, 2:52 p.m. UTC 
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 <vidyas@nvidia.com>
---
 drivers/pci/hotplug/pciehp_pci.c | 13 ++++++++++++-
 drivers/pci/pci.c                | 20 ++++++++++++++++++++
 include/linux/pci.h              |  6 ++++++
 3 files changed, 38 insertions(+), 1 deletion(-)

Comments

kernel test robot Jan. 11, 2023, 5:11 p.m. UTC | #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: 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 <lkp@intel.com>

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
kernel test robot Jan. 11, 2023, 5:11 p.m. UTC | #2 
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 <lkp@intel.com>

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
diff mbox series

Patch

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