On Monday, March 14, 2011, Naga Chumbalkar wrote:
>
> For servers whose hardware cannot handle ASPM the BIOS ought to set the
> FADT bit shown below:
>
> In Sec 5.2.9.3 (IA-PC Boot Arch. Flags) of ACPI4.0a Specification, please
> see Table 5-11:
> PCIe ASPM Controls :
> If set, indicates to OSPM that it must not enable OPSM ASPM control
> on this platform.
>
> However there are shipping servers whose BIOS did not set this bit. (An
> example is the HP ProLiant DL385 G6 and this is being addressed in a
> Maintenance BIOS). For such servers commit
> 28eb5f274a305bf3a13b2c80c4804d4515d05c64 checks to see if the BIOS is
> willing to grant control for certain services via _OSC. If the BIOS
> refuses then ASPM is disabled by calling pcie_no_aspm(). Commit
> fe31e69740eddc7316071ed5165fed6703c8cd12 modified that code so the
> relevant portion looks like this:
> In ./drivers/pci/pcie/portdrv_core.c:
>
> 357 /* Get and check PCI Express port services */
> 358 capabilities = get_port_device_capability(dev);
> 359 if (!capabilities) {
> 360 pcie_no_aspm();
> 361 return 0;
> 362 }
>
> However, if a user boots the server and sets the ASPM "policy" to
> "powersave" via /sys then pcie_aspm_set_policy() will run through the
> "link_list" and re-configure ASPM policy on devices that advertise
> ASPM L0s/L1 capability:
> # echo powersave > /sys/module/pcie_aspm/parameters/policy
> # cat /sys/module/pcie_aspm/parameters/policy
> default performance [powersave]
>
> However that will cause NMIs since the hardware doesn't play well with
> ASPM:
> [ 1651.906015] NMI: PCI system error (SERR) for reason b1 on CPU 0.
> [ 1651.906015] Dazed and confused, but trying to continue
>
> I agree the BIOS should have set that FADT bit in the first place but we
> could be a bit more robust - especially given the fact that Windows doesn't
> cause NMIs in the above scenario.
>
> Let's not allow a user to modify ASPM policy on hardware where the BIOS
> doesn't grant _OSC control:
>
> Signed-off-by: Naga Chumbalkar <nagananda.chumbalkar@xxxxxx>
Acked-by: Rafael J. Wysocki <rjw@xxxxxxx>
However, I'd appreciate it if you used commit subjects in addition to the
commit names in your changelogs (that makes things easier to read without
going to git an searching for the given commit).
Also, if you want the patch to be merged, please resubmit to linux-pci with
a CC to Jesse Barnes.
Thanks,
Rafael
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 3188cd9..40cb4a0 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -747,6 +747,8 @@ static int pcie_aspm_set_policy(const char *val, struct kernel_param *kp)
> int i;
> struct pcie_link_state *link;
>
> + if (aspm_disabled)
> + return;
> for (i = 0; i < ARRAY_SIZE(policy_str); i++)
> if (!strncmp(val, policy_str[i], strlen(policy_str[i])))
> break;
> @@ -801,6 +803,8 @@ static ssize_t link_state_store(struct device *dev,
> struct pcie_link_state *link, *root = pdev->link_state->root;
> u32 val = buf[0] - '0', state = 0;
>
> + if (aspm_disabled)
> + return;
> if (n < 1 || val > 3)
> return -EINVAL;
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
