All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
Cc: "Bjorn Helgaas" <bhelgaas@google.com>,
	"Jürgen Groß" <jgross@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>,
	"Boris Ostrovsky" <boris.ostrovsky@oracle.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	linux-kernel@vger.kernel.org, regressions@lists.linux.dev,
	"Felix Fietkau" <nbd@nbd.name>,
	"Lorenzo Bianconi" <lorenzo@kernel.org>,
	"Ryder Lee" <ryder.lee@mediatek.com>,
	linux-pci@vger.kernel.org
Subject: Re: Config space access to Mediatek MT7922 doesn't work after device reset in Xen PV dom0 (regression, Linux 6.12)
Date: Tue, 28 Jan 2025 21:03:15 -0600	[thread overview]
Message-ID: <20250129030315.GA392478@bhelgaas> (raw)
In-Reply-To: <Z5mOKQUrgeF_r6te@mail-itl>

[+cc linux-pci]

On Wed, Jan 29, 2025 at 03:10:49AM +0100, Marek Marczykowski-Górecki wrote:
> On Tue, Jan 28, 2025 at 07:15:26PM -0600, Bjorn Helgaas wrote:
> > On Fri, Jan 17, 2025 at 01:05:30PM +0100, Marek Marczykowski-Górecki wrote:
> > > After updating PV dom0 to Linux 6.12, The Mediatek MT7922 device reports
> > > all 0xff when accessing its config space. This happens only after device
> > > reset (which is also triggered when binding the device to the
> > > xen-pciback driver).
> > 
> > Thanks for the report and for all the debugging you've already done!
> > 
> > > Reproducer:
> > > 
> > >     # lspci -xs 01:00.0
> > >     01:00.0 Network controller: MEDIATEK Corp. MT7922 802.11ax PCI Express Wireless Network Adapter
> > >     00: c3 14 16 06 00 00 10 00 00 00 80 02 10 00 00 00
> > >     ...
> > >     # echo 1 > /sys/bus/pci/devices/0000:01:00.0/reset
> > >     # lspci -xs 01:00.0
> > >     01:00.0 Network controller: MEDIATEK Corp. MT7922 802.11ax PCI Express Wireless Network Adapter
> > >     00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > >
> > > The same operation done on Linux 6.12 running without Xen works fine.
> > > 
> > > git bisect points at:
> > > 
> > >     commit d591f6804e7e1310881c9224d72247a2b65039af
> > >     Author: Bjorn Helgaas <bhelgaas@google.com>
> > >     Date:   Tue Aug 27 18:48:46 2024 -0500
> > > 
> > >     PCI: Wait for device readiness with Configuration RRS
> > > 
> > > part of that commit:
> > > @@ -1311,9 +1320,15 @@ static int pci_dev_wait(struct pci_dev *dev, char *reset_type, int timeout)
> > >                         return -ENOTTY;
> > >                 }
> > >  
> > > -               pci_read_config_dword(dev, PCI_COMMAND, &id);
> > > -               if (!PCI_POSSIBLE_ERROR(id))
> > > -                       break;
> > > +               if (root && root->config_crs_sv) {
> > > +                       pci_read_config_dword(dev, PCI_VENDOR_ID, &id);
> > > +                       if (!pci_bus_crs_vendor_id(id))
> > > +                               break;
> > > +               } else {
> > > +                       pci_read_config_dword(dev, PCI_COMMAND, &id);
> > > +                       if (!PCI_POSSIBLE_ERROR(id))
> > > +                               break;
> > > +               }
> > >  
> > >     
> > > Adding some debugging, the PCI_VENDOR_ID read in pci_dev_wait() returns
> > > initially 0xffffffff. If I extend the condition with
> > > "&& !PCI_POSSIBLE_ERROR(id)", then the issue disappear. But reading the
> > > patch description, it would break VF.
> > > I'm not sure where the issue is, but given it breaks only when running
> > > with Xen, I guess something is wrong with "Configuration RRS Software
> > > Visibility" in that case.
> > 
> > I'm missing something.  If you get 0xffffffff, that is not the 0x0001
> > Vendor ID, so pci_dev_wait() should exit immediately.  
> 
> I'm not sure what is going on there either, but my _guess_ is that the
> loop exits too early due to the above. And it makes some further actions
> to fail.

When RRS SV is enabled, reading PCI_VENDOR_ID should always return
0x0001 (if the device isn't ready and responds with RRS status) or the
valid Vendor ID.  I don't think it should ever return 0xffff (unless
the device is powered off, unplugged, or broken, of course).

> > But the log at
> > https://github.com/QubesOS/qubes-issues/issues/9689#issuecomment-2582927149
> > says it *doesn't* exit and eventually times out.
> 
> Note this log is from "working" kernel, so that timeout must be
> something else.

I saw it was labeled "NO BUG" but I'm not sure it's labeled correctly
since there are no interesting messages from the "BUG PRESENT" part.
Awfully funny coincidence if it's unrelated.

> > And the lspci above shows ~0 data for much of the header, even though
> > the device must be ready by then.
> > 
> > I don't have any good ideas, but since the problem only happens with
> > Xen, and it seems to affect more than just the Vendor ID, maybe you
> > could instrument xen_pcibk_config_read() and see if there's something
> > wonky going on there?
> 
> This one is used when pcifront (from a different PV VM) is asking pciback
> to read something. I see the issue even before starting any other VM and
> not even attaching the device to the xen-pciback driver...

The report claims the problem only happens with Xen.  I'm not a Xen
person, and I don't know how to find the relevant config accessors.
The snippets of kernel messages I see at [1] all mention pciback, so
that's my only clue of where to look.  Bottom line, I have no idea
what the config accessor path is, and maybe we could learn something
by looking at whatever it is.

[1] https://github.com/QubesOS/qubes-issues/issues/9689#issuecomment-2582927149

  reply	other threads:[~2025-01-29  3:03 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-17 12:05 Config space access to Mediatek MT7922 doesn't work after device reset in Xen PV dom0 (regression, Linux 6.12) Marek Marczykowski-Górecki
2025-01-29  1:15 ` Bjorn Helgaas
2025-01-29  2:10   ` Marek Marczykowski-Górecki
2025-01-29  3:03     ` Bjorn Helgaas [this message]
2025-01-29  3:22       ` Marek Marczykowski-Górecki
2025-01-29  3:40         ` Bjorn Helgaas
2025-01-29  3:47           ` Marek Marczykowski-Górecki
2025-01-29 13:32             ` Bjorn Helgaas
2025-01-29 13:52               ` Jan Beulich
2025-01-29 14:50                 ` Bjorn Helgaas
2025-01-29  9:17         ` Jan Beulich
2025-01-29 11:53           ` Marek Marczykowski-Górecki
2025-01-29 12:49             ` Jan Beulich
2025-01-29 13:28           ` Bjorn Helgaas
2025-01-29 13:54             ` Jan Beulich
2025-01-29 18:48     ` Bjorn Helgaas
2025-01-30  4:55       ` Marek Marczykowski-Górecki
2025-01-30  9:30         ` Jan Beulich
2025-01-30 21:31           ` Bjorn Helgaas
2025-01-31  7:13             ` Jan Beulich
2025-01-31  8:36               ` Marek Marczykowski-Górecki
2025-02-05 22:14             ` Marek Marczykowski-Górecki
2025-02-07 22:00               ` Bjorn Helgaas
2025-02-07 22:10                 ` Marek Marczykowski-Górecki
2025-02-07 22:23               ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250129030315.GA392478@bhelgaas \
    --to=helgaas@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=jgross@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=marmarek@invisiblethingslab.com \
    --cc=nbd@nbd.name \
    --cc=regressions@lists.linux.dev \
    --cc=roger.pau@citrix.com \
    --cc=ryder.lee@mediatek.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.