This patch reads and stores host bridge resources reported by
ACPI BIOS for x86_64 systems. This is needed since ACPI hotplug
code now uses the PCI core for resource management. This patch
simply adds the boot parameter (acpi=root_resources) to enable
the functionality that is implemented in arch/i386.
Signed-off-by: Rajesh Shah <[email protected]>
Index: linux-2.6.12-rc4-mm2/include/asm-x86_64/acpi.h
===================================================================
--- linux-2.6.12-rc4-mm2.orig/include/asm-x86_64/acpi.h
+++ linux-2.6.12-rc4-mm2/include/asm-x86_64/acpi.h
@@ -143,10 +143,16 @@ static inline void acpi_disable_pci(void
acpi_noirq_set();
}
extern int acpi_irq_balance_set(char *str);
+extern int acpi_read_root_resources;
+static inline void acpi_set_read_root_resources(void)
+{
+ acpi_read_root_resources = 1;
+}
#else
static inline void acpi_noirq_set(void) { }
static inline void acpi_disable_pci(void) { }
static inline int acpi_irq_balance_set(char *str) { return 0; }
+static inline void acpi_set_read_root_resources(void) { }
#endif
#ifdef CONFIG_ACPI_SLEEP
Index: linux-2.6.12-rc4-mm2/arch/x86_64/kernel/setup.c
===================================================================
--- linux-2.6.12-rc4-mm2.orig/arch/x86_64/kernel/setup.c
+++ linux-2.6.12-rc4-mm2/arch/x86_64/kernel/setup.c
@@ -319,6 +319,9 @@ static __init void parse_cmdline_early (
acpi_disable_pci();
else if (!memcmp(from, "acpi=noirq", 10))
acpi_noirq_set();
+ /* Use ACPI to read host bridge resources */
+ else if (!memcmp(from, "acpi=root_resources", 19))
+ acpi_set_read_root_resources();
else if (!memcmp(from, "acpi_sci=edge", 13))
acpi_sci_flags.trigger = 1;
--
On Fri, May 20, 2005 at 05:42:41PM -0700, [email protected] wrote:
> This patch reads and stores host bridge resources reported by
> ACPI BIOS for x86_64 systems. This is needed since ACPI hotplug
> code now uses the PCI core for resource management. This patch
> simply adds the boot parameter (acpi=root_resources) to enable
> the functionality that is implemented in arch/i386.
>
This means all hot plug users have to pass this strange parameter?
That does not sound very user friendly. Especially since you usually
only need pci hotplug in emergencies, and then you likely didnt pass it.
Cant you find a way to do this without parameters? Any reason
to not make it default?
-Andi
On Mon, May 23, 2005 at 06:15:07PM +0200, Andi Kleen wrote:
> On Fri, May 20, 2005 at 05:42:41PM -0700, [email protected] wrote:
> > This patch reads and stores host bridge resources reported by
> > ACPI BIOS for x86_64 systems. This is needed since ACPI hotplug
> > code now uses the PCI core for resource management. This patch
> > simply adds the boot parameter (acpi=root_resources) to enable
> > the functionality that is implemented in arch/i386.
> >
>
> This means all hot plug users have to pass this strange parameter?
> That does not sound very user friendly. Especially since you usually
> only need pci hotplug in emergencies, and then you likely didnt pass it.
>
> Cant you find a way to do this without parameters? Any reason
> to not make it default?
>
I found several systems in which the host bridge was decoding 6+
resource ranges. In the pci_bus structure, I only have room for 4,
so I'm forced to drop some ranges that are in fact being passed
down. For such cases, if I enable this by default, I see boot time
failures for devices that attempted to claim these dropped resources.
These devices were otherwise properly configured by BIOS, and work
fine with today's (incorrect) assumption that all host bridges
decode all unclaimed resources. I didn't want the patch to break
existing systems, hence the boot parameter.
Another option I'd thought of but never really pursued was to
implement this as a late_initcall. I'll look into that some more.
In that case, we'd continue to think that all host bridges decode
all unclaimed resources at boot time and depend on BIOS to program
resources for boot time devices correctly. Later, we'd collect the
more accurate host bridge resource picture to make hotplug work
correctly. Kind of hackish, but I can't think of another way to
avoid the boot parameter.
Rajesh
On Mon, May 23, 2005 at 05:57:08PM -0700, Rajesh Shah wrote:
> On Mon, May 23, 2005 at 06:15:07PM +0200, Andi Kleen wrote:
> > On Fri, May 20, 2005 at 05:42:41PM -0700, [email protected] wrote:
> > > This patch reads and stores host bridge resources reported by
> > > ACPI BIOS for x86_64 systems. This is needed since ACPI hotplug
> > > code now uses the PCI core for resource management. This patch
> > > simply adds the boot parameter (acpi=root_resources) to enable
> > > the functionality that is implemented in arch/i386.
> > >
> >
> > This means all hot plug users have to pass this strange parameter?
> > That does not sound very user friendly. Especially since you usually
> > only need pci hotplug in emergencies, and then you likely didnt pass it.
> >
> > Cant you find a way to do this without parameters? Any reason
> > to not make it default?
> >
> I found several systems in which the host bridge was decoding 6+
> resource ranges. In the pci_bus structure, I only have room for 4,
> so I'm forced to drop some ranges that are in fact being passed
How about you allocate an extended structure with kmalloc in this case?
Or if it is only 6 ranges max (it is not, is it?) you could extend
the array.
I doubt this information will need *that* much memory, so it should
be reasonable to just teach the PCI subsystem about it.
> Another option I'd thought of but never really pursued was to
> implement this as a late_initcall. I'll look into that some more.
> In that case, we'd continue to think that all host bridges decode
> all unclaimed resources at boot time and depend on BIOS to program
> resources for boot time devices correctly. Later, we'd collect the
> more accurate host bridge resource picture to make hotplug work
> correctly. Kind of hackish, but I can't think of another way to
> avoid the boot parameter.
It sounds preferable to me to just give PCI the full picture from
the beginning instead of using such hacks which will likely
come back later to hurt us.
-Andi
On Tue, May 24, 2005 at 02:05:27PM +0200, Andi Kleen wrote:
> How about you allocate an extended structure with kmalloc in this case?
This would lead to quite a few changes in the PCI subsystem.
Looks good as a long-term solution though.
> Or if it is only 6 ranges max (it is not, is it?) you could extend
> the array.
>
> I doubt this information will need *that* much memory, so it should
> be reasonable to just teach the PCI subsystem about it.
Agreed. As a bonus, extending the PCI_BUS_NUM_RESOURCES to 6 would
cleanly resolve problems with "transparent" PCI bridges - the bus
might have 3 "native" + 3 parent bus ranges in that case.
Ivan.
On Tue, May 24, 2005 at 06:58:56PM +0400, Ivan Kokshaysky wrote:
> On Tue, May 24, 2005 at 02:05:27PM +0200, Andi Kleen wrote:
> > How about you allocate an extended structure with kmalloc in this case?
>
> This would lead to quite a few changes in the PCI subsystem.
> Looks good as a long-term solution though.
>
Yes, I did look at that and this would be a big change that would
affect almost all architectures. I was thinking something like
this would be more appropriate as part of the PCI rewrite that
Adam Belay had proposed.
> > Or if it is only 6 ranges max (it is not, is it?) you could extend
> > the array.
> >
No, 6 is not guaranteed but would cover a larger percentage of
systems. 8 would probably cover all but a few special cases.
> > I doubt this information will need *that* much memory, so it should
> > be reasonable to just teach the PCI subsystem about it.
>
> Agreed. As a bonus, extending the PCI_BUS_NUM_RESOURCES to 6 would
> cleanly resolve problems with "transparent" PCI bridges - the bus
> might have 3 "native" + 3 parent bus ranges in that case.
>
The concern here isn't just increasing the size of pci_bus. The
resource pointers in pci_bus point to resource structures in the
corresponding pci_dev structure for p2p bridges. If we want to
maintain this scheme, we'd have to increase the number of resources
in the pci_dev structure too, which increases it for every single
pci device in the system. Probably ok for big server machines, but
would others (e.g. embedded folks) complain?
I just realized that I did not explicitly CC Greg in my original
post. Doing that now, to see what he thinks.
Rajesh
On Tue, May 24, 2005 at 08:45:36AM -0700, Rajesh Shah wrote:
> The concern here isn't just increasing the size of pci_bus. The
> resource pointers in pci_bus point to resource structures in the
> corresponding pci_dev structure for p2p bridges. If we want to
> maintain this scheme, we'd have to increase the number of resources
> in the pci_dev structure too, which increases it for every single
> pci device in the system.
No. The pci_bus resource pointers are just pointers to _some_ resources
and generally aren't tied to particular pci device. For example, the
root pci buses often don't even have corresponding bus->self structure,
and bus resources are pointers to global io[mem,port]_resource.
And definitely we must not touch resource layout in struct pci_dev -
it's defined by pci specs.
> Probably ok for big server machines, but
> would others (e.g. embedded folks) complain?
Increasing the size of pci_bus by 8 or 16 bytes shouldn't be a problem -
I don't think embedded machines have a lot of pci buses. :-)
Anyway, the default PCI_BUS_NUM_RESOURCES value can be overridden in
arch specific headers.
Ivan.
On Tue, May 24, 2005 at 08:58:55PM +0400, Ivan Kokshaysky wrote:
> On Tue, May 24, 2005 at 08:45:36AM -0700, Rajesh Shah wrote:
> > The concern here isn't just increasing the size of pci_bus. The
> > resource pointers in pci_bus point to resource structures in the
> > corresponding pci_dev structure for p2p bridges. If we want to
> > maintain this scheme, we'd have to increase the number of resources
> > in the pci_dev structure too, which increases it for every single
> > pci device in the system.
>
> No. The pci_bus resource pointers are just pointers to _some_ resources
> and generally aren't tied to particular pci device. For example, the
> root pci buses often don't even have corresponding bus->self structure,
> and bus resources are pointers to global io[mem,port]_resource.
For the transparent p2p bridge problem you mentioned, wouldn't you
be dealing with p2p bridges, and therefore expect the pci_bus
resource pointers to point to the corresponding pci_dev resources?
Or are you proposing to decouple pci_bus resource pointers from
pci_dev completely? From quick code inspection, that seems to be
not too much trouble to increase from 4 then.
Rajesh
On Tue, May 24, 2005 at 10:37:25AM -0700, Rajesh Shah wrote:
> For the transparent p2p bridge problem you mentioned, wouldn't you
> be dealing with p2p bridges, and therefore expect the pci_bus
> resource pointers to point to the corresponding pci_dev resources?
The problem is that for subtractive decode bridges we assume
full "transparency" and completely ignore standard p2p bridge
resources (i.e. windows) just using first 3 parent bus pointers
whatever they are.
This model does work in most cases, but there are potential problems
with peer-to-peer DMA behind such bridges, poor performance for MMIO
ranges outside bridge windows, prefetchable vs. non-prefetchable issues
and so on.
If we had 6 or more resource pointers in struct pci_bus, then the
appended patch would fix that.
> Or are you proposing to decouple pci_bus resource pointers from
> pci_dev completely?
Actually no. Low-level bridge drivers (p2p, cardbus or particular
host bridge) certainly know about resource layout of the respective
device. But generic resource management code doesn't make any
assumptions about that and looks only at resource types.
> From quick code inspection, that seems to be
> not too much trouble to increase from 4 then.
No trouble at all, I guess.
Ivan.
--- linux/drivers/pci/probe.c.orig Sat May 7 09:20:31 2005
+++ linux/drivers/pci/probe.c Wed May 25 18:31:34 2005
@@ -239,9 +239,8 @@ void __devinit pci_read_bridge_bases(str
if (dev->transparent) {
printk(KERN_INFO "PCI: Transparent bridge - %s\n", pci_name(dev));
- for(i = 0; i < PCI_BUS_NUM_RESOURCES; i++)
- child->resource[i] = child->parent->resource[i];
- return;
+ for(i = 3; i < PCI_BUS_NUM_RESOURCES; i++)
+ child->resource[i] = child->parent->resource[i - 3];
}
for(i=0; i<3; i++)