Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: Lost PCIe PME after a914ff2d78ce ("PCI/ASPM: Don't select
 CONFIG_PCIEASPM by default")
From:   Heiner Kallweit <hkallweit1@gmail.com>
To:     Bjorn Helgaas <helgaas@kernel.org>,
        "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc:     Bjorn Helgaas <bhelgaas@google.com>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org
References: <20200529192143.GA448525@bjorn-Precision-5520>
 <2d3944ea-f46c-037b-2395-859c4240f1fb@gmail.com>
Message-ID: <bdc33be8-1db6-b147-cbc4-90fa0dc3d999@gmail.com>
Date:   Fri, 29 May 2020 22:09:08 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.1
MIME-Version: 1.0
In-Reply-To: <2d3944ea-f46c-037b-2395-859c4240f1fb@gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 29.05.2020 21:40, Heiner Kallweit wrote:
> On 29.05.2020 21:21, Bjorn Helgaas wrote:
>> [+cc Rafael, linux-kernel]
>>
>> On Fri, May 29, 2020 at 08:50:46PM +0200, Heiner Kallweit wrote:
>>> On 28.05.2020 23:44, Heiner Kallweit wrote:
>>>> For whatever reason with this change (and losing ASPM control) I also
>>>> loose the PCIe PME interrupts. This prevents my network card from
>>>> resuming from runtime-suspend.
>>>> Reverting the change brings back ASPM control and the PCIe PME irq's.
>>>>
>>>> Affected system is a Zotac MiniPC with a N3450 CPU:
>>>> PCI bridge: Intel Corporation Celeron N3350/Pentium N4200/Atom E3900 Series PCI Express Port A #1 (rev fb)
>>>>
>>> I checked a little bit further and w/o ASPM control the root ports
>>> don't have the PME service bit set in their capabilities.
>>> Not sure whether this is a chipset bug or whether there's a better
>>> explanation. However more chipsets may have such a behavior.
>>
>> Hmm.  Is the difference simply changing the PCIEASPM config symbol, or
>> are you booting with command-line arguments like "pcie_aspm=off"?
>>
> Only difference is the config symbol. My command line is plain and simple:
> 
> Command line: initrd=\intel-ucode.img initrd=\initramfs-linux.img root=/dev/sda2 rw
> 
>> What's the specific PME bit that changes in the root ports?  Can you
>> collect the "sudo lspci -vvxxxx" output with and without ASPM?
>>
>> The capability bits are generally read-only as far as the PCI spec is
>> concerned, but devices have implementation-specific knobs that the
>> BIOS may use to change things.  Without CONFIG_PCIEASPM, Linux will
>> not request control of LTR, and that could cause the BIOS to change
>> something.  You should be able to see the LTR control difference in
>> the dmesg logging about _OSC.
>>
>>> W/o the "default y" for ASPM control we also have the situation now
>>> that the config option description says "When in doubt, say Y."
>>> but it takes the EXPERT mode to enable it. This seems to be a little
>>> bit inconsistent.
>>
>> We should probably remove the "if EXPERT" from the PCIEASPM kconfig.
>> But I would expect PME to work correctly regardless of PCIEASPM, so
>> removing "if EXPERT" doesn't solve the underlying problem.
>>
>> Rafael, does this ring any bells for you?  I don't remember a
>> connection between PME and ASPM, but maybe there is one.
>>
>>> To cut a long story short:
>>> At least on some systems this change has unwanted side effects.
> 
> lspci output w/ and w/o ASPM is attached incl. a diff.
> Here comes the _OSC difference.
> 
> w/o ASPM
> 
> [    0.386063] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig Segments MSI HPX-Type3]
> [    0.386918] acpi PNP0A08:00: _OSC: not requesting OS control; OS requires [ExtendedConfig ASPM ClockPM MSI]
> 
> w/ ASPM
> [    0.388141] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI HPX-Type3]
> [    0.393648] acpi PNP0A08:00: _OSC: OS now controls [PME AER PCIeCapability LTR]
> 
> It's at least interesting that w/o ASPM OS doesn't control PME and AER.
> 

This was the right entry point, also w/o ASPM control OS states to ACPI that it
needs ASPM and ClockPM. The following patch fixes the PME issue for me.
See also the _OSC part below.


diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 9e235c1a7..8df1fa728 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -38,10 +38,15 @@ static int acpi_pci_root_scan_dependent(struct acpi_device *adev)
 	return 0;
 }
 
+#ifdef CONFIG_PCIEASPM
 #define ACPI_PCIE_REQ_SUPPORT (OSC_PCI_EXT_CONFIG_SUPPORT \
 				| OSC_PCI_ASPM_SUPPORT \
 				| OSC_PCI_CLOCK_PM_SUPPORT \
 				| OSC_PCI_MSI_SUPPORT)
+#else
+#define ACPI_PCIE_REQ_SUPPORT (OSC_PCI_EXT_CONFIG_SUPPORT \
+				| OSC_PCI_MSI_SUPPORT)
+#endif
 
 static const struct acpi_device_id root_device_ids[] = {
 	{"PNP0A03", 0},
-- 
2.26.2


[    0.387527] acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig Segments MSI HPX-Type3]
[    0.393033] acpi PNP0A08:00: _OSC: OS now controls [PME AER PCIeCapability]