diff mbox series

[2/2] PCI: vmd: Override ASPM on TGL/ADL VMD devices

Message ID 20211120015756.1396263-2-david.e.box@linux.intel.com (mailing list archive)
State Superseded
Delegated to: Bjorn Helgaas
Headers show
Series [1/2] PCI/ASPM: Add ASPM BIOS override function | expand

Commit Message

David E. Box Nov. 20, 2021, 1:57 a.m. UTC 
From: Michael Bottini <michael.a.bottini@linux.intel.com>

On Tiger Lake and Alder Lake platforms, VMD controllers do not have ASPM
enabled nor LTR values set by BIOS. This leads high power consumption on
these platforms when VMD is enabled as reported in bugzilla [1].  Enable
these features in the VMD driver using pcie_aspm_policy_override() to set
the ASPM policy for the root ports.

To do this, add an additional flag in VMD features to specify devices that
must have their respective policies overridden.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=213717

Signed-off-by: Michael Bottini <michael.a.bottini@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 drivers/pci/controller/vmd.c | 40 +++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

Comments

kernel test robot Nov. 20, 2021, 5:13 a.m. UTC | #1 
Hi "David,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on v5.16-rc1 next-20211118]
[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]

url:    https://github.com/0day-ci/linux/commits/David-E-Box/PCI-ASPM-Add-ASPM-BIOS-override-function/20211120-095959
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-a012-20211118 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/d0452407e2d5bf22bd1094654d7e868311b7c94e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-E-Box/PCI-ASPM-Add-ASPM-BIOS-override-function/20211120-095959
        git checkout d0452407e2d5bf22bd1094654d7e868311b7c94e
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/vmd.c:676:5: warning: no previous prototype for 'vmd_enable_aspm' [-Wmissing-prototypes]
     676 | int vmd_enable_aspm(struct pci_dev *pdev, void *userdata)
         |     ^~~~~~~~~~~~~~~


vim +/vmd_enable_aspm +676 drivers/pci/controller/vmd.c

   671	
   672	/*
   673	 * Override the BIOS ASPM policy and set the LTR value for PCI storage
   674	 * devices on the VMD bride.
   675	 */
 > 676	int vmd_enable_aspm(struct pci_dev *pdev, void *userdata)
   677	{
   678		int features = *(int *)userdata;
   679	
   680		if (features & VMD_FEAT_QUIRK_OVERRIDE_ASPM &&
   681		    pdev->class == PCI_CLASS_STORAGE_EXPRESS) {
   682			int pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
   683	
   684			if (pos) {
   685				pci_write_config_word(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, 0x1003);
   686				pci_write_config_word(pdev, pos + PCI_LTR_MAX_NOSNOOP_LAT, 0x1003);
   687				pcie_aspm_policy_override(pdev);
   688			}
   689		}
   690		return 0;
   691	}
   692	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Nov. 20, 2021, 4:35 p.m. UTC | #2 
Hi "David,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on helgaas-pci/next]
[also build test WARNING on v5.16-rc1 next-20211118]
[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]

url:    https://github.com/0day-ci/linux/commits/David-E-Box/PCI-ASPM-Add-ASPM-BIOS-override-function/20211120-095959
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-buildonly-randconfig-r001-20211118 (attached as .config)
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/0day-ci/linux/commit/d0452407e2d5bf22bd1094654d7e868311b7c94e
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review David-E-Box/PCI-ASPM-Add-ASPM-BIOS-override-function/20211120-095959
        git checkout d0452407e2d5bf22bd1094654d7e868311b7c94e
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/pci/controller/vmd.c:676:5: warning: no previous prototype for function 'vmd_enable_aspm' [-Wmissing-prototypes]
   int vmd_enable_aspm(struct pci_dev *pdev, void *userdata)
       ^
   drivers/pci/controller/vmd.c:676:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int vmd_enable_aspm(struct pci_dev *pdev, void *userdata)
   ^
   static 
   1 warning generated.


vim +/vmd_enable_aspm +676 drivers/pci/controller/vmd.c

   671	
   672	/*
   673	 * Override the BIOS ASPM policy and set the LTR value for PCI storage
   674	 * devices on the VMD bride.
   675	 */
 > 676	int vmd_enable_aspm(struct pci_dev *pdev, void *userdata)
   677	{
   678		int features = *(int *)userdata;
   679	
   680		if (features & VMD_FEAT_QUIRK_OVERRIDE_ASPM &&
   681		    pdev->class == PCI_CLASS_STORAGE_EXPRESS) {
   682			int pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
   683	
   684			if (pos) {
   685				pci_write_config_word(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, 0x1003);
   686				pci_write_config_word(pdev, pos + PCI_LTR_MAX_NOSNOOP_LAT, 0x1003);
   687				pcie_aspm_policy_override(pdev);
   688			}
   689		}
   690		return 0;
   691	}
   692	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index a45e8e59d3d4..0d0b8888cd90 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -20,6 +20,8 @@ 
 
 #include <asm/irqdomain.h>
 
+#include "../pci.h"
+
 #define VMD_CFGBAR	0
 #define VMD_MEMBAR1	2
 #define VMD_MEMBAR2	4
@@ -67,6 +69,12 @@  enum vmd_features {
 	 * interrupt handling.
 	 */
 	VMD_FEAT_CAN_BYPASS_MSI_REMAP		= (1 << 4),
+
+	/*
+	 * Device must have ASPM policy overridden, as its default policy is
+	 * incorrect.
+	 */
+	VMD_FEAT_QUIRK_OVERRIDE_ASPM		= (1 << 5),
 };
 
 static DEFINE_IDA(vmd_instance_ida);
@@ -661,6 +669,27 @@  static int vmd_alloc_irqs(struct vmd_dev *vmd)
 	return 0;
 }
 
+/*
+ * Override the BIOS ASPM policy and set the LTR value for PCI storage
+ * devices on the VMD bride.
+ */
+int vmd_enable_aspm(struct pci_dev *pdev, void *userdata)
+{
+	int features = *(int *)userdata;
+
+	if (features & VMD_FEAT_QUIRK_OVERRIDE_ASPM &&
+	    pdev->class == PCI_CLASS_STORAGE_EXPRESS) {
+		int pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
+
+		if (pos) {
+			pci_write_config_word(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, 0x1003);
+			pci_write_config_word(pdev, pos + PCI_LTR_MAX_NOSNOOP_LAT, 0x1003);
+			pcie_aspm_policy_override(pdev);
+		}
+	}
+	return 0;
+}
+
 static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 {
 	struct pci_sysdata *sd = &vmd->sysdata;
@@ -807,6 +836,8 @@  static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 	pci_scan_child_bus(vmd->bus);
 	pci_assign_unassigned_bus_resources(vmd->bus);
 
+	pci_walk_bus(vmd->bus, vmd_enable_aspm, &features);
+
 	/*
 	 * VMD root buses are virtual and don't return true on pci_is_pcie()
 	 * and will fail pcie_bus_configure_settings() early. It can instead be
@@ -948,15 +979,18 @@  static const struct pci_device_id vmd_ids[] = {
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x467f),
 		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
 				VMD_FEAT_HAS_BUS_RESTRICTIONS |
-				VMD_FEAT_OFFSET_FIRST_VECTOR,},
+				VMD_FEAT_OFFSET_FIRST_VECTOR |
+				VMD_FEAT_QUIRK_OVERRIDE_ASPM,},
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, 0x4c3d),
 		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
 				VMD_FEAT_HAS_BUS_RESTRICTIONS |
-				VMD_FEAT_OFFSET_FIRST_VECTOR,},
+				VMD_FEAT_OFFSET_FIRST_VECTOR |
+				VMD_FEAT_QUIRK_OVERRIDE_ASPM,},
 	{PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B),
 		.driver_data = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
 				VMD_FEAT_HAS_BUS_RESTRICTIONS |
-				VMD_FEAT_OFFSET_FIRST_VECTOR,},
+				VMD_FEAT_OFFSET_FIRST_VECTOR |
+				VMD_FEAT_QUIRK_OVERRIDE_ASPM,},
 	{0,}
 };
 MODULE_DEVICE_TABLE(pci, vmd_ids);