All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Grumbach, Emmanuel" <emmanuel.grumbach@intel.com>
To: "helgaas@kernel.org" <helgaas@kernel.org>
Cc: "thierry.reding@kernel.org" <thierry.reding@kernel.org>,
	"mmaddireddy@nvidia.com" <mmaddireddy@nvidia.com>,
	"lukas@wunner.de" <lukas@wunner.de>,
	"sashiko-reviews@lists.linux.dev"
	<sashiko-reviews@lists.linux.dev>,
	"Korenblit, Miriam Rachel" <miriam.rachel.korenblit@intel.com>,
	"treding@nvidia.com" <treding@nvidia.com>,
	"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: [PATCH] PCI: Disable NoSnoop and Relaxed ordering for Intel wireless BE200
Date: Thu, 30 Jul 2026 20:16:33 +0000	[thread overview]
Message-ID: <b314134d7e39eb497a17283ce15467914aa054ae.camel@intel.com> (raw)
In-Reply-To: <20260730200819.GA1520846@bhelgaas>

On Thu, 2026-07-30 at 15:08 -0500, Bjorn Helgaas wrote:
> On Thu, Jul 30, 2026 at 06:45:29PM +0000, Grumbach, Emmanuel wrote:
> > On Thu, 2026-07-30 at 12:28 -0500, Bjorn Helgaas wrote:
> > > On Thu, Jul 30, 2026 at 10:21:06AM +0530, Manikanta Maddireddy
> > > wrote:
> > > > On 29/07/26 12:19 pm, Grumbach, Emmanuel wrote:
> > > > > On Tue, 2026-07-28 at 15:18 -0500, Bjorn Helgaas wrote:
> > > > > > On Tue, Jul 28, 2026 at 07:47:04PM +0000, Grumbach,
> > > > > > Emmanuel
> > > > > > wrote:
> > > > > > > On Tue, 2026-07-28 at 14:24 -0500, Bjorn Helgaas wrote:
> > > > > > > > On Tue, Jul 28, 2026 at 06:21:23PM +0000, Grumbach,
> > > > > > > > Emmanuel
> > > > > > > > wrote:
> > > > > > > > > On Tue, 2026-07-28 at 13:03 -0500, Bjorn Helgaas
> > > > > > > > > wrote:
> > > > > > > > > ...
> > > > > > > > 
> > > > > > > > > > AFAICS we still don't know why Tegra264 would set
> > > > > > > > > > NoSnoop and/or RlxdOrd in the MMIO read.
> > > > > > > > > 
> > > > > > > > > Me neither but... I can't comment on that. And de-
> > > > > > > > > facto,
> > > > > > > > > once it does that, the BE200 replies with the NoSnoop
> > > > > > > > > and
> > > > > > > > > RlxOrd clear in the TLP which is then considered as a
> > > > > > > > > malformed TLP.
> > > > > > > > > 
> > > > > > > > > > Unless iwlwifi asked for NoSnoop and/or RlxdOrd to
> > > > > > > > > > be
> > > > > > > > > > set, I think it's a potential problem for drivers
> > > > > > > > > > if
> > > > > > > > > > Tegra264 sets them.
> > > > > > > > > 
> > > > > > > > > I ... don't think we would do that. The driver would
> > > > > > > > > certainly not do that... Regarding the hardware
> > > > > > > > > itself, I
> > > > > > > > > can't comment, but I can check internally.
> > > > > > > > 
> > > > > > > > Forgot to ask, can you collect the output of "sudo
> > > > > > > > lspci
> > > > > > > > -vv", at least for the RP and the BE200?
> > > > > > > > 
> > > > > > > > And maybe the dmesg log when booted with
> > > > > > > > "log_buf_len=16M
> > > > > > > > pci=earlydump" as well, so we can see if attributes
> > > > > > > > were
> > > > > > > > enabled by a bootloader?
> > > > > > > 
> > > > > > > I assume you want all this without the quirk, right?
> > > > > > 
> > > > > > Right.
> > > > > 
> > > > > Attached.
> > > > > Note that we used an out-of-tree driver in those attempts so
> > > > > the
> > > > > kernel is tainted, but this does not impact the issue we're
> > > > > discussing here.
> > > > 
> > > > PCI_EXP_DEVCTL_RELAX_EN and PCI_EXP_DEVCTL_NOSNOOP_EN are set
> > > > by HW
> > > > default value, this is inline with PCIe spec. It says "the
> > > > default
> > > > value of this bit is 1b" in both cases.
> > > 
> > > Huh, you're right.  I was assuming 0b was the default, but indeed
> > > 1b
> > > is the default unless they're hardwired to 0.  And they're both
> > > set
> > > to
> > > 1b on my x86 (AMD) Root Ports as well.  
> > 
> > Not on my Intel Root port:
> > 
> > 00:1d.0 PCI bridge: Intel Corporation Alder Lake PCI Express Root
> > Port 
> > 
> > <Snip>
> > 
> >         Capabilities: [40] Express (v2) Root Port (Slot+),
> > IntMsgNum 0
> >                 DevCap: MaxPayload 256 bytes, PhantFunc 0
> >                         ExtTag- RBE+ TEE-IO-
> >                 DevCtl: CorrErr- NonFatalErr- FatalErr- UnsupReq-
> >                         RlxdOrd- ExtTag- PhantFunc- AuxPwr-
> > NoSnoop-
> >                         MaxPayload 128 bytes, MaxReadReq 128 bytes
> 
> Super interesting.

