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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 3/26/2024 2:08 PM, Bjorn Helgaas wrote:
On Tue, Mar 26, 2024 at 08:51:45AM -0700, Paul M Stillwell Jr wrote:
On 3/25/2024 6:59 PM, Kai-Heng Feng wrote:
On Tue, Mar 26, 2024 at 8:17 AM Nirmal Patel
<nirmal.patel@xxxxxxxxxxxxxxx> wrote:

On Fri, 22 Mar 2024 18:36:37 -0500 Bjorn Helgaas
<helgaas@xxxxxxxxxx> 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@xxxxxxxxxxxxx> 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.

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.

There isn't any official documentation for this, it's just the way VMD
is designed :). VMD assumes that everything behind it is hotpluggable so
the bits don't *really* matter. In other words, VMD supports hotplug and
you can't turn that off so hotplug is always on regardless of what the
bits say.

This whole discussion is about who *owns* the feature, not whether the
hardware supports the feature.


I'm not disagreeing about who owns the feature :) I'm trying to find a solution when the data that the driver gets is wrong.

The general rule is "if _OSC covers the feature, the platform owns the
feature and the OS shouldn't touch it unless the OS requests and is
granted control of it."


The code is fine in a bare metal system, but completely broken in a hypervisor because all the _OSC bits are set to 0. So I am trying to find a solution where the hotplug bits can be set correctly in this situation.

VMD wants special treatment, and we'd like a simple description of
what that treatment is.  Right now it sounds like you want the OS to
own *hotplug* below VMD regardless of what _OSC says.


What I want is for hotplug to be enabled in a hypervisor :) The slot capability register has the correct bits set, but it doesn't make sense (to me) to use a register in a root port to set the bridge settings. That's why this patch just removed the hotplug bits from the code. Everything is set up correctly (as far as I can tell) in both bare metal and hypervisors if we omit those 2 lines.

Does anyone have a better suggestion on how to handle the case where the _OSC bits are incorrect?

Paul

But I still have no idea about other features like AER, etc., so we're
kind of in no man's land about how to handle them.

Bjorn





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux