All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
	Len Brown <lenb@kernel.org>, Bjorn Helgaas <bhelgaas@google.com>,
	linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org
Subject: Re: [RFC 1/1] PCI/ACPI: Make acpi_pci_root_validate_resources() reject IOMEM resources which start at address 0
Date: Wed, 16 Jun 2021 17:57:05 -0500	[thread overview]
Message-ID: <20210616225705.GA3014869@bjorn-Precision-5520> (raw)
In-Reply-To: <8065c303-fc11-93f2-64a5-39048b7501fd@redhat.com>

On Wed, Jun 16, 2021 at 08:43:12PM +0200, Hans de Goede wrote:
> On 6/15/21 10:23 PM, Bjorn Helgaas wrote:
> > On Tue, Jun 15, 2021 at 12:25:55PM +0200, Hans de Goede wrote:

> I've 2 dmesgs from runs both with and without pci=nocrs, the one
> with a clean kernel commandline (no special options) yields:
> 
> [    0.312333] pci_bus 0000:00: root bus resource [io  0x0000-0x0cf7 window]
> [    0.312335] pci_bus 0000:00: root bus resource [io  0x0d00-0xffff window]
> [    0.312336] pci_bus 0000:00: root bus resource [mem 0x000a0000-0x000bffff window]
> [    0.312337] pci_bus 0000:00: root bus resource [mem 0x65400000-0xbfffffff window]
> [    0.312338] pci_bus 0000:00: root bus resource [bus 00-fe]
> 
> Where as the one with pci=nocrs on the kernel commandline gives:
> 
> [    0.271766] pci_bus 0000:00: root bus resource [io  0x0000-0xffff]
> [    0.271767] pci_bus 0000:00: root bus resource [mem 0x00000000-0x7fffffffff]
> [    0.271768] pci_bus 0000:00: root bus resource [bus 00-fe]
> 
> Hmm, so assuming that you are right that pci=nocrs only influences
> the root resources (and I believe you are), and given that the problem is
> that we are getting these errors:
> 
> [    0.655335] pci 0000:00:15.0: BAR 0: no space for [mem size 0x00001000 64bit]
> [    0.655337] pci 0000:00:15.0: BAR 0: failed to assign [mem size 0x00001000 64bit]
> [    0.655339] pci 0000:00:15.1: BAR 0: no space for [mem size 0x00001000 64bit]
> [    0.655340] pci 0000:00:15.1: BAR 0: failed to assign [mem size 0x00001000 64bit]
> [    0.655342] pci 0000:00:1f.5: BAR 0: no space for [mem size 0x00001000]
> 
> Instead of getting this:
> 
> [    0.355716] pci 0000:00:15.0: BAR 0: assigned [mem 0x29c000000-0x29c000fff 64bit]
> [    0.355783] pci 0000:00:15.1: BAR 0: assigned [mem 0x29c001000-0x29c001fff 64bit]
> 
> So now I believe that my initial theory for this is probably completely wrong; and
> I wonder if the issue is that the _CRS returned root IOMEM window is big enough
> to exactly hold the BIOS assigned mappings, but it does not have any free space
> allowing the kernel to assign space for the 0000:00:15.0 and 0000:00:15.1
> devices ?
>
> Assuming that that theory is right, how could we work around this problem?
> Or at least do a quick debug patch to confirm that indeed the window is "full" ?

I'd be pretty surprised if the host bridge window actually full --
[mem 0x65400000-0xbfffffff] is a pretty big range and these devices
only need 4K each.

But maybe we aren't smart enough when trying to allocate space.
Places like __pci_bus_size_bridges() and __pci_assign_resource() are
full of assumptions about what PCI BARs can go where, depending on
64bit-ness, prefetchability, etc.

Maybe instrumenting those allocation paths would give some insight.
Possibly we should go ahead and merge some permanent pci_dbg() stuff
there too.

I do note that the working "pci=nocrs" case puts these devices above
4GB.  _CRS only told you about host bridge windows *below* 4GB, and
Linux will never assign space that's outside the windows (except in
the "pci=nocrs" case, of course).

> >> This happens specifically for the designware I2C PCI devices on these
> >> laptops, causing I2C-HID attached touchpads/touchscreens to not work.
> >>
> >> Booting with nocrs on these devices results in the kernel itself
> >> assigning memory to these devices, fixing things:
> > 
> > "pci=nocrs" to help people repro this or try the same workaround
> > elsewhere.
> 
> Not sure what you are trying to say here.

Sorry, I just meant that instead of "Booting with nocrs ...", I'd like
the commit log to say "Booting with 'pci=nocrs' ..." so that
non-expert users reading it will have a bit of a head start on how to
try this themselves.

Bjorn

  reply	other threads:[~2021-06-16 22:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-15 10:25 [RFC 0/1] PCI/ACPI: Make acpi_pci_root_validate_resources() reject IOMEM resources which start at address 0 Hans de Goede
2021-06-15 10:25 ` [RFC 1/1] " Hans de Goede
2021-06-15 10:59   ` Rafael J. Wysocki
2021-06-15 11:33     ` Hans de Goede
2021-06-15 11:52       ` Rafael J. Wysocki
2021-06-15 20:23   ` Bjorn Helgaas
2021-06-16 18:43     ` Hans de Goede
2021-06-16 22:57       ` Bjorn Helgaas [this message]
2021-06-21 11:49         ` Hans de Goede
2021-06-21 12:37           ` Bjorn Helgaas
2021-07-01 14:21           ` Hans de Goede

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=20210616225705.GA3014869@bjorn-Precision-5520 \
    --to=helgaas@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=hdegoede@redhat.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=rjw@rjwysocki.net \
    /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.