public inbox for linux-pci@vger.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Michael Walle <michael@walle.cc>
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	intel-wired-lan@lists.osuosl.org,
	Bjorn Helgaas <bhelgaas@google.com>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	Paul Menzel <pmenzel@molgen.mpg.de>
Subject: Re: [PATCH v2] PCI: Fix Intel i210 by avoiding overlapping of BARs
Date: Tue, 12 Jan 2021 16:58:02 -0600	[thread overview]
Message-ID: <20210112225802.GA1859984@bjorn-Precision-5520> (raw)
In-Reply-To: <642eb96b495f5ad7d2d14410fedcd1ad@walle.cc>

On Sat, Jan 09, 2021 at 07:31:46PM +0100, Michael Walle wrote:
> Hi Bjorn,
> 
> Am 2021-01-08 22:20, schrieb Bjorn Helgaas:
> > On Wed, Dec 30, 2020 at 07:53:17PM +0100, Michael Walle wrote:
> > > The Intel i210 doesn't work if the Expansion ROM BAR overlaps with
> > > another BAR. Networking won't work at all and once a packet is sent
> > > the
> > > netdev watchdog will bite:
> > 
> > 1) Is this a regression?  It sounds like you don't know for sure
> > because earlier kernels don't support your platform.
> 
> Whats the background of the question? The board is offially supported
> since 5.8. I doubt that the code responsible to not touch the ExpROM
> BAR in pci_std_update_resource() were recently changed/added; the
> comment refers to a mail from 2005. So no I don't think it is a
> regression per se.

Just asking because it affects the urgency.  If we added a regression
during the v5.11 merge window, we'd try hard to fix it before
v5.11-final.  But it sounds like the problem has been there a long
time, so a fix could wait until v5.12.

> It is just that some combination of hardware and firmware will program
> the BARs in away so that this bug is triggered. And chances of this
> happing are very unlikely.
> 
> Do we agree that it should be irrelevant how the firmware programs and
> enables the BARs in this case? I.e. you could "fix" u-boot to match the
> way linux will assign addresses to the BARs. But that would just work
> around the real issue here. IMO.

I agree, Linux should work correctly regardless of how firmware
programmed the BARs.

> > 2) Can you open a bugzilla at https://bugzilla.kernel.org and attach
> > the complete dmesg and "sudo lspci -vv" output?  I want to see whether
> > Linux is assigning something incorrectly or this is a consequence of
> > some firmware initialization.
> 
> Sure, but you wouldn't even see the error with "lspci -vv" because
> lspci will just show the mapping linux assigned to it. But not whats
> written to the actual BAR for the PCI card. I'll also include a
> "lspci -xx". I've enabled CONFIG_PCI_DEBUG, too.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=211105
> 
> > 3) If the Intel i210 is defective in how it handles an Expansion ROM
> > that overlaps another BAR, a quirk might be the right fix. But my
> > guess is the device is working correctly per spec and there's
> > something wrong in how firmware/Linux is assigning things.  That would
> > mean we need a more generic fix that's not a quirk and not tied to the
> > Intel i210.
> 
> Agreed, but as you already stated (and I've also found that in the PCI
> spec) the Expansion ROM address decoder can be shared by the other BARs
> and it shouldn't matter as long as the ExpROM BAR is disabled, which is
> the case here.

My point is just that if this could theoretically affect devices other
than the i210, the fix should not be an i210-specific quirk.

I'll assume this is a general problem and wait for a generic PCI core
solution unless it's i210-specific.

Bjorn

  reply	other threads:[~2021-01-12 22:59 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-30 18:53 [PATCH v2] PCI: Fix Intel i210 by avoiding overlapping of BARs Michael Walle
2021-01-08 21:20 ` Bjorn Helgaas
2021-01-09 18:31   ` Michael Walle
2021-01-12 22:58     ` Bjorn Helgaas [this message]
2021-01-12 23:32       ` Michael Walle
2021-01-15 23:57         ` Bjorn Helgaas
2021-01-17 19:27           ` Michael Walle
2021-02-01 19:49             ` Michael Walle
2021-02-01 22:20               ` Bjorn Helgaas
2021-03-15 21:51                 ` Michael Walle
2021-08-20 15:12                   ` Michael Walle
2021-12-20 17:43                     ` Michael Walle
2021-12-21 17:48                       ` Bjorn Helgaas
2021-12-23  9:27                         ` Michael Walle
2021-12-23 16:37                           ` Bjorn Helgaas
2021-12-23 18:12                             ` Michael Walle
2022-01-12 14:50                               ` 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=20210112225802.GA1859984@bjorn-Precision-5520 \
    --to=helgaas@kernel.org \
    --cc=anthony.l.nguyen@intel.com \
    --cc=bhelgaas@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jesse.brandeburg@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=michael@walle.cc \
    --cc=pmenzel@molgen.mpg.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox