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:   Tue, 18 Jul 2023 14:24:50 -0500
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     "Limonciello, Mario" <mario.limonciello@amd.com>
Cc:     Kai-Heng Feng <kai.heng.feng@canonical.com>,
        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
Message-ID: <20230718192450.GA489825@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <dc44cb41-b306-f18a-c9fc-3d956777f722@amd.com>
Precedence: bulk

On Mon, Jul 17, 2023 at 11:51:32AM -0500, Limonciello, Mario wrote:
> On 7/16/2023 10:34 PM, Kai-Heng Feng wrote:
> > On Sat, Jul 15, 2023 at 12:37 AM Mario Limonciello <mario.limonciello@amd.com> wrote:
> > > On 7/14/23 03:17, Kai-Heng Feng wrote:

> > > > The main point is OS should stick to the BIOS default, which is the
> > > > only ASPM setting tested before putting hardware to the market.
> > > 
> > > Unfortunately; I don't think you can jump to this conclusion.

I think using the BIOS default as a limit is problematic.  I think it
would be perfectly reasonable for a BIOS to (a) configure only devices
it needs for console and boot, leaving others at power-on defaults,
and (b) configure devices in the safest configuration possible on the
assumption that an OS can decide the runtime policy itself.

Obviously I'm not a BIOS writer (though I sure wish I could talk to
some!), so maybe these are bad assumptions.

> > > A big difference in the Windows world to Linux world is that OEMs ship
> > > with a factory Windows image that may set policies like this.  OEM
> > > "platform" drivers can set registry keys too.

I suppose this means that the OEM image contains drivers that aren't
in the Microsoft media, and those drivers may set constraints on ASPM
usage?

If you boot the Microsoft media that lacks those drivers, maybe it
doesn't bother to configure ASPM for those devices?  Linux currently
configures ASPM for everything at enumeration-time, so we do it even
if there's no driver.

> > I wonder if there's any particular modification should be improved for
> > this patch?
> 
> Knowing this information I personally think the original patch that started
> this thread makes a lot of sense.

I'm still opposed to using dev_is_removable() as a predicate because I
don't think it has any technical connection to ASPM configuration.

Bjorn