Re: [PATCH v2] PCI: vmd: Enable Hotplug based on BIOS setting on VMD rootports - Kai-Heng Feng

From: Kai-Heng Feng <kai.heng.feng@canonical.com>
To: Nirmal Patel <nirmal.patel@linux.intel.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	linux-pci@vger.kernel.org,
	 "Rafael J. Wysocki" <rjw@rjwysocki.net>
Subject: Re: [PATCH v2] PCI: vmd: Enable Hotplug based on BIOS setting on VMD rootports
Date: Thu, 7 Mar 2024 14:44:23 +0800	[thread overview]
Message-ID: <CAAd53p4j3MwigsFXpftuDdSfhn7EH_-cOOjP2DqnqeAuD+Fb=Q@mail.gmail.com> (raw)
In-Reply-To: <46b737db266f08c6dc98c77044bab12491a4d971.camel@linux.intel.com>

Hi Nirmal,

On Thu, Mar 7, 2024 at 6:20 AM Nirmal Patel
<nirmal.patel@linux.intel.com> wrote:
>
> On Tue, 2024-02-13 at 10:47 -0700, Nirmal Patel wrote:
> > On Wed, 2024-02-07 at 12:55 -0600, Bjorn Helgaas wrote:
> > > On Tue, Feb 06, 2024 at 05:27:29PM -0700, Nirmal Patel wrote:
> > > > ...
> > > > Did you have a chance to look at my response on January 16th to
> > > > your
> > > > questions? I tried to summarize all of the potential problems and
> > > > issues with different fixes. Please let me know if it is easier
> > > > if
> > > > I
> > > > resend the explanation. Thanks.
> > >
> > > I did see your Jan 16 response, thanks.
> > >
> > > I had more questions after reading it, but they're more about
> > > understanding the topology seen by the host and the guest:
> > >   Jan 16:
> > > https://lore.kernel.org/r/20240117004933.GA108810@bhelgaas
> > >   Feb  1:
> > > https://lore.kernel.org/r/20240201211620.GA650432@bhelgaas
> > >
> > > As I mentioned in my second Feb 1 response
> > > (https://lore.kernel.org/r/20240201222245.GA650725@bhelgaas), the
> > > usual plan envisioned by the PCI Firmware spec is that an OS can
> > > use
> > > a
> > > PCIe feature if the platform has granted the OS ownership via _OSC
> > > and
> > > a device advertises the feature via a Capability in config space.
> > >
> > > My main concern with the v2 patch
> > > (
> > > https://lore.kernel.org/r/20231127211729.42668-1-nirmal.patel@linux.intel.com
> > > )
> > > is that it overrides _OSC for native_pcie_hotplug, but not for any
> > > of
> > > the other features (AER, PME, LTR, DPC, etc.)
> > >
> > > I think it's hard to read the specs and conclude that PCIe hotplug
> > > is
> > > a special case, and I think we're likely to have similar issues
> > > with
> > > other features in the future.
> > >
> > > But if you think this is the best solution, I'm OK with merging it.
> Hi Bjorn,
>
> We tried some other ways to pass the information about all of the flags
> but I couldn't retrieve it in guest OS and VMD hardware doesn't have
> empty registers to write this information. So as of now we have this
> solution which only overwrites Hotplug flag. If you can accept it that
> would be great.

My original commit "PCI: vmd: Honor ACPI _OSC on PCIe features" was a
logically conclusion based on VMD ports are just apertures.
Apparently there are more nuances than that, and people outside of
Intel can't possibly know. And yes VMD creates lots of headaches
during hardware enablement.

So is it possible to document the right way to use _OSC, as Bjorn suggested?

Kai-Heng

> > In the host OS, this overrides whatever was negotiated via _OSC, I
> > guess on the principle that _OSC doesn't apply because the host BIOS
> > doesn't know about the Root Ports below the VMD.  (I'm not sure it's
> > 100% resolved that _OSC doesn't apply to them, so we should mention
> > that assumption here.)
> _OSC still controls every flag including Hotplug. I have observed that
> slot capabilities register has hotplug enabled only when platform has
> enabled the hotplug. So technically not overriding it in the host.
> It overrides in guest because _OSC is passing wrong/different
> information than the _OSC information in Host.
> Also like I mentioned, slot capabilities registers are not changed in
> Guest because vmd is passthrough.
> >
> > In the guest OS, this still overrides whatever was negotiated via
> > _OSC, but presumably the guest BIOS *does* know about the Root Ports,
> > so I don't think the same principle about overriding _OSC applies
> > there.
> >
> > I think it's still a problem that we handle
> > host_bridge->native_pcie_hotplug differently than all the other
> > features negotiated via _OSC.  We should at least explain this
> > somehow.
> Right now this is the only way to know in Guest OS if platform has
> enabled Hotplug or not. We have many customers complaining about
> Hotplug being broken in Guest. So just trying to honor _OSC but also
> fixing its limitation in Guest.
>
> Thanks
> nirmal.
>
> > I sincerely apologize for late responses. I just found out that my
> > evolution mail client is automatically sending linux-pci emails to
> > junk
> > since January 2024.
> >
> > At the moment Hotplug is an exception for us, but I do share your
> > concern about other flags. We have done lot of testing and so far
> > patch
> > v2 is the best solution we have.
> >
> > Thanks
> > nirmal
> > > Bjorn
>