Re: [Intel-wired-lan] [PATCH] PCI/ASPM: Enable ASPM on external PCIe devices

From: Mario Limonciello <mario.limonciello@amd.com>
To: Bjorn Helgaas <helgaas@kernel.org>,
	Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc: Kuppuswamy Sathyanarayanan
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	linux-pci@vger.kernel.org,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	linux-kernel@vger.kernel.org, Vidya Sagar <vidyas@nvidia.com>,
	Michael Bottini <michael.a.bottini@linux.intel.com>,
	intel-wired-lan@osuosl.org, bhelgaas@google.com,
	Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: Re: [Intel-wired-lan] [PATCH] PCI/ASPM: Enable ASPM on external PCIe devices
Date: Wed, 5 Jul 2023 23:07:34 -0500	[thread overview]
Message-ID: <9d1095ab-23e5-3df3-58d6-b2974f87ee72@amd.com> (raw)
In-Reply-To: <20230705200617.GA72825@bhelgaas>

On 7/5/23 15:06, Bjorn Helgaas wrote:
> On Wed, Jun 28, 2023 at 01:09:49PM +0800, Kai-Heng Feng wrote:
>> On Wed, Jun 28, 2023 at 4:54 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>>> On Tue, Jun 27, 2023 at 04:35:25PM +0800, Kai-Heng Feng wrote:
>>>> On Fri, Jun 23, 2023 at 7:06 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>>>>> On Tue, Jun 20, 2023 at 01:36:59PM -0500, Limonciello, Mario wrote:
> 
>>> It's perfectly fine for the IP to support PCI features that are not
>>> and can not be enabled in a system design.  But I expect that
>>> strapping or firmware would disable those features so they are not
>>> advertised in config space.
>>>
>>> If BIOS leaves features disabled because they cannot work, but at the
>>> same time leaves them advertised in config space, I'd say that's a
>>> BIOS defect.  In that case, we should have a DMI quirk or something to
>>> work around the defect.
>>
>> That means most if not all BIOS are defected.
>> BIOS vendors and ODM never bothered (and probably will not) to change
>> the capabilities advertised by config space because "it already works
>> under Windows".
> 
> This is what seems strange to me.  Are you saying that Windows never
> enables these power-saving features?  Or that Windows includes quirks
> for all these broken BIOSes?  Neither idea seems very convincing.
> 

I see your point.  I was looking through Microsoft documentation for 
hints and came across this:

https://learn.microsoft.com/en-us/windows-hardware/customize/power-settings/pci-express-settings-link-state-power-management

They have a policy knob to globally set L0 or L1 for PCIe links.

They don't explicitly say it, but surely it's based on what the devices 
advertise in the capabilities registers.

>>>> So the logic is to ignore the capability and trust the default set
>>>> by BIOS.
>>>
>>> I think limiting ASPM support to whatever BIOS configured at boot-time
>>> is problematic.  I don't think we can assume that all platforms have
>>> firmware that configures ASPM as aggressively as possible, and
>>> obviously firmware won't configure hot-added devices at all (in
>>> general; I know ACPI _HPX can do some of that).
>>
>> Totally agree. I was not suggesting to limiting the setting at all.
>> A boot-time parameter to flip ASPM setting is very useful. If none has
>> been set, default to BIOS setting.
> 
> A boot-time parameter for debugging and workarounds is fine.  IMO,
> needing a boot-time parameter in the course of normal operation is
> not OK.
> 
> Bjorn

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan