| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 1 Aug 2013 15:55:48 +0000
From: "Wyborny, Carolyn" <carolyn.wyborny@...el.com>
To: Pavel Machek <pavel@....cz>
CC: Bjorn Helgaas <bhelgaas@...gle.com>, Greg KH <greg@...ah.com>,
kernel list <linux-kernel@...r.kernel.org>,
Joe Lawrence <joe.lawrence@...atus.com>,
Myron Stowe <myron.stowe@...hat.com>,
"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
"Allan, Bruce W" <bruce.w.allan@...el.com>,
"Skidmore, Donald C" <donald.c.skidmore@...el.com>,
"Rose, Gregory V" <gregory.v.rose@...el.com>,
"Waskiewicz Jr, Peter P" <peter.p.waskiewicz.jr@...el.com>,
"Duyck, Alexander H" <alexander.h.duyck@...el.com>,
"Ronciak, John" <john.ronciak@...el.com>,
"Dave, Tushar N" <tushar.n.dave@...el.com>,
"e1000-devel@...ts.sourceforge.net"
<e1000-devel@...ts.sourceforge.net>
Subject: RE: /sys/module/pcie_aspm/parameters/policy not writable?
> -----Original Message-----
> From: Wyborny, Carolyn
> Sent: Thursday, August 01, 2013 7:56 AM
> To: 'Pavel Machek'
> Cc: Bjorn Helgaas; Greg KH; kernel list; Joe Lawrence; Myron Stowe; Kirsher,
> Jeffrey T; Brandeburg, Jesse; Allan, Bruce W; Skidmore, Donald C; Rose, Gregory
> V; Waskiewicz Jr, Peter P; Duyck, Alexander H; Ronciak, John; Dave, Tushar N;
> e1000-devel@...ts.sourceforge.net
> Subject: RE: /sys/module/pcie_aspm/parameters/policy not writable?
>
> [..]
> > If there's patch, I'll gladly try it :-). Thanks,
> >
> Pavel
> Thanks Pavel,
Minor fix. Missed a bracket removal Please use this for your testing instead.
Thanks,
Carolyn
>
> It ended up taking more time than I thought. The checking of whether ASPM is
> really enabled or not is, unfortunately, not as straightforward as I thought
> initially, so we ended up rewriting the function a bit. In this patch we try to use
> the pci_disable_link_state_locked() call, but if it fails, we then write to pci config
> space of the device to disable it.
>
> Please let me know if this actually disables the ASPM for your 82574 parts or
> not. We can still continue the discussion on whether this is the best approach or
> not, or what else could be done.
>
> This patch attempts to work around a problem found with some systems where
> the call to pci_diable_link_state_locked() fails. As a result, ASPM is not, in fact,
> disabled. Changing disable aspm code to check if state actually is disabled after
> the call and, if not, try another way to disable it.
>
> Signed-off-by: Carolyn Wyborny <carolyn.wyborny@...el.com>
> ---
>
> drivers/net/ethernet/intel/e1000e/netdev.c | 86 ++++++++++++++++++++------
> --
> 1 files changed, 60 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> index e6d2c0f..bc3025b 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -64,8 +64,6 @@ static int debug = -1; module_param(debug, int, 0);
> MODULE_PARM_DESC(debug, "Debug level (0=none,...,16=all)");
>
> -static void e1000e_disable_aspm(struct pci_dev *pdev, u16 state);
> -
> static const struct e1000_info *e1000_info_tbl[] = {
> [board_82571] = &e1000_82571_info,
> [board_82572] = &e1000_82572_info,
> @@ -6019,38 +6017,74 @@ static int __e1000_shutdown(struct pci_dev *pdev,
> bool runtime)
> return 0;
> }
>
> -#ifdef CONFIG_PCIEASPM
> -static void __e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
> +/**
> + * e1000e_disable_aspm - Disable ASPM states
> + * @pdev: pointer to PCI device struct
> + * @state: bit-mask of ASPM states to disable
> + *
> + * Some devices *must* have certain ASPM states disabled per hardware
> errata.
> + **/
> +static void e1000e_disable_aspm(struct pci_dev *pdev, u16 state)
> {
> + struct pci_dev *parent = pdev->bus->self;
> + u16 aspm_dis_mask = 0;
> + u16 pdev_aspmc, parent_aspmc;
> +
> + switch (state) {
> + case PCIE_LINK_STATE_L0S:
> + case PCIE_LINK_STATE_L0S + PCIE_LINK_STATE_L1:
> + aspm_dis_mask |= PCI_EXP_LNKCTL_ASPM_L0S;
> + /* fall-through - can't have L1 without L0s */
> + case PCIE_LINK_STATE_L1:
> + aspm_dis_mask |= PCI_EXP_LNKCTL_ASPM_L1;
> + break;
> + default:
> + return;
> + }
> +
> + pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &pdev_aspmc);
> + pdev_aspmc &= PCI_EXP_LNKCTL_ASPMC;
> +
> + if (parent) {
> + pcie_capability_read_word(parent, PCI_EXP_LNKCTL,
> + &parent_aspmc);
> + parent_aspmc &= PCI_EXP_LNKCTL_ASPMC;
> + }
> +
> + /* Nothing to do if the ASPM states to be disabled already are */
> + if (!(pdev_aspmc & aspm_dis_mask) &&
> + (!parent || !(parent_aspmc & aspm_dis_mask)))
> + return;
> +
> + dev_info(&pdev->dev, "Disabling ASPM %s %s\n",
> + (aspm_dis_mask & pdev_aspmc &
> PCI_EXP_LNKCTL_ASPM_L0S) ?
> + "L0s" : "",
> + (aspm_dis_mask & pdev_aspmc & PCI_EXP_LNKCTL_ASPM_L1)
> ?
> + "L1" : "");
> +
> +#ifdef CONFIG_PCIEASPM
> pci_disable_link_state_locked(pdev, state); -} -#else -static void
> __e1000e_disable_aspm(struct pci_dev *pdev, u16 state) -{
> - u16 aspm_ctl = 0;
>
> - if (state & PCIE_LINK_STATE_L0S)
> - aspm_ctl |= PCI_EXP_LNKCTL_ASPM_L0S;
> - if (state & PCIE_LINK_STATE_L1)
> - aspm_ctl |= PCI_EXP_LNKCTL_ASPM_L1;
> + /* Double-check ASPM control. If not disabled by the above, the
> + * BIOS is preventing that from happening (or CONFIG_PCIEASPM is
> + * not enabled); override by writing PCI config space directly.
> + */
> + pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &pdev_aspmc);
> + pdev_aspmc &= PCI_EXP_LNKCTL_ASPMC;
> +
> + if (!(aspm_dis_mask & pdev_aspmc))
> + return;
> +#endif
>
> /* Both device and parent should have the same ASPM setting.
> * Disable ASPM in downstream component first and then upstream.
> */
> - pcie_capability_clear_word(pdev, PCI_EXP_LNKCTL, aspm_ctl);
> -
> - if (pdev->bus->self)
> - pcie_capability_clear_word(pdev->bus->self, PCI_EXP_LNKCTL,
> - aspm_ctl);
> -}
> -#endif
> -static void e1000e_disable_aspm(struct pci_dev *pdev, u16 state) -{
> - dev_info(&pdev->dev, "Disabling ASPM %s %s\n",
> - (state & PCIE_LINK_STATE_L0S) ? "L0s" : "",
> - (state & PCIE_LINK_STATE_L1) ? "L1" : "");
> + pcie_capability_clear_word(pdev, PCI_EXP_LNKCTL, aspm_dis_mask);
>
> - __e1000e_disable_aspm(pdev, state);
> + if (parent)
> + pcie_capability_clear_word(parent, PCI_EXP_LNKCTL,
> + aspm_dis_mask);
> }
>
> #ifdef CONFIG_PM
Content of type "message/rfc822" skipped
Powered by blists - more mailing lists