Thread (19 messages) 19 messages, 8 authors, 2016-11-29

[PATCH] PCI: Add information about describing PCI in ACPI

From: Gabriele Paoloni <hidden>
Date: 2016-11-21 08:53:14
Also in: linux-acpi, linux-pci, lkml

Hi Bjorn
-----Original Message-----
From: Bjorn Helgaas [mailto:helgaas at kernel.org]
Sent: 18 November 2016 17:54
To: Gabriele Paoloni
Cc: Bjorn Helgaas; linux-pci at vger.kernel.org; linux-
acpi at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
kernel at lists.infradead.org; linaro-acpi at lists.linaro.org
Subject: Re: [PATCH] PCI: Add information about describing PCI in ACPI

On Fri, Nov 18, 2016 at 05:17:34PM +0000, Gabriele Paoloni wrote:
quoted
quoted
-----Original Message-----
From: linux-kernel-owner at vger.kernel.org [mailto:linux-kernel-
owner at vger.kernel.org] On Behalf Of Bjorn Helgaas
Sent: 17 November 2016 18:00
quoted
quoted
+Static tables like MCFG, HPET, ECDT, etc., are *not* mechanisms
for
quoted
quoted
+reserving address space!  The static tables are for things the OS
+needs to know early in boot, before it can parse the ACPI
namespace.
quoted
quoted
+If a new table is defined, an old OS needs to operate correctly
even
quoted
quoted
+though it ignores the table.  _CRS allows that because it is
generic
quoted
quoted
+and understood by the old OS; a static table does not.
Right so if my understanding is correct you are saying that resources
described in the MCFG table should also be declared in PNP0C02
devices
quoted
so that the PNP driver can reserve these resources.
Yes.
quoted
On the other side the PCI Root bridge driver should not reserve such
resources.

Well if my understanding is correct I think we have a problem here:
http://lxr.free-electrons.com/source/drivers/pci/ecam.c#L74

As you can see pci_ecam_create() will conflict with the pnp driver
as it will try to reserve the resources from the MCFG table...

Maybe we need to rework pci_ecam_create() ?
I think it's OK as it is.

The pnp/system.c driver does try to reserve PNP0C02 resources, and it
marks them as "not busy".  That way they appear in /proc/iomem and
won't be allocated for anything else, but they can still be requested
by drivers, e.g., pci/ecam.c, which will mark them "busy".

This is analogous to what the PCI core does in pci_claim_resource().
This is really a function of the ACPI/PNP *core*, which should reserve
all _CRS resources for all devices (not just PNP0C02 devices).  But
it's done by pnp/system.c, and only for PNP0C02, because there's a
bunch of historical baggage there.

You'll also notice that in this case, things are out of order:
logically the pnp/system.c reservation should happen first, but in
fact the pci/ecam.c request happens *before* the pnp/system.c one.
That means the pnp/system.c one might fail and complain "[mem ...]
could not be reserved".
Correct me if I am wrong...

So currently we are relying on the fact that pci_ecam_create() is called
before the pnp driver.
If the pnp driver came first we would end up in pci_ecam_create() failing
here:
http://lxr.free-electrons.com/source/drivers/pci/ecam.c#L76

I am not sure but it seems to me like a bit weak condition to rely on...
what about removing the error condition in pci_ecam_create() and logging
just a dev_info()?

Thanks

Gab

Bjorn
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help