Every one and his own areas of interest ;)

> 
> I guess Alder Lake isn't one of the RPs affected by
> quirk_relaxedordering_disable()?  I think 87e09cdec4da ("PCI: Disable
> Relaxed Ordering for some Intel processors") added all the existing
> uses of that quirk for Intel RPs, and the commit log only mentions
> Broadwell and Haswell.

Well... that commit is from 2017. Alder lake was shipped in 2021.

> 
> I wonder if the PCI core should disable RO and NoSnoop on Root Ports
> across the board?
> 
> I think this would mainly affect MMIO accesses performed by drivers,
> and I suspect drivers expect those writes to be performed in order,
> e.g., if the driver writes DMA descriptors to the device followed by
> a
> write to a doorbell, those should stay in order.  If the RP has RO
> enabled, those writes may be reordered.

As a device writer I'd say we'd be required to add a memory barrier in
that case, but we use writel usually.
Does that include a write vs write ordering? Hm...

> 
> > > So I would think we'd see this Malformed TLP error on pretty much
> > > every system with BE200 in it.
> > 
> > I guess that BE200 is almost only plugged into Intel systems which
> > is
> > why this bug went unnoticed until now..

  reply	other threads:[~2026-07-30 20:16 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-21  6:54 [PATCH] PCI: Disable NoSnoop and Relaxed ordering for Intel wireless BE200 Emmanuel Grumbach
2026-06-21  7:12 ` sashiko-bot
2026-06-21  7:30   ` Grumbach, Emmanuel
2026-07-22 14:56     ` Bjorn Helgaas
2026-07-22 15:14       ` Grumbach, Emmanuel
2026-07-22 17:04         ` Bjorn Helgaas
2026-07-23  9:49           ` Manikanta Maddireddy
2026-07-23 11:35             ` Grumbach, Emmanuel
2026-07-24 10:54               ` Manikanta Maddireddy
2026-07-28 18:03               ` Bjorn Helgaas
2026-07-28 18:21                 ` Grumbach, Emmanuel
2026-07-28 19:11                   ` Bjorn Helgaas
2026-07-28 19:16                     ` Grumbach, Emmanuel
2026-07-28 19:24                   ` Bjorn Helgaas
2026-07-28 19:47                     ` Grumbach, Emmanuel
2026-07-28 20:18                       ` Bjorn Helgaas
2026-07-29  6:49                         ` Grumbach, Emmanuel
2026-07-30  4:51                           ` Manikanta Maddireddy
2026-07-30 17:28                             ` Bjorn Helgaas
2026-07-30 18:45                               ` Grumbach, Emmanuel
2026-07-30 20:08                                 ` Bjorn Helgaas
2026-07-30 20:16                                   ` Grumbach, Emmanuel [this message]
2026-07-23  6:47   ` Lukas Wunner
2026-06-21  8:10 ` Lukas Wunner
2026-06-21  8:28   ` Grumbach, Emmanuel
2026-07-22 11:01   ` Grumbach, Emmanuel
2026-07-22 14:36     ` Lukas Wunner

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=b314134d7e39eb497a17283ce15467914aa054ae.camel@intel.com \
    --to=emmanuel.grumbach@intel.com \
    --cc=helgaas@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=miriam.rachel.korenblit@intel.com \
    --cc=mmaddireddy@nvidia.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=thierry.reding@kernel.org \
    --cc=treding@nvidia.com \
    /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.