On Fri, 2024-01-12 at 16:55 -0600, Bjorn Helgaas wrote:
> [+cc Rafael, Kai-Heng]
>
> On Thu, Dec 14, 2023 at 03:22:21PM -0700, Nirmal Patel wrote:
> > On Thu, 2023-12-14 at 13:23 -0600, Bjorn Helgaas wrote:
> > > On Wed, Dec 13, 2023 at 06:07:02PM -0700, Nirmal Patel wrote:
> > > > On Tue, 2023-12-12 at 15:13 -0600, Bjorn Helgaas wrote:
> > > > > On Mon, Dec 11, 2023 at 04:05:25PM -0700, Nirmal Patel wrote:
> > > > > > On Tue, 2023-12-05 at 18:27 -0600, Bjorn Helgaas wrote:
> > > > > > > On Tue, Dec 05, 2023 at 03:20:27PM -0700, Nirmal Patel
> > > > > > > wrote:
> > > > > > > ...
> > > > > > > > Currently Hotplug is controlled by _OSC in both host
> > > > > > > > and
> > > > > > > > Guest. In case of guest, it seems guest BIOS hasn't
> > > > > > > > implemented _OSC since all the flags are set to 0 which
> > > > > > > > is not the case in host.
> > > > > > >
> > > > > > > I think you want pciehp to work on the VMD Root Ports in
> > > > > > > the Guest, no matter what, on the assumption that any
> > > > > > > _OSC
> > > > > > > that applies to host bridge A does not apply to the host
> > > > > > > bridge created by the vmd driver.
> > > > > > >
> > > > > > > If so, we should just revert 04b12ef163d1 ("PCI: vmd:
> > > > > > > Honor ACPI _OSC on PCIe features"). Since host bridge B
> > > > > > > was not enumerated via ACPI, the OS owns all those
> > > > > > > features under B by default, so reverting 04b12ef163d1
> > > > > > > would leave that state.
> > > > > > >
> > > > > > > Obviously we'd have to first figure out another solution
> > > > > > > for the AER message flood that 04b12ef163d1 resolved.
> > > > > >
> > > > > > If we revert 04b12ef163d1, then VMD driver will still
> > > > > > enable
> > > > > > AER blindly which is a bug. So we need to find a way to
> > > > > > make
> > > > > > VMD driver aware of AER platform setting and use that
> > > > > > information to enable or disable AER for its child devices.
> > > > > >
> > > > > > There is a setting in BIOS that allows us to enable OS
> > > > > > native AER support on the platform. This setting is located
> > > > > > in EDK Menu -> Platform configuration -> system event log
> > > > > > ->
> > > > > > IIO error enabling -> OS native AER support.
> > > > > >
> > > > > > This setting is assigned to VMD bridge by
> > > > > > vmd_copy_host_bridge_flags in patch 04b12ef163d1. Without
> > > > > > the patch 04b12ef163d1, VMD driver will enable AER even if
> > > > > > platform has disabled OS native AER support as mentioned
> > > > > > earlier. This will result in an AER flood mentioned in
> > > > > > 04b12ef163d1 since there is no AER handlers.
> > >
> > > I missed this before. What does "there are no AER handlers"
> > > mean?
> > > Do you mean there are no *firmware* AER handlers?
> >
> > Sorry for confusing wordings. Your understanding is correct. The
> > patch
> > 04b12ef163d1 is used to disable AER on vmd and avoid the AER
> > flooding
> > by applying _OSC settings since vmd driver doesn't give a choice to
> > toggle AER, DPC, LTR, etc.
> > > The dmesg log at
> > > https://bugzilla.kernel.org/attachment.cgi?id=299571
> > > (from https://bugzilla.kernel.org/show_bug.cgi?id=215027, the bug
> > > fixed by 04b12ef163d1), shows this:
> > >
> > > acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM
> > > Segments MSI HPX-Type3]
> > > acpi PNP0A08:00: _OSC: platform does not support [AER]
> > > acpi PNP0A08:00: _OSC: OS now controls [PCIeHotplug SHPCHotplug
> > > PME
> > > PCIeCapability LTR]
> > >
> > > so the firmware did not grant AER control to the OS (I think
> > > "platform does not support" is a confusing description).
> > >
> > > Prior to 04b12ef163d1, Linux applied _OSC above the VMD bridge
> > > but
> > > not below it, so Linux had native control below VMD and it did
> > > handle AER interrupts from the 10000:e0:06.0 Root Port below VMD:
> > >
> > > vmd 0000:00:0e.0: PCI host bridge to bus 10000:e0
> > > pcieport 10000:e0:06.0: AER: Corrected error received:
> > > 10000:e1:00.0
> > >
> > > After 04b12ef163d1, Linux applied _OSC below the VMD bridge as
> > > well,
> > > so it did not enable or handle any AER interrupts. I suspect the
> > > platform didn't handle AER interrupts below VMD either, so those
> > > errors were probably just ignored.
> > >
> > > > > > If VMD is aware of OS native AER support setting, then we
> > > > > > will not see AER flooding issue.
> > > > > >
> > > > > > Do you have any suggestion on how to make VMD driver aware
> > > > > > of AER setting and keep it in sync with platform setting.
> > > > >
> > > > > Well, this is the whole problem. IIUC, you're saying we
> > > > > should use _OSC to negotiate for AER control below the VMD
> > > > > bridge, but ignore _OSC for hotplug control.
> > > >
> > > > Because VMD has its own hotplug BIOS setting which allows vmd
> > > > to
> > > > enable or disable hotplug on its domain. However we don't have
> > > > VMD specific AER, DPC, LTR settings.
> > >
> > > I don't quite follow. The OS knows nothing about whether BIOS
> > > settings exist, so they can't be used to determine where _OSC
> > > applies.
> > >
> > > > Is it okay if we include an additional check for hotplug? i.e.
> > > > Hotplug capable bit in SltCap register which reflects VMD BIOS
> > > > hotplug setting.
> > >
> > > I don't know what check you have in mind, but the OS can
> > > definitely use SltCap to decide what features to enable.
> >
> > Yes I agree. That is what I am also suggesting in this patch.
>
> I should have said "the OS can use SltCap to *help* decide what
> features to enable." For ACPI host bridges, _OSC determines whether
> hotplug should be operated by the platform or the OS.
>
> I think we're converging on the idea that since VMD is effectively
> *not* an ACPI host bridge and doesn't have its own _OSC, the _OSC
> that
> applies to the VMD endpoint does *not* apply to the hierarchy below
> the VMD. In that case, the default is that the OS owns all the
> features (hotplug, AER, etc) below the VMD.
Well there will be few problems if VMD owns/assigns all the flags by itself. We discussed all of the potential problems but due to the holidays I think I should summarize them again.
#1 : Currently there is no way to pass the information about AER, DPC,
etc to VMD driver from BIOS or from boot parameter. For example, If VMD
blindly enables AER and platform has AER disabled, then we will see
AERs from devices under VMD but user have no way to toggle it since we
rejected idea of adding boot parameter for AER, DPC under VMD. I
believe you also didn't like the idea of sysfs knob suggested by Kai-
Heng.
#2 : Even if we use VMD hardware register to store AER, DPC and make
UEFI VMD driver to write information about Hotplug, DPC, AER, we still
dont have a way to change the setting if user wants to alter them. Also
the issue will still persist in client platforms since we don't own
their UEFI VMD driver. It will be a huge effort.
#3 : At this moment, user can enable/disable only Hotplug in VMD BIOS
settings (meaning no AER, DPC, LTR, etc)and VMD driver can read it from
SltCap register. This means BIOS needs to add other settings and VMD
needs to figure out to read them which at this moment VMD can't do it.
#4 : consistency with Host OS and Guest OS.
I believe the current propesed patch is the best option which requires
minimal changes without breaking other platform features and unblock
our customers. This issues has been a blocker for us.
For your concerns regarding how VMD can own all the _OSC features, i am
open to other ideas and will discuss with the architect if they have
any suggestions.
>
> > > You suggest above that you want vmd to be aware of AER ownership
> > > per
> > > _OSC, but it sounds more like you just want AER disabled below
> > > VMD
> > > regardless. Or do you have platforms that can actually handle
> > > AER
> > > interrupts from Root Ports below VMD?
> >
> > No I dont want AER disabled below VMD all the time. I am suggesting
> > vmd should be in sync with all the _OSC flags since vmd doesn't
> > give
> > a choice to toggle.
>
> This is what I don't understand. If we decide that _OSC doesn't
> apply
> below VMD because the VMD host bridge isn't described in ACPI, the
> idea of being in sync with _OSC doesn't make sense.
>
> > However, the issue arises in guest where _OSC setting for hotplug
> > is
> > always 0 even though hotplug is 1 in host and hotplug enable bit in
> > SltCap register is 1 in both host and guest. So we can use that to
> > enable hotplug in guest. Like you suggested in your above comment.
>
> All this got paged out over the holidays, so I need to refresh my
> understanding here. Maybe it will help if we can make the situation
> more concrete. I'm basing this on the logs at
> https://bugzilla.kernel.org/show_bug.cgi?id=215027. I assume the
> 10000:e0:06.0 Root Port and the 10000:e1:00.0 NVMe device are both
> passed through to the guest. I'm sure I got lots wrong here, so
> please correct me :)
>
> Host OS sees:
>
> PNP0A08 host bridge to 0000 [bus 00-ff]
> _OSC applies to domain 0000
> OS owns [PCIeHotplug SHPCHotplug PME PCIeCapability LTR] in
> domain 0000
> vmd 0000:00:0e.0: PCI host bridge to domain 10000 [bus e0-ff]
> no _OSC applies in domain 10000;
> host OS owns all PCIe features in domain 10000
> pci 10000:e0:06.0: [8086:464d] # VMD root port
> pci 10000:e0:06.0: PCI bridge to [bus e1]
> pci 10000:e0:06.0: SltCap: HotPlug+ # Hotplug Capable
> pci 10000:e1:00.0: [144d:a80a] # nvme
>
> Guest OS sees:
>
> PNP0A03 host bridge to 0000 [bus 00-ff]
> _OSC applies to domain 0000
> platform owns [PCIeHotplug ...] # _OSC doesn't grant
> to OS
> pci 0000:e0:06.0: [8086:464d] # VMD root port
> pci 0000:e0:06.0: PCI bridge to [bus e1]
> pci 0000:e0:06.0: SltCap: HotPlug+ # Hotplug Capable
> pci 0000:e1:00.0: [144d:a80a] # nvme
>
> So the guest OS sees that the VMD Root Port supports hotplug, but it
> can't use it because it was not granted ownership of the feature?
You are correct about _OSC not granting access in Guest. This is what I
see on my setup.
Host OS:
ACPI: PCI Root Bridge [PC11] (domain 0000 [bus e2-fa])
acpi PNP0A08:0b: _OSC: OS supports [ExtendedConfig ASPM ClockPM
Segments MSI EDR HPX-Type3]
acpi PNP0A08:0b: _OSC: platform does not support [SHPCHotplug
AER DPC]
acpi PNP0A08:0b: _OSC: OS now controls [PCIeHotplug PME
PCIeCapability LTR]
PCI host bridge to bus 0000:e2
vmd 0000:e2:00.5: PCI host bridge to bus 10007:00
vmd 0000:e2:00.5: Bound to PCI domain 10007
Guest OS:
ACPI: PCI Root Bridge [PC0G] (domain 0000 [bus 03])
acpi PNP0A08:01: _OSC: OS supports [ExtendedConfig ASPM ClockPM
Segments MSI EDR HPX-Type3]
acpi PNP0A08:01: _OSC: platform does not support [PCIeHotplug
SHPCHotplug PME AER LTR DPC]
acpi PNP0A08:01: _OSC: OS now controls [PCIeCapability]
vmd 0000:03:00.0: Bound to PCI domain 10000
SltCap: PwrCtrl- MRL- AttnInd- PwrInd- HotPlug- Surprise-
>
> Bjorn
