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>,
	Paul M Stillwell Jr <paul.m.stillwell.jr@intel.com>,
	 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: Tue, 26 Mar 2024 09:59:24 +0800	[thread overview]
Message-ID: <CAAd53p6JmJoGjiC5nqRbA4x6fNZY5xCyGELXj0-9ux2LWVhroA@mail.gmail.com> (raw)
In-Reply-To: <20240325171743.000013e6@linux.intel.com>

On Tue, Mar 26, 2024 at 8:17 AM Nirmal Patel
<nirmal.patel@linux.intel.com> wrote:
>
> On Fri, 22 Mar 2024 18:36:37 -0500
> Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> > On Fri, Mar 22, 2024 at 03:43:27PM -0700, Paul M Stillwell Jr wrote:
> > > On 3/22/2024 2:37 PM, Bjorn Helgaas wrote:
> > > > On Fri, Mar 22, 2024 at 01:57:00PM -0700, Nirmal Patel wrote:
> > > > > On Fri, 15 Mar 2024 09:29:32 +0800
> > > > > Kai-Heng Feng <kai.heng.feng@canonical.com> wrote:
> > > > > ...
> > > >
> > > > > > If there's an official document on intel.com, it can make
> > > > > > many things clearer and easier.
> > > > > > States what VMD does and what VMD expect OS to do can be
> > > > > > really helpful. Basically put what you wrote in an official
> > > > > > document.
> > > > >
> > > > > Thanks for the suggestion. I can certainly find official VMD
> > > > > architecture document and add that required information to
> > > > > Documentation/PCI/controller/vmd.rst. Will that be okay?
> > > >
> > > > I'd definitely be interested in whatever you can add to illuminate
> > > > these issues.
> > > >
> > > > > I also need your some help/suggestion on following alternate
> > > > > solution. We have been looking at VMD HW registers to find some
> > > > > empty registers. Cache Line Size register offset OCh is not
> > > > > being used by VMD. This is the explanation in PCI spec 5.0
> > > > > section 7.5.1.1.7: "This read-write register is implemented for
> > > > > legacy compatibility purposes but has no effect on any PCI
> > > > > Express device behavior." Can these registers be used for
> > > > > passing _OSC settings from BIOS to VMD OS driver?
> > > > >
> > > > > These 8 bits are more than enough for UEFI VMD driver to store
> > > > > all _OSC flags and VMD OS driver can read it during OS boot up.
> > > > > This will solve all of our issues.
> > > >
> > > > Interesting idea.  I think you'd have to do some work to separate
> > > > out the conventional PCI devices, where PCI_CACHE_LINE_SIZE is
> > > > still relevant, to make sure nothing breaks.  But I think we
> > > > overwrite it in some cases even for PCIe devices where it's
> > > > pointless, and it would be nice to clean that up.
> > >
> > > I think the suggestion here is to use the VMD devices Cache Line
> > > Size register, not the other PCI devices. In that case we don't
> > > have to worry about conventional PCI devices because we aren't
> > > touching them.
> >
> > Yes, but there is generic code that writes PCI_CACHE_LINE_SIZE for
> > every device in some cases.  If we wrote the VMD PCI_CACHE_LINE_SIZE,
> > it would obliterate whatever you want to pass there.
> >
> > Bjorn
> Our initial testing showed no change in value by overwrite from pci. The
> registers didn't change in Host as well as in Guest OS when some data
> was written from BIOS. I will perform more testing and also make sure
> to write every register just in case.

If the VMD hardware is designed in this way and there's an official
document states that "VMD ports should follow _OSC expect for
hotplugging" then IMO there's no need to find alternative.

Kai-Heng

> Thanks for the response.
>
> -nirmal
>