* [PATCH V3 1/2] PCI/ASPM: Add ASPM BIOS override function @ 2021-12-09 20:12 David E. Box 2021-12-09 20:12 ` [PATCH V3 2/2] PCI: vmd: Override ASPM on TGL/ADL VMD devices David E. Box 0 siblings, 1 reply; 4+ messages in thread From: David E. Box @ 2021-12-09 20:12 UTC (permalink / raw) To: nirmal.patel, jonathan.derrick, lorenzo.pieralisi, robh, kw, bhelgaas, david.e.box, michael.a.bottini, rafael Cc: linux-pci, linux-kernel From: Michael Bottini <michael.a.bottini@linux.intel.com> Devices that appear under the Intel VMD host bridge are not visible to BIOS and therefore not programmed by BIOS with ASPM settings. For these devices, it is necessary for the driver to configure ASPM. Since ASPM settings are adjustable at runtime by module parameter, use the same mechanism to allow drivers to override the default (in this case never configured) BIOS policy to ASPM_STATE_ALL. Then, reconfigure ASPM on the link. Do not override if ASPM control is disabled. Signed-off-by: Michael Bottini <michael.a.bottini@linux.intel.com> Signed-off-by: David E. Box <david.e.box@linux.intel.com> --- V3 - Fix missing semicolon in static inline function. V2 - Change return type to int so caller can determine if override was successful - Return immediately if link is not found so that lock it not unecessarily taken, suggested by kw@linux.com - Don't override if aspm_disabled is true drivers/pci/pci.h | 2 ++ drivers/pci/pcie/aspm.c | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 3d60cabde1a1..c9c55d43cd8a 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -562,11 +562,13 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev); void pcie_aspm_exit_link_state(struct pci_dev *pdev); void pcie_aspm_pm_state_change(struct pci_dev *pdev); void pcie_aspm_powersave_config_link(struct pci_dev *pdev); +int pcie_aspm_policy_override(struct pci_dev *dev); #else static inline void pcie_aspm_init_link_state(struct pci_dev *pdev) { } static inline void pcie_aspm_exit_link_state(struct pci_dev *pdev) { } static inline void pcie_aspm_pm_state_change(struct pci_dev *pdev) { } static inline void pcie_aspm_powersave_config_link(struct pci_dev *pdev) { } +static inline int pcie_aspm_policy_override(struct pci_dev *dev) { return -EINVAL; } #endif #ifdef CONFIG_PCIE_ECRC diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 52c74682601a..e2c61e14e724 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -1140,6 +1140,25 @@ int pci_disable_link_state(struct pci_dev *pdev, int state) } EXPORT_SYMBOL(pci_disable_link_state); +int pcie_aspm_policy_override(struct pci_dev *pdev) +{ + struct pcie_link_state *link = pcie_aspm_get_link(pdev); + + if (!link || aspm_disabled) + return -EINVAL; + + down_read(&pci_bus_sem); + mutex_lock(&aspm_lock); + link->aspm_default = ASPM_STATE_ALL; + pcie_config_aspm_link(link, policy_to_aspm_state(link)); + pcie_set_clkpm(link, policy_to_clkpm_state(link)); + mutex_unlock(&aspm_lock); + up_read(&pci_bus_sem); + + return 0; +} +EXPORT_SYMBOL(pcie_aspm_policy_override); + static int pcie_aspm_set_policy(const char *val, const struct kernel_param *kp) { -- 2.25.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH V3 2/2] PCI: vmd: Override ASPM on TGL/ADL VMD devices 2021-12-09 20:12 [PATCH V3 1/2] PCI/ASPM: Add ASPM BIOS override function David E. Box @ 2021-12-09 20:12 ` David E. Box 2021-12-14 7:56 ` Christoph Hellwig 0 siblings, 1 reply; 4+ messages in thread From: David E. Box @ 2021-12-09 20:12 UTC (permalink / raw) To: nirmal.patel, jonathan.derrick, lorenzo.pieralisi, robh, kw, bhelgaas, david.e.box, michael.a.bottini, rafael Cc: linux-pci, linux-kernel, Adhitya Mohan 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> Tested-by: Adhitya Mohan <me@adhityamohan.in> --- V3 - No changes V2 - Use return status to print pci_info message if ASPM cannot be enabled. - Add missing static declaration, caught by lkp@intel.com drivers/pci/controller/vmd.c | 42 +++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index a45e8e59d3d4..b4f18af7733f 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,29 @@ 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. + */ +static 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); + if (pcie_aspm_policy_override(pdev)) + pci_info(pdev, "Unable of override ASPM policy\n"); + } + } + + return 0; +} + static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features) { struct pci_sysdata *sd = &vmd->sysdata; @@ -807,6 +838,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 +981,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); -- 2.25.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH V3 2/2] PCI: vmd: Override ASPM on TGL/ADL VMD devices 2021-12-09 20:12 ` [PATCH V3 2/2] PCI: vmd: Override ASPM on TGL/ADL VMD devices David E. Box @ 2021-12-14 7:56 ` Christoph Hellwig 2021-12-14 17:33 ` David E. Box 0 siblings, 1 reply; 4+ messages in thread From: Christoph Hellwig @ 2021-12-14 7:56 UTC (permalink / raw) To: David E. Box Cc: nirmal.patel, jonathan.derrick, lorenzo.pieralisi, robh, kw, bhelgaas, michael.a.bottini, rafael, linux-pci, linux-kernel, Adhitya Mohan On Thu, Dec 09, 2021 at 12:12:35PM -0800, David E. Box wrote: > +static 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); > + if (pcie_aspm_policy_override(pdev)) > + pci_info(pdev, "Unable of override ASPM policy\n"); > + } This is completely unredable due to the long lines. Just return early on the two conditionals and it will all be fine. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH V3 2/2] PCI: vmd: Override ASPM on TGL/ADL VMD devices 2021-12-14 7:56 ` Christoph Hellwig @ 2021-12-14 17:33 ` David E. Box 0 siblings, 0 replies; 4+ messages in thread From: David E. Box @ 2021-12-14 17:33 UTC (permalink / raw) To: Christoph Hellwig Cc: nirmal.patel, jonathan.derrick, lorenzo.pieralisi, robh, kw, bhelgaas, michael.a.bottini, rafael, linux-pci, linux-kernel, Adhitya Mohan On Mon, 2021-12-13 at 23:56 -0800, Christoph Hellwig wrote: > On Thu, Dec 09, 2021 at 12:12:35PM -0800, David E. Box wrote: > > +static 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); > > + if (pcie_aspm_policy_override(pdev)) > > + pci_info(pdev, "Unable of override ASPM > > policy\n"); > > + } > > This is completely unredable due to the long lines. Just return early on > the two conditionals and it will all be fine. Sure. Thanks. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-12-14 17:33 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-09 20:12 [PATCH V3 1/2] PCI/ASPM: Add ASPM BIOS override function David E. Box 2021-12-09 20:12 ` [PATCH V3 2/2] PCI: vmd: Override ASPM on TGL/ADL VMD devices David E. Box 2021-12-14 7:56 ` Christoph Hellwig 2021-12-14 17:33 ` David E. Box
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.