Re: [PATCH] PCI: Clear PCI_STATUS when setting up the device

From: Bjorn Helgaas
Date: Mon Nov 07 2022 - 16:37:48 EST

On Fri, Nov 04, 2022 at 10:53:39AM -0500, Bjorn Helgaas wrote:
> On Tue, May 17, 2022 at 12:37:38PM +0800, Kai-Heng Feng wrote:
> > We are seeing Master Abort bit is set on Intel I350 ethernet device and its
> > root port right after boot, probably happened during BIOS phase:
> >
> > 00:06.0 PCI bridge [0604]: Intel Corporation Device [8086:464d] (rev 05) (prog-if 00 [Normal decode])
> > Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
> > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ >SERR- <PERR- INTx-
> >
> > 6e:00.0 Ethernet controller [0200]: Intel Corporation I350 Gigabit Network Connection [8086:1521] (rev 01)
> > Subsystem: Intel Corporation Ethernet Server Adapter I350-T2 [8086:00a2]
> > Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
> > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ >SERR- <PERR- INTx-
> >
> > 6e:00.1 Ethernet controller [0200]: Intel Corporation I350 Gigabit Network Connection [8086:1521] (rev 01)
> > Subsystem: Intel Corporation Ethernet Server Adapter I350-T2 [8086:00a2]
> > Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx+
> > Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort+ >SERR- <PERR- INTx-
> >
> > And the Master Abort bit is cleared after S3.
> >
> > Since there's no functional impact found, clear the PCI_STATUS to treat
> > it anew at setting up.
> >
> > Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=215989
> > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
>
> This patch appeared in v6.0 as 6cd514e58f12 ("PCI: Clear PCI_STATUS
> when setting up device"). Christophe reported in the bugzilla that it
> causes boot failures:
>
> > --- Comment #3 from Christophe Fergeau (cfergeau@xxxxxxxxxx) ---
> > This commit
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6c
> > d514e58f12b211d638dbf6f791fa18d854f09c
> > references this issue.
> > This commit causes boot failures when trying to start linux guests with Apple's
> > hypervisor framework (for example using https://github.com/evansm7/vftool ).
> > After reverting it, I can successfully boot 6.1-rc kernels on my macOS12 x86_64
> > macbook. With this commit, the VM fails to start, additional details in
> > https://bugzilla.redhat.com/show_bug.cgi?id=2137803

I queued up a revert for v6.2. Obviously I would prefer if we could
figure out how to clear PCI_STATUS while still letting the guests
boot, but I have no idea how to debug the boot failures.

commit 44e985938e85 ("Revert "PCI: Clear PCI_STATUS when setting up device"")
Author: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Date: Mon Nov 7 15:31:08 2022 -0600

Revert "PCI: Clear PCI_STATUS when setting up device"

This reverts commit 6cd514e58f12b211d638dbf6f791fa18d854f09c.

Christophe Fergeau reported that 6cd514e58f12 ("PCI: Clear PCI_STATUS when
setting up device") causes boot failures when trying to start linux guests
with Apple's virtualization framework (for example using
https://developer.apple.com/documentation/virtualization/running_linux_in_a_virtual_machine?language=objc)

6cd514e58f12 only solved a cosmetic problem, so revert it to fix the boot
failures.

Link: https://bugzilla.redhat.com/show_bug.cgi?id=2137803
Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b66fa42c4b1f..1d6f7b502020 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1891,9 +1891,6 @@ int pci_setup_device(struct pci_dev *dev)

dev->broken_intx_masking = pci_intx_mask_broken(dev);

- /* Clear errors left from system firmware */
- pci_write_config_word(dev, PCI_STATUS, 0xffff);
-
switch (dev->hdr_type) { /* header type */
case PCI_HEADER_TYPE_NORMAL: /* standard header */
if (class == PCI_CLASS_BRIDGE_PCI)

> > ---
> > drivers/pci/probe.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index 17a969942d370..414f659dc8735 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1890,6 +1890,9 @@ int pci_setup_device(struct pci_dev *dev)
> >
> > dev->broken_intx_masking = pci_intx_mask_broken(dev);
> >
> > + /* Clear errors left from system firmware */
> > + pci_write_config_word(dev, PCI_STATUS, 0xffff);
> > +
> > switch (dev->hdr_type) { /* header type */
> > case PCI_HEADER_TYPE_NORMAL: /* standard header */
> > if (class == PCI_CLASS_BRIDGE_PCI)
> > --
> > 2.34.1
> >