Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 5 Jul 2023 15:06:17 -0500
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     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>,
        "Limonciello, Mario" <mario.limonciello@amd.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
Message-ID: <20230705200617.GA72825@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAAd53p5YjKbOkzBUmhu=EctusO7d38tHhg_b+zzvfVt97hLuOA@mail.gmail.com>
Precedence: bulk

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.

> > > 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