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;
MIME-Version: 1.0
References: <dc44cb41-b306-f18a-c9fc-3d956777f722@amd.com> <20230718192450.GA489825@bhelgaas>
In-Reply-To: <20230718192450.GA489825@bhelgaas>
From:   Kai-Heng Feng <kai.heng.feng@canonical.com>
Date:   Fri, 11 Aug 2023 16:34:52 +0800
Message-ID: <CAAd53p5PAhX6OO0xzaF5TKJ4qT6=nMjQqv5vZM=7rFKtgr-H=A@mail.gmail.com>
Subject: Re: [Intel-wired-lan] [PATCH] PCI/ASPM: Enable ASPM on external PCIe devices
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     "Limonciello, Mario" <mario.limonciello@amd.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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Wed, Jul 19, 2023 at 3:24=E2=80=AFAM Bjorn Helgaas <helgaas@kernel.org> =
wrote:
>
> 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=E2=80=AFAM Mario Limonciello <mario.lim=
onciello@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 t=
he
> > > > > 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.

This is not using BIOS as a "limit". OS is still capable of changing
the ASPM policy at boot time or runtime.
The main point is to find a "sane" setting for devices where BIOS
can't program ASPM.

>
> 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 s=
hip
> > > > 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.

This can be another topic to explore. But sounds like it can break things.

>
> > > I wonder if there's any particular modification should be improved fo=
r
> > > this patch?
> >
> > Knowing this information I personally think the original patch that sta=
rted
> > 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.

OK. So what should we do instead? Checking if the device is connected
to TBT switch?

Kai-Heng


>
> Bjorn