On 4/25/2024 3:32 PM, Bjorn Helgaas wrote:
On Thu, Apr 25, 2024 at 02:43:07PM -0700, Paul M Stillwell Jr wrote:
On 4/25/2024 10:24 AM, Bjorn Helgaas wrote:
On Wed, Apr 24, 2024 at 02:29:16PM -0700, Paul M Stillwell Jr wrote:
On 4/23/2024 5:47 PM, Bjorn Helgaas wrote:
...
_OSC is the only mechanism for negotiating ownership of these
features, and PCI Firmware r3.3, sec 4.5.1, is pretty clear that _OSC
only applies to the hierarchy originated by the PNP0A03/PNP0A08 host
bridge that contains the _OSC method. AFAICT, there's no
global/device-specific thing here.
The BIOS may have a single user-visible setting, and it may apply that
setting to all host bridge _OSC methods, but that's just part of the
BIOS UI, not part of the firmware/OS interface.
Fair, but we are still left with the question of how to set the _OSC bits
for the VMD bridge. This would normally happen using ACPI AFAICT and we
don't have that for the devices behind VMD.
In the absence of a mechanism for negotiating ownership, e.g., an ACPI
host bridge device for the hierarchy, the OS owns all the PCIe
features.
I'm new to this space so I don't know what it means for the OS to
own the features. In other words, how would the vmd driver figure
out what features are supported?
There are three things that go into this:
- Does the OS support the feature, e.g., is CONFIG_PCIEAER enabled?
- Has the platform granted permission to the OS to use the feature,
either explicitly via _OSC or implicitly because there's no
mechanism to negotiate ownership?
- Does the device advertise the feature, e.g., does it have an AER
Capability?
If all three are true, Linux enables the feature.
I think vmd has implicit ownership of all features because there is no
ACPI host bridge device for the VMD domain, and (IMO) that means there
is no way to negotiate ownership in that domain.
So the VMD domain starts with all the native_* bits set, meaning Linux
owns the features. If the vmd driver doesn't want some feature to be
used, it could clear the native_* bit for it.
I don't think vmd should unilaterally claim ownership of features by
*setting* native_* bits because that will lead to conflicts with
firmware.
This is the crux of the problem IMO. I'm happy to set the native_* bits
using some knowledge about what the firmware wants, but we don't have a
mechanism to do it AFAICT. I think that's what commit 04b12ef163d1
("PCI: vmd: Honor ACPI _OSC on PCIe features") was trying to do: use a
domain that ACPI had run on and negotiated features and apply them to
the vmd domain.
Using the 3 criteria you described above, could we do this for the
hotplug feature for VMD:
1. Use IS_ENABLED(CONFIG_<whatever hotplug setting we need>) to check to
see if the hotplug feature is enabled
2. We know that for VMD we want hotplug enabled so that is the implicit
permission
3. Look at the root ports below VMD and see if hotplug capability is set
If 1 & 3 are true, then we set the native_* bits for hotplug (we can
look for surprise hotplug as well in the capability to set the
native_shpc_hotplug bit corrrectly) to 1. This feels like it would solve
the problem of "what if there is a hotplug issue on the platform"
because the user would have disabled hotplug for VMD and the root ports
below it would have the capability turned off.
In theory we could do this same thing for all the features, but we don't
know what the firmware wants for features other than hotplug (because we
implicitly know that vmd wants hotplug). I feel like 04b12ef163d1 is a
good compromise for the other features, but I hear your issues with it.
I'm happy to "do the right thing" for the other features, I just can't
figure out what that thing is :)
Paul
04b12ef163d1 basically asserted "the platform knows about a hardware
issue between VMD and this NVMe and avoided it by disabling AER in
domain 0000; therefore we should also disable AER in the VMD domain."
Your patch at
https://lore.kernel.org/linux-pci/20240408183927.135-1-paul.m.stillwell.jr@xxxxxxxxx/
says "vmd users *always* want hotplug enabled." What happens when a
platform knows about a hotplug hardware issue and avoids it by
disabling hotplug in domain 0000?
I was thinking about this also and I could look at all the root ports
underneath vmd and see if hotplug is set for any of them. If it is then we
could set the native_*hotplug bits based on that.
No. "Hotplug is set" means the device advertises the feature via
config space, in this case, it has the Hot-Plug Capable bit in the
PCIe Slot Capabilities set. That just means the device has hardware
support for the feature.
On ACPI systems, the OS can only use pciehp on the device if firmware
has granted ownership to the OS via _OSC because the firmware may want
to use the feature itself. If both OS and firmware think they own the
feature, they will conflict with each other.
If firmware retains owership of hotplug, it can field hotplug events
itself and notify the OS via the ACPI hotplug mechanism. The acpiphp
driver handles those events for PCI.
Firmware may do this if it wants to work around hardware defects it
knows about, or if it wants to do OS-independent logging (more
applicable for AER), or if it wants to intercept hotplug events to do
some kind of initialization, etc.
Bjorn
