[PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Gary Hade]

From: Gary Hade <garyhade@us.ibm.com>

Contention for scarce PCI memory resources has been growing
due to an increasing number of PCI slots in large multi-node
systems.  The kernel currently attempts by default to
allocate memory for all PCI expansion ROMs so there has
also been an increasing number of PCI memory allocation
failures seen on these systems.  This occurs because the
BIOS either (1) provides insufficient PCI memory resource
for all the expansion ROMs or (2) provides adequate PCI
memory resource for expansion ROMs but provides the
space in kernel unexpected BIOS assigned P2P non-prefetch
windows.

The resulting PCI memory allocation failures may be benign
when related to memory requests for expansion ROMs themselves
but in some cases they can occur when attempting to allocate
space for more critical BARs.  This can happen when a successful
expansion ROM allocation request consumes memory resource
that was intended for a non-ROM BAR.  We have seen this
happen during PCI hotplug of an adapter that contains a
P2P bridge where successful memory allocation for an
expansion ROM BAR on device behind the bridge consumed
memory that was intended for a non-ROM BAR on the P2P bridge.
In all cases the allocation failure messages can be very
confusing for users.

This patch provides a new 'pci=norom' kernel boot parameter
that can be used to disable the default PCI expansion ROM memory
resource allocation.  This provides a way to avoid the above
described issues on systems that do not contain PCI devices
for which drivers or user-level applications depend on the 
default PCI expansion ROM memory resource allocation behavior.

Signed-off-by: Gary Hade <garyhade@us.ibm.com>
---

--- linux-2.6.26-rc2/arch/x86/pci/pci.h.orig	2008-05-12 10:53:04.000000000 -0700
+++ linux-2.6.26-rc2/arch/x86/pci/pci.h	2008-05-12 10:54:39.000000000 -0700
@@ -27,6 +27,7 @@
 #define PCI_CAN_SKIP_ISA_ALIGN	0x8000
 #define PCI_USE__CRS		0x10000
 #define PCI_CHECK_ENABLE_AMD_MMCONF	0x20000
+#define PCI_NOASSIGN_ROMS	0x40000
 
 extern unsigned int pci_probe;
 extern unsigned long pirq_table_addr;
--- linux-2.6.26-rc2/Documentation/kernel-parameters.txt.orig	2008-05-12 10:55:30.000000000 -0700
+++ linux-2.6.26-rc2/Documentation/kernel-parameters.txt	2008-05-12 10:57:49.000000000 -0700
@@ -1493,6 +1493,9 @@ and is between 256 and 4096 characters. 
 				Use with caution as certain devices share
 				address decoders between ROMs and other
 				resources.
+		norom		[X86-32,X86_64] Do not assign address space to
+				expansion ROMs that do not already have
+				BIOS assigned address ranges.
 		irqmask=0xMMMM	[X86-32] Set a bit mask of IRQs allowed to be
 				assigned automatically to PCI devices. You can
 				make the kernel exclude IRQs of your ISA cards
--- linux-2.6.26-rc2/arch/x86/pci/common.c.orig	2008-05-12 10:59:58.000000000 -0700
+++ linux-2.6.26-rc2/arch/x86/pci/common.c	2008-05-12 11:22:05.000000000 -0700
@@ -121,6 +121,21 @@ void __init dmi_check_skip_isa_align(voi
 	dmi_check_system(can_skip_pciprobe_dmi_table);
 }
 
+static void __devinit pcibios_fixup_device_resources(struct pci_dev *dev)
+{
+	struct resource *rom_r = &dev->resource[PCI_ROM_RESOURCE];
+
+	if (pci_probe & PCI_NOASSIGN_ROMS) {
+		if (rom_r->parent)
+			return;
+		if (rom_r->start) {
+			/* we deal with BIOS assigned ROM later */
+			return;
+		}
+		rom_r->start = rom_r->end = rom_r->flags = 0;
+	}
+}
+
 /*
  *  Called after each bus is probed, but before its children
  *  are examined.
@@ -128,7 +143,11 @@ void __init dmi_check_skip_isa_align(voi
 
 void __devinit  pcibios_fixup_bus(struct pci_bus *b)
 {
+	struct pci_dev *dev;
+
 	pci_read_bridge_bases(b);
+	list_for_each_entry(dev, &b->devices, bus_list)
+		pcibios_fixup_device_resources(dev);
 }
 
 /*
@@ -483,6 +502,9 @@ char * __devinit  pcibios_setup(char *st
 	else if (!strcmp(str, "rom")) {
 		pci_probe |= PCI_ASSIGN_ROMS;
 		return NULL;
+	} else if (!strcmp(str, "norom")) {
+		pci_probe |= PCI_NOASSIGN_ROMS;
+		return NULL;
 	} else if (!strcmp(str, "assign-busses")) {
 		pci_probe |= PCI_ASSIGN_ALL_BUSSES;
 		return NULL;

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Yinghai Lu]

On Mon, May 12, 2008 at 1:57 PM, Gary Hade <garyhade@us.ibm.com> wrote:
> From: Gary Hade <garyhade@us.ibm.com>
>
>  Contention for scarce PCI memory resources has been growing
>  due to an increasing number of PCI slots in large multi-node
>  systems.  The kernel currently attempts by default to
>  allocate memory for all PCI expansion ROMs so there has
>  also been an increasing number of PCI memory allocation
>  failures seen on these systems.  This occurs because the
>  BIOS either (1) provides insufficient PCI memory resource
>  for all the expansion ROMs or (2) provides adequate PCI
>  memory resource for expansion ROMs but provides the
>  space in kernel unexpected BIOS assigned P2P non-prefetch
>  windows.
>
>  The resulting PCI memory allocation failures may be benign
>  when related to memory requests for expansion ROMs themselves
>  but in some cases they can occur when attempting to allocate
>  space for more critical BARs.  This can happen when a successful
>  expansion ROM allocation request consumes memory resource
>  that was intended for a non-ROM BAR.  We have seen this
>  happen during PCI hotplug of an adapter that contains a
>  P2P bridge where successful memory allocation for an
>  expansion ROM BAR on device behind the bridge consumed
>  memory that was intended for a non-ROM BAR on the P2P bridge.
>  In all cases the allocation failure messages can be very
>  confusing for users.
>
>  This patch provides a new 'pci=norom' kernel boot parameter
>  that can be used to disable the default PCI expansion ROM memory
>  resource allocation.  This provides a way to avoid the above
>  described issues on systems that do not contain PCI devices
>  for which drivers or user-level applications depend on the
>  default PCI expansion ROM memory resource allocation behavior.
>
>  Signed-off-by: Gary Hade <garyhade@us.ibm.com>
>  ---
>
>  --- linux-2.6.26-rc2/arch/x86/pci/pci.h.orig    2008-05-12 10:53:04.000000000 -0700
>  +++ linux-2.6.26-rc2/arch/x86/pci/pci.h 2008-05-12 10:54:39.000000000 -0700
>  @@ -27,6 +27,7 @@
>   #define PCI_CAN_SKIP_ISA_ALIGN 0x8000
>   #define PCI_USE__CRS           0x10000
>   #define PCI_CHECK_ENABLE_AMD_MMCONF    0x20000
>  +#define PCI_NOASSIGN_ROMS      0x40000
>
>   extern unsigned int pci_probe;
>   extern unsigned long pirq_table_addr;
>  --- linux-2.6.26-rc2/Documentation/kernel-parameters.txt.orig   2008-05-12 10:55:30.000000000 -0700
>  +++ linux-2.6.26-rc2/Documentation/kernel-parameters.txt        2008-05-12 10:57:49.000000000 -0700
>  @@ -1493,6 +1493,9 @@ and is between 256 and 4096 characters.
>                                 Use with caution as certain devices share
>                                 address decoders between ROMs and other
>                                 resources.
>  +               norom           [X86-32,X86_64] Do not assign address space to
>  +                               expansion ROMs that do not already have
>  +                               BIOS assigned address ranges.
>                 irqmask=0xMMMM  [X86-32] Set a bit mask of IRQs allowed to be
>                                 assigned automatically to PCI devices. You can
>                                 make the kernel exclude IRQs of your ISA cards
>  --- linux-2.6.26-rc2/arch/x86/pci/common.c.orig 2008-05-12 10:59:58.000000000 -0700
>  +++ linux-2.6.26-rc2/arch/x86/pci/common.c      2008-05-12 11:22:05.000000000 -0700
>  @@ -121,6 +121,21 @@ void __init dmi_check_skip_isa_align(voi
>         dmi_check_system(can_skip_pciprobe_dmi_table);
>   }
>
>  +static void __devinit pcibios_fixup_device_resources(struct pci_dev *dev)
>  +{
>  +       struct resource *rom_r = &dev->resource[PCI_ROM_RESOURCE];
>  +
>  +       if (pci_probe & PCI_NOASSIGN_ROMS) {
>  +               if (rom_r->parent)
>  +                       return;
>  +               if (rom_r->start) {
>  +                       /* we deal with BIOS assigned ROM later */
>  +                       return;
>  +               }
>  +               rom_r->start = rom_r->end = rom_r->flags = 0;
>  +       }
>  +}
>  +
>   /*
>   *  Called after each bus is probed, but before its children
>   *  are examined.
>  @@ -128,7 +143,11 @@ void __init dmi_check_skip_isa_align(voi
>
>   void __devinit  pcibios_fixup_bus(struct pci_bus *b)
>   {
>  +       struct pci_dev *dev;
>  +
>         pci_read_bridge_bases(b);
>  +       list_for_each_entry(dev, &b->devices, bus_list)
>  +               pcibios_fixup_device_resources(dev);
>   }

or put check
+       if (pci_probe & PCI_NOASSIGN_ROMS) {

out of loop?

YH

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Gary Hade]

On Mon, May 12, 2008 at 02:43:44PM -0700, Yinghai Lu wrote:
> On Mon, May 12, 2008 at 1:57 PM, Gary Hade <garyhade@us.ibm.com> wrote:
	<snip>
> >  --- linux-2.6.26-rc2/arch/x86/pci/common.c.orig 2008-05-12 10:59:58.000000000 -0700
> >  +++ linux-2.6.26-rc2/arch/x86/pci/common.c      2008-05-12 11:22:05.000000000 -0700
> >  @@ -121,6 +121,21 @@ void __init dmi_check_skip_isa_align(voi
> >         dmi_check_system(can_skip_pciprobe_dmi_table);
> >   }
> >
> >  +static void __devinit pcibios_fixup_device_resources(struct pci_dev *dev)
> >  +{
> >  +       struct resource *rom_r = &dev->resource[PCI_ROM_RESOURCE];
> >  +
> >  +       if (pci_probe & PCI_NOASSIGN_ROMS) {
> >  +               if (rom_r->parent)
> >  +                       return;
> >  +               if (rom_r->start) {
> >  +                       /* we deal with BIOS assigned ROM later */
> >  +                       return;
> >  +               }
> >  +               rom_r->start = rom_r->end = rom_r->flags = 0;
> >  +       }
> >  +}
> >  +
> >   /*
> >   *  Called after each bus is probed, but before its children
> >   *  are examined.
> >  @@ -128,7 +143,11 @@ void __init dmi_check_skip_isa_align(voi
> >
> >   void __devinit  pcibios_fixup_bus(struct pci_bus *b)
> >   {
> >  +       struct pci_dev *dev;
> >  +
> >         pci_read_bridge_bases(b);
> >  +       list_for_each_entry(dev, &b->devices, bus_list)
> >  +               pcibios_fixup_device_resources(dev);
> >   }
> 
> or put check
> +       if (pci_probe & PCI_NOASSIGN_ROMS) {
> 
> out of loop?

I could certainly do that but I had intended that the new
pcibios_fixup_device_resources function act as a container where
other kinds of fixups could be added later.  Do you (or others)
think the additional cycles consumed by this approach are an
issue here?

Thanks,
Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Yinghai Lu]

On Mon, May 12, 2008 at 3:30 PM, Gary Hade <garyhade@us.ibm.com> wrote:
> On Mon, May 12, 2008 at 02:43:44PM -0700, Yinghai Lu wrote:
>  > On Mon, May 12, 2008 at 1:57 PM, Gary Hade <garyhade@us.ibm.com> wrote:
>         <snip>
>
> > >  --- linux-2.6.26-rc2/arch/x86/pci/common.c.orig 2008-05-12 10:59:58.000000000 -0700
>  > >  +++ linux-2.6.26-rc2/arch/x86/pci/common.c      2008-05-12 11:22:05.000000000 -0700
>  > >  @@ -121,6 +121,21 @@ void __init dmi_check_skip_isa_align(voi
>  > >         dmi_check_system(can_skip_pciprobe_dmi_table);
>  > >   }
>  > >
>  > >  +static void __devinit pcibios_fixup_device_resources(struct pci_dev *dev)
>  > >  +{
>  > >  +       struct resource *rom_r = &dev->resource[PCI_ROM_RESOURCE];
>  > >  +
>  > >  +       if (pci_probe & PCI_NOASSIGN_ROMS) {
>  > >  +               if (rom_r->parent)
>  > >  +                       return;
>  > >  +               if (rom_r->start) {
>  > >  +                       /* we deal with BIOS assigned ROM later */
>  > >  +                       return;
>  > >  +               }
>  > >  +               rom_r->start = rom_r->end = rom_r->flags = 0;
>  > >  +       }
>  > >  +}
>  > >  +
>  > >   /*
>  > >   *  Called after each bus is probed, but before its children
>  > >   *  are examined.
>  > >  @@ -128,7 +143,11 @@ void __init dmi_check_skip_isa_align(voi
>  > >
>  > >   void __devinit  pcibios_fixup_bus(struct pci_bus *b)
>  > >   {
>  > >  +       struct pci_dev *dev;
>  > >  +
>  > >         pci_read_bridge_bases(b);
>  > >  +       list_for_each_entry(dev, &b->devices, bus_list)
>  > >  +               pcibios_fixup_device_resources(dev);
>  > >   }
>  >
>  > or put check
>  > +       if (pci_probe & PCI_NOASSIGN_ROMS) {
>  >
>  > out of loop?
>
>  I could certainly do that but I had intended that the new
>  pcibios_fixup_device_resources function act as a container where
>  other kinds of fixups could be added later.  Do you (or others)
>  think the additional cycles consumed by this approach are an
>  issue here?

ok, then wonder if we can don't assign roms for x86_64 by default.

can we use pci rom in 64 bit kernel?

YH

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Gary Hade]

On Tue, May 13, 2008 at 10:48:03AM -0700, Yinghai Lu wrote:
> On Mon, May 12, 2008 at 3:30 PM, Gary Hade <garyhade@us.ibm.com> wrote:
> > On Mon, May 12, 2008 at 02:43:44PM -0700, Yinghai Lu wrote:
> >  > On Mon, May 12, 2008 at 1:57 PM, Gary Hade <garyhade@us.ibm.com> wrote:
> >         <snip>
> >
> > > >  --- linux-2.6.26-rc2/arch/x86/pci/common.c.orig 2008-05-12 10:59:58.000000000 -0700
> >  > >  +++ linux-2.6.26-rc2/arch/x86/pci/common.c      2008-05-12 11:22:05.000000000 -0700
> >  > >  @@ -121,6 +121,21 @@ void __init dmi_check_skip_isa_align(voi
> >  > >         dmi_check_system(can_skip_pciprobe_dmi_table);
> >  > >   }
> >  > >
> >  > >  +static void __devinit pcibios_fixup_device_resources(struct pci_dev *dev)
> >  > >  +{
> >  > >  +       struct resource *rom_r = &dev->resource[PCI_ROM_RESOURCE];
> >  > >  +
> >  > >  +       if (pci_probe & PCI_NOASSIGN_ROMS) {
> >  > >  +               if (rom_r->parent)
> >  > >  +                       return;
> >  > >  +               if (rom_r->start) {
> >  > >  +                       /* we deal with BIOS assigned ROM later */
> >  > >  +                       return;
> >  > >  +               }
> >  > >  +               rom_r->start = rom_r->end = rom_r->flags = 0;
> >  > >  +       }
> >  > >  +}
> >  > >  +
> >  > >   /*
> >  > >   *  Called after each bus is probed, but before its children
> >  > >   *  are examined.
> >  > >  @@ -128,7 +143,11 @@ void __init dmi_check_skip_isa_align(voi
> >  > >
> >  > >   void __devinit  pcibios_fixup_bus(struct pci_bus *b)
> >  > >   {
> >  > >  +       struct pci_dev *dev;
> >  > >  +
> >  > >         pci_read_bridge_bases(b);
> >  > >  +       list_for_each_entry(dev, &b->devices, bus_list)
> >  > >  +               pcibios_fixup_device_resources(dev);
> >  > >   }
> >  >
> >  > or put check
> >  > +       if (pci_probe & PCI_NOASSIGN_ROMS) {
> >  >
> >  > out of loop?
> >
> >  I could certainly do that but I had intended that the new
> >  pcibios_fixup_device_resources function act as a container where
> >  other kinds of fixups could be added later.  Do you (or others)
> >  think the additional cycles consumed by this approach are an
> >  issue here?
> 
> ok, then wonder if we can don't assign roms for x86_64 by default.

No, I don't think so.  ..at least not by me! :)

There were some lkml/linux-pci visible discussions back in
November-December of last year where I floated the idea
of making PCI expansion ROM memory non-assignment the default:
  http://marc.info/?l=linux-kernel&m=119742188215024&w=2
  http://marc.info/?l=linux-kernel&m=119689499508369&w=2
I did not hear any objections so I went ahead and submitted
the change which entered mainline at 2.6.25-rc1 but it was
sternly evicted last week because of a reported regression
that it had caused:
  http://marc.info/?l=linux-kernel&m=121029093331908&w=2
Attachments to http://bugs.freedesktop.org/show_bug.cgi?id=15523
indicate that a 2.6.25-rc2 x86_64 kernel was being used.

> 
> can we use pci rom in 64 bit kernel?

Sorry, not sure if I understand this question.  I hope the
above answers it.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Yinghai Lu]

On Tue, May 13, 2008 at 2:00 PM, Gary Hade <garyhade@us.ibm.com> wrote:
>
> On Tue, May 13, 2008 at 10:48:03AM -0700, Yinghai Lu wrote:
>  > On Mon, May 12, 2008 at 3:30 PM, Gary Hade <garyhade@us.ibm.com> wrote:
>  > > On Mon, May 12, 2008 at 02:43:44PM -0700, Yinghai Lu wrote:
>  > >  > On Mon, May 12, 2008 at 1:57 PM, Gary Hade <garyhade@us.ibm.com> wrote:
>  > >         <snip>
>  > >
>  > > > >  --- linux-2.6.26-rc2/arch/x86/pci/common.c.orig 2008-05-12 10:59:58.000000000 -0700
>  > >  > >  +++ linux-2.6.26-rc2/arch/x86/pci/common.c      2008-05-12 11:22:05.000000000 -0700
>  > >  > >  @@ -121,6 +121,21 @@ void __init dmi_check_skip_isa_align(voi
>  > >  > >         dmi_check_system(can_skip_pciprobe_dmi_table);
>  > >  > >   }
>  > >  > >
>  > >  > >  +static void __devinit pcibios_fixup_device_resources(struct pci_dev *dev)
>  > >  > >  +{
>  > >  > >  +       struct resource *rom_r = &dev->resource[PCI_ROM_RESOURCE];
>  > >  > >  +
>  > >  > >  +       if (pci_probe & PCI_NOASSIGN_ROMS) {
>  > >  > >  +               if (rom_r->parent)
>  > >  > >  +                       return;
>  > >  > >  +               if (rom_r->start) {
>  > >  > >  +                       /* we deal with BIOS assigned ROM later */
>  > >  > >  +                       return;
>  > >  > >  +               }
>  > >  > >  +               rom_r->start = rom_r->end = rom_r->flags = 0;
>  > >  > >  +       }
>  > >  > >  +}
>  > >  > >  +
>  > >  > >   /*
>  > >  > >   *  Called after each bus is probed, but before its children
>  > >  > >   *  are examined.
>  > >  > >  @@ -128,7 +143,11 @@ void __init dmi_check_skip_isa_align(voi
>  > >  > >
>  > >  > >   void __devinit  pcibios_fixup_bus(struct pci_bus *b)
>  > >  > >   {
>  > >  > >  +       struct pci_dev *dev;
>  > >  > >  +
>  > >  > >         pci_read_bridge_bases(b);
>  > >  > >  +       list_for_each_entry(dev, &b->devices, bus_list)
>  > >  > >  +               pcibios_fixup_device_resources(dev);
>  > >  > >   }
>  > >  >
>  > >  > or put check
>  > >  > +       if (pci_probe & PCI_NOASSIGN_ROMS) {
>  > >  >
>  > >  > out of loop?
>  > >
>  > >  I could certainly do that but I had intended that the new
>  > >  pcibios_fixup_device_resources function act as a container where
>  > >  other kinds of fixups could be added later.  Do you (or others)
>  > >  think the additional cycles consumed by this approach are an
>  > >  issue here?
>  >
>  > ok, then wonder if we can don't assign roms for x86_64 by default.
>
>  No, I don't think so.  ..at least not by me! :)
>
>  There were some lkml/linux-pci visible discussions back in
>  November-December of last year where I floated the idea
>  of making PCI expansion ROM memory non-assignment the default:
>   http://marc.info/?l=linux-kernel&m=119742188215024&w=2
>   http://marc.info/?l=linux-kernel&m=119689499508369&w=2
>  I did not hear any objections so I went ahead and submitted
>  the change which entered mainline at 2.6.25-rc1 but it was
>  sternly evicted last week because of a reported regression
>  that it had caused:
>   http://marc.info/?l=linux-kernel&m=121029093331908&w=2
>  Attachments to http://bugs.freedesktop.org/show_bug.cgi?id=15523
>  indicate that a 2.6.25-rc2 x86_64 kernel was being used.
>
>
>  >
>  > can we use pci rom in 64 bit kernel?
>
>  Sorry, not sure if I understand this question.  I hope the
>  above answers it.

ok, i think the driver could reload fw in the option rom to reset the
controlller in pci card.
i don't think there is other usage for the option rom after OS loaded,
except option rom contain other run-time code...

so could disable them all, and use pci-quirks to enable that for the
device/driver need it.

YH

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Yinghai Lu]

On Tue, May 13, 2008 at 2:11 PM, Yinghai Lu <yhlu.kernel@gmail.com> wrote:
>
> On Tue, May 13, 2008 at 2:00 PM, Gary Hade <garyhade@us.ibm.com> wrote:
>  >
>  > On Tue, May 13, 2008 at 10:48:03AM -0700, Yinghai Lu wrote:
>  >  > On Mon, May 12, 2008 at 3:30 PM, Gary Hade <garyhade@us.ibm.com> wrote:
>  >  > > On Mon, May 12, 2008 at 02:43:44PM -0700, Yinghai Lu wrote:
>  >  > >  > On Mon, May 12, 2008 at 1:57 PM, Gary Hade <garyhade@us.ibm.com> wrote:
>  >  > >         <snip>
>  >  > >
>  >  > > > >  --- linux-2.6.26-rc2/arch/x86/pci/common.c.orig 2008-05-12 10:59:58.000000000 -0700
>  >  > >  > >  +++ linux-2.6.26-rc2/arch/x86/pci/common.c      2008-05-12 11:22:05.000000000 -0700
>  >  > >  > >  @@ -121,6 +121,21 @@ void __init dmi_check_skip_isa_align(voi
>  >  > >  > >         dmi_check_system(can_skip_pciprobe_dmi_table);
>  >  > >  > >   }
>  >  > >  > >
>  >  > >  > >  +static void __devinit pcibios_fixup_device_resources(struct pci_dev *dev)
>  >  > >  > >  +{
>  >  > >  > >  +       struct resource *rom_r = &dev->resource[PCI_ROM_RESOURCE];
>  >  > >  > >  +
>  >  > >  > >  +       if (pci_probe & PCI_NOASSIGN_ROMS) {
>  >  > >  > >  +               if (rom_r->parent)
>  >  > >  > >  +                       return;
>  >  > >  > >  +               if (rom_r->start) {
>  >  > >  > >  +                       /* we deal with BIOS assigned ROM later */
>  >  > >  > >  +                       return;
>  >  > >  > >  +               }
>  >  > >  > >  +               rom_r->start = rom_r->end = rom_r->flags = 0;
>  >  > >  > >  +       }
>  >  > >  > >  +}
>  >  > >  > >  +
>  >  > >  > >   /*
>  >  > >  > >   *  Called after each bus is probed, but before its children
>  >  > >  > >   *  are examined.
>  >  > >  > >  @@ -128,7 +143,11 @@ void __init dmi_check_skip_isa_align(voi
>  >  > >  > >
>  >  > >  > >   void __devinit  pcibios_fixup_bus(struct pci_bus *b)
>  >  > >  > >   {
>  >  > >  > >  +       struct pci_dev *dev;
>  >  > >  > >  +
>  >  > >  > >         pci_read_bridge_bases(b);
>  >  > >  > >  +       list_for_each_entry(dev, &b->devices, bus_list)
>  >  > >  > >  +               pcibios_fixup_device_resources(dev);
>  >  > >  > >   }
>  >  > >  >
>  >  > >  > or put check
>  >  > >  > +       if (pci_probe & PCI_NOASSIGN_ROMS) {
>  >  > >  >
>  >  > >  > out of loop?
>  >  > >
>  >  > >  I could certainly do that but I had intended that the new
>  >  > >  pcibios_fixup_device_resources function act as a container where
>  >  > >  other kinds of fixups could be added later.  Do you (or others)
>  >  > >  think the additional cycles consumed by this approach are an
>  >  > >  issue here?
>  >  >
>  >  > ok, then wonder if we can don't assign roms for x86_64 by default.
>  >
>  >  No, I don't think so.  ..at least not by me! :)
>  >
>  >  There were some lkml/linux-pci visible discussions back in
>  >  November-December of last year where I floated the idea
>  >  of making PCI expansion ROM memory non-assignment the default:
>  >   http://marc.info/?l=linux-kernel&m=119742188215024&w=2
>  >   http://marc.info/?l=linux-kernel&m=119689499508369&w=2
>  >  I did not hear any objections so I went ahead and submitted
>  >  the change which entered mainline at 2.6.25-rc1 but it was
>  >  sternly evicted last week because of a reported regression
>  >  that it had caused:
>  >   http://marc.info/?l=linux-kernel&m=121029093331908&w=2
>  >  Attachments to http://bugs.freedesktop.org/show_bug.cgi?id=15523
>  >  indicate that a 2.6.25-rc2 x86_64 kernel was being used.
>  >
>  >
>  >  >
>  >  > can we use pci rom in 64 bit kernel?
>  >
>  >  Sorry, not sure if I understand this question.  I hope the
>  >  above answers it.
>
>  ok, i think the driver could reload fw in the option rom to reset the
>  controlller in pci card.
>  i don't think there is other usage for the option rom after OS loaded,
>  except option rom contain other run-time code...
>
>  so could disable them all, and use pci-quirks to enable that for the
>  device/driver need it.

interesting x log
good:
(II) RADEON(3): RADEONScreenInit e8000000 67108864 0
(==) RADEON(3): Write-combining range (0xec000000,0x4000000)
(WW) RADEON(3): Cannot read colourmap from VGA.  Will restore with default
Entering TV Save
Save TV timing tables
saveTimingTables: reading timing tables
TV Save done
disable montype: 1
(II) RADEON(3): RADEONInitMemoryMap() :
(II) RADEON(3):   mem_size         : 0x08000000
(II) RADEON(3):   MC_FB_LOCATION   : 0xefffe800
(II) RADEON(3):   MC_AGP_LOCATION  : 0xffffffc0
(II) RADEON(3): Depth moves disabled by default
(II) RADEON(3): Memory manager initialized to (0,0) (1280,8191)
(II) RADEON(3): Reserved area from (0,1200) to (1280,1202)
(II) RADEON(3): Largest offscreen area available: 1280 x 6989

bad:
(II) RADEON(2): RADEONScreenInit e8000000 0 0
(==) RADEON(2): Write-combining range (0xe8000000,0x400000)
Entering TV Save
Save TV timing tables
saveTimingTables: reading timing tables
TV Save done
disable montype: 3
(II) RADEON(2): RADEONInitMemoryMap() :
(II) RADEON(2):   mem_size         : 0x04000000
(II) RADEON(2):   MC_FB_LOCATION   : 0xebffe800
(II) RADEON(2):   MC_AGP_LOCATION  : 0xffffffc0
(II) RADEON(2): Depth moves disabled by default
(EE) RADEON(2): Memory manager initialization to (0,0) (1280,819) failed

Fatal server error:
AddScreen/ScreenInit failed for driver 2


so what is 67108864? the rom BAR?

YH

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Jesse Barnes]

> >  There were some lkml/linux-pci visible discussions back in
> >  November-December of last year where I floated the idea
> >  of making PCI expansion ROM memory non-assignment the default:
> >   http://marc.info/?l=linux-kernel&m=119742188215024&w=2
> >   http://marc.info/?l=linux-kernel&m=119689499508369&w=2
> >  I did not hear any objections so I went ahead and submitted
> >  the change which entered mainline at 2.6.25-rc1 but it was
> >  sternly evicted last week because of a reported regression
> >  that it had caused:
> >   http://marc.info/?l=linux-kernel&m=121029093331908&w=2
> >  Attachments to http://bugs.freedesktop.org/show_bug.cgi?id=15523
> >  indicate that a 2.6.25-rc2 x86_64 kernel was being used.
> >
> >  > can we use pci rom in 64 bit kernel?
> >
> >  Sorry, not sure if I understand this question.  I hope the
> >  above answers it.
>
> ok, i think the driver could reload fw in the option rom to reset the
> controlller in pci card.
> i don't think there is other usage for the option rom after OS loaded,
> except option rom contain other run-time code...

On graphics devices it's used for more than just POSTing (in fact POSTing is 
the least of its uses these days).  The ROM often contains tables describing 
the specific hardware configuration of a given board, including output 
information, extra supported modes, etc.

On other devices, it can be used to store default settings (this is common for 
SCSI cards iirc, though I'm not sure how much Linux drivers use this info).

> so could disable them all, and use pci-quirks to enable that for the
> device/driver need it.

Since it's probably only big systems that really need the extra address space, 
I'd rather to the opposite:  allocate ROM space by default and let a boot 
option avoid it.

Jesse

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Yinghai Lu]

On Tue, May 13, 2008 at 3:28 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > >  There were some lkml/linux-pci visible discussions back in
>  > >  November-December of last year where I floated the idea
>  > >  of making PCI expansion ROM memory non-assignment the default:
>  > >   http://marc.info/?l=linux-kernel&m=119742188215024&w=2
>  > >   http://marc.info/?l=linux-kernel&m=119689499508369&w=2
>  > >  I did not hear any objections so I went ahead and submitted
>  > >  the change which entered mainline at 2.6.25-rc1 but it was
>  > >  sternly evicted last week because of a reported regression
>  > >  that it had caused:
>  > >   http://marc.info/?l=linux-kernel&m=121029093331908&w=2
>  > >  Attachments to http://bugs.freedesktop.org/show_bug.cgi?id=15523
>  > >  indicate that a 2.6.25-rc2 x86_64 kernel was being used.
>  > >
>  > >  > can we use pci rom in 64 bit kernel?
>  > >
>  > >  Sorry, not sure if I understand this question.  I hope the
>  > >  above answers it.
>  >
>  > ok, i think the driver could reload fw in the option rom to reset the
>  > controlller in pci card.
>  > i don't think there is other usage for the option rom after OS loaded,
>  > except option rom contain other run-time code...
>
>  On graphics devices it's used for more than just POSTing (in fact POSTing is
>  the least of its uses these days).  The ROM often contains tables describing
>  the specific hardware configuration of a given board, including output
>  information, extra supported modes, etc.
>
>  On other devices, it can be used to store default settings (this is common for
>  SCSI cards iirc, though I'm not sure how much Linux drivers use this info).
>
>
>  > so could disable them all, and use pci-quirks to enable that for the
>  > device/driver need it.
>
>  Since it's probably only big systems that really need the extra address space,
>  I'd rather to the opposite:  allocate ROM space by default and let a boot
>  option avoid it.

that boot option will get rid of that ROM BAR for all devices...
big system could use several graphical cards that need ROM BAR and
several cards that don't need ROM BAR.

YH

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Gary Hade]

On Tue, May 13, 2008 at 04:23:55PM -0700, Yinghai Lu wrote:
> On Tue, May 13, 2008 at 3:28 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > >  There were some lkml/linux-pci visible discussions back in
> >  > >  November-December of last year where I floated the idea
> >  > >  of making PCI expansion ROM memory non-assignment the default:
> >  > >   http://marc.info/?l=linux-kernel&m=119742188215024&w=2
> >  > >   http://marc.info/?l=linux-kernel&m=119689499508369&w=2
> >  > >  I did not hear any objections so I went ahead and submitted
> >  > >  the change which entered mainline at 2.6.25-rc1 but it was
> >  > >  sternly evicted last week because of a reported regression
> >  > >  that it had caused:
> >  > >   http://marc.info/?l=linux-kernel&m=121029093331908&w=2
> >  > >  Attachments to http://bugs.freedesktop.org/show_bug.cgi?id=15523
> >  > >  indicate that a 2.6.25-rc2 x86_64 kernel was being used.
> >  > >
> >  > >  > can we use pci rom in 64 bit kernel?
> >  > >
> >  > >  Sorry, not sure if I understand this question.  I hope the
> >  > >  above answers it.
> >  >
> >  > ok, i think the driver could reload fw in the option rom to reset the
> >  > controlller in pci card.
> >  > i don't think there is other usage for the option rom after OS loaded,
> >  > except option rom contain other run-time code...
> >
> >  On graphics devices it's used for more than just POSTing (in fact POSTing is
> >  the least of its uses these days).  The ROM often contains tables describing
> >  the specific hardware configuration of a given board, including output
> >  information, extra supported modes, etc.
> >
> >  On other devices, it can be used to store default settings (this is common for
> >  SCSI cards iirc, though I'm not sure how much Linux drivers use this info).
> >
> >
> >  > so could disable them all, and use pci-quirks to enable that for the
> >  > device/driver need it.
> >
> >  Since it's probably only big systems that really need the extra address space,
> >  I'd rather to the opposite:  allocate ROM space by default and let a boot
> >  option avoid it.
> 
> that boot option will get rid of that ROM BAR for all devices...
> big system could use several graphical cards that need ROM BAR and
> several cards that don't need ROM BAR.

Correct, not a perfect solution but better than we have right now.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Gary Hade]

On Tue, May 13, 2008 at 05:07:02PM -0700, Gary Hade wrote:
> On Tue, May 13, 2008 at 04:23:55PM -0700, Yinghai Lu wrote:
> > On Tue, May 13, 2008 at 3:28 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > > > >  There were some lkml/linux-pci visible discussions back in
> > >  > >  November-December of last year where I floated the idea
> > >  > >  of making PCI expansion ROM memory non-assignment the default:
> > >  > >   http://marc.info/?l=linux-kernel&m=119742188215024&w=2
> > >  > >   http://marc.info/?l=linux-kernel&m=119689499508369&w=2
> > >  > >  I did not hear any objections so I went ahead and submitted
> > >  > >  the change which entered mainline at 2.6.25-rc1 but it was
> > >  > >  sternly evicted last week because of a reported regression
> > >  > >  that it had caused:
> > >  > >   http://marc.info/?l=linux-kernel&m=121029093331908&w=2
> > >  > >  Attachments to http://bugs.freedesktop.org/show_bug.cgi?id=15523
> > >  > >  indicate that a 2.6.25-rc2 x86_64 kernel was being used.
> > >  > >
> > >  > >  > can we use pci rom in 64 bit kernel?
> > >  > >
> > >  > >  Sorry, not sure if I understand this question.  I hope the
> > >  > >  above answers it.
> > >  >
> > >  > ok, i think the driver could reload fw in the option rom to reset the
> > >  > controlller in pci card.
> > >  > i don't think there is other usage for the option rom after OS loaded,
> > >  > except option rom contain other run-time code...
> > >
> > >  On graphics devices it's used for more than just POSTing (in fact POSTing is
> > >  the least of its uses these days).  The ROM often contains tables describing
> > >  the specific hardware configuration of a given board, including output
> > >  information, extra supported modes, etc.
> > >
> > >  On other devices, it can be used to store default settings (this is common for
> > >  SCSI cards iirc, though I'm not sure how much Linux drivers use this info).
> > >
> > >
> > >  > so could disable them all, and use pci-quirks to enable that for the
> > >  > device/driver need it.
> > >
> > >  Since it's probably only big systems that really need the extra address space,
> > >  I'd rather to the opposite:  allocate ROM space by default and let a boot
> > >  option avoid it.
> > 
> > that boot option will get rid of that ROM BAR for all devices...
> > big system could use several graphical cards that need ROM BAR and
> > several cards that don't need ROM BAR.
> 
> Correct, not a perfect solution but better than we have right now.

...but don't let this comment make you think that I am not in
favor of something like you are suggesting.  I am interested
in getting the 'pci=norom' change into mainline quickly but it
is obviously less than ideal for the long term.

Jesse, any idea how many different devices absolutely need
memory allocated for their expansion ROMs?

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: [RFC] which drivers need to map ROM BARs?

[Jesse Barnes]

On Wednesday, May 14, 2008 9:20 am Gary Hade wrote:
> > > that boot option will get rid of that ROM BAR for all devices...
> > > big system could use several graphical cards that need ROM BAR and
> > > several cards that don't need ROM BAR.
> >
> > Correct, not a perfect solution but better than we have right now.
>
> ...but don't let this comment make you think that I am not in
> favor of something like you are suggesting.  I am interested
> in getting the 'pci=norom' change into mainline quickly but it
> is obviously less than ideal for the long term.
>
> Jesse, any idea how many different devices absolutely need
> memory allocated for their expansion ROMs?

Well video devices are one obvious type.  Beyond that you'd have to audit the 
drivers in the tree looking for ones that map the ROM BAR (iirc qla1280 did 
this?).

Jesse

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Jesse Barnes]

On Wednesday, May 14, 2008 9:20 am Gary Hade wrote:
> > Correct, not a perfect solution but better than we have right now.
>
> ...but don't let this comment make you think that I am not in
> favor of something like you are suggesting.  I am interested
> in getting the 'pci=norom' change into mainline quickly but it
> is obviously less than ideal for the long term.
>
> Jesse, any idea how many different devices absolutely need
> memory allocated for their expansion ROMs?

Unless there's a ton of demand, I'd rather go with the norom option, but 
either way, I'd like to push the fix early in the 2.6.27 cycle rather than 
trying to get it into 2.6.26 at the last minute...

So assuming you're ok with your last patch, I'll stuff it into linux-next.

Thanks,
Jesse

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Gary Hade]

On Tue, May 20, 2008 at 10:57:49AM -0700, Jesse Barnes wrote:
> On Wednesday, May 14, 2008 9:20 am Gary Hade wrote:
> > > Correct, not a perfect solution but better than we have right now.
> >
> > ...but don't let this comment make you think that I am not in
> > favor of something like you are suggesting.  I am interested
> > in getting the 'pci=norom' change into mainline quickly but it
> > is obviously less than ideal for the long term.
> >
> > Jesse, any idea how many different devices absolutely need
> > memory allocated for their expansion ROMs?
> 
> Unless there's a ton of demand, I'd rather go with the norom option, but 
> either way, I'd like to push the fix early in the 2.6.27 cycle rather than 
> trying to get it into 2.6.26 at the last minute...

This is fine.  I would also like to see the pci=norom option
added as-is with the thought that we may improve later by either
modifying pci=norom to exclude devices that need memory mapped
to their expansion ROMs or by adding another option (pci=minrom ?)
that would do the same thing.

> 
> So assuming you're ok with your last patch, I'll stuff it into linux-next.

Works for me.  Thanks.

Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Jesse Barnes]

On Tuesday, May 20, 2008 1:00 pm Gary Hade wrote:
> On Tue, May 20, 2008 at 10:57:49AM -0700, Jesse Barnes wrote:
> > On Wednesday, May 14, 2008 9:20 am Gary Hade wrote:
> > > > Correct, not a perfect solution but better than we have right now.
> > >
> > > ...but don't let this comment make you think that I am not in
> > > favor of something like you are suggesting.  I am interested
> > > in getting the 'pci=norom' change into mainline quickly but it
> > > is obviously less than ideal for the long term.
> > >
> > > Jesse, any idea how many different devices absolutely need
> > > memory allocated for their expansion ROMs?
> >
> > Unless there's a ton of demand, I'd rather go with the norom option, but
> > either way, I'd like to push the fix early in the 2.6.27 cycle rather
> > than trying to get it into 2.6.26 at the last minute...
>
> This is fine.  I would also like to see the pci=norom option
> added as-is with the thought that we may improve later by either
> modifying pci=norom to exclude devices that need memory mapped
> to their expansion ROMs or by adding another option (pci=minrom ?)
> that would do the same thing.
>
> > So assuming you're ok with your last patch, I'll stuff it into
> > linux-next.
>
> Works for me.  Thanks.

Ok, just pushed the norom patch to linux-next.  I'll test it out, but it would 
be good if you could try the tree out out on one of the problem machines too.

Thanks,
Jesse

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Gary Hade]

On Tue, May 20, 2008 at 01:16:33PM -0700, Jesse Barnes wrote:
> On Tuesday, May 20, 2008 1:00 pm Gary Hade wrote:
> > On Tue, May 20, 2008 at 10:57:49AM -0700, Jesse Barnes wrote:
> > > On Wednesday, May 14, 2008 9:20 am Gary Hade wrote:
> > > > > Correct, not a perfect solution but better than we have right now.
> > > >
> > > > ...but don't let this comment make you think that I am not in
> > > > favor of something like you are suggesting.  I am interested
> > > > in getting the 'pci=norom' change into mainline quickly but it
> > > > is obviously less than ideal for the long term.
> > > >
> > > > Jesse, any idea how many different devices absolutely need
> > > > memory allocated for their expansion ROMs?
> > >
> > > Unless there's a ton of demand, I'd rather go with the norom option, but
> > > either way, I'd like to push the fix early in the 2.6.27 cycle rather
> > > than trying to get it into 2.6.26 at the last minute...
> >
> > This is fine.  I would also like to see the pci=norom option
> > added as-is with the thought that we may improve later by either
> > modifying pci=norom to exclude devices that need memory mapped
> > to their expansion ROMs or by adding another option (pci=minrom ?)
> > that would do the same thing.
> >
> > > So assuming you're ok with your last patch, I'll stuff it into
> > > linux-next.
> >
> > Works for me.  Thanks.
> 
> Ok, just pushed the norom patch to linux-next.  I'll test it out, but it would 
> be good if you could try the tree out out on one of the problem machines too.

Jesse, I just tried 2.6.26-rc3-next-20080521 on one of
those systems and the pci=norom option worked as expected.

Thanks,
Gary

-- 
Gary Hade
System x Enablement
IBM Linux Technology Center
503-578-4503  IBM T/L: 775-4503
garyhade@us.ibm.com
http://www.ibm.com/linux/ltc

Re: [PATCH] PCI: boot parameter to avoid expansion ROM memory allocation

[Jesse Barnes]

On Wednesday, May 21, 2008 10:29 am Gary Hade wrote:
> > > > So assuming you're ok with your last patch, I'll stuff it into
> > > > linux-next.
> > >
> > > Works for me.  Thanks.
> >
> > Ok, just pushed the norom patch to linux-next.  I'll test it out, but it
> > would be good if you could try the tree out out on one of the problem
> > machines too.
>
> Jesse, I just tried 2.6.26-rc3-next-20080521 on one of
> those systems and the pci=norom option worked as expected.

Great, thanks.  I'll push it to Linus in the next merge window.

Jesse