Linux-PCI Archive on lore.kernel.org
 help / color / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Bjorn Helgaas <helgaas@kernel.org>, Sinan Kaya <okaya@kernel.org>,
	linux-pci <linux-pci@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH v2] arm64: acpi/pci: invoke _DSM whether to preserve firmware PCI setup
Date: Fri, 14 Jun 2019 20:36:27 +1000
Message-ID: <906b2576756e82a54b584c3de2d8362602de07ce.camel@kernel.crashing.org> (raw)
In-Reply-To: <20190614095742.GA27188@e121166-lin.cambridge.arm.com>

On Fri, 2019-06-14 at 10:57 +0100, Lorenzo Pieralisi wrote:
> > Also using "probe_only" for _DSM #5 = 0 isn't a good idea, at least as
> > implemented today in the rest of the kernel, probe_only also means we
> > shouldn't assign what was left unassigned. However _DSM #5 allows this.
> 
> I am not sure about this. PCI_PROBE_ONLY cannot stop an OS from
> reassigning BARs that are clearly misconfigured, it does not make
> any sense.

PCI_PROBE_ONLY is a linux thing which, as implemented today, implies no
assignment at all. I believe it originates as a merge of variants of
the same thing, at least one of them being one I created for powerpc
back in the days due to our proprietary hypervisor not letting you
touch any of the PCI config space.

If a device looks broken, disable it, don't use it, but don't reassign
either. At least that the semantics we have today. And as such they
don't match _DSM #5 = 0.

> It can't stop an OS from writing those BARs anyway,
> since they must be sized, why firmware would prevent an OS from
> reassigning BARs that are programmed with values that can be
> deemed 100% bogus ? Or put it differently, why must an OS preserve
> those values willy-nilly ?

Don't ask me ... IBM firmware :-) At least that was the idea back then.

That said I suppose some platforms may also have set that flag to
indicate that they aren't sure what other "ghost" things might be
in the address space, ie Linux doesnt have a clear view of what's
free to allocate devices to for example.

> For me, PCI_PROBE_ONLY and _DSM == 0 on a host bridge must be considered
> equivalent.

Well, that's not what PCI_PROBE_ONLY is today in Linux. It might be
what you would like it to be but it's not what it is :-) And I'd like
to avoid making arm64 different than everybody else here because I want
to consolidate things.

Fundamentally, is what _DSM #5 == 0 does any different from our
standard (not PROBE_ONLY) mode of operation on server platforms anyway
? Ie, we read what's there, and we leave it alone unless it's broken or
unassigned ? This is precisely the definition of _DSM #5 == 0 no ?

PROBE_ONLY is .. something else.

> I agree with Bjorn on his reading of _DSM #5 and I think that
> the original patch that claims on _DSM #5 == 0 is a good
> starting point.

The original patch is a good starting point, we agree. The only point
of disagreement with Bjorn at this stage is what the "default" is in
absence of _DSM #5.

The spec says it should be the same as _DSM #5 == 0, but we know today
it will introduce a much wider ranging change to arm64 to treat it that
way. At the very least, changing the default should be a different
patch.

>  I would like to make it a default even without
> _DSM #5 == 0 so that claim and reassign on claim failure works
> irrespective of _DSM #5, it is now or never, I think we can give
> it a shot, with an incremental patch.

We should. In fact, I was thinking about it on the way home tonight and
was going to ask you and Ard to try this out and send me the debug
level log output of anything that looks wrong on any platform:

diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index bb85e2f4603f..0af1f1b4e4d8 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -193,8 +193,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
 	if (!bus)
 		return NULL;
 
-	pci_bus_size_bridges(bus);
-	pci_bus_assign_resources(bus);
+	pci_bus_claim_resources(bus);
+	pci_assign_unassigned_root_bus_resources(bus);
 
 	list_for_each_entry(child, &bus->children, node)
 		pcie_bus_configure_settings(child);

Cheers,
Ben.



  reply index

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-13  7:54 Benjamin Herrenschmidt
2019-06-13 19:02 ` Bjorn Helgaas
2019-06-13 21:59   ` Benjamin Herrenschmidt
2019-06-13 23:07   ` Benjamin Herrenschmidt
2019-06-14  7:42     ` Ard Biesheuvel
2019-06-14  8:36       ` Benjamin Herrenschmidt
2019-06-14  9:57         ` Lorenzo Pieralisi
2019-06-14 10:36           ` Benjamin Herrenschmidt [this message]
2019-06-14 10:43             ` Benjamin Herrenschmidt
2019-06-14 13:12               ` Bjorn Helgaas
2019-06-14 13:48                 ` Benjamin Herrenschmidt
2019-06-14 20:13                   ` Bjorn Helgaas
2019-06-15  1:18                     ` Benjamin Herrenschmidt
2019-06-14 13:09         ` Bjorn Helgaas
2019-06-14 13:46           ` Benjamin Herrenschmidt

Reply instructions:

You may reply publically 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=906b2576756e82a54b584c3de2d8362602de07ce.camel@kernel.crashing.org \
    --to=benh@kernel.crashing.org \
    --cc=ard.biesheuvel@linaro.org \
    --cc=helgaas@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=okaya@kernel.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

Linux-PCI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pci/0 linux-pci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pci linux-pci/ https://lore.kernel.org/linux-pci \
		linux-pci@vger.kernel.org linux-pci@archiver.kernel.org
	public-inbox-index linux-pci


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pci


AGPL code for this site: git clone https://public-inbox.org/ public-inbox