The first two are intended as a bugfix for a very old regression:
https://bugzilla.kernel.org/show_bug.cgi?id=68591 "pci=nocrs required to
boot Asrock M3A UCC". Without "pci=nocrs", the system hangs during boot
when the PCI core moves the HPET. Unfortunately I do not have testing
results from the reporter (ping, Bodo :)).
The third and fourth are minor cleanups, no functional change intended
except for the format of some AGP printks.
I intend these for v3.16.
---
Bjorn Helgaas (4):
x86/PCI: Don't try to move IORESOURCE_PCI_FIXED resources
x86/PCI: Mark HPET BAR as IORESOURCE_PCI_FIXED
x86/PCI: Move pcibios_assign_resources() annotation to definition
x86/gart: Tidy messages and add bridge device info
arch/x86/kernel/aperture_64.c | 20 ++++++++++++--------
arch/x86/pci/fixup.c | 12 ++++++++++++
arch/x86/pci/i386.c | 27 ++++++++++++++++-----------
3 files changed, 40 insertions(+), 19 deletions(-)
Move the pcibios_assign_resources() fs_initcall annotation next to the
function definition. No functional change.
Signed-off-by: Bjorn Helgaas <[email protected]>
---
arch/x86/pci/i386.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index 6db58d68feb5..a19ed92e74e4 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -361,6 +361,12 @@ static int __init pcibios_assign_resources(void)
return 0;
}
+/**
+ * called in fs_initcall (one below subsys_initcall),
+ * give a chance for motherboard reserve resources
+ */
+fs_initcall(pcibios_assign_resources);
+
void pcibios_resource_survey_bus(struct pci_bus *bus)
{
dev_printk(KERN_DEBUG, &bus->dev, "Allocating resources\n");
@@ -397,12 +403,6 @@ void __init pcibios_resource_survey(void)
ioapic_insert_resources();
}
-/**
- * called in fs_initcall (one below subsys_initcall),
- * give a chance for motherboard reserve resources
- */
-fs_initcall(pcibios_assign_resources);
-
static const struct vm_operations_struct pci_mmap_ops = {
.access = generic_access_phys,
};
Print the AGP bridge info the same way as the rest of the kernel, e.g.,
"0000:00:04.0" instead of "00:04:00".
Also print the AGP aperture address range the same way we print resources,
and label it explicitly as a bus address range.
No functional change.
Signed-off-by: Bjorn Helgaas <[email protected]>
---
arch/x86/kernel/aperture_64.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
index 9fa8aa051f54..4dd76d3df056 100644
--- a/arch/x86/kernel/aperture_64.c
+++ b/arch/x86/kernel/aperture_64.c
@@ -126,10 +126,12 @@ static u32 __init read_agp(int bus, int slot, int func, int cap, u32 *order)
u64 aper;
u32 old_order;
- printk(KERN_INFO "AGP bridge at %02x:%02x:%02x\n", bus, slot, func);
+ printk(KERN_INFO "pci 0000:%02x:%02x.%d: AGP bridge\n", bus, slot,
+ func);
apsizereg = read_pci_config_16(bus, slot, func, cap + 0x14);
if (apsizereg == 0xffffffff) {
- printk(KERN_ERR "APSIZE in AGP bridge unreadable\n");
+ printk(KERN_ERR "pci 0000:%02x:%02x.%d: APSIZE unreadable\n",
+ bus, slot, func);
return 0;
}
@@ -153,16 +155,18 @@ static u32 __init read_agp(int bus, int slot, int func, int cap, u32 *order)
* On some sick chips, APSIZE is 0. It means it wants 4G
* so let double check that order, and lets trust AMD NB settings:
*/
- printk(KERN_INFO "Aperture from AGP @ %Lx old size %u MB\n",
- aper, 32 << old_order);
+ printk(KERN_INFO "pci 0000:%02x:%02x.%d: AGP aperture [bus %Lx-%Lx] old size %u MB\n",
+ bus, slot, func, aper, aper + (32 << old_order) - 1,
+ 32 << old_order);
if (aper + (32ULL<<(20 + *order)) > 0x100000000ULL) {
- printk(KERN_INFO "Aperture size %u MB (APSIZE %x) is not right, using settings from NB\n",
- 32 << *order, apsizereg);
+ printk(KERN_INFO "pci 0000:%02x:%02x.%d: AGP aperture size %u MB (APSIZE %x) is not right, using settings from NB\n",
+ bus, slot, func, 32 << *order, apsizereg);
*order = old_order;
}
- printk(KERN_INFO "Aperture from AGP @ %Lx size %u MB (APSIZE %x)\n",
- aper, 32 << *order, apsizereg);
+ printk(KERN_INFO "pci 0000:%02x:%02x.%d: AGP aperture [bus %Lx-%Lx] size %u MB (APSIZE %x)\n",
+ bus, slot, func, aper, aper + (32 << *order) - 1, 32 << *order,
+ apsizereg);
if (!aperture_valid(aper, (32*1024*1024) << *order, 32<<20))
return 0;
Bodo reported that on the Asrock M3A UCC, v3.12.6 hangs during boot unless
he uses "pci=nocrs". This regression was caused by 7bc5e3f2be32 ("x86/PCI:
use host bridge _CRS info by default on 2008 and newer machines"), which
appeared in v2.6.34.
The reason is that the HPET address appears in a PCI device BAR, and this
address is not contained in any of the host bridge windows. Linux moves
the PCI BAR into a window, but the original address was published via the
HPET table and an ACPI device, so changing the BAR is a bad idea:
ACPI: HPET id: 0x43538301 base: 0xfed00000
pci_root PNP0A03:00: host bridge window [mem 0xd0000000-0xdfffffff]
pci_root PNP0A03:00: host bridge window [mem 0xf0000000-0xfebfffff]
pci 0000:00:14.0: [1002:4385] type 0 class 0x000c05
pci 0000:00:14.0: reg 14: [mem 0xfed00000-0xfed003ff]
hpet0: at MMIO 0xfed00000, IRQs 2, 8, 0, 0
pnp 00:06: Plug and Play ACPI device, IDs PNP0103 (active)
pnp 00:06: [mem 0xfed00000-0xfed003ff]
This patch marks the BAR as IORESOURCE_PCI_FIXED to prevent Linux from
moving it. This depends on a previous patch ("x86/PCI: Don't try to move
IORESOURCE_PCI_FIXED resources") to check for this flag when
pci_claim_resource() fails.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=68591
Reported-by: Bodo Eggert <[email protected]>
Signed-off-by: Bjorn Helgaas <[email protected]>
---
arch/x86/pci/fixup.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index 94ae9ae9574f..d4359f9d74b6 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -6,6 +6,7 @@
#include <linux/dmi.h>
#include <linux/pci.h>
#include <linux/vgaarb.h>
+#include <asm/hpet.h>
#include <asm/pci_x86.h>
static void pci_fixup_i450nx(struct pci_dev *d)
@@ -526,6 +527,17 @@ static void sb600_disable_hpet_bar(struct pci_dev *dev)
}
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x4385, sb600_disable_hpet_bar);
+static void sb600_hpet_quirk(struct pci_dev *dev)
+{
+ struct resource *r = &dev->resource[0];
+
+ if (r->flags & IORESOURCE_MEM && r->start == hpet_address) {
+ r->flags |= IORESOURCE_PCI_FIXED;
+ dev_info(&dev->dev, "reg 0x10 contains HPET; making it immovable\n");
+ }
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_ATI, 0x4385, sb600_hpet_quirk);
+
/*
* Twinhead H12Y needs us to block out a region otherwise we map devices
* there and any access kills the box.
Don't attempt to move resource marked IORESOURCE_PCI_FIXED, even if
pci_claim_resource() fails. In some cases, these are legacy resources that
cannot be moved.
Signed-off-by: Bjorn Helgaas <[email protected]>
---
arch/x86/pci/i386.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index db6b1ab43255..6db58d68feb5 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -271,11 +271,16 @@ static void pcibios_allocate_dev_resources(struct pci_dev *dev, int pass)
"BAR %d: reserving %pr (d=%d, p=%d)\n",
idx, r, disabled, pass);
if (pci_claim_resource(dev, idx) < 0) {
- /* We'll assign a new address later */
- pcibios_save_fw_addr(dev,
- idx, r->start);
- r->end -= r->start;
- r->start = 0;
+ if (r->flags & IORESOURCE_PCI_FIXED) {
+ dev_info(&dev->dev, "BAR %d %pR is immovable\n",
+ idx, r);
+ } else {
+ /* We'll assign a new address later */
+ pcibios_save_fw_addr(dev,
+ idx, r->start);
+ r->end -= r->start;
+ r->start = 0;
+ }
}
}
}
* Bjorn Helgaas <[email protected]> wrote:
> Print the AGP bridge info the same way as the rest of the kernel, e.g.,
> "0000:00:04.0" instead of "00:04:00".
>
> Also print the AGP aperture address range the same way we print resources,
> and label it explicitly as a bus address range.
>
> No functional change.
>
> Signed-off-by: Bjorn Helgaas <[email protected]>
> ---
> arch/x86/kernel/aperture_64.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
> index 9fa8aa051f54..4dd76d3df056 100644
> --- a/arch/x86/kernel/aperture_64.c
> +++ b/arch/x86/kernel/aperture_64.c
> @@ -126,10 +126,12 @@ static u32 __init read_agp(int bus, int slot, int func, int cap, u32 *order)
> u64 aper;
> u32 old_order;
>
> - printk(KERN_INFO "AGP bridge at %02x:%02x:%02x\n", bus, slot, func);
> + printk(KERN_INFO "pci 0000:%02x:%02x.%d: AGP bridge\n", bus, slot,
> + func);
Please leave it on a single line - checkpatch warning nonwithstanding.
pr_info() is also useful.
> apsizereg = read_pci_config_16(bus, slot, func, cap + 0x14);
> if (apsizereg == 0xffffffff) {
> - printk(KERN_ERR "APSIZE in AGP bridge unreadable\n");
> + printk(KERN_ERR "pci 0000:%02x:%02x.%d: APSIZE unreadable\n",
> + bus, slot, func);
Here too you can also use pr_err() to shorten the line.
> return 0;
> }
>
> @@ -153,16 +155,18 @@ static u32 __init read_agp(int bus, int slot, int func, int cap, u32 *order)
> * On some sick chips, APSIZE is 0. It means it wants 4G
> * so let double check that order, and lets trust AMD NB settings:
> */
> - printk(KERN_INFO "Aperture from AGP @ %Lx old size %u MB\n",
> - aper, 32 << old_order);
> + printk(KERN_INFO "pci 0000:%02x:%02x.%d: AGP aperture [bus %Lx-%Lx] old size %u MB\n",
> + bus, slot, func, aper, aper + (32 << old_order) - 1,
> + 32 << old_order);
> if (aper + (32ULL<<(20 + *order)) > 0x100000000ULL) {
> - printk(KERN_INFO "Aperture size %u MB (APSIZE %x) is not right, using settings from NB\n",
> - 32 << *order, apsizereg);
> + printk(KERN_INFO "pci 0000:%02x:%02x.%d: AGP aperture size %u MB (APSIZE %x) is not right, using settings from NB\n",
> + bus, slot, func, 32 << *order, apsizereg);
> *order = old_order;
> }
>
> - printk(KERN_INFO "Aperture from AGP @ %Lx size %u MB (APSIZE %x)\n",
> - aper, 32 << *order, apsizereg);
> + printk(KERN_INFO "pci 0000:%02x:%02x.%d: AGP aperture [bus %Lx-%Lx] size %u MB (APSIZE %x)\n",
> + bus, slot, func, aper, aper + (32 << *order) - 1, 32 << *order,
> + apsizereg);
>
> if (!aperture_valid(aper, (32*1024*1024) << *order, 32<<20))
> return 0;
Also, you could add:
#define PREFIX "PCI/gart: "
or something like that, to define a standard prefix for these
messages.
Btw., this file could be moved to arch/x86/pci/?
Otherwise:
Acked-by: Ingo Molnar <[email protected]>
Thanks,
Ingo
On Sat, Apr 26, 2014 at 12:01 AM, Ingo Molnar <[email protected]> wrote:
>
> * Bjorn Helgaas <[email protected]> wrote:
>
>> Print the AGP bridge info the same way as the rest of the kernel, e.g.,
>> "0000:00:04.0" instead of "00:04:00".
>>
>> Also print the AGP aperture address range the same way we print resources,
>> and label it explicitly as a bus address range.
>>
>> No functional change.
>>
>> Signed-off-by: Bjorn Helgaas <[email protected]>
>> ---
>> arch/x86/kernel/aperture_64.c | 20 ++++++++++++--------
>> 1 file changed, 12 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/x86/kernel/aperture_64.c b/arch/x86/kernel/aperture_64.c
>> index 9fa8aa051f54..4dd76d3df056 100644
>> --- a/arch/x86/kernel/aperture_64.c
>> +++ b/arch/x86/kernel/aperture_64.c
>> @@ -126,10 +126,12 @@ static u32 __init read_agp(int bus, int slot, int func, int cap, u32 *order)
>> u64 aper;
>> u32 old_order;
>>
>> - printk(KERN_INFO "AGP bridge at %02x:%02x:%02x\n", bus, slot, func);
>> + printk(KERN_INFO "pci 0000:%02x:%02x.%d: AGP bridge\n", bus, slot,
>> + func);
>
> Please leave it on a single line - checkpatch warning nonwithstanding.
> pr_info() is also useful.
> ...
> Also, you could add:
>
> #define PREFIX "PCI/gart: "
>
> or something like that, to define a standard prefix for these
> messages.
I converted to pr_info(), etc. and added pr_fmt for an "AGP: " prefix.
> Btw., this file could be moved to arch/x86/pci/?
Yes, I'd be glad to do this. It looks like all the following files
might be candidates:
amd_gart_64.c (CONFIG_GART_IOMMU)
aperture_64.c (CONFIG_GART_IOMMU)
pci-calgary_64.c (CONFIG_CALGARY_IOMMU)
tce_64.c (CONFIG_CALGARY_IOMMU)
mmconf-fam10h_64.c (CONFIG_PCI_MMCONFIG)
What do you think? Move all, move none, move only aperture_64.c?
Thanks for your comments!
Bjorn