Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Andi Kleen <[EMAIL PROTECTED]> writes: > On Thu, Dec 13, 2007 at 09:39:22AM -0500, Neil Horman wrote: >> >> Ok, new patch attached, taking into account Andi's request for a cleaner > method > > Sorry for not noticing that earlier, but was there a specific reason this > needs > to be an early quirk at all? kexec can only happen after the standard quirks > ran. > I think it should be fine as a standard "late" quirk. Just to document things. The important thing is this quirk happens before calibrate_delay(). Which is still before the normal pci subsystem gets initialized. So that seems to require an early_quirk as the pci subsystem is not initialized by that point. The only case we are likely to hit this is kdump because BIOS almost always boot us on the cpu with apic id == 0. However in the case of this bug if we happen to boot on a cpu with apic id >= 16 we won't be able to boot the linux kernel either, because calibrate_delay will fail. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Mon, Dec 17, 2007 at 04:16:42PM +0100, Ingo Molnar wrote: > > * Neil Horman <[EMAIL PROTECTED]> wrote: > > > Ok, new patch attached, taking into account Andi's request for a > > cleaner method to implement single application quirks. I've spoken > > with Ben, who is continuing to retest, and reports that clean > > methodical testing results in success with this patch. > > thanks, i've added your patch to x86.git#mm - queued up for v2.6.25 > merging. But v2.6.24 merging would be too risky i think: it's not a > regression fix and for a regular fix it's a bit late in the cycle for a > bugfix that changes so many aspects of the early quirk code. > > Ingo Copy that, thanks Ingo! Neil -- /*** *Neil Horman *Software Engineer *Red Hat, Inc. [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
* Neil Horman <[EMAIL PROTECTED]> wrote: > Ok, new patch attached, taking into account Andi's request for a > cleaner method to implement single application quirks. I've spoken > with Ben, who is continuing to retest, and reports that clean > methodical testing results in success with this patch. thanks, i've added your patch to x86.git#mm - queued up for v2.6.25 merging. But v2.6.24 merging would be too risky i think: it's not a regression fix and for a regular fix it's a bit late in the cycle for a bugfix that changes so many aspects of the early quirk code. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 13, 2007 at 10:32:22AM -0500, Neil Horman wrote: > On Thu, Dec 13, 2007 at 04:16:29PM +0100, Andi Kleen wrote: > > On Thu, Dec 13, 2007 at 09:39:22AM -0500, Neil Horman wrote: > > > > > > Ok, new patch attached, taking into account Andi's request for a cleaner > > > method > > > > Sorry for not noticing that earlier, but was there a specific reason this > > needs > > to be an early quirk at all? kexec can only happen after the standard > > quirks ran. > > I think it should be fine as a standard "late" quirk. > > > > -Andi > > > Early quirk seemed like the right thing to do to me. Starting from boot up, > this (mis)configuration by the bios can mean that come cpus just don't get > interrupts. I could imagine situations like serial console not working if the > serial port interrupt was routed to a cpu that used extended APIC id. I've > never actually observed it happening, but making sure that all cpus were > eligible to get interrupts early in the boot process made sense to me. > > Neil > Sorry to push on this, but do we have a consensus on this fix? Andi, do you still feel this needs to be a late quirk given my previous arguments? Regards Neil -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 13, 2007 at 10:32:22AM -0500, Neil Horman wrote: On Thu, Dec 13, 2007 at 04:16:29PM +0100, Andi Kleen wrote: On Thu, Dec 13, 2007 at 09:39:22AM -0500, Neil Horman wrote: Ok, new patch attached, taking into account Andi's request for a cleaner method Sorry for not noticing that earlier, but was there a specific reason this needs to be an early quirk at all? kexec can only happen after the standard quirks ran. I think it should be fine as a standard late quirk. -Andi Early quirk seemed like the right thing to do to me. Starting from boot up, this (mis)configuration by the bios can mean that come cpus just don't get interrupts. I could imagine situations like serial console not working if the serial port interrupt was routed to a cpu that used extended APIC id. I've never actually observed it happening, but making sure that all cpus were eligible to get interrupts early in the boot process made sense to me. Neil Sorry to push on this, but do we have a consensus on this fix? Andi, do you still feel this needs to be a late quirk given my previous arguments? Regards Neil -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
* Neil Horman [EMAIL PROTECTED] wrote: Ok, new patch attached, taking into account Andi's request for a cleaner method to implement single application quirks. I've spoken with Ben, who is continuing to retest, and reports that clean methodical testing results in success with this patch. thanks, i've added your patch to x86.git#mm - queued up for v2.6.25 merging. But v2.6.24 merging would be too risky i think: it's not a regression fix and for a regular fix it's a bit late in the cycle for a bugfix that changes so many aspects of the early quirk code. Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Mon, Dec 17, 2007 at 04:16:42PM +0100, Ingo Molnar wrote: * Neil Horman [EMAIL PROTECTED] wrote: Ok, new patch attached, taking into account Andi's request for a cleaner method to implement single application quirks. I've spoken with Ben, who is continuing to retest, and reports that clean methodical testing results in success with this patch. thanks, i've added your patch to x86.git#mm - queued up for v2.6.25 merging. But v2.6.24 merging would be too risky i think: it's not a regression fix and for a regular fix it's a bit late in the cycle for a bugfix that changes so many aspects of the early quirk code. Ingo Copy that, thanks Ingo! Neil -- /*** *Neil Horman *Software Engineer *Red Hat, Inc. [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Andi Kleen [EMAIL PROTECTED] writes: On Thu, Dec 13, 2007 at 09:39:22AM -0500, Neil Horman wrote: Ok, new patch attached, taking into account Andi's request for a cleaner method Sorry for not noticing that earlier, but was there a specific reason this needs to be an early quirk at all? kexec can only happen after the standard quirks ran. I think it should be fine as a standard late quirk. Just to document things. The important thing is this quirk happens before calibrate_delay(). Which is still before the normal pci subsystem gets initialized. So that seems to require an early_quirk as the pci subsystem is not initialized by that point. The only case we are likely to hit this is kdump because BIOS almost always boot us on the cpu with apic id == 0. However in the case of this bug if we happen to boot on a cpu with apic id = 16 we won't be able to boot the linux kernel either, because calibrate_delay will fail. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 13, 2007 at 04:16:29PM +0100, Andi Kleen wrote: > On Thu, Dec 13, 2007 at 09:39:22AM -0500, Neil Horman wrote: > > > > Ok, new patch attached, taking into account Andi's request for a cleaner > > method > > Sorry for not noticing that earlier, but was there a specific reason this > needs > to be an early quirk at all? kexec can only happen after the standard quirks > ran. > I think it should be fine as a standard "late" quirk. > > -Andi > Early quirk seemed like the right thing to do to me. Starting from boot up, this (mis)configuration by the bios can mean that come cpus just don't get interrupts. I could imagine situations like serial console not working if the serial port interrupt was routed to a cpu that used extended APIC id. I've never actually observed it happening, but making sure that all cpus were eligible to get interrupts early in the boot process made sense to me. Neil > ___ > kexec mailing list > [EMAIL PROTECTED] > http://lists.infradead.org/mailman/listinfo/kexec -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 13, 2007 at 09:39:22AM -0500, Neil Horman wrote: > > Ok, new patch attached, taking into account Andi's request for a cleaner > method Sorry for not noticing that earlier, but was there a specific reason this needs to be an early quirk at all? kexec can only happen after the standard quirks ran. I think it should be fine as a standard "late" quirk. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Ok, new patch attached, taking into account Andi's request for a cleaner method to implement single application quirks. I've spoken with Ben, who is continuing to retest, and reports that clean methodical testing results in success with this patch. Summary: Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 17 of the hypertransport transaction config register, which indicated that the mask for the destination field of interrupt packets accross the ht bus (see section 3.3.9 of http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it will not recieve interrupts during the kdump kernel boot, and this hang will be the result. The fix is to add this patch, whcih add an early pci quirk check, to forcibly enable this bit in the httcfg register. This enables all cpus on a system to receive interrupts, and allows kdump kernel bootup to procede normally. Regards Neil Signed-off-by: Neil Horman <[EMAIL PROTECTED]> early-quirks.c | 86 +++-- 1 file changed, 65 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..f4ed3d1 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,31 @@ #include #endif -static void __init via_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg & (1 << 18)) { + printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n"); + if ((htcfg & (1 << 17)) == 0) { + printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n"); + printk(KERN_INFO "Note this is a bios bug, please contact your hw vendor\n"); + htcfg |= (1 << 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init via_bugs(int num, int slot, int func) +{ #ifdef CONFIG_GART_IOMMU if ((end_pfn > MAX_DMA32_PFN || force_iommu) && !gart_iommu_aperture_allowed) { @@ -44,8 +67,8 @@ #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +95,8 @@ } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -83,15 +106,27 @@ #endif } +#define QFLAG_APPLY_ONCE 0x1 +#define QFLAG_APPLIED 0x2 +#define QFLAG_DONE (QFLAG_APPLY_ONCE|QFLAG_APPLIED) struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + u32 flags; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, + PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, + PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, + PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, + PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, 0, fix_hypertransport_config }, {} }; @@ -106,27 +141,36 @@ for (num = 0; num < 32; num++) { for (slot = 0; slot < 32; slot++) { for (func = 0; func < 8; func++) { - u32 class; - u32 vendor; + u16 class; + u16 vendor; + u16 device; u8 type; int i; - class =
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Ok, new patch attached, taking into account Andi's request for a cleaner method to implement single application quirks. I've spoken with Ben, who is continuing to retest, and reports that clean methodical testing results in success with this patch. Summary: Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 17 of the hypertransport transaction config register, which indicated that the mask for the destination field of interrupt packets accross the ht bus (see section 3.3.9 of http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it will not recieve interrupts during the kdump kernel boot, and this hang will be the result. The fix is to add this patch, whcih add an early pci quirk check, to forcibly enable this bit in the httcfg register. This enables all cpus on a system to receive interrupts, and allows kdump kernel bootup to procede normally. Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 86 +++-- 1 file changed, 65 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..f4ed3d1 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,31 @@ #include asm/gart.h #endif -static void __init via_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init via_bugs(int num, int slot, int func) +{ #ifdef CONFIG_GART_IOMMU if ((end_pfn MAX_DMA32_PFN || force_iommu) !gart_iommu_aperture_allowed) { @@ -44,8 +67,8 @@ #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +95,8 @@ } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -83,15 +106,27 @@ #endif } +#define QFLAG_APPLY_ONCE 0x1 +#define QFLAG_APPLIED 0x2 +#define QFLAG_DONE (QFLAG_APPLY_ONCE|QFLAG_APPLIED) struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + u32 flags; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, + PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, + PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, + PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, + PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, 0, fix_hypertransport_config }, {} }; @@ -106,27 +141,36 @@ for (num = 0; num 32; num++) { for (slot = 0; slot 32; slot++) { for (func = 0; func 8; func++) { - u32 class; - u32 vendor; + u16 class; + u16 vendor; + u16 device; u8 type; int i; - class =
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 13, 2007 at 09:39:22AM -0500, Neil Horman wrote: Ok, new patch attached, taking into account Andi's request for a cleaner method Sorry for not noticing that earlier, but was there a specific reason this needs to be an early quirk at all? kexec can only happen after the standard quirks ran. I think it should be fine as a standard late quirk. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 13, 2007 at 04:16:29PM +0100, Andi Kleen wrote: On Thu, Dec 13, 2007 at 09:39:22AM -0500, Neil Horman wrote: Ok, new patch attached, taking into account Andi's request for a cleaner method Sorry for not noticing that earlier, but was there a specific reason this needs to be an early quirk at all? kexec can only happen after the standard quirks ran. I think it should be fine as a standard late quirk. -Andi Early quirk seemed like the right thing to do to me. Starting from boot up, this (mis)configuration by the bios can mean that come cpus just don't get interrupts. I could imagine situations like serial console not working if the serial port interrupt was routed to a cpu that used extended APIC id. I've never actually observed it happening, but making sure that all cpus were eligible to get interrupts early in the boot process made sense to me. Neil ___ kexec mailing list [EMAIL PROTECTED] http://lists.infradead.org/mailman/listinfo/kexec -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman <[EMAIL PROTECTED]> writes: > I think this just leaves us with deciding on a mechanism for how to do > single-application quirks. I take Andi's point that adding a flag set to the > quirk data structure is a fine solution, but I'm really ok with static > integers > in individual functions. Do we have consensus on how to handle that? I'm > happy > either way, but I'd rather have agreement on how to handle it before I post > another iteration of this patch. As long as the solution is simple, small and concise I don't care. And since what will make Andi happy seems to meet those criteria, that should be fine. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Wed, Dec 12, 2007 at 12:43:34PM -0700, Eric W. Biederman wrote: > Andi Kleen <[EMAIL PROTECTED]> writes: > > > > It could enable the extended APIC IDs but not use them? > > In which case complaining is still correct (the BIOS was out of sync), > enabling bit 17 is still correct and we are just in overkill mode. > > > Anyways I haven't got docs on that NV bridge so I might be wrong. > > This has everything to do with how AMD coherent hypertransport works and > little if anything to do with how the NV bridge operated. > > Basically the NV bridge seems to be sending a standard hypertransport > x86 legacy interrupt packet (that doesn't have any target information) > and when that packet hits the coherent hypertransport domain it isn't > being converted into whatever would send it to all cpus. > > . > > The real practical problem is if somehow the BIOS goofs up this way > and it then decides to ask us to boot on one of these cpus with > an extended apic id. We will hang in calibrate_delay. So far > this only seems to happen in the kdump case but in theory the BIOS > could be completely crazy. > I think this just leaves us with deciding on a mechanism for how to do single-application quirks. I take Andi's point that adding a flag set to the quirk data structure is a fine solution, but I'm really ok with static integers in individual functions. Do we have consensus on how to handle that? I'm happy either way, but I'd rather have agreement on how to handle it before I post another iteration of this patch. Thanks & Regards Neil > Eric -- /*** *Neil Horman [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Andi Kleen <[EMAIL PROTECTED]> writes: > > It could enable the extended APIC IDs but not use them? In which case complaining is still correct (the BIOS was out of sync), enabling bit 17 is still correct and we are just in overkill mode. > Anyways I haven't got docs on that NV bridge so I might be wrong. This has everything to do with how AMD coherent hypertransport works and little if anything to do with how the NV bridge operated. Basically the NV bridge seems to be sending a standard hypertransport x86 legacy interrupt packet (that doesn't have any target information) and when that packet hits the coherent hypertransport domain it isn't being converted into whatever would send it to all cpus. . The real practical problem is if somehow the BIOS goofs up this way and it then decides to ask us to boot on one of these cpus with an extended apic id. We will hang in calibrate_delay. So far this only seems to happen in the kdump case but in theory the BIOS could be completely crazy. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Wed, Dec 12, 2007 at 10:55:15AM -0500, Neil Horman wrote: > On Wed, Dec 12, 2007 at 03:21:32PM +0100, Andi Kleen wrote: > > > + htcfg = read_pci_config(num, slot, func, 0x68); > > > + if (htcfg & (1 << 18)) { > > > + printk(KERN_INFO "Detected use of extended apic ids on > > > hypertransport bus\n"); > > > + if ((htcfg & (1 << 17)) == 0) { > > > + printk(KERN_INFO "Enabling hypertransport extended apic > > > interrupt broadcast\n"); > > > + printk(KERN_INFO "Note this is a bios bug, please > > > contact your hw vendor\n"); > > > > I'm not convinced the message is correct. e.g. on a system with only a dual > > core not enabling > > that is fine, but the extended IDs might be still set. > > > I'm not sure that would be fine. In the situation you describe, not setting > this bit means the second core won't receive interrupts. If we crash on that > core and boot the kdump kernel with it, we get exactly the same problem that > we > currently see. It could enable the extended APIC IDs but not use them? Anyways I haven't got docs on that NV bridge so I might be wrong. > > > #endif /* CONFIG_ACPI */ > > > > > > -static void __init nvidia_bugs(void) > > > +static void __init nvidia_bugs(int num, int slot, int func) > > > { > > > + static int fix_applied = 0; > > > + > > > + if (fix_applied++) > > > + return; > > > > This looks like the wrong place to do this. Better add a flag or something > > in the structure. Dito others. > > > I suppose I can, but I'm not sure what benefit that provides. Can you > elaborate? The code would be smaller and cleaner. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Wed, Dec 12, 2007 at 03:21:32PM +0100, Andi Kleen wrote: > > + htcfg = read_pci_config(num, slot, func, 0x68); > > + if (htcfg & (1 << 18)) { > > + printk(KERN_INFO "Detected use of extended apic ids on > > hypertransport bus\n"); > > + if ((htcfg & (1 << 17)) == 0) { > > + printk(KERN_INFO "Enabling hypertransport extended apic > > interrupt broadcast\n"); > > + printk(KERN_INFO "Note this is a bios bug, please > > contact your hw vendor\n"); > > I'm not convinced the message is correct. e.g. on a system with only a dual > core not enabling > that is fine, but the extended IDs might be still set. > I'm not sure that would be fine. In the situation you describe, not setting this bit means the second core won't receive interrupts. If we crash on that core and boot the kdump kernel with it, we get exactly the same problem that we currently see. > > #endif /* CONFIG_ACPI */ > > > > -static void __init nvidia_bugs(void) > > +static void __init nvidia_bugs(int num, int slot, int func) > > { > > + static int fix_applied = 0; > > + > > + if (fix_applied++) > > + return; > > This looks like the wrong place to do this. Better add a flag or something > in the structure. Dito others. > I suppose I can, but I'm not sure what benefit that provides. Can you elaborate? > Also while not a problem here in general it's bad style to add potential > wrapping bugs like this. Never use ++ for flags. > I can fix that up. I'll hold off though until ben redoes all his testing. He mentioned earlier this morning, that some of the results he was getting may have been caused by a kexec utility bug. He's re-confirming that this patch solves the reported problem. Once he does, I'll repost. Thanks & Regards Neil > -Andi -- /*** *Neil Horman [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
> + htcfg = read_pci_config(num, slot, func, 0x68); > + if (htcfg & (1 << 18)) { > + printk(KERN_INFO "Detected use of extended apic ids on > hypertransport bus\n"); > + if ((htcfg & (1 << 17)) == 0) { > + printk(KERN_INFO "Enabling hypertransport extended apic > interrupt broadcast\n"); > + printk(KERN_INFO "Note this is a bios bug, please > contact your hw vendor\n"); I'm not convinced the message is correct. e.g. on a system with only a dual core not enabling that is fine, but the extended IDs might be still set. > #endif /* CONFIG_ACPI */ > > -static void __init nvidia_bugs(void) > +static void __init nvidia_bugs(int num, int slot, int func) > { > + static int fix_applied = 0; > + > + if (fix_applied++) > + return; This looks like the wrong place to do this. Better add a flag or something in the structure. Dito others. Also while not a problem here in general it's bad style to add potential wrapping bugs like this. Never use ++ for flags. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
+ htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); I'm not convinced the message is correct. e.g. on a system with only a dual core not enabling that is fine, but the extended IDs might be still set. #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; This looks like the wrong place to do this. Better add a flag or something in the structure. Dito others. Also while not a problem here in general it's bad style to add potential wrapping bugs like this. Never use ++ for flags. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Wed, Dec 12, 2007 at 03:21:32PM +0100, Andi Kleen wrote: + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); I'm not convinced the message is correct. e.g. on a system with only a dual core not enabling that is fine, but the extended IDs might be still set. I'm not sure that would be fine. In the situation you describe, not setting this bit means the second core won't receive interrupts. If we crash on that core and boot the kdump kernel with it, we get exactly the same problem that we currently see. #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; This looks like the wrong place to do this. Better add a flag or something in the structure. Dito others. I suppose I can, but I'm not sure what benefit that provides. Can you elaborate? Also while not a problem here in general it's bad style to add potential wrapping bugs like this. Never use ++ for flags. I can fix that up. I'll hold off though until ben redoes all his testing. He mentioned earlier this morning, that some of the results he was getting may have been caused by a kexec utility bug. He's re-confirming that this patch solves the reported problem. Once he does, I'll repost. Thanks Regards Neil -Andi -- /*** *Neil Horman [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Wed, Dec 12, 2007 at 10:55:15AM -0500, Neil Horman wrote: On Wed, Dec 12, 2007 at 03:21:32PM +0100, Andi Kleen wrote: + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); I'm not convinced the message is correct. e.g. on a system with only a dual core not enabling that is fine, but the extended IDs might be still set. I'm not sure that would be fine. In the situation you describe, not setting this bit means the second core won't receive interrupts. If we crash on that core and boot the kdump kernel with it, we get exactly the same problem that we currently see. It could enable the extended APIC IDs but not use them? Anyways I haven't got docs on that NV bridge so I might be wrong. #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; This looks like the wrong place to do this. Better add a flag or something in the structure. Dito others. I suppose I can, but I'm not sure what benefit that provides. Can you elaborate? The code would be smaller and cleaner. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Andi Kleen [EMAIL PROTECTED] writes: It could enable the extended APIC IDs but not use them? In which case complaining is still correct (the BIOS was out of sync), enabling bit 17 is still correct and we are just in overkill mode. Anyways I haven't got docs on that NV bridge so I might be wrong. This has everything to do with how AMD coherent hypertransport works and little if anything to do with how the NV bridge operated. Basically the NV bridge seems to be sending a standard hypertransport x86 legacy interrupt packet (that doesn't have any target information) and when that packet hits the coherent hypertransport domain it isn't being converted into whatever would send it to all cpus. . The real practical problem is if somehow the BIOS goofs up this way and it then decides to ask us to boot on one of these cpus with an extended apic id. We will hang in calibrate_delay. So far this only seems to happen in the kdump case but in theory the BIOS could be completely crazy. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Wed, Dec 12, 2007 at 12:43:34PM -0700, Eric W. Biederman wrote: Andi Kleen [EMAIL PROTECTED] writes: It could enable the extended APIC IDs but not use them? In which case complaining is still correct (the BIOS was out of sync), enabling bit 17 is still correct and we are just in overkill mode. Anyways I haven't got docs on that NV bridge so I might be wrong. This has everything to do with how AMD coherent hypertransport works and little if anything to do with how the NV bridge operated. Basically the NV bridge seems to be sending a standard hypertransport x86 legacy interrupt packet (that doesn't have any target information) and when that packet hits the coherent hypertransport domain it isn't being converted into whatever would send it to all cpus. . The real practical problem is if somehow the BIOS goofs up this way and it then decides to ask us to boot on one of these cpus with an extended apic id. We will hang in calibrate_delay. So far this only seems to happen in the kdump case but in theory the BIOS could be completely crazy. I think this just leaves us with deciding on a mechanism for how to do single-application quirks. I take Andi's point that adding a flag set to the quirk data structure is a fine solution, but I'm really ok with static integers in individual functions. Do we have consensus on how to handle that? I'm happy either way, but I'd rather have agreement on how to handle it before I post another iteration of this patch. Thanks Regards Neil Eric -- /*** *Neil Horman [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman [EMAIL PROTECTED] writes: I think this just leaves us with deciding on a mechanism for how to do single-application quirks. I take Andi's point that adding a flag set to the quirk data structure is a fine solution, but I'm really ok with static integers in individual functions. Do we have consensus on how to handle that? I'm happy either way, but I'd rather have agreement on how to handle it before I post another iteration of this patch. As long as the solution is simple, small and concise I don't care. And since what will make Andi happy seems to meet those criteria, that should be fine. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 11, 2007 4:52 PM, Neil Horman <[EMAIL PROTECTED]> wrote: > On Tue, Dec 11, 2007 at 04:16:32PM -0800, Ben Woodard wrote: > > We may need to go back and do some additional work on this. It doesn't > > seem to be quite as cut and dried as we initially thought. > > > > This quirk doesn't appear to work on virtually the same motherboard with > > the barcelona processors in it. It also may be sensitive to the firmware > > version. More extensive testing on a larger number of pre-production is > > not showing it to be as effective as it appeared to be initially on the > > testbed. > > > > I'm doing some retesting to figure out what exact situations and > > collection of patches were able to make it work before. > > > Ben, please lets be clear about this. You say this patch doesn't help on a > new > system. Even thought its almost the exact same system, its not the same > system. > Does this patch work consistently on the system you initially reported the > problem on? I've done enough work on this at this point that I'm invested in > not abandoning this fix. If this solves the problem on dual core system, but > not quad core, I'd much rather move forward with this fix and address your > quad > core problem as a separate issue. > > Neil > > > > -ben > > > > > > > > Neil Horman wrote: > > > Recently a kdump bug was discovered in which a system would hang inside > > > calibrate_delay during the booting of the kdump kernel. This was caused > > > by the > > > fact that the jiffies counter was not being incremented during timer > > > calibration. The root cause of this problem was found to be a bios > > > misconfiguration of the hypertransport bus. On system affected by this > > > hang, > > > the bios had assigned APIC ids which used extended apic bits (more than > > > the > > > nominal 4 bit ids's), but failed to configure bit 17 of the hypertransport > > > transaction config register, which indicated that the mask for the > > > destination > > > field of interrupt packets accross the ht bus (see section 3.3.9 of > > > http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). > > > If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it > > > will > > > not recieve interrupts during the kdump kernel boot, and this hang will > > > be the > > > result. The fix is to add this patch, whcih add an early pci quirk > > > check, to > > > forcibly enable this bit in the httcfg register. This enables all cpus > > > on a > > > system to receive interrupts, and allows kdump kernel bootup to procede > > > normally. > > > > > > Regards > > > Neil > > > > > > > > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > > > ... > > > static struct chipset early_qrk[] __initdata = { > > > - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, > > > - { PCI_VENDOR_ID_VIA, via_bugs }, > > > - { PCI_VENDOR_ID_ATI, ati_bugs }, > > > + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, > > > nvidia_bugs }, > > > + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, > > > via_bugs }, > > > + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, > > > ati_bugs }, > > > + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, > > > PCI_ANY_ID, fix_hypertransport_config }, ==> + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, + { PCI_VENDOR_ID_AMD, 0x1200 , PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, I still think good way is that you ask Supermicro to update their BIOS to use newer code from AMD. YH -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Tue, Dec 11, 2007 at 04:16:32PM -0800, Ben Woodard wrote: > We may need to go back and do some additional work on this. It doesn't > seem to be quite as cut and dried as we initially thought. > > This quirk doesn't appear to work on virtually the same motherboard with > the barcelona processors in it. It also may be sensitive to the firmware > version. More extensive testing on a larger number of pre-production is > not showing it to be as effective as it appeared to be initially on the > testbed. > > I'm doing some retesting to figure out what exact situations and > collection of patches were able to make it work before. > Ben, please lets be clear about this. You say this patch doesn't help on a new system. Even thought its almost the exact same system, its not the same system. Does this patch work consistently on the system you initially reported the problem on? I've done enough work on this at this point that I'm invested in not abandoning this fix. If this solves the problem on dual core system, but not quad core, I'd much rather move forward with this fix and address your quad core problem as a separate issue. Neil > -ben > > > > Neil Horman wrote: > > Recently a kdump bug was discovered in which a system would hang inside > > calibrate_delay during the booting of the kdump kernel. This was caused by > > the > > fact that the jiffies counter was not being incremented during timer > > calibration. The root cause of this problem was found to be a bios > > misconfiguration of the hypertransport bus. On system affected by this > > hang, > > the bios had assigned APIC ids which used extended apic bits (more than the > > nominal 4 bit ids's), but failed to configure bit 17 of the hypertransport > > transaction config register, which indicated that the mask for the > > destination > > field of interrupt packets accross the ht bus (see section 3.3.9 of > > http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). > > If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it > > will > > not recieve interrupts during the kdump kernel boot, and this hang will be > > the > > result. The fix is to add this patch, whcih add an early pci quirk check, > > to > > forcibly enable this bit in the httcfg register. This enables all cpus on a > > system to receive interrupts, and allows kdump kernel bootup to procede > > normally. > > > > Regards > > Neil > > > > > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > > > > > > early-quirks.c | 90 > > +++-- > > 1 file changed, 69 insertions(+), 21 deletions(-) > > > > > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > > index 88bb83e..c0d0c69 100644 > > --- a/arch/x86/kernel/early-quirks.c > > +++ b/arch/x86/kernel/early-quirks.c > > @@ -21,8 +21,36 @@ > > #include > > #endif > > > > -static void __init via_bugs(void) > > +static void __init fix_hypertransport_config(int num, int slot, int func) > > { > > + u32 htcfg; > > + /* > > +*we found a hypertransport bus > > +*make sure that are broadcasting > > +*interrupts to all cpus on the ht bus > > +*if we're using extended apic ids > > +*/ > > + htcfg = read_pci_config(num, slot, func, 0x68); > > + if (htcfg & (1 << 18)) { > > + printk(KERN_INFO "Detected use of extended apic ids on > > hypertransport bus\n"); > > + if ((htcfg & (1 << 17)) == 0) { > > + printk(KERN_INFO "Enabling hypertransport extended apic > > interrupt broadcast\n"); > > + printk(KERN_INFO "Note this is a bios bug, please > > contact your hw vendor\n"); > > + htcfg |= (1 << 17); > > + write_pci_config(num, slot, func, 0x68, htcfg); > > + } > > + } > > + > > + > > +} > > + > > +static void __init via_bugs(int num, int slot, int func) > > +{ > > + static int fix_applied = 0; > > + > > + if (fix_applied++) > > + return; > > + > > #ifdef CONFIG_GART_IOMMU > > if ((end_pfn > MAX_DMA32_PFN || force_iommu) && > > !gart_iommu_aperture_allowed) { > > @@ -44,8 +72,13 @@ static int __init nvidia_hpet_check(struct > > acpi_table_header *header) > > #endif /* CONFIG_X86_IO_APIC */ > > #endif /* CONFIG_ACPI */ > > > > -static void __init nvidia_bugs(void) > > +static void __init nvidia_bugs(int num, int slot, int func) > > { > > + static int fix_applied = 0; > > + > > + if (fix_applied++) > > + return; > > + > > #ifdef CONFIG_ACPI > > #ifdef CONFIG_X86_IO_APIC > > /* > > @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) > > > > } > > > > -static void __init ati_bugs(void) > > +static void __init ati_bugs(int num, int slot, int func) > > { > > + static int fix_applied = 0; > > + > > + if (fix_applied++) > > + return; > > + > > #ifdef CONFIG_X86_IO_APIC > > if
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
We may need to go back and do some additional work on this. It doesn't seem to be quite as cut and dried as we initially thought. This quirk doesn't appear to work on virtually the same motherboard with the barcelona processors in it. It also may be sensitive to the firmware version. More extensive testing on a larger number of pre-production is not showing it to be as effective as it appeared to be initially on the testbed. I'm doing some retesting to figure out what exact situations and collection of patches were able to make it work before. -ben Neil Horman wrote: Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 17 of the hypertransport transaction config register, which indicated that the mask for the destination field of interrupt packets accross the ht bus (see section 3.3.9 of http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it will not recieve interrupts during the kdump kernel boot, and this hang will be the result. The fix is to add this patch, whcih add an early pci quirk check, to forcibly enable this bit in the httcfg register. This enables all cpus on a system to receive interrupts, and allows kdump kernel bootup to procede normally. Regards Neil Signed-off-by: Neil Horman <[EMAIL PROTECTED]> early-quirks.c | 90 +++-- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..c0d0c69 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,36 @@ #include #endif -static void __init via_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg & (1 << 18)) { + printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n"); + if ((htcfg & (1 << 17)) == 0) { + printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n"); + printk(KERN_INFO "Note this is a bios bug, please contact your hw vendor\n"); + htcfg |= (1 << 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init via_bugs(int num, int slot, int func) +{ + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_GART_IOMMU if ((end_pfn > MAX_DMA32_PFN || force_iommu) && !gart_iommu_aperture_allowed) { @@ -44,8 +72,13 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -84,14 +122,18 @@ static void __init ati_bugs(void) } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, {} }; @@ -106,27 +148,33 @@
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 17 of the hypertransport transaction config register, which indicated that the mask for the destination field of interrupt packets accross the ht bus (see section 3.3.9 of http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it will not recieve interrupts during the kdump kernel boot, and this hang will be the result. The fix is to add this patch, whcih add an early pci quirk check, to forcibly enable this bit in the httcfg register. This enables all cpus on a system to receive interrupts, and allows kdump kernel bootup to procede normally. Regards Neil Signed-off-by: Neil Horman <[EMAIL PROTECTED]> early-quirks.c | 90 +++-- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..c0d0c69 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,36 @@ #include #endif -static void __init via_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg & (1 << 18)) { + printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n"); + if ((htcfg & (1 << 17)) == 0) { + printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n"); + printk(KERN_INFO "Note this is a bios bug, please contact your hw vendor\n"); + htcfg |= (1 << 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init via_bugs(int num, int slot, int func) +{ + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_GART_IOMMU if ((end_pfn > MAX_DMA32_PFN || force_iommu) && !gart_iommu_aperture_allowed) { @@ -44,8 +72,13 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -84,14 +122,18 @@ static void __init ati_bugs(void) } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, {} }; @@ -106,27 +148,33 @@ void __init early_quirks(void) for (num = 0; num < 32; num++) { for (slot = 0; slot < 32; slot++) { for (func = 0; func < 8; func++) { - u32 class; - u32 vendor; + u16 class; + u16 vendor; + u16 device; u8 type; int i; - class = read_pci_config(num,slot,func, + + class =
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 11, 2007 11:24 AM, Neil Horman <[EMAIL PROTECTED]> wrote: > On Tue, Dec 11, 2007 at 11:46:34AM -0700, Eric W. Biederman wrote: > > Neil Horman <[EMAIL PROTECTED]> writes: > > > > Ok. My only remaining nit to pick is that fix_hypertransport_config > > is right in the middle of the nvidia quirks, which can be a bit > > confusing when reading through the code. Otherwise I think this > > is a version that we can merge. > > > Sure, I'll move it to the top of the file > > > Let's get a clean description on this thing and send it to the > > current x86 maintainers. Thomas, Ingo, and HPA > > > > Clean Summary: > > Recently a kdump bug was discovered in which a system would hang inside > calibrate_delay during the booting of the kdump kernel. This was caused by > the > fact that the jiffies counter was not being incremented during timer > calibration. The root cause of this problem was found to be a bios > misconfiguration of the hypertransport bus. On system affected by this hang, > the bios had assigned APIC ids which used extended apic bits (more than the > nominal 4 bit ids's), but failed to configure bit 18 of the hypertransport should be bit 17. YH -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Tue, Dec 11, 2007 at 11:46:34AM -0700, Eric W. Biederman wrote: > Neil Horman <[EMAIL PROTECTED]> writes: > > Ok. My only remaining nit to pick is that fix_hypertransport_config > is right in the middle of the nvidia quirks, which can be a bit > confusing when reading through the code. Otherwise I think this > is a version that we can merge. > Sure, I'll move it to the top of the file > Let's get a clean description on this thing and send it to the > current x86 maintainers. Thomas, Ingo, and HPA > Clean Summary: Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 18 of the hypertransport transaction config register, which indicated that the mask for the destination field of interrupt packets accross the ht bus (see section 3.3.9 of http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it will not recieve interrupts during the kdump kernel boot, and this hang will be the result. The fix is to add this patch, whcih add an early pci quirk check, to forcibly enable this bit in the httcfg register. This enables all cpus on a system to receive interrupts, and allows kdump kernel bootup to procede normally. > > > Thanks & Regards > > Neil > > > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > Acked-by: "Eric W. Biederman" <[EMAIL PROTECTED]> > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> early-quirks.c | 90 +++-- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..c0d0c69 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,36 @@ #include #endif -static void __init via_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg & (1 << 18)) { + printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n"); + if ((htcfg & (1 << 17)) == 0) { + printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n"); + printk(KERN_INFO "Note this is a bios bug, please contact your hw vendor\n"); + htcfg |= (1 << 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init via_bugs(int num, int slot, int func) +{ + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_GART_IOMMU if ((end_pfn > MAX_DMA32_PFN || force_iommu) && !gart_iommu_aperture_allowed) { @@ -44,8 +72,13 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -84,14 +122,18 @@ static void __init ati_bugs(void) } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST,
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman <[EMAIL PROTECTED]> writes: > On Tue, Dec 11, 2007 at 08:29:20AM -0700, Eric W. Biederman wrote: >> Neil Horman <[EMAIL PROTECTED]> writes: >> >> > On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: >> >> Neil Horman <[EMAIL PROTECTED]> writes: >> >> Ok. I just looked at read_pci_config. It doesn't do the right thing for >> a non-aligned 32bit access. (Not that I am convinced there is a right >> thing we can do). Please make this read_pci_config_16 instead >> and you won't need the shift. >> >> Either that or as I earlier suggested just do a 32bit read from offset 0 >> and use shifts and masks to get vendor and device fields. >> > > > The former seems like a reasonable solution to me. Corrected in this updated > patch. > >> You almost got YH's comment. You need return 2 for the old functions >> so we don't try and apply a per chipset fixup for every device in >> the system. >> >> I'm actually inclined to remove the return magic and just do something >> like: >> static fix_applied; >> if (fix_applied++) >> return; >> In those functions that should be called only once. >> > > I like the latter approach better. It seems less convoluted to me. > > New patch attached. Ok. My only remaining nit to pick is that fix_hypertransport_config is right in the middle of the nvidia quirks, which can be a bit confusing when reading through the code. Otherwise I think this is a version that we can merge. Let's get a clean description on this thing and send it to the current x86 maintainers. Thomas, Ingo, and HPA > Thanks & Regards > Neil > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> Acked-by: "Eric W. Biederman" <[EMAIL PROTECTED]> > > > early-quirks.c | 90 +++-- > 1 file changed, 69 insertions(+), 21 deletions(-) > > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > index 88bb83e..f307285 100644 > --- a/arch/x86/kernel/early-quirks.c > +++ b/arch/x86/kernel/early-quirks.c > @@ -21,8 +21,13 @@ > #include > #endif > > -static void __init via_bugs(void) > +static void __init via_bugs(int num, int slot, int func) > { > + static int fix_applied = 0; > + > + if (fix_applied++) > + return; > + > #ifdef CONFIG_GART_IOMMU > if ((end_pfn > MAX_DMA32_PFN || force_iommu) && > !gart_iommu_aperture_allowed) { > @@ -44,8 +49,36 @@ static int __init nvidia_hpet_check(struct > acpi_table_header > *header) > #endif /* CONFIG_X86_IO_APIC */ > #endif /* CONFIG_ACPI */ > > -static void __init nvidia_bugs(void) > +static void __init fix_hypertransport_config(int num, int slot, int func) > { > + u32 htcfg; > + /* > + *we found a hypertransport bus > + *make sure that are broadcasting > + *interrupts to all cpus on the ht bus > + *if we're using extended apic ids > + */ > + htcfg = read_pci_config(num, slot, func, 0x68); > + if (htcfg & (1 << 18)) { > + printk(KERN_INFO "Detected use of extended apic ids on hypertransport > bus\n"); > + if ((htcfg & (1 << 17)) == 0) { > + printk(KERN_INFO "Enabling hypertransport extended apic interrupt > broadcast\n"); > + printk(KERN_INFO "Note this is a bios bug, please contact your hw > vendor\n"); > + htcfg |= (1 << 17); > + write_pci_config(num, slot, func, 0x68, htcfg); > + } > + } > + > + > +} > + > +static void __init nvidia_bugs(int num, int slot, int func) > +{ > + static int fix_applied = 0; > + > + if (fix_applied++) > + return; > + > #ifdef CONFIG_ACPI > #ifdef CONFIG_X86_IO_APIC > /* > @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) > > } > > -static void __init ati_bugs(void) > +static void __init ati_bugs(int num, int slot, int func) > { > + static int fix_applied = 0; > + > + if (fix_applied++) > + return; > + > #ifdef CONFIG_X86_IO_APIC > if (timer_over_8254 == 1) { > timer_over_8254 = 0; > @@ -84,14 +122,18 @@ static void __init ati_bugs(void) > } > > struct chipset { > - u16 vendor; > - void (*f)(void); > + u32 vendor; > + u32 device; > + u32 class; > + u32 class_mask; > + void (*f)(int num, int slot, int func); > }; > > static struct chipset early_qrk[] __initdata = { > - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, > - { PCI_VENDOR_ID_VIA, via_bugs }, > - { PCI_VENDOR_ID_ATI, ati_bugs }, > + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, > nvidia_bugs }, > + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs > }, > + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs > }, > + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, > PCI_ANY_ID, fix_hypertransport_config }, > {} > }; > > @@ -106,27 +148,33 @@ void __init early_quirks(void) >
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 11, 2007 10:29 AM, Neil Horman <[EMAIL PROTECTED]> wrote: > > On Tue, Dec 11, 2007 at 10:00:00AM -0800, Yinghai Lu wrote: > > On Dec 11, 2007 7:29 AM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > > > > > > I'm actually inclined to remove the return magic and just do something > > > like: > > > static fix_applied; > > > if (fix_applied++) > > > return; > > > In those functions that should be called only once. > > > > it seems we need to have two tables. one for northbridge (sweep all > > the NB_K8) and another for SB ( like Nvidia, ati..., one touch and > > leave) > > > > YH > > > I like Erics idea better I think. My origional patch had two tables, and it > seems that it made the early quirk detection logic that much more convoluted. > This way each quirk can determine if it needs to be applied to more than one > pci > device. > nvidia or ati chip will come first, and then amd NB ( K8). So you need to make sure "fix_applied return" is not going to skip your fix to K8_NB. YH -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Tue, Dec 11, 2007 at 10:00:00AM -0800, Yinghai Lu wrote: > On Dec 11, 2007 7:29 AM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > > > > Neil Horman <[EMAIL PROTECTED]> writes: > > > > > On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: > > >> Neil Horman <[EMAIL PROTECTED]> writes: > > >> > > >> Almost there. > > > cool! :) > > > > > > > > >> > > >> We should not need check_hypertransport_config as the generic loop > > >> now does the work for us. > > >> > + > > >> > static void __init nvidia_bugs(void) > > >> > { > > >> > #ifdef CONFIG_ACPI > > >> > @@ -83,15 +127,25 @@ static void __init ati_bugs(void) > > >> > #endif > > >> > } > > >> > > > >> > +static void __init amd_host_bugs(void) > > >> > +{ > > >> > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); > > >> > + check_hypertransport_config(); > > >> > +} > > >> > > >> Likewise this function is unneeded and the printk is likely confusing > > >> for users. > > >> > > > Copy that. Fixed > > > > > > > > >> >{} > > >> So make that fix_hypertransport_config and we should be good. > > > Done > > > > > >> > > >> We don't need to shift device. Although we can do: > > >> device_vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID); > > >> device = device_vendor >> 16; > > >> vendor = device_vendor & 0x; > > >> > > > I'm not so sure about this. In my testing, it was clear that I needed to > > > do a > > > shift on device to make valid comparisons to the defined PCI_DEVICE_* > > > macros. > > > The origional code had to do the same thing with the class field, which is > > > simmilarly positioned in the pci config space. > > > > Ok. I just looked at read_pci_config. It doesn't do the right thing for > > a non-aligned 32bit access. (Not that I am convinced there is a right > > thing we can do). Please make this read_pci_config_16 instead > > and you won't need the shift. > > > > Either that or as I earlier suggested just do a 32bit read from offset 0 > > and use shifts and masks to get vendor and device fields. > > > > The current code doing a shift where none should be needed (because > > we ignore the two low order bits in our read) is totally weird > > when looking at it. > > > > > Other than that, new patch attached. Enables the detection of AMD > > > hypertransport functions and checks for the proper quirk just as before, > > > and > > > incoporates your comments above Eric, as well as yours Yinghai. > > > > You almost got YH's comment. You need return 2 for the old functions > > so we don't try and apply a per chipset fixup for every device in > > the system. > > > > I'm actually inclined to remove the return magic and just do something > > like: > > static fix_applied; > > if (fix_applied++) > > return; > > In those functions that should be called only once. > > it seems we need to have two tables. one for northbridge (sweep all > the NB_K8) and another for SB ( like Nvidia, ati..., one touch and > leave) > > YH > I like Erics idea better I think. My origional patch had two tables, and it seems that it made the early quirk detection logic that much more convoluted. This way each quirk can determine if it needs to be applied to more than one pci device. Neil > ___ > kexec mailing list > [EMAIL PROTECTED] > http://lists.infradead.org/mailman/listinfo/kexec -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Tue, Dec 11, 2007 at 08:29:20AM -0700, Eric W. Biederman wrote: > Neil Horman <[EMAIL PROTECTED]> writes: > > > On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: > >> Neil Horman <[EMAIL PROTECTED]> writes: > > Ok. I just looked at read_pci_config. It doesn't do the right thing for > a non-aligned 32bit access. (Not that I am convinced there is a right > thing we can do). Please make this read_pci_config_16 instead > and you won't need the shift. > > Either that or as I earlier suggested just do a 32bit read from offset 0 > and use shifts and masks to get vendor and device fields. > The former seems like a reasonable solution to me. Corrected in this updated patch. > You almost got YH's comment. You need return 2 for the old functions > so we don't try and apply a per chipset fixup for every device in > the system. > > I'm actually inclined to remove the return magic and just do something > like: > static fix_applied; > if (fix_applied++) > return; > In those functions that should be called only once. > I like the latter approach better. It seems less convoluted to me. New patch attached. Thanks & Regards Neil Signed-off-by: Neil Horman <[EMAIL PROTECTED]> early-quirks.c | 90 +++-- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..f307285 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,13 @@ #include #endif -static void __init via_bugs(void) +static void __init via_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_GART_IOMMU if ((end_pfn > MAX_DMA32_PFN || force_iommu) && !gart_iommu_aperture_allowed) { @@ -44,8 +49,36 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg & (1 << 18)) { + printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n"); + if ((htcfg & (1 << 17)) == 0) { + printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n"); + printk(KERN_INFO "Note this is a bios bug, please contact your hw vendor\n"); + htcfg |= (1 << 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init nvidia_bugs(int num, int slot, int func) +{ + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -84,14 +122,18 @@ static void __init ati_bugs(void) } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, {} }; @@ -106,27 +148,33 @@ void __init early_quirks(void) for (num = 0; num < 32; num++) { for (slot = 0; slot < 32; slot++) { for (func = 0; func < 8; func++) { - u32 class; - u32 vendor; + u16 class; + u16 vendor; + u16 device; u8 type; int i; - class = read_pci_config(num,slot,func, + +
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 11, 2007 7:29 AM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > > Neil Horman <[EMAIL PROTECTED]> writes: > > > On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: > >> Neil Horman <[EMAIL PROTECTED]> writes: > >> > >> Almost there. > > cool! :) > > > > > >> > >> We should not need check_hypertransport_config as the generic loop > >> now does the work for us. > >> > + > >> > static void __init nvidia_bugs(void) > >> > { > >> > #ifdef CONFIG_ACPI > >> > @@ -83,15 +127,25 @@ static void __init ati_bugs(void) > >> > #endif > >> > } > >> > > >> > +static void __init amd_host_bugs(void) > >> > +{ > >> > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); > >> > + check_hypertransport_config(); > >> > +} > >> > >> Likewise this function is unneeded and the printk is likely confusing > >> for users. > >> > > Copy that. Fixed > > > > > >> >{} > >> So make that fix_hypertransport_config and we should be good. > > Done > > > >> > >> We don't need to shift device. Although we can do: > >> device_vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID); > >> device = device_vendor >> 16; > >> vendor = device_vendor & 0x; > >> > > I'm not so sure about this. In my testing, it was clear that I needed to > > do a > > shift on device to make valid comparisons to the defined PCI_DEVICE_* > > macros. > > The origional code had to do the same thing with the class field, which is > > simmilarly positioned in the pci config space. > > Ok. I just looked at read_pci_config. It doesn't do the right thing for > a non-aligned 32bit access. (Not that I am convinced there is a right > thing we can do). Please make this read_pci_config_16 instead > and you won't need the shift. > > Either that or as I earlier suggested just do a 32bit read from offset 0 > and use shifts and masks to get vendor and device fields. > > The current code doing a shift where none should be needed (because > we ignore the two low order bits in our read) is totally weird > when looking at it. > > > Other than that, new patch attached. Enables the detection of AMD > > hypertransport functions and checks for the proper quirk just as before, and > > incoporates your comments above Eric, as well as yours Yinghai. > > You almost got YH's comment. You need return 2 for the old functions > so we don't try and apply a per chipset fixup for every device in > the system. > > I'm actually inclined to remove the return magic and just do something > like: > static fix_applied; > if (fix_applied++) > return; > In those functions that should be called only once. it seems we need to have two tables. one for northbridge (sweep all the NB_K8) and another for SB ( like Nvidia, ati..., one touch and leave) YH -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman <[EMAIL PROTECTED]> writes: > On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: >> Neil Horman <[EMAIL PROTECTED]> writes: >> >> Almost there. > cool! :) > > >> >> We should not need check_hypertransport_config as the generic loop >> now does the work for us. >> > + >> > static void __init nvidia_bugs(void) >> > { >> > #ifdef CONFIG_ACPI >> > @@ -83,15 +127,25 @@ static void __init ati_bugs(void) >> > #endif >> > } >> > >> > +static void __init amd_host_bugs(void) >> > +{ >> > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); >> > + check_hypertransport_config(); >> > +} >> >> Likewise this function is unneeded and the printk is likely confusing >> for users. >> > Copy that. Fixed > > >> >{} >> So make that fix_hypertransport_config and we should be good. > Done > >> >> We don't need to shift device. Although we can do: >> device_vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID); >> device = device_vendor >> 16; >> vendor = device_vendor & 0x; >> > I'm not so sure about this. In my testing, it was clear that I needed to do a > shift on device to make valid comparisons to the defined PCI_DEVICE_* macros. > The origional code had to do the same thing with the class field, which is > simmilarly positioned in the pci config space. Ok. I just looked at read_pci_config. It doesn't do the right thing for a non-aligned 32bit access. (Not that I am convinced there is a right thing we can do). Please make this read_pci_config_16 instead and you won't need the shift. Either that or as I earlier suggested just do a 32bit read from offset 0 and use shifts and masks to get vendor and device fields. The current code doing a shift where none should be needed (because we ignore the two low order bits in our read) is totally weird when looking at it. > Other than that, new patch attached. Enables the detection of AMD > hypertransport functions and checks for the proper quirk just as before, and > incoporates your comments above Eric, as well as yours Yinghai. You almost got YH's comment. You need return 2 for the old functions so we don't try and apply a per chipset fixup for every device in the system. I'm actually inclined to remove the return magic and just do something like: static fix_applied; if (fix_applied++) return; In those functions that should be called only once. Eric > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > > > early-quirks.c | 76 + > 1 file changed, 61 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > index 88bb83e..e13c999 100644 > --- a/arch/x86/kernel/early-quirks.c > +++ b/arch/x86/kernel/early-quirks.c > @@ -21,7 +21,7 @@ > #include > #endif > > -static void __init via_bugs(void) > +static int __init via_bugs(int num, int slot, int func) > { > #ifdef CONFIG_GART_IOMMU > if ((end_pfn > MAX_DMA32_PFN || force_iommu) && > @@ -32,6 +32,7 @@ static void __init via_bugs(void) > gart_iommu_aperture_disabled = 1; > } > #endif > + return 1; return 2; > } > > #ifdef CONFIG_ACPI > @@ -44,7 +45,31 @@ static int __init nvidia_hpet_check(struct > acpi_table_header > *header) > #endif /* CONFIG_X86_IO_APIC */ > #endif /* CONFIG_ACPI */ > > -static void __init nvidia_bugs(void) > +static int __init fix_hypertransport_config(int num, int slot, int func) > +{ > + u32 htcfg; > + /* > + *we found a hypertransport bus > + *make sure that are broadcasting > + *interrupts to all cpus on the ht bus > + *if we're using extended apic ids > + */ > + htcfg = read_pci_config(num, slot, func, 0x68); > + if (htcfg & (1 << 18)) { > + printk(KERN_INFO "Detected use of extended apic ids on hypertransport > bus\n"); > + if ((htcfg & (1 << 17)) == 0) { > + printk(KERN_INFO "Enabling hypertransport extended apic interrupt > broadcast\n"); > + printk(KERN_INFO "Note this is a bios bug, please contact your hw > vendor\n"); > + htcfg |= (1 << 17); > + write_pci_config(num, slot, func, 0x68, htcfg); > + } > + } > + > + return 1; > + > +} > + Hmm. I don't think we want this code positioned in the middle of the nvidia bug checks. > +static int __init nvidia_bugs(int num, int slot, int func) > { > #ifdef CONFIG_ACPI > #ifdef CONFIG_X86_IO_APIC > @@ -56,7 +81,7 @@ static void __init nvidia_bugs(void) >* at least allow a command line override. >*/ > if (acpi_use_timer_override) > - return; > + return 1; > > if (acpi_table_parse(ACPI_SIG_HPET, nvidia_hpet_check)) { > acpi_skip_timer_override = 1; > @@ -70,9 +95,10 @@ static void __init nvidia_bugs(void) > #endif > /* RED-PEN skip them on mptables too? */ > > + return 1; return 2; > } > > -static void
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: > Neil Horman <[EMAIL PROTECTED]> writes: > > Almost there. cool! :) > > We should not need check_hypertransport_config as the generic loop > now does the work for us. > > + > > static void __init nvidia_bugs(void) > > { > > #ifdef CONFIG_ACPI > > @@ -83,15 +127,25 @@ static void __init ati_bugs(void) > > #endif > > } > > > > +static void __init amd_host_bugs(void) > > +{ > > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); > > + check_hypertransport_config(); > > +} > > Likewise this function is unneeded and the printk is likely confusing > for users. > Copy that. Fixed > > {} > So make that fix_hypertransport_config and we should be good. Done > > We don't need to shift device. Although we can do: > device_vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID); > device = device_vendor >> 16; > vendor = device_vendor & 0x; > I'm not so sure about this. In my testing, it was clear that I needed to do a shift on device to make valid comparisons to the defined PCI_DEVICE_* macros. The origional code had to do the same thing with the class field, which is simmilarly positioned in the pci config space. Other than that, new patch attached. Enables the detection of AMD hypertransport functions and checks for the proper quirk just as before, and incoporates your comments above Eric, as well as yours Yinghai. Thanks & Regards Neil Signed-off-by: Neil Horman <[EMAIL PROTECTED]> early-quirks.c | 76 + 1 file changed, 61 insertions(+), 15 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..e13c999 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,7 +21,7 @@ #include #endif -static void __init via_bugs(void) +static int __init via_bugs(int num, int slot, int func) { #ifdef CONFIG_GART_IOMMU if ((end_pfn > MAX_DMA32_PFN || force_iommu) && @@ -32,6 +32,7 @@ static void __init via_bugs(void) gart_iommu_aperture_disabled = 1; } #endif + return 1; } #ifdef CONFIG_ACPI @@ -44,7 +45,31 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static int __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg & (1 << 18)) { + printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n"); + if ((htcfg & (1 << 17)) == 0) { + printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n"); + printk(KERN_INFO "Note this is a bios bug, please contact your hw vendor\n"); + htcfg |= (1 << 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + return 1; + +} + +static int __init nvidia_bugs(int num, int slot, int func) { #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC @@ -56,7 +81,7 @@ static void __init nvidia_bugs(void) * at least allow a command line override. */ if (acpi_use_timer_override) - return; + return 1; if (acpi_table_parse(ACPI_SIG_HPET, nvidia_hpet_check)) { acpi_skip_timer_override = 1; @@ -70,9 +95,10 @@ static void __init nvidia_bugs(void) #endif /* RED-PEN skip them on mptables too? */ + return 1; } -static void __init ati_bugs(void) +static int __init ati_bugs(int num, int slot, int func) { #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { @@ -81,17 +107,22 @@ static void __init ati_bugs(void) "ATI board detected. Disabling timer routing over 8254.\n"); } #endif + return 1; } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + int (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, {} }; @@ -108,25
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: Almost there. cool! :) snip We should not need check_hypertransport_config as the generic loop now does the work for us. + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,15 +127,25 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} Likewise this function is unneeded and the printk is likely confusing for users. Copy that. Fixed snip {} So make that fix_hypertransport_config and we should be good. Done We don't need to shift device. Although we can do: device_vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID); device = device_vendor 16; vendor = device_vendor 0x; I'm not so sure about this. In my testing, it was clear that I needed to do a shift on device to make valid comparisons to the defined PCI_DEVICE_* macros. The origional code had to do the same thing with the class field, which is simmilarly positioned in the pci config space. Other than that, new patch attached. Enables the detection of AMD hypertransport functions and checks for the proper quirk just as before, and incoporates your comments above Eric, as well as yours Yinghai. Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 76 + 1 file changed, 61 insertions(+), 15 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..e13c999 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,7 +21,7 @@ #include asm/gart.h #endif -static void __init via_bugs(void) +static int __init via_bugs(int num, int slot, int func) { #ifdef CONFIG_GART_IOMMU if ((end_pfn MAX_DMA32_PFN || force_iommu) @@ -32,6 +32,7 @@ static void __init via_bugs(void) gart_iommu_aperture_disabled = 1; } #endif + return 1; } #ifdef CONFIG_ACPI @@ -44,7 +45,31 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static int __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + return 1; + +} + +static int __init nvidia_bugs(int num, int slot, int func) { #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC @@ -56,7 +81,7 @@ static void __init nvidia_bugs(void) * at least allow a command line override. */ if (acpi_use_timer_override) - return; + return 1; if (acpi_table_parse(ACPI_SIG_HPET, nvidia_hpet_check)) { acpi_skip_timer_override = 1; @@ -70,9 +95,10 @@ static void __init nvidia_bugs(void) #endif /* RED-PEN skip them on mptables too? */ + return 1; } -static void __init ati_bugs(void) +static int __init ati_bugs(int num, int slot, int func) { #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { @@ -81,17 +107,22 @@ static void __init ati_bugs(void) ATI board detected. Disabling timer routing over 8254.\n); } #endif + return 1; } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + int (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, {} }; @@ -108,25 +139,40 @@ void __init early_quirks(void)
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman [EMAIL PROTECTED] writes: On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: Almost there. cool! :) snip We should not need check_hypertransport_config as the generic loop now does the work for us. + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,15 +127,25 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} Likewise this function is unneeded and the printk is likely confusing for users. Copy that. Fixed snip {} So make that fix_hypertransport_config and we should be good. Done We don't need to shift device. Although we can do: device_vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID); device = device_vendor 16; vendor = device_vendor 0x; I'm not so sure about this. In my testing, it was clear that I needed to do a shift on device to make valid comparisons to the defined PCI_DEVICE_* macros. The origional code had to do the same thing with the class field, which is simmilarly positioned in the pci config space. Ok. I just looked at read_pci_config. It doesn't do the right thing for a non-aligned 32bit access. (Not that I am convinced there is a right thing we can do). Please make this read_pci_config_16 instead and you won't need the shift. Either that or as I earlier suggested just do a 32bit read from offset 0 and use shifts and masks to get vendor and device fields. The current code doing a shift where none should be needed (because we ignore the two low order bits in our read) is totally weird when looking at it. Other than that, new patch attached. Enables the detection of AMD hypertransport functions and checks for the proper quirk just as before, and incoporates your comments above Eric, as well as yours Yinghai. You almost got YH's comment. You need return 2 for the old functions so we don't try and apply a per chipset fixup for every device in the system. I'm actually inclined to remove the return magic and just do something like: static fix_applied; if (fix_applied++) return; In those functions that should be called only once. Eric Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 76 + 1 file changed, 61 insertions(+), 15 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..e13c999 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,7 +21,7 @@ #include asm/gart.h #endif -static void __init via_bugs(void) +static int __init via_bugs(int num, int slot, int func) { #ifdef CONFIG_GART_IOMMU if ((end_pfn MAX_DMA32_PFN || force_iommu) @@ -32,6 +32,7 @@ static void __init via_bugs(void) gart_iommu_aperture_disabled = 1; } #endif + return 1; return 2; } #ifdef CONFIG_ACPI @@ -44,7 +45,31 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static int __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* + *we found a hypertransport bus + *make sure that are broadcasting + *interrupts to all cpus on the ht bus + *if we're using extended apic ids + */ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + return 1; + +} + Hmm. I don't think we want this code positioned in the middle of the nvidia bug checks. +static int __init nvidia_bugs(int num, int slot, int func) { #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC @@ -56,7 +81,7 @@ static void __init nvidia_bugs(void) * at least allow a command line override. */ if (acpi_use_timer_override) - return; + return 1; if (acpi_table_parse(ACPI_SIG_HPET, nvidia_hpet_check)) { acpi_skip_timer_override = 1; @@ -70,9 +95,10 @@ static void __init nvidia_bugs(void) #endif /* RED-PEN skip them on mptables too? */ + return 1; return 2; } -static void __init ati_bugs(void) +static int __init ati_bugs(int num, int slot, int func) { #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { @@ -81,17 +107,22 @@ static void __init
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 11, 2007 7:29 AM, Eric W. Biederman [EMAIL PROTECTED] wrote: Neil Horman [EMAIL PROTECTED] writes: On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: Almost there. cool! :) snip We should not need check_hypertransport_config as the generic loop now does the work for us. + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,15 +127,25 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} Likewise this function is unneeded and the printk is likely confusing for users. Copy that. Fixed snip {} So make that fix_hypertransport_config and we should be good. Done We don't need to shift device. Although we can do: device_vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID); device = device_vendor 16; vendor = device_vendor 0x; I'm not so sure about this. In my testing, it was clear that I needed to do a shift on device to make valid comparisons to the defined PCI_DEVICE_* macros. The origional code had to do the same thing with the class field, which is simmilarly positioned in the pci config space. Ok. I just looked at read_pci_config. It doesn't do the right thing for a non-aligned 32bit access. (Not that I am convinced there is a right thing we can do). Please make this read_pci_config_16 instead and you won't need the shift. Either that or as I earlier suggested just do a 32bit read from offset 0 and use shifts and masks to get vendor and device fields. The current code doing a shift where none should be needed (because we ignore the two low order bits in our read) is totally weird when looking at it. Other than that, new patch attached. Enables the detection of AMD hypertransport functions and checks for the proper quirk just as before, and incoporates your comments above Eric, as well as yours Yinghai. You almost got YH's comment. You need return 2 for the old functions so we don't try and apply a per chipset fixup for every device in the system. I'm actually inclined to remove the return magic and just do something like: static fix_applied; if (fix_applied++) return; In those functions that should be called only once. it seems we need to have two tables. one for northbridge (sweep all the NB_K8) and another for SB ( like Nvidia, ati..., one touch and leave) YH -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Tue, Dec 11, 2007 at 08:29:20AM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: Ok. I just looked at read_pci_config. It doesn't do the right thing for a non-aligned 32bit access. (Not that I am convinced there is a right thing we can do). Please make this read_pci_config_16 instead and you won't need the shift. Either that or as I earlier suggested just do a 32bit read from offset 0 and use shifts and masks to get vendor and device fields. The former seems like a reasonable solution to me. Corrected in this updated patch. You almost got YH's comment. You need return 2 for the old functions so we don't try and apply a per chipset fixup for every device in the system. I'm actually inclined to remove the return magic and just do something like: static fix_applied; if (fix_applied++) return; In those functions that should be called only once. I like the latter approach better. It seems less convoluted to me. New patch attached. Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 90 +++-- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..f307285 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,13 @@ #include asm/gart.h #endif -static void __init via_bugs(void) +static void __init via_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_GART_IOMMU if ((end_pfn MAX_DMA32_PFN || force_iommu) !gart_iommu_aperture_allowed) { @@ -44,8 +49,36 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init nvidia_bugs(int num, int slot, int func) +{ + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -84,14 +122,18 @@ static void __init ati_bugs(void) } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, {} }; @@ -106,27 +148,33 @@ void __init early_quirks(void) for (num = 0; num 32; num++) { for (slot = 0; slot 32; slot++) { for (func = 0; func 8; func++) { - u32 class; - u32 vendor; + u16 class; + u16 vendor; + u16 device; u8 type; int i; - class = read_pci_config(num,slot,func, + + class =
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Tue, Dec 11, 2007 at 10:00:00AM -0800, Yinghai Lu wrote: On Dec 11, 2007 7:29 AM, Eric W. Biederman [EMAIL PROTECTED] wrote: Neil Horman [EMAIL PROTECTED] writes: On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: Almost there. cool! :) snip We should not need check_hypertransport_config as the generic loop now does the work for us. + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,15 +127,25 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} Likewise this function is unneeded and the printk is likely confusing for users. Copy that. Fixed snip {} So make that fix_hypertransport_config and we should be good. Done We don't need to shift device. Although we can do: device_vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID); device = device_vendor 16; vendor = device_vendor 0x; I'm not so sure about this. In my testing, it was clear that I needed to do a shift on device to make valid comparisons to the defined PCI_DEVICE_* macros. The origional code had to do the same thing with the class field, which is simmilarly positioned in the pci config space. Ok. I just looked at read_pci_config. It doesn't do the right thing for a non-aligned 32bit access. (Not that I am convinced there is a right thing we can do). Please make this read_pci_config_16 instead and you won't need the shift. Either that or as I earlier suggested just do a 32bit read from offset 0 and use shifts and masks to get vendor and device fields. The current code doing a shift where none should be needed (because we ignore the two low order bits in our read) is totally weird when looking at it. Other than that, new patch attached. Enables the detection of AMD hypertransport functions and checks for the proper quirk just as before, and incoporates your comments above Eric, as well as yours Yinghai. You almost got YH's comment. You need return 2 for the old functions so we don't try and apply a per chipset fixup for every device in the system. I'm actually inclined to remove the return magic and just do something like: static fix_applied; if (fix_applied++) return; In those functions that should be called only once. it seems we need to have two tables. one for northbridge (sweep all the NB_K8) and another for SB ( like Nvidia, ati..., one touch and leave) YH I like Erics idea better I think. My origional patch had two tables, and it seems that it made the early quirk detection logic that much more convoluted. This way each quirk can determine if it needs to be applied to more than one pci device. Neil ___ kexec mailing list [EMAIL PROTECTED] http://lists.infradead.org/mailman/listinfo/kexec -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 11, 2007 10:29 AM, Neil Horman [EMAIL PROTECTED] wrote: On Tue, Dec 11, 2007 at 10:00:00AM -0800, Yinghai Lu wrote: On Dec 11, 2007 7:29 AM, Eric W. Biederman [EMAIL PROTECTED] wrote: I'm actually inclined to remove the return magic and just do something like: static fix_applied; if (fix_applied++) return; In those functions that should be called only once. it seems we need to have two tables. one for northbridge (sweep all the NB_K8) and another for SB ( like Nvidia, ati..., one touch and leave) YH I like Erics idea better I think. My origional patch had two tables, and it seems that it made the early quirk detection logic that much more convoluted. This way each quirk can determine if it needs to be applied to more than one pci device. nvidia or ati chip will come first, and then amd NB ( K8). So you need to make sure fix_applied return is not going to skip your fix to K8_NB. YH -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman [EMAIL PROTECTED] writes: On Tue, Dec 11, 2007 at 08:29:20AM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: On Mon, Dec 10, 2007 at 09:48:11PM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: Ok. I just looked at read_pci_config. It doesn't do the right thing for a non-aligned 32bit access. (Not that I am convinced there is a right thing we can do). Please make this read_pci_config_16 instead and you won't need the shift. Either that or as I earlier suggested just do a 32bit read from offset 0 and use shifts and masks to get vendor and device fields. The former seems like a reasonable solution to me. Corrected in this updated patch. You almost got YH's comment. You need return 2 for the old functions so we don't try and apply a per chipset fixup for every device in the system. I'm actually inclined to remove the return magic and just do something like: static fix_applied; if (fix_applied++) return; In those functions that should be called only once. I like the latter approach better. It seems less convoluted to me. New patch attached. Ok. My only remaining nit to pick is that fix_hypertransport_config is right in the middle of the nvidia quirks, which can be a bit confusing when reading through the code. Otherwise I think this is a version that we can merge. Let's get a clean description on this thing and send it to the current x86 maintainers. Thomas, Ingo, and HPA Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] Acked-by: Eric W. Biederman [EMAIL PROTECTED] early-quirks.c | 90 +++-- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..f307285 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,13 @@ #include asm/gart.h #endif -static void __init via_bugs(void) +static void __init via_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_GART_IOMMU if ((end_pfn MAX_DMA32_PFN || force_iommu) !gart_iommu_aperture_allowed) { @@ -44,8 +49,36 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* + *we found a hypertransport bus + *make sure that are broadcasting + *interrupts to all cpus on the ht bus + *if we're using extended apic ids + */ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init nvidia_bugs(int num, int slot, int func) +{ + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -84,14 +122,18 @@ static void __init ati_bugs(void) } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, {} }; @@ -106,27 +148,33 @@ void __init early_quirks(void) for (num = 0; num 32; num++) { for (slot = 0; slot 32; slot++) { for (func = 0; func 8; func++) { - u32 class; -
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Tue, Dec 11, 2007 at 11:46:34AM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: Ok. My only remaining nit to pick is that fix_hypertransport_config is right in the middle of the nvidia quirks, which can be a bit confusing when reading through the code. Otherwise I think this is a version that we can merge. Sure, I'll move it to the top of the file Let's get a clean description on this thing and send it to the current x86 maintainers. Thomas, Ingo, and HPA Clean Summary: Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 18 of the hypertransport transaction config register, which indicated that the mask for the destination field of interrupt packets accross the ht bus (see section 3.3.9 of http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it will not recieve interrupts during the kdump kernel boot, and this hang will be the result. The fix is to add this patch, whcih add an early pci quirk check, to forcibly enable this bit in the httcfg register. This enables all cpus on a system to receive interrupts, and allows kdump kernel bootup to procede normally. Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] Acked-by: Eric W. Biederman [EMAIL PROTECTED] Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 90 +++-- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..c0d0c69 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,36 @@ #include asm/gart.h #endif -static void __init via_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init via_bugs(int num, int slot, int func) +{ + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_GART_IOMMU if ((end_pfn MAX_DMA32_PFN || force_iommu) !gart_iommu_aperture_allowed) { @@ -44,8 +72,13 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -84,14 +122,18 @@ static void __init ati_bugs(void) } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 11, 2007 11:24 AM, Neil Horman [EMAIL PROTECTED] wrote: On Tue, Dec 11, 2007 at 11:46:34AM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: Ok. My only remaining nit to pick is that fix_hypertransport_config is right in the middle of the nvidia quirks, which can be a bit confusing when reading through the code. Otherwise I think this is a version that we can merge. Sure, I'll move it to the top of the file Let's get a clean description on this thing and send it to the current x86 maintainers. Thomas, Ingo, and HPA Clean Summary: Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 18 of the hypertransport should be bit 17. YH -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 17 of the hypertransport transaction config register, which indicated that the mask for the destination field of interrupt packets accross the ht bus (see section 3.3.9 of http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it will not recieve interrupts during the kdump kernel boot, and this hang will be the result. The fix is to add this patch, whcih add an early pci quirk check, to forcibly enable this bit in the httcfg register. This enables all cpus on a system to receive interrupts, and allows kdump kernel bootup to procede normally. Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 90 +++-- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..c0d0c69 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,36 @@ #include asm/gart.h #endif -static void __init via_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init via_bugs(int num, int slot, int func) +{ + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_GART_IOMMU if ((end_pfn MAX_DMA32_PFN || force_iommu) !gart_iommu_aperture_allowed) { @@ -44,8 +72,13 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -84,14 +122,18 @@ static void __init ati_bugs(void) } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, {} }; @@ -106,27 +148,33 @@ void __init early_quirks(void) for (num = 0; num 32; num++) { for (slot = 0; slot 32; slot++) { for (func = 0; func 8; func++) { - u32 class; - u32 vendor; + u16 class; + u16 vendor; + u16 device; u8 type; int i; - class = read_pci_config(num,slot,func, + + class =
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
We may need to go back and do some additional work on this. It doesn't seem to be quite as cut and dried as we initially thought. This quirk doesn't appear to work on virtually the same motherboard with the barcelona processors in it. It also may be sensitive to the firmware version. More extensive testing on a larger number of pre-production is not showing it to be as effective as it appeared to be initially on the testbed. I'm doing some retesting to figure out what exact situations and collection of patches were able to make it work before. -ben Neil Horman wrote: Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 17 of the hypertransport transaction config register, which indicated that the mask for the destination field of interrupt packets accross the ht bus (see section 3.3.9 of http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it will not recieve interrupts during the kdump kernel boot, and this hang will be the result. The fix is to add this patch, whcih add an early pci quirk check, to forcibly enable this bit in the httcfg register. This enables all cpus on a system to receive interrupts, and allows kdump kernel bootup to procede normally. Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 90 +++-- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..c0d0c69 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,36 @@ #include asm/gart.h #endif -static void __init via_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init via_bugs(int num, int slot, int func) +{ + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_GART_IOMMU if ((end_pfn MAX_DMA32_PFN || force_iommu) !gart_iommu_aperture_allowed) { @@ -44,8 +72,13 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -84,14 +122,18 @@ static void __init ati_bugs(void) } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32 class_mask; + void (*f)(int num, int slot, int func); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, {} }; @@ -106,27 +148,33 @@ void __init
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Tue, Dec 11, 2007 at 04:16:32PM -0800, Ben Woodard wrote: We may need to go back and do some additional work on this. It doesn't seem to be quite as cut and dried as we initially thought. This quirk doesn't appear to work on virtually the same motherboard with the barcelona processors in it. It also may be sensitive to the firmware version. More extensive testing on a larger number of pre-production is not showing it to be as effective as it appeared to be initially on the testbed. I'm doing some retesting to figure out what exact situations and collection of patches were able to make it work before. Ben, please lets be clear about this. You say this patch doesn't help on a new system. Even thought its almost the exact same system, its not the same system. Does this patch work consistently on the system you initially reported the problem on? I've done enough work on this at this point that I'm invested in not abandoning this fix. If this solves the problem on dual core system, but not quad core, I'd much rather move forward with this fix and address your quad core problem as a separate issue. Neil -ben Neil Horman wrote: Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 17 of the hypertransport transaction config register, which indicated that the mask for the destination field of interrupt packets accross the ht bus (see section 3.3.9 of http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it will not recieve interrupts during the kdump kernel boot, and this hang will be the result. The fix is to add this patch, whcih add an early pci quirk check, to forcibly enable this bit in the httcfg register. This enables all cpus on a system to receive interrupts, and allows kdump kernel bootup to procede normally. Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 90 +++-- 1 file changed, 69 insertions(+), 21 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..c0d0c69 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -21,8 +21,36 @@ #include asm/gart.h #endif -static void __init via_bugs(void) +static void __init fix_hypertransport_config(int num, int slot, int func) { + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + printk(KERN_INFO Note this is a bios bug, please contact your hw vendor\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + + +} + +static void __init via_bugs(int num, int slot, int func) +{ + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_GART_IOMMU if ((end_pfn MAX_DMA32_PFN || force_iommu) !gart_iommu_aperture_allowed) { @@ -44,8 +72,13 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ -static void __init nvidia_bugs(void) +static void __init nvidia_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_ACPI #ifdef CONFIG_X86_IO_APIC /* @@ -72,8 +105,13 @@ static void __init nvidia_bugs(void) } -static void __init ati_bugs(void) +static void __init ati_bugs(int num, int slot, int func) { + static int fix_applied = 0; + + if (fix_applied++) + return; + #ifdef CONFIG_X86_IO_APIC if (timer_over_8254 == 1) { timer_over_8254 = 0; @@ -84,14 +122,18 @@ static void __init ati_bugs(void) } struct chipset { - u16 vendor; - void (*f)(void); + u32 vendor; + u32 device; + u32 class; + u32
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 11, 2007 4:52 PM, Neil Horman [EMAIL PROTECTED] wrote: On Tue, Dec 11, 2007 at 04:16:32PM -0800, Ben Woodard wrote: We may need to go back and do some additional work on this. It doesn't seem to be quite as cut and dried as we initially thought. This quirk doesn't appear to work on virtually the same motherboard with the barcelona processors in it. It also may be sensitive to the firmware version. More extensive testing on a larger number of pre-production is not showing it to be as effective as it appeared to be initially on the testbed. I'm doing some retesting to figure out what exact situations and collection of patches were able to make it work before. Ben, please lets be clear about this. You say this patch doesn't help on a new system. Even thought its almost the exact same system, its not the same system. Does this patch work consistently on the system you initially reported the problem on? I've done enough work on this at this point that I'm invested in not abandoning this fix. If this solves the problem on dual core system, but not quad core, I'd much rather move forward with this fix and address your quad core problem as a separate issue. Neil -ben Neil Horman wrote: Recently a kdump bug was discovered in which a system would hang inside calibrate_delay during the booting of the kdump kernel. This was caused by the fact that the jiffies counter was not being incremented during timer calibration. The root cause of this problem was found to be a bios misconfiguration of the hypertransport bus. On system affected by this hang, the bios had assigned APIC ids which used extended apic bits (more than the nominal 4 bit ids's), but failed to configure bit 17 of the hypertransport transaction config register, which indicated that the mask for the destination field of interrupt packets accross the ht bus (see section 3.3.9 of http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/26094.PDF). If a crash occurs on a cpu with an APIC id that extends beyond 4 bits, it will not recieve interrupts during the kdump kernel boot, and this hang will be the result. The fix is to add this patch, whcih add an early pci quirk check, to forcibly enable this bit in the httcfg register. This enables all cpus on a system to receive interrupts, and allows kdump kernel bootup to procede normally. Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] ... static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, == + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, + { PCI_VENDOR_ID_AMD, 0x1200 , PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, fix_hypertransport_config }, I still think good way is that you ask Supermicro to update their BIOS to use newer code from AMD. YH -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 10, 2007 8:48 PM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > Neil Horman <[EMAIL PROTECTED]> writes: > > Almost there. > > > > > On Mon, Dec 10, 2007 at 06:08:03PM -0700, Eric W. Biederman wrote: > >> Neil Horman <[EMAIL PROTECTED]> writes: > >> > > > >> > >> Ok. This test is broken. Please remove the == 1. You are looking > >> for == (1 << 18). So just saying: "if (htcfg & (1 << 18))" should be > >> clearer. > >> > > Fixed. Thanks! > > > >> > + printk(KERN_INFO "Detected use of extended apic ids on hypertransport > > bus\n"); > >> > + if ((htcfg & (1 << 17)) == 0) { > >> > + printk(KERN_INFO "Enabling hypertransport extended apic interrupt > >> > broadcast\n"); > >> > + htcfg |= (1 << 17); > >> > + write_pci_config(num, slot, func, 0x68, htcfg); > >> > + } > >> > + } > >> > + > >> > +} > >> > >> The rest of this quirk looks fine, include the fact it is only intended > >> to be applied to PCI_VENDOR_ID_AMD PCI_DEVICE_ID_AMD_K8_NB. > >> > > Copy that. > > > >> > >> For what is below I don't like the way the infrastructure has been > >> extended as what you are doing quickly devolves into a big mess. > >> > >> Please extend struct chipset to be something like: > >> struct chipset { > >> u16 vendor; > >> u16 device; > >> u32 class, class_mask; > >> void (*f)(void); > >> }; > >> > >> And then the test for matching the chipset can be something like: > >> if ((id->vendor == PCI_ANY_ID || id->vendor == dev->vendor) && > >> (id->device == PCI_ANY_ID || id->device == dev->device) && > >> !((id->class ^ dev->class) & id->class_mask)) > >> > >> Essentially a subset of pci_match_one_device from drivers/pci/pci.h > >> > >> That way you don't need to increase the number of tables or the > >> number of passes through the pci busses, just update the early_qrk > >> table with a few more bits of information. > >> > > copy that. Fixed. Thanks! > > > >> The extended form should be much more maintainable in the long > >> run. Given that we may want this before we enable the timer > >> which is very early doing this in the pci early quirks seems > >> to make sense. > >> > >> Eric > > > > > > New patch attached, with suggestions incorporated. > > > > Thanks & regards > > Neil > > > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > > > > > > early-quirks.c | 82 > > ++--- > > 1 file changed, 73 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > > index 88bb83e..4b0cee1 100644 > > --- a/arch/x86/kernel/early-quirks.c > > +++ b/arch/x86/kernel/early-quirks.c > > @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct > > acpi_table_header > > *header) > > #endif /* CONFIG_X86_IO_APIC */ > > #endif /* CONFIG_ACPI */ > > > > +static void __init fix_hypertransport_config(int num, int slot, int func) > > +{ > > + u32 htcfg; > > + /* > > + *we found a hypertransport bus > > + *make sure that are broadcasting > > + *interrupts to all cpus on the ht bus > > + *if we're using extended apic ids > > + */ > > + htcfg = read_pci_config(num, slot, func, 0x68); > > + if (htcfg & (1 << 18)) { > > + printk(KERN_INFO "Detected use of extended apic ids on hypertransport > > bus\n"); > > + if ((htcfg & (1 << 17)) == 0) { > > + printk(KERN_INFO "Enabling hypertransport extended apic interrupt > > broadcast\n"); > > + htcfg |= (1 << 17); > > + write_pci_config(num, slot, func, 0x68, htcfg); > > + } > > + } > > + > > +} > > + > > +static void __init check_hypertransport_config() > > +{ > > + int num, slot, func; > > + u32 device, vendor; > > + func = 0; > > + for (num = 0; num < 32; num++) { > > + for (slot = 0; slot < 32; slot++) { > > + vendor = read_pci_config(num,slot,func, > > + PCI_VENDOR_ID); > > + device = read_pci_config(num,slot,func, > > + PCI_DEVICE_ID); > > + vendor &= 0x; > > + device >>= 16; > > + if ((vendor == PCI_VENDOR_ID_AMD) && > > + (device == PCI_DEVICE_ID_AMD_K8_NB)) > > + fix_hypertransport_config(num,slot,func); > > + } > > + } > > + > > + return; > > + > > +} > > We should not need check_hypertransport_config as the generic loop > now does the work for us. > > + > > static void __init nvidia_bugs(void) > > { > > #ifdef CONFIG_ACPI > > @@ -83,15 +127,25 @@ static void __init ati_bugs(void) > > #endif > > } > > > > +static void __init amd_host_bugs(void) > > +{ > > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); > > + check_hypertransport_config(); > > +} > > Likewise
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman <[EMAIL PROTECTED]> writes: Almost there. > On Mon, Dec 10, 2007 at 06:08:03PM -0700, Eric W. Biederman wrote: >> Neil Horman <[EMAIL PROTECTED]> writes: >> > >> >> Ok. This test is broken. Please remove the == 1. You are looking >> for == (1 << 18). So just saying: "if (htcfg & (1 << 18))" should be >> clearer. >> > Fixed. Thanks! > >> > + printk(KERN_INFO "Detected use of extended apic ids on hypertransport > bus\n"); >> > + if ((htcfg & (1 << 17)) == 0) { >> > + printk(KERN_INFO "Enabling hypertransport extended apic interrupt >> > broadcast\n"); >> > + htcfg |= (1 << 17); >> > + write_pci_config(num, slot, func, 0x68, htcfg); >> > + } >> > + } >> > + >> > +} >> >> The rest of this quirk looks fine, include the fact it is only intended >> to be applied to PCI_VENDOR_ID_AMD PCI_DEVICE_ID_AMD_K8_NB. >> > Copy that. > >> >> For what is below I don't like the way the infrastructure has been >> extended as what you are doing quickly devolves into a big mess. >> >> Please extend struct chipset to be something like: >> struct chipset { >> u16 vendor; >> u16 device; >> u32 class, class_mask; >> void (*f)(void); >> }; >> >> And then the test for matching the chipset can be something like: >> if ((id->vendor == PCI_ANY_ID || id->vendor == dev->vendor) && >> (id->device == PCI_ANY_ID || id->device == dev->device) && >> !((id->class ^ dev->class) & id->class_mask)) >> >> Essentially a subset of pci_match_one_device from drivers/pci/pci.h >> >> That way you don't need to increase the number of tables or the >> number of passes through the pci busses, just update the early_qrk >> table with a few more bits of information. >> > copy that. Fixed. Thanks! > >> The extended form should be much more maintainable in the long >> run. Given that we may want this before we enable the timer >> which is very early doing this in the pci early quirks seems >> to make sense. >> >> Eric > > > New patch attached, with suggestions incorporated. > > Thanks & regards > Neil > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > > > early-quirks.c | 82 ++--- > 1 file changed, 73 insertions(+), 9 deletions(-) > > > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > index 88bb83e..4b0cee1 100644 > --- a/arch/x86/kernel/early-quirks.c > +++ b/arch/x86/kernel/early-quirks.c > @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct > acpi_table_header > *header) > #endif /* CONFIG_X86_IO_APIC */ > #endif /* CONFIG_ACPI */ > > +static void __init fix_hypertransport_config(int num, int slot, int func) > +{ > + u32 htcfg; > + /* > + *we found a hypertransport bus > + *make sure that are broadcasting > + *interrupts to all cpus on the ht bus > + *if we're using extended apic ids > + */ > + htcfg = read_pci_config(num, slot, func, 0x68); > + if (htcfg & (1 << 18)) { > + printk(KERN_INFO "Detected use of extended apic ids on hypertransport > bus\n"); > + if ((htcfg & (1 << 17)) == 0) { > + printk(KERN_INFO "Enabling hypertransport extended apic interrupt > broadcast\n"); > + htcfg |= (1 << 17); > + write_pci_config(num, slot, func, 0x68, htcfg); > + } > + } > + > +} > + > +static void __init check_hypertransport_config() > +{ > + int num, slot, func; > + u32 device, vendor; > + func = 0; > + for (num = 0; num < 32; num++) { > + for (slot = 0; slot < 32; slot++) { > + vendor = read_pci_config(num,slot,func, > + PCI_VENDOR_ID); > + device = read_pci_config(num,slot,func, > + PCI_DEVICE_ID); > + vendor &= 0x; > + device >>= 16; > + if ((vendor == PCI_VENDOR_ID_AMD) && > + (device == PCI_DEVICE_ID_AMD_K8_NB)) > + fix_hypertransport_config(num,slot,func); > + } > + } > + > + return; > + > +} We should not need check_hypertransport_config as the generic loop now does the work for us. > + > static void __init nvidia_bugs(void) > { > #ifdef CONFIG_ACPI > @@ -83,15 +127,25 @@ static void __init ati_bugs(void) > #endif > } > > +static void __init amd_host_bugs(void) > +{ > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); > + check_hypertransport_config(); > +} Likewise this function is unneeded and the printk is likely confusing for users. > struct chipset { > u16 vendor; > + u16 device; > + u32 class; > + u32 class_mask; > void (*f)(void); > }; > > static struct chipset early_qrk[] __initdata = { > - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, > - { PCI_VENDOR_ID_VIA,
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Mon, Dec 10, 2007 at 06:08:03PM -0700, Eric W. Biederman wrote: > Neil Horman <[EMAIL PROTECTED]> writes: > > > Ok. This test is broken. Please remove the == 1. You are looking > for == (1 << 18). So just saying: "if (htcfg & (1 << 18))" should be clearer. > Fixed. Thanks! > > + printk(KERN_INFO "Detected use of extended apic ids on hypertransport > > bus\n"); > > + if ((htcfg & (1 << 17)) == 0) { > > + printk(KERN_INFO "Enabling hypertransport extended apic interrupt > > broadcast\n"); > > + htcfg |= (1 << 17); > > + write_pci_config(num, slot, func, 0x68, htcfg); > > + } > > + } > > + > > +} > > The rest of this quirk looks fine, include the fact it is only intended > to be applied to PCI_VENDOR_ID_AMD PCI_DEVICE_ID_AMD_K8_NB. > Copy that. > > For what is below I don't like the way the infrastructure has been > extended as what you are doing quickly devolves into a big mess. > > Please extend struct chipset to be something like: > struct chipset { > u16 vendor; > u16 device; > u32 class, class_mask; > void (*f)(void); > }; > > And then the test for matching the chipset can be something like: > if ((id->vendor == PCI_ANY_ID || id->vendor == dev->vendor) && > (id->device == PCI_ANY_ID || id->device == dev->device) && > !((id->class ^ dev->class) & id->class_mask)) > > Essentially a subset of pci_match_one_device from drivers/pci/pci.h > > That way you don't need to increase the number of tables or the > number of passes through the pci busses, just update the early_qrk > table with a few more bits of information. > copy that. Fixed. Thanks! > The extended form should be much more maintainable in the long > run. Given that we may want this before we enable the timer > which is very early doing this in the pci early quirks seems > to make sense. > > Eric New patch attached, with suggestions incorporated. Thanks & regards Neil Signed-off-by: Neil Horman <[EMAIL PROTECTED]> early-quirks.c | 82 ++--- 1 file changed, 73 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..4b0cee1 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg & (1 << 18)) { + printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n"); + if ((htcfg & (1 << 17)) == 0) { + printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n"); + htcfg |= (1 << 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num < 32; num++) { + for (slot = 0; slot < 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor &= 0x; + device >>= 16; + if ((vendor == PCI_VENDOR_ID_AMD) && + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,15 +127,25 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); + check_hypertransport_config(); +} + struct chipset { u16 vendor; + u16 device; + u32 class; + u32 class_mask; void (*f)(void); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + {
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
> Sorry to reply to myself, but do we have consensus on this patch? I'd like to > figure out its disposition if possible. What the patch tries to do looks like the right thing. So if we can get a version that is clean and actually works we should merge it. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman <[EMAIL PROTECTED]> writes: > On Fri, Dec 07, 2007 at 09:21:44AM -0500, Neil Horman wrote: >> On Fri, Dec 07, 2007 at 01:22:04AM -0800, Yinghai Lu wrote: >> > On Dec 7, 2007 12:50 AM, Yinghai Lu <[EMAIL PROTECTED]> wrote: >> > > >> > > On Dec 6, 2007 4:33 PM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: >> > ... >> > > > >> > > > My feel is that if it is for legacy interrupts only it should not be a > problem. >> > > > Let's investigate and see if we can unconditionally enable this quirk >> > > > for all opteron systems. >> > > >> > > i checked that bit >> > > >> > > > http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596=markup > >> > >> > it should be bit 18 (HTTC_APIC_EXT_ID) >> > >> > >> > YH >> >> this seems reasonable, I can reroll the patch for this. As I think about it > I'm >> also going to update the patch to make this check occur for any pci class >> 0600 >> device from vendor AMD, since its possible that more than just nvidia >> chipsets >> can be affected. >> >> I'll repost as soon as I've tested, thanks! >> Neil > > > Ok, New patch attached. It preforms the same function as previously > described, > but is more restricted in its application. As Yinghai pointed out, the > broadcast mask bit (bit 17 in the htcfg register) should only be enabled, if > the > extened apic id bit (bit 18 in the same register) is also set. So this patch > now check for that bit to be turned on first. Also, this patch now adds an > independent quirk check for all AMD hypertransport host controllers, since its > possible for this misconfiguration to be present in systems other than > nvidias. > The net effect of these changes is, that its now applicable to all AMD systems > containing hypertransport busses, and is only activated if extended apic ids > are > in use, meaning that this quirk guarantees that all processors in a system are > elligible to receive interrupts from the ioapic, even if their apicid extends > beyond the nominal 4 bit limitation. Tested successfully by me. > > Thanks & Regards > Neil > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > > > early-quirks.c | 83 - > 1 file changed, 76 insertions(+), 7 deletions(-) > > > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > index 88bb83e..d5a7b30 100644 > --- a/arch/x86/kernel/early-quirks.c > +++ b/arch/x86/kernel/early-quirks.c > @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct > acpi_table_header > *header) > #endif /* CONFIG_X86_IO_APIC */ > #endif /* CONFIG_ACPI */ > > +static void __init fix_hypertransport_config(int num, int slot, int func) > +{ > + u32 htcfg; > + /* > + *we found a hypertransport bus > + *make sure that are broadcasting > + *interrupts to all cpus on the ht bus > + *if we're using extended apic ids > + */ > + htcfg = read_pci_config(num, slot, func, 0x68); > + if ((htcfg & (1 << 18)) == 1) { Ok. This test is broken. Please remove the == 1. You are looking for == (1 << 18). So just saying: "if (htcfg & (1 << 18))" should be clearer. > + printk(KERN_INFO "Detected use of extended apic ids on hypertransport > bus\n"); > + if ((htcfg & (1 << 17)) == 0) { > + printk(KERN_INFO "Enabling hypertransport extended apic interrupt > broadcast\n"); > + htcfg |= (1 << 17); > + write_pci_config(num, slot, func, 0x68, htcfg); > + } > + } > + > +} The rest of this quirk looks fine, include the fact it is only intended to be applied to PCI_VENDOR_ID_AMD PCI_DEVICE_ID_AMD_K8_NB. For what is below I don't like the way the infrastructure has been extended as what you are doing quickly devolves into a big mess. Please extend struct chipset to be something like: struct chipset { u16 vendor; u16 device; u32 class, class_mask; void (*f)(void); }; And then the test for matching the chipset can be something like: if ((id->vendor == PCI_ANY_ID || id->vendor == dev->vendor) && (id->device == PCI_ANY_ID || id->device == dev->device) && !((id->class ^ dev->class) & id->class_mask)) Essentially a subset of pci_match_one_device from drivers/pci/pci.h That way you don't need to increase the number of tables or the number of passes through the pci busses, just update the early_qrk table with a few more bits of information. The extended form should be much more maintainable in the long run. Given that we may want this before we enable the timer which is very early doing this in the pci early quirks seems to make sense. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Mon, Dec 10, 2007 at 10:39:59AM -0500, Neil Horman wrote: > On Fri, Dec 07, 2007 at 12:58:32PM -0500, Neil Horman wrote: > > > > Ok, New patch attached. It preforms the same function as previously > > described, > > but is more restricted in its application. As Yinghai pointed out, the > > broadcast mask bit (bit 17 in the htcfg register) should only be enabled, > > if the > > extened apic id bit (bit 18 in the same register) is also set. So this > > patch > > now check for that bit to be turned on first. Also, this patch now adds an > > independent quirk check for all AMD hypertransport host controllers, since > > its > > possible for this misconfiguration to be present in systems other than > > nvidias. > > The net effect of these changes is, that its now applicable to all AMD > > systems > > containing hypertransport busses, and is only activated if extended apic > > ids are > > in use, meaning that this quirk guarantees that all processors in a system > > are > > elligible to receive interrupts from the ioapic, even if their apicid > > extends > > beyond the nominal 4 bit limitation. Tested successfully by me. > > > > Thanks & Regards > > Neil > > > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > > > > > > early-quirks.c | 83 > > - > > 1 file changed, 76 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > > index 88bb83e..d5a7b30 100644 > > --- a/arch/x86/kernel/early-quirks.c > > +++ b/arch/x86/kernel/early-quirks.c > > @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct > > acpi_table_header *header) > > #endif /* CONFIG_X86_IO_APIC */ > > #endif /* CONFIG_ACPI */ > > > > +static void __init fix_hypertransport_config(int num, int slot, int func) > > +{ > > + u32 htcfg; > > + /* > > +*we found a hypertransport bus > > +*make sure that are broadcasting > > +*interrupts to all cpus on the ht bus > > +*if we're using extended apic ids > > +*/ > > + htcfg = read_pci_config(num, slot, func, 0x68); > > + if ((htcfg & (1 << 18)) == 1) { > > + printk(KERN_INFO "Detected use of extended apic ids on > > hypertransport bus\n"); > > + if ((htcfg & (1 << 17)) == 0) { > > + printk(KERN_INFO "Enabling hypertransport extended apic > > interrupt broadcast\n"); > > + htcfg |= (1 << 17); > > + write_pci_config(num, slot, func, 0x68, htcfg); > > + } > > + } > > + > > +} > > + > > +static void __init check_hypertransport_config() > > +{ > > + int num, slot, func; > > + u32 device, vendor; > > + func = 0; > > + for (num = 0; num < 32; num++) { > > + for (slot = 0; slot < 32; slot++) { > > + vendor = read_pci_config(num,slot,func, > > + PCI_VENDOR_ID); > > + device = read_pci_config(num,slot,func, > > + PCI_DEVICE_ID); > > + vendor &= 0x; > > + device >>= 16; > > + if ((vendor == PCI_VENDOR_ID_AMD) && > > + (device == PCI_DEVICE_ID_AMD_K8_NB)) > > + fix_hypertransport_config(num,slot,func); > > + } > > + } > > + > > + return; > > + > > +} > > + > > static void __init nvidia_bugs(void) > > { > > #ifdef CONFIG_ACPI > > @@ -83,6 +127,12 @@ static void __init ati_bugs(void) > > #endif > > } > > > > +static void __init amd_host_bugs(void) > > +{ > > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); > > + check_hypertransport_config(); > > +} > > + > > struct chipset { > > u16 vendor; > > void (*f)(void); > > @@ -95,9 +145,16 @@ static struct chipset early_qrk[] __initdata = { > > {} > > }; > > > > +static struct chipset early_host_qrk[] __initdata = { > > + { PCI_VENDOR_ID_AMD, amd_host_bugs}, > > + {} > > +}; > > + > > void __init early_quirks(void) > > { > > int num, slot, func; > > + u8 found_bridge = 0; > > + u8 found_host = 0; > > > > if (!early_pci_allowed()) > > return; > > @@ -115,18 +172,30 @@ void __init early_quirks(void) > > if (class == 0x) > > break; > > > > - if ((class >> 16) != PCI_CLASS_BRIDGE_PCI) > > + class >>= 16; > > + if ((class != PCI_CLASS_BRIDGE_PCI) && > > + (class != PCI_CLASS_BRIDGE_HOST)) > > continue; > > > > vendor = read_pci_config(num, slot, func, > > PCI_VENDOR_ID); > > vendor &= 0x; > > - > > - for (i = 0; early_qrk[i].f; i++) > > -
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 12:58:32PM -0500, Neil Horman wrote: > > Ok, New patch attached. It preforms the same function as previously > described, > but is more restricted in its application. As Yinghai pointed out, the > broadcast mask bit (bit 17 in the htcfg register) should only be enabled, if > the > extened apic id bit (bit 18 in the same register) is also set. So this patch > now check for that bit to be turned on first. Also, this patch now adds an > independent quirk check for all AMD hypertransport host controllers, since its > possible for this misconfiguration to be present in systems other than > nvidias. > The net effect of these changes is, that its now applicable to all AMD systems > containing hypertransport busses, and is only activated if extended apic ids > are > in use, meaning that this quirk guarantees that all processors in a system are > elligible to receive interrupts from the ioapic, even if their apicid extends > beyond the nominal 4 bit limitation. Tested successfully by me. > > Thanks & Regards > Neil > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > > > early-quirks.c | 83 > - > 1 file changed, 76 insertions(+), 7 deletions(-) > > > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > index 88bb83e..d5a7b30 100644 > --- a/arch/x86/kernel/early-quirks.c > +++ b/arch/x86/kernel/early-quirks.c > @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct > acpi_table_header *header) > #endif /* CONFIG_X86_IO_APIC */ > #endif /* CONFIG_ACPI */ > > +static void __init fix_hypertransport_config(int num, int slot, int func) > +{ > + u32 htcfg; > + /* > + *we found a hypertransport bus > + *make sure that are broadcasting > + *interrupts to all cpus on the ht bus > + *if we're using extended apic ids > + */ > + htcfg = read_pci_config(num, slot, func, 0x68); > + if ((htcfg & (1 << 18)) == 1) { > + printk(KERN_INFO "Detected use of extended apic ids on > hypertransport bus\n"); > + if ((htcfg & (1 << 17)) == 0) { > + printk(KERN_INFO "Enabling hypertransport extended apic > interrupt broadcast\n"); > + htcfg |= (1 << 17); > + write_pci_config(num, slot, func, 0x68, htcfg); > + } > + } > + > +} > + > +static void __init check_hypertransport_config() > +{ > + int num, slot, func; > + u32 device, vendor; > + func = 0; > + for (num = 0; num < 32; num++) { > + for (slot = 0; slot < 32; slot++) { > + vendor = read_pci_config(num,slot,func, > + PCI_VENDOR_ID); > + device = read_pci_config(num,slot,func, > + PCI_DEVICE_ID); > + vendor &= 0x; > + device >>= 16; > + if ((vendor == PCI_VENDOR_ID_AMD) && > + (device == PCI_DEVICE_ID_AMD_K8_NB)) > + fix_hypertransport_config(num,slot,func); > + } > + } > + > + return; > + > +} > + > static void __init nvidia_bugs(void) > { > #ifdef CONFIG_ACPI > @@ -83,6 +127,12 @@ static void __init ati_bugs(void) > #endif > } > > +static void __init amd_host_bugs(void) > +{ > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); > + check_hypertransport_config(); > +} > + > struct chipset { > u16 vendor; > void (*f)(void); > @@ -95,9 +145,16 @@ static struct chipset early_qrk[] __initdata = { > {} > }; > > +static struct chipset early_host_qrk[] __initdata = { > + { PCI_VENDOR_ID_AMD, amd_host_bugs}, > + {} > +}; > + > void __init early_quirks(void) > { > int num, slot, func; > + u8 found_bridge = 0; > + u8 found_host = 0; > > if (!early_pci_allowed()) > return; > @@ -115,18 +172,30 @@ void __init early_quirks(void) > if (class == 0x) > break; > > - if ((class >> 16) != PCI_CLASS_BRIDGE_PCI) > + class >>= 16; > + if ((class != PCI_CLASS_BRIDGE_PCI) && > + (class != PCI_CLASS_BRIDGE_HOST)) > continue; > > vendor = read_pci_config(num, slot, func, >PCI_VENDOR_ID); > vendor &= 0x; > - > - for (i = 0; early_qrk[i].f; i++) > - if (early_qrk[i].vendor == vendor) { > - early_qrk[i].f(); > - return; > - } > +
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 12:58:32PM -0500, Neil Horman wrote: Ok, New patch attached. It preforms the same function as previously described, but is more restricted in its application. As Yinghai pointed out, the broadcast mask bit (bit 17 in the htcfg register) should only be enabled, if the extened apic id bit (bit 18 in the same register) is also set. So this patch now check for that bit to be turned on first. Also, this patch now adds an independent quirk check for all AMD hypertransport host controllers, since its possible for this misconfiguration to be present in systems other than nvidias. The net effect of these changes is, that its now applicable to all AMD systems containing hypertransport busses, and is only activated if extended apic ids are in use, meaning that this quirk guarantees that all processors in a system are elligible to receive interrupts from the ioapic, even if their apicid extends beyond the nominal 4 bit limitation. Tested successfully by me. Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 83 - 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..d5a7b30 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* + *we found a hypertransport bus + *make sure that are broadcasting + *interrupts to all cpus on the ht bus + *if we're using extended apic ids + */ + htcfg = read_pci_config(num, slot, func, 0x68); + if ((htcfg (1 18)) == 1) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num 32; num++) { + for (slot = 0; slot 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor = 0x; + device = 16; + if ((vendor == PCI_VENDOR_ID_AMD) + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,6 +127,12 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} + struct chipset { u16 vendor; void (*f)(void); @@ -95,9 +145,16 @@ static struct chipset early_qrk[] __initdata = { {} }; +static struct chipset early_host_qrk[] __initdata = { + { PCI_VENDOR_ID_AMD, amd_host_bugs}, + {} +}; + void __init early_quirks(void) { int num, slot, func; + u8 found_bridge = 0; + u8 found_host = 0; if (!early_pci_allowed()) return; @@ -115,18 +172,30 @@ void __init early_quirks(void) if (class == 0x) break; - if ((class 16) != PCI_CLASS_BRIDGE_PCI) + class = 16; + if ((class != PCI_CLASS_BRIDGE_PCI) + (class != PCI_CLASS_BRIDGE_HOST)) continue; vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID); vendor = 0x; - - for (i = 0; early_qrk[i].f; i++) - if (early_qrk[i].vendor == vendor) { - early_qrk[i].f(); - return; - } + if ((class == PCI_CLASS_BRIDGE_PCI) (!found_bridge)) { + for (i = 0; early_qrk[i].f; i++) +
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Mon, Dec 10, 2007 at 10:39:59AM -0500, Neil Horman wrote: On Fri, Dec 07, 2007 at 12:58:32PM -0500, Neil Horman wrote: Ok, New patch attached. It preforms the same function as previously described, but is more restricted in its application. As Yinghai pointed out, the broadcast mask bit (bit 17 in the htcfg register) should only be enabled, if the extened apic id bit (bit 18 in the same register) is also set. So this patch now check for that bit to be turned on first. Also, this patch now adds an independent quirk check for all AMD hypertransport host controllers, since its possible for this misconfiguration to be present in systems other than nvidias. The net effect of these changes is, that its now applicable to all AMD systems containing hypertransport busses, and is only activated if extended apic ids are in use, meaning that this quirk guarantees that all processors in a system are elligible to receive interrupts from the ioapic, even if their apicid extends beyond the nominal 4 bit limitation. Tested successfully by me. Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 83 - 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..d5a7b30 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if ((htcfg (1 18)) == 1) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num 32; num++) { + for (slot = 0; slot 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor = 0x; + device = 16; + if ((vendor == PCI_VENDOR_ID_AMD) + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,6 +127,12 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} + struct chipset { u16 vendor; void (*f)(void); @@ -95,9 +145,16 @@ static struct chipset early_qrk[] __initdata = { {} }; +static struct chipset early_host_qrk[] __initdata = { + { PCI_VENDOR_ID_AMD, amd_host_bugs}, + {} +}; + void __init early_quirks(void) { int num, slot, func; + u8 found_bridge = 0; + u8 found_host = 0; if (!early_pci_allowed()) return; @@ -115,18 +172,30 @@ void __init early_quirks(void) if (class == 0x) break; - if ((class 16) != PCI_CLASS_BRIDGE_PCI) + class = 16; + if ((class != PCI_CLASS_BRIDGE_PCI) + (class != PCI_CLASS_BRIDGE_HOST)) continue; vendor = read_pci_config(num, slot, func, PCI_VENDOR_ID); vendor = 0x; - - for (i = 0; early_qrk[i].f; i++) - if (early_qrk[i].vendor == vendor) { - early_qrk[i].f(); - return; - } + if ((class == PCI_CLASS_BRIDGE_PCI) (!found_bridge)) {
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman [EMAIL PROTECTED] writes: On Fri, Dec 07, 2007 at 09:21:44AM -0500, Neil Horman wrote: On Fri, Dec 07, 2007 at 01:22:04AM -0800, Yinghai Lu wrote: On Dec 7, 2007 12:50 AM, Yinghai Lu [EMAIL PROTECTED] wrote: On Dec 6, 2007 4:33 PM, Eric W. Biederman [EMAIL PROTECTED] wrote: ... My feel is that if it is for legacy interrupts only it should not be a problem. Let's investigate and see if we can unconditionally enable this quirk for all opteron systems. i checked that bit http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596view=markup snip it should be bit 18 (HTTC_APIC_EXT_ID) YH this seems reasonable, I can reroll the patch for this. As I think about it I'm also going to update the patch to make this check occur for any pci class 0600 device from vendor AMD, since its possible that more than just nvidia chipsets can be affected. I'll repost as soon as I've tested, thanks! Neil Ok, New patch attached. It preforms the same function as previously described, but is more restricted in its application. As Yinghai pointed out, the broadcast mask bit (bit 17 in the htcfg register) should only be enabled, if the extened apic id bit (bit 18 in the same register) is also set. So this patch now check for that bit to be turned on first. Also, this patch now adds an independent quirk check for all AMD hypertransport host controllers, since its possible for this misconfiguration to be present in systems other than nvidias. The net effect of these changes is, that its now applicable to all AMD systems containing hypertransport busses, and is only activated if extended apic ids are in use, meaning that this quirk guarantees that all processors in a system are elligible to receive interrupts from the ioapic, even if their apicid extends beyond the nominal 4 bit limitation. Tested successfully by me. Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 83 - 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..d5a7b30 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* + *we found a hypertransport bus + *make sure that are broadcasting + *interrupts to all cpus on the ht bus + *if we're using extended apic ids + */ + htcfg = read_pci_config(num, slot, func, 0x68); + if ((htcfg (1 18)) == 1) { Ok. This test is broken. Please remove the == 1. You are looking for == (1 18). So just saying: if (htcfg (1 18)) should be clearer. + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} The rest of this quirk looks fine, include the fact it is only intended to be applied to PCI_VENDOR_ID_AMD PCI_DEVICE_ID_AMD_K8_NB. For what is below I don't like the way the infrastructure has been extended as what you are doing quickly devolves into a big mess. Please extend struct chipset to be something like: struct chipset { u16 vendor; u16 device; u32 class, class_mask; void (*f)(void); }; And then the test for matching the chipset can be something like: if ((id-vendor == PCI_ANY_ID || id-vendor == dev-vendor) (id-device == PCI_ANY_ID || id-device == dev-device) !((id-class ^ dev-class) id-class_mask)) Essentially a subset of pci_match_one_device from drivers/pci/pci.h That way you don't need to increase the number of tables or the number of passes through the pci busses, just update the early_qrk table with a few more bits of information. The extended form should be much more maintainable in the long run. Given that we may want this before we enable the timer which is very early doing this in the pci early quirks seems to make sense. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Sorry to reply to myself, but do we have consensus on this patch? I'd like to figure out its disposition if possible. What the patch tries to do looks like the right thing. So if we can get a version that is clean and actually works we should merge it. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Mon, Dec 10, 2007 at 06:08:03PM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: snip Ok. This test is broken. Please remove the == 1. You are looking for == (1 18). So just saying: if (htcfg (1 18)) should be clearer. Fixed. Thanks! + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} The rest of this quirk looks fine, include the fact it is only intended to be applied to PCI_VENDOR_ID_AMD PCI_DEVICE_ID_AMD_K8_NB. Copy that. For what is below I don't like the way the infrastructure has been extended as what you are doing quickly devolves into a big mess. Please extend struct chipset to be something like: struct chipset { u16 vendor; u16 device; u32 class, class_mask; void (*f)(void); }; And then the test for matching the chipset can be something like: if ((id-vendor == PCI_ANY_ID || id-vendor == dev-vendor) (id-device == PCI_ANY_ID || id-device == dev-device) !((id-class ^ dev-class) id-class_mask)) Essentially a subset of pci_match_one_device from drivers/pci/pci.h That way you don't need to increase the number of tables or the number of passes through the pci busses, just update the early_qrk table with a few more bits of information. copy that. Fixed. Thanks! The extended form should be much more maintainable in the long run. Given that we may want this before we enable the timer which is very early doing this in the pci early quirks seems to make sense. Eric New patch attached, with suggestions incorporated. Thanks regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 82 ++--- 1 file changed, 73 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..4b0cee1 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num 32; num++) { + for (slot = 0; slot 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor = 0x; + device = 16; + if ((vendor == PCI_VENDOR_ID_AMD) + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,15 +127,25 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} + struct chipset { u16 vendor; + u16 device; + u32 class; + u32 class_mask; void (*f)(void); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, ati_bugs }, + { PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_K8_NB, PCI_CLASS_BRIDGE_HOST, PCI_ANY_ID, amd_host_bugs }, {} }; @@ -108,25
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman [EMAIL PROTECTED] writes: Almost there. On Mon, Dec 10, 2007 at 06:08:03PM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: snip Ok. This test is broken. Please remove the == 1. You are looking for == (1 18). So just saying: if (htcfg (1 18)) should be clearer. Fixed. Thanks! + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} The rest of this quirk looks fine, include the fact it is only intended to be applied to PCI_VENDOR_ID_AMD PCI_DEVICE_ID_AMD_K8_NB. Copy that. For what is below I don't like the way the infrastructure has been extended as what you are doing quickly devolves into a big mess. Please extend struct chipset to be something like: struct chipset { u16 vendor; u16 device; u32 class, class_mask; void (*f)(void); }; And then the test for matching the chipset can be something like: if ((id-vendor == PCI_ANY_ID || id-vendor == dev-vendor) (id-device == PCI_ANY_ID || id-device == dev-device) !((id-class ^ dev-class) id-class_mask)) Essentially a subset of pci_match_one_device from drivers/pci/pci.h That way you don't need to increase the number of tables or the number of passes through the pci busses, just update the early_qrk table with a few more bits of information. copy that. Fixed. Thanks! The extended form should be much more maintainable in the long run. Given that we may want this before we enable the timer which is very early doing this in the pci early quirks seems to make sense. Eric New patch attached, with suggestions incorporated. Thanks regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 82 ++--- 1 file changed, 73 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..4b0cee1 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* + *we found a hypertransport bus + *make sure that are broadcasting + *interrupts to all cpus on the ht bus + *if we're using extended apic ids + */ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num 32; num++) { + for (slot = 0; slot 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor = 0x; + device = 16; + if ((vendor == PCI_VENDOR_ID_AMD) + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} We should not need check_hypertransport_config as the generic loop now does the work for us. + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,15 +127,25 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} Likewise this function is unneeded and the printk is likely confusing for users. struct chipset { u16 vendor; + u16 device; + u32 class; + u32 class_mask; void (*f)(void); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + { PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, nvidia_bugs }, + { PCI_VENDOR_ID_VIA, PCI_ANY_ID, PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, via_bugs }, + { PCI_VENDOR_ID_ATI, PCI_ANY_ID,
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 10, 2007 8:48 PM, Eric W. Biederman [EMAIL PROTECTED] wrote: Neil Horman [EMAIL PROTECTED] writes: Almost there. On Mon, Dec 10, 2007 at 06:08:03PM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: snip Ok. This test is broken. Please remove the == 1. You are looking for == (1 18). So just saying: if (htcfg (1 18)) should be clearer. Fixed. Thanks! + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} The rest of this quirk looks fine, include the fact it is only intended to be applied to PCI_VENDOR_ID_AMD PCI_DEVICE_ID_AMD_K8_NB. Copy that. For what is below I don't like the way the infrastructure has been extended as what you are doing quickly devolves into a big mess. Please extend struct chipset to be something like: struct chipset { u16 vendor; u16 device; u32 class, class_mask; void (*f)(void); }; And then the test for matching the chipset can be something like: if ((id-vendor == PCI_ANY_ID || id-vendor == dev-vendor) (id-device == PCI_ANY_ID || id-device == dev-device) !((id-class ^ dev-class) id-class_mask)) Essentially a subset of pci_match_one_device from drivers/pci/pci.h That way you don't need to increase the number of tables or the number of passes through the pci busses, just update the early_qrk table with a few more bits of information. copy that. Fixed. Thanks! The extended form should be much more maintainable in the long run. Given that we may want this before we enable the timer which is very early doing this in the pci early quirks seems to make sense. Eric New patch attached, with suggestions incorporated. Thanks regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 82 ++--- 1 file changed, 73 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..4b0cee1 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* + *we found a hypertransport bus + *make sure that are broadcasting + *interrupts to all cpus on the ht bus + *if we're using extended apic ids + */ + htcfg = read_pci_config(num, slot, func, 0x68); + if (htcfg (1 18)) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num 32; num++) { + for (slot = 0; slot 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor = 0x; + device = 16; + if ((vendor == PCI_VENDOR_ID_AMD) + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} We should not need check_hypertransport_config as the generic loop now does the work for us. + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,15 +127,25 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} Likewise this function is unneeded and the printk is likely confusing for users. struct chipset { u16 vendor; + u16 device; + u32 class; + u32 class_mask; void (*f)(void); }; static struct chipset early_qrk[] __initdata = { - { PCI_VENDOR_ID_NVIDIA, nvidia_bugs }, - { PCI_VENDOR_ID_VIA, via_bugs }, - { PCI_VENDOR_ID_ATI, ati_bugs }, + {
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 11:19:10AM -0800, yhlu wrote: > On Dec 7, 2007 9:58 AM, Neil Horman <[EMAIL PROTECTED]> wrote: > > On Fri, Dec 07, 2007 at 09:21:44AM -0500, Neil Horman wrote: > > > On Fri, Dec 07, 2007 at 01:22:04AM -0800, Yinghai Lu wrote: > > > > On Dec 7, 2007 12:50 AM, Yinghai Lu <[EMAIL PROTECTED]> wrote: > > > > > > > > > > On Dec 6, 2007 4:33 PM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > > > > ... > > > > > > > > > > > > My feel is that if it is for legacy interrupts only it should not > > > > > > be a problem. > > > > > > Let's investigate and see if we can unconditionally enable this > > > > > > quirk > > > > > > for all opteron systems. > > > > > > > > > > i checked that bit > > > > > > > > > > http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596=markup > > > > > > > > > > it should be bit 18 (HTTC_APIC_EXT_ID) > > > > > > > > > > > > YH > > > > > > this seems reasonable, I can reroll the patch for this. As I think about > > > it I'm > > > also going to update the patch to make this check occur for any pci class > > > 0600 > > > device from vendor AMD, since its possible that more than just nvidia > > > chipsets > > > can be affected. > > > > > > I'll repost as soon as I've tested, thanks! > > > Neil > > > > > > Ok, New patch attached. It preforms the same function as previously > > described, > > but is more restricted in its application. As Yinghai pointed out, the > > broadcast mask bit (bit 17 in the htcfg register) should only be enabled, > > if the > > extened apic id bit (bit 18 in the same register) is also set. So this > > patch > > now check for that bit to be turned on first. Also, this patch now adds an > > independent quirk check for all AMD hypertransport host controllers, since > > its > > possible for this misconfiguration to be present in systems other than > > nvidias. > > The net effect of these changes is, that its now applicable to all AMD > > systems > > containing hypertransport busses, and is only activated if extended apic > > ids are > > in use, meaning that this quirk guarantees that all processors in a system > > are > > elligible to receive interrupts from the ioapic, even if their apicid > > extends > > beyond the nominal 4 bit limitation. Tested successfully by me. > > > > Thanks & Regards > > Neil > > > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > > > > > > early-quirks.c | 83 > > - > > 1 file changed, 76 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > > index 88bb83e..d5a7b30 100644 > > --- a/arch/x86/kernel/early-quirks.c > > +++ b/arch/x86/kernel/early-quirks.c > > @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct > > acpi_table_header *header) > > #endif /* CONFIG_X86_IO_APIC */ > > #endif /* CONFIG_ACPI */ > > > > +static void __init fix_hypertransport_config(int num, int slot, int func) > > +{ > > + u32 htcfg; > > + /* > > +*we found a hypertransport bus > > +*make sure that are broadcasting > > +*interrupts to all cpus on the ht bus > > +*if we're using extended apic ids > > +*/ > > + htcfg = read_pci_config(num, slot, func, 0x68); > > + if ((htcfg & (1 << 18)) == 1) { > > + printk(KERN_INFO "Detected use of extended apic ids on > > hypertransport bus\n"); > > + if ((htcfg & (1 << 17)) == 0) { > > + printk(KERN_INFO "Enabling hypertransport extended > > apic interrupt broadcast\n"); > > + htcfg |= (1 << 17); > > + write_pci_config(num, slot, func, 0x68, htcfg); > > + } > > + } > > + > > +} > > + > > +static void __init check_hypertransport_config() > > +{ > > + int num, slot, func; > > + u32 device, vendor; > > + func = 0; > > + for (num = 0; num < 32; num++) { > > + for (slot = 0; slot < 32; slot++) { > > + vendor = read_pci_config(num,slot,func, > > + PCI_VENDOR_ID); > > + device = read_pci_config(num,slot,func, > > + PCI_DEVICE_ID); > > + vendor &= 0x; > > + device >>= 16; > > + if ((vendor == PCI_VENDOR_ID_AMD) && > > + (device == PCI_DEVICE_ID_AMD_K8_NB)) > > + fix_hypertransport_config(num,slot,func); > > + } > > + } > > + > > + return; > > + > > +} > > + > > static void __init nvidia_bugs(void) > > { > > #ifdef CONFIG_ACPI > > @@ -83,6 +127,12 @@ static void __init ati_bugs(void) > > #endif > > } > > > > +static void __init amd_host_bugs(void) > > +{ > > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); > >
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 7, 2007 9:58 AM, Neil Horman <[EMAIL PROTECTED]> wrote: > On Fri, Dec 07, 2007 at 09:21:44AM -0500, Neil Horman wrote: > > On Fri, Dec 07, 2007 at 01:22:04AM -0800, Yinghai Lu wrote: > > > On Dec 7, 2007 12:50 AM, Yinghai Lu <[EMAIL PROTECTED]> wrote: > > > > > > > > On Dec 6, 2007 4:33 PM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > > > ... > > > > > > > > > > My feel is that if it is for legacy interrupts only it should not be > > > > > a problem. > > > > > Let's investigate and see if we can unconditionally enable this quirk > > > > > for all opteron systems. > > > > > > > > i checked that bit > > > > > > > > http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596=markup > > > > > > > it should be bit 18 (HTTC_APIC_EXT_ID) > > > > > > > > > YH > > > > this seems reasonable, I can reroll the patch for this. As I think about > > it I'm > > also going to update the patch to make this check occur for any pci class > > 0600 > > device from vendor AMD, since its possible that more than just nvidia > > chipsets > > can be affected. > > > > I'll repost as soon as I've tested, thanks! > > Neil > > > Ok, New patch attached. It preforms the same function as previously > described, > but is more restricted in its application. As Yinghai pointed out, the > broadcast mask bit (bit 17 in the htcfg register) should only be enabled, if > the > extened apic id bit (bit 18 in the same register) is also set. So this patch > now check for that bit to be turned on first. Also, this patch now adds an > independent quirk check for all AMD hypertransport host controllers, since its > possible for this misconfiguration to be present in systems other than > nvidias. > The net effect of these changes is, that its now applicable to all AMD systems > containing hypertransport busses, and is only activated if extended apic ids > are > in use, meaning that this quirk guarantees that all processors in a system are > elligible to receive interrupts from the ioapic, even if their apicid extends > beyond the nominal 4 bit limitation. Tested successfully by me. > > Thanks & Regards > Neil > > Signed-off-by: Neil Horman <[EMAIL PROTECTED]> > > > early-quirks.c | 83 > - > 1 file changed, 76 insertions(+), 7 deletions(-) > > > > diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c > index 88bb83e..d5a7b30 100644 > --- a/arch/x86/kernel/early-quirks.c > +++ b/arch/x86/kernel/early-quirks.c > @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct > acpi_table_header *header) > #endif /* CONFIG_X86_IO_APIC */ > #endif /* CONFIG_ACPI */ > > +static void __init fix_hypertransport_config(int num, int slot, int func) > +{ > + u32 htcfg; > + /* > +*we found a hypertransport bus > +*make sure that are broadcasting > +*interrupts to all cpus on the ht bus > +*if we're using extended apic ids > +*/ > + htcfg = read_pci_config(num, slot, func, 0x68); > + if ((htcfg & (1 << 18)) == 1) { > + printk(KERN_INFO "Detected use of extended apic ids on > hypertransport bus\n"); > + if ((htcfg & (1 << 17)) == 0) { > + printk(KERN_INFO "Enabling hypertransport extended > apic interrupt broadcast\n"); > + htcfg |= (1 << 17); > + write_pci_config(num, slot, func, 0x68, htcfg); > + } > + } > + > +} > + > +static void __init check_hypertransport_config() > +{ > + int num, slot, func; > + u32 device, vendor; > + func = 0; > + for (num = 0; num < 32; num++) { > + for (slot = 0; slot < 32; slot++) { > + vendor = read_pci_config(num,slot,func, > + PCI_VENDOR_ID); > + device = read_pci_config(num,slot,func, > + PCI_DEVICE_ID); > + vendor &= 0x; > + device >>= 16; > + if ((vendor == PCI_VENDOR_ID_AMD) && > + (device == PCI_DEVICE_ID_AMD_K8_NB)) > + fix_hypertransport_config(num,slot,func); > + } > + } > + > + return; > + > +} > + > static void __init nvidia_bugs(void) > { > #ifdef CONFIG_ACPI > @@ -83,6 +127,12 @@ static void __init ati_bugs(void) > #endif > } > > +static void __init amd_host_bugs(void) > +{ > + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); > + check_hypertransport_config(); > +} > + > struct chipset { > u16 vendor; > void (*f)(void); > @@ -95,9 +145,16 @@ static struct chipset early_qrk[] __initdata = { > {} > }; > > +static struct chipset early_host_qrk[] __initdata = { > + { PCI_VENDOR_ID_AMD, amd_host_bugs}, > + {} > +}; > + > void __init
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman <[EMAIL PROTECTED]> writes: > this seems reasonable, I can reroll the patch for this. As I think about it > I'm > also going to update the patch to make this check occur for any pci class 0600 > device from vendor AMD, since its possible that more than just nvidia chipsets > can be affected. > > I'll repost as soon as I've tested, thanks! Thanks. Neil in your testing please confirm the preconditions for setting the Apic Extended Broadcast flag (bit 17) are present. If that is the case it makes sense to always set that bit on conforming systems but we will also want to print a message noting that the BIOS has a bug, and we are working around it. Thanks, Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 11:36:58AM -0700, Eric W. Biederman wrote: > Neil Horman <[EMAIL PROTECTED]> writes: > > > this seems reasonable, I can reroll the patch for this. As I think about > > it I'm > > also going to update the patch to make this check occur for any pci class > > 0600 > > device from vendor AMD, since its possible that more than just nvidia > > chipsets > > can be affected. > > > > I'll repost as soon as I've tested, thanks! > > Thanks. > > Neil in your testing please confirm the preconditions for setting > the Apic Extended Broadcast flag (bit 17) are present. > The systems that I have here do _not_ in fact have that precondition, but the systems from Ben, who originoally reported the problem do have that precondition, and he has reported that this fixes the hang in the kdump boot. > If that is the case it makes sense to always set that bit on conforming > systems but we will also want to print a message noting that the > BIOS has a bug, and we are working around it. > I've got two printk's in this patch, one that indicates that Extended APIC ID's are in use, and a second that indicates that there is a mismatch between the use of extended APIC ids (bit 18) and the lack of an extended APIC id dest mask for interrupt packets (bit 17). Not sure if that meets you're requirements, but I think its sufficient. If you disagree, let me know and we can enhance them. Thanks Neil -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman <[EMAIL PROTECTED]> writes: >> Does LAPIC allow to disable a specific vector and not accept interrupts? I >> don't think so. If a timer interrupt is broadcasted to every cpu I think >> everybody will accept it (like broadcast IPI). That's why intelligence >> is built into IOAPIC and direct interrupts to a cpu or group of cpu. >> > See disable_APIC_timer(). It seems to set the mask bit in the APIC_LVTT > entry. Yes. The local allows us to choose to accept ExtInt interrupts or not. We can't do it per vector but we can do by interrupt delivery path. External Interrupt. External NMI. Logical Apic Bus (Although I don't know if we can disable this one). > >> I am just trying to understand the functionality better. Can somebody help me >> understand how do we make sure that same timer interrupt is not processed by >> all cpus (assuming hypertransport is broadcasting it)? >> > I understand your desire, but clearly, something prevents it. Note our > earlier > conversation, this bit doesn't actually force a unicast of an interrupt > packet, > but simply masks the destination field. When set to zero, it simply means > that > the ht interrupt packet destination field is restricted to 4 bits rather than > 8. > So its not like when its set to zero we are guaranteed that it is forced to a > single processor anyway. All setting this bit does is ensure that if any > apics > out on a system are addresed using an extended apic id, that interrupts can > reach them. Thats why it was suggested that this bit only be forcibly set if > bit 18 is also set. We should only have a single cpus local apic configured to accept the interrupt. Further it would not surprise me if there was some first come first served logic with accepting the interrupt in there somewhere. >> Again for my understanding, I got few questions. >> >> - Why does nvidia choose not to broadcast the interrupts and still works >> fine? Does that mean nvidia chipse will not work the extended cpu apic >> ids? >> > It doesn't! When booting normally getting interrupts to apics that use 4 bit > apic ids is sufficient since cpu0 is in that set, but if we crash on a cpu > with > an extended id, we hang. Yes. This sounds like a BIOS bug at present. A chipset feature residing in the CPUs was not enabled properly. >> - Why do I need to broadcast the interrupts and not target specific cpus? >> > Its not a forced broadcast, its a mask on the apic id. The IOAPIC still > addresses specific cpus. This is a broadcast for legacy mode interrupts which don't have a cpu destination field. Once we are running the timer through the ioapic we have a destination field and the broadcast logic should not matter. >> - If I am broadcasting interrupts, how do I make sure only one cpu >> picks it up. >> > The IOAPIC handles that. Well the local apics. If you are in ioapic mode and give the cpus a choice of which cpu to deliver to it is a hardware anycast irq transmission. That is the hardware magically picks one cpu of the set of allow cpus to deliver the irq to. How that happens is implementation specific. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 09:21:44AM -0500, Neil Horman wrote: > On Fri, Dec 07, 2007 at 01:22:04AM -0800, Yinghai Lu wrote: > > On Dec 7, 2007 12:50 AM, Yinghai Lu <[EMAIL PROTECTED]> wrote: > > > > > > On Dec 6, 2007 4:33 PM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > > ... > > > > > > > > My feel is that if it is for legacy interrupts only it should not be a > > > > problem. > > > > Let's investigate and see if we can unconditionally enable this quirk > > > > for all opteron systems. > > > > > > i checked that bit > > > > > > http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596=markup > > > > it should be bit 18 (HTTC_APIC_EXT_ID) > > > > > > YH > > this seems reasonable, I can reroll the patch for this. As I think about it > I'm > also going to update the patch to make this check occur for any pci class 0600 > device from vendor AMD, since its possible that more than just nvidia chipsets > can be affected. > > I'll repost as soon as I've tested, thanks! > Neil Ok, New patch attached. It preforms the same function as previously described, but is more restricted in its application. As Yinghai pointed out, the broadcast mask bit (bit 17 in the htcfg register) should only be enabled, if the extened apic id bit (bit 18 in the same register) is also set. So this patch now check for that bit to be turned on first. Also, this patch now adds an independent quirk check for all AMD hypertransport host controllers, since its possible for this misconfiguration to be present in systems other than nvidias. The net effect of these changes is, that its now applicable to all AMD systems containing hypertransport busses, and is only activated if extended apic ids are in use, meaning that this quirk guarantees that all processors in a system are elligible to receive interrupts from the ioapic, even if their apicid extends beyond the nominal 4 bit limitation. Tested successfully by me. Thanks & Regards Neil Signed-off-by: Neil Horman <[EMAIL PROTECTED]> early-quirks.c | 83 - 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..d5a7b30 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if ((htcfg & (1 << 18)) == 1) { + printk(KERN_INFO "Detected use of extended apic ids on hypertransport bus\n"); + if ((htcfg & (1 << 17)) == 0) { + printk(KERN_INFO "Enabling hypertransport extended apic interrupt broadcast\n"); + htcfg |= (1 << 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num < 32; num++) { + for (slot = 0; slot < 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor &= 0x; + device >>= 16; + if ((vendor == PCI_VENDOR_ID_AMD) && + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,6 +127,12 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT "IN AMD_HOST_BUGS\n"); + check_hypertransport_config(); +} + struct chipset { u16 vendor; void (*f)(void); @@ -95,9 +145,16 @@ static struct chipset early_qrk[] __initdata = { {} }; +static struct chipset early_host_qrk[] __initdata = { + { PCI_VENDOR_ID_AMD, amd_host_bugs}, + {} +}; + void __init early_quirks(void) { int num, slot, func; + u8 found_bridge = 0; + u8 found_host = 0; if (!early_pci_allowed()) return; @@ -115,18 +172,30 @@ void __init early_quirks(void) if (class == 0x) break; -
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 10:16:23AM -0500, Vivek Goyal wrote: > On Fri, Dec 07, 2007 at 09:53:15AM -0500, Neil Horman wrote: > > On Fri, Dec 07, 2007 at 09:39:44AM -0500, Vivek Goyal wrote: > > > On Thu, Dec 06, 2007 at 07:10:23PM -0500, Neil Horman wrote: > > > > On Thu, Dec 06, 2007 at 05:11:43PM -0500, Vivek Goyal wrote: > > > > > On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: > > > > > > On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: > > > > > > > On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: > > > > > > > > > > > > > > > > > > > > Thats what I'm doing at the moment. I'm working on a RHEL5 patch > > > > > > > at the moment > > > > > > > (since thats whats on the production system thats failing), and > > > > > > > will forward > > > > > > > port it once its working > > > > > > > > > > > > > > And not to split hairs, but techically thats not our _only_ > > > > > > > choice. We could > > > > > > > force kdump boots on cpu0 as well ;) > > > > > > > > > > > > > > Thanks > > > > > > > Neil > > > > > > > > > > > > > > > Thanks > > > > > > > > Vivek > > > > > > > > > > > > > > > > > > > > > > > > > Sorry to have been quiet on this issue for a few days. Interesting > > > > > > news to > > > > > > report, though. So I was working on a patch to do early apic > > > > > > enabling on > > > > > > x86_64, and had something working for the old 2.6.18 kernel that we > > > > > > were > > > > > > origionally testing on. Unfortunately while it worked on 2.6.18 it > > > > > > failed > > > > > > miserably on 2.6.24-rc3-mm2, causing check_timer to consistently > > > > > > report that the > > > > > > timer interrupt wasn't getting received (even though we could > > > > > > successfully run > > > > > > calibrate_delay). Vivek and I were digging into this, when I ran > > > > > > accross the > > > > > > description of the hypertransport configuration register in the > > > > > > opteron > > > > > > specification. It contains a bit that, suprise, configures the ht > > > > > > bus to either > > > > > > unicast interrupts delivered accross the ht bus to a single cpu, or > > > > > > to broadcast > > > > > > it to all cpus. Since it seemed more likely that the 8259 in the > > > > > > nvidia > > > > > > southbridge was transporting legacy mode interrupts over the ht bus > > > > > > than > > > > > > directly to cpu0 via an actual wire, I wrote the attached patch to > > > > > > add a quirk > > > > > > for nvidia chipsets, which scanned for hypertransport controllers, > > > > > > and ensured > > > > > > that that broadcast bit was set. Test results indicate that this > > > > > > solves the > > > > > > problem, and kdump kernels boot just fine on the affected system. > > > > > > > > > > > > > > > > Hi Neil, > > > > > > > > > > Should we disable this broadcasting feature once we are through? > > > > > Otherwise > > > > > in normal systems it might mean extra traffic on hypertransport. There > > > > > is no need for every interrupt to be broadcasted in normal systems? > > > > > > > > > > Thanks > > > > > Vivek > > > > > > > > No, I don't think thats necessecary. Once the apics are enabled, > > > > interrupts > > > > shouldn't travel accross the hypertransport bus anyway, opting instead > > > > to use > > > > the dedicated apic bus (at least thats my understanding). > > > > > > I think all interrupt message travel on hypertransport. Even after APICS > > > have been enabled. > > > > > > Look at the following document. > > > > > > http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24674.pdf > > > > > > Have a look at figure 1, figure 2 and section 3.4.2.2 and 3.4.2.3 > > > > > > That's a different thing that once IOAPIC has formed the vectored message, > > > Hypertransport might not touch the destination field. > > > > > Ok, that might be the case then. > > > > > Having said that, I am wondering what will happen if a system continues > > > to operate the timer through IOAPIC in ExtInt mode. Will hypertransport > > > keep on broadcasting that interrupt to every cpu? And every cpu will > > > process that interrupt. > > > > > I don't think so. IIRC once the other cpus are started they all disable the > > timer interrupt, except for one cpu, opting instead to get the timer tick > > via > > ipi, So while they all might see the interrupt packet on the ht bus, only > > one > > cpu will process it. > > > > Does LAPIC allow to disable a specific vector and not accept interrupts? I > don't think so. If a timer interrupt is broadcasted to every cpu I think > everybody will accept it (like broadcast IPI). That's why intelligence > is built into IOAPIC and direct interrupts to a cpu or group of cpu. > See disable_APIC_timer(). It seems to set the mask bit in the APIC_LVTT entry. > I am just trying to understand the functionality better. Can somebody help me > understand how do we make sure that same timer interrupt is not processed by
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 09:53:15AM -0500, Neil Horman wrote: > On Fri, Dec 07, 2007 at 09:39:44AM -0500, Vivek Goyal wrote: > > On Thu, Dec 06, 2007 at 07:10:23PM -0500, Neil Horman wrote: > > > On Thu, Dec 06, 2007 at 05:11:43PM -0500, Vivek Goyal wrote: > > > > On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: > > > > > On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: > > > > > > On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: > > > > > > > > > > > > > > > > > Thats what I'm doing at the moment. I'm working on a RHEL5 patch > > > > > > at the moment > > > > > > (since thats whats on the production system thats failing), and > > > > > > will forward > > > > > > port it once its working > > > > > > > > > > > > And not to split hairs, but techically thats not our _only_ choice. > > > > > > We could > > > > > > force kdump boots on cpu0 as well ;) > > > > > > > > > > > > Thanks > > > > > > Neil > > > > > > > > > > > > > Thanks > > > > > > > Vivek > > > > > > > > > > > > > > > > > > > > > Sorry to have been quiet on this issue for a few days. Interesting > > > > > news to > > > > > report, though. So I was working on a patch to do early apic > > > > > enabling on > > > > > x86_64, and had something working for the old 2.6.18 kernel that we > > > > > were > > > > > origionally testing on. Unfortunately while it worked on 2.6.18 it > > > > > failed > > > > > miserably on 2.6.24-rc3-mm2, causing check_timer to consistently > > > > > report that the > > > > > timer interrupt wasn't getting received (even though we could > > > > > successfully run > > > > > calibrate_delay). Vivek and I were digging into this, when I ran > > > > > accross the > > > > > description of the hypertransport configuration register in the > > > > > opteron > > > > > specification. It contains a bit that, suprise, configures the ht > > > > > bus to either > > > > > unicast interrupts delivered accross the ht bus to a single cpu, or > > > > > to broadcast > > > > > it to all cpus. Since it seemed more likely that the 8259 in the > > > > > nvidia > > > > > southbridge was transporting legacy mode interrupts over the ht bus > > > > > than > > > > > directly to cpu0 via an actual wire, I wrote the attached patch to > > > > > add a quirk > > > > > for nvidia chipsets, which scanned for hypertransport controllers, > > > > > and ensured > > > > > that that broadcast bit was set. Test results indicate that this > > > > > solves the > > > > > problem, and kdump kernels boot just fine on the affected system. > > > > > > > > > > > > > Hi Neil, > > > > > > > > Should we disable this broadcasting feature once we are through? > > > > Otherwise > > > > in normal systems it might mean extra traffic on hypertransport. There > > > > is no need for every interrupt to be broadcasted in normal systems? > > > > > > > > Thanks > > > > Vivek > > > > > > No, I don't think thats necessecary. Once the apics are enabled, > > > interrupts > > > shouldn't travel accross the hypertransport bus anyway, opting instead to > > > use > > > the dedicated apic bus (at least thats my understanding). > > > > I think all interrupt message travel on hypertransport. Even after APICS > > have been enabled. > > > > Look at the following document. > > > > http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24674.pdf > > > > Have a look at figure 1, figure 2 and section 3.4.2.2 and 3.4.2.3 > > > > That's a different thing that once IOAPIC has formed the vectored message, > > Hypertransport might not touch the destination field. > > > Ok, that might be the case then. > > > Having said that, I am wondering what will happen if a system continues > > to operate the timer through IOAPIC in ExtInt mode. Will hypertransport > > keep on broadcasting that interrupt to every cpu? And every cpu will > > process that interrupt. > > > I don't think so. IIRC once the other cpus are started they all disable the > timer interrupt, except for one cpu, opting instead to get the timer tick via > ipi, So while they all might see the interrupt packet on the ht bus, only one > cpu will process it. > Does LAPIC allow to disable a specific vector and not accept interrupts? I don't think so. If a timer interrupt is broadcasted to every cpu I think everybody will accept it (like broadcast IPI). That's why intelligence is built into IOAPIC and direct interrupts to a cpu or group of cpu. I am just trying to understand the functionality better. Can somebody help me understand how do we make sure that same timer interrupt is not processed by all cpus (assuming hypertransport is broadcasting it)? > > Hence, I feel it is safe to restore the broadcast bit back to BIOS value > > once > > we are through calibrate_delay(). > > > I disagree. Looking at what Yinghai said, the default setting for the > broadcast > bit isn't actually to unicast the interrupt, its just to set the broadcast > mask > to 0xF,
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 09:39:44AM -0500, Vivek Goyal wrote: > On Thu, Dec 06, 2007 at 07:10:23PM -0500, Neil Horman wrote: > > On Thu, Dec 06, 2007 at 05:11:43PM -0500, Vivek Goyal wrote: > > > On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: > > > > On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: > > > > > On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: > > > > > > > > > > > > > > Thats what I'm doing at the moment. I'm working on a RHEL5 patch at > > > > > the moment > > > > > (since thats whats on the production system thats failing), and will > > > > > forward > > > > > port it once its working > > > > > > > > > > And not to split hairs, but techically thats not our _only_ choice. > > > > > We could > > > > > force kdump boots on cpu0 as well ;) > > > > > > > > > > Thanks > > > > > Neil > > > > > > > > > > > Thanks > > > > > > Vivek > > > > > > > > > > > > > > > > > Sorry to have been quiet on this issue for a few days. Interesting news > > > > to > > > > report, though. So I was working on a patch to do early apic enabling > > > > on > > > > x86_64, and had something working for the old 2.6.18 kernel that we were > > > > origionally testing on. Unfortunately while it worked on 2.6.18 it > > > > failed > > > > miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report > > > > that the > > > > timer interrupt wasn't getting received (even though we could > > > > successfully run > > > > calibrate_delay). Vivek and I were digging into this, when I ran > > > > accross the > > > > description of the hypertransport configuration register in the opteron > > > > specification. It contains a bit that, suprise, configures the ht bus > > > > to either > > > > unicast interrupts delivered accross the ht bus to a single cpu, or to > > > > broadcast > > > > it to all cpus. Since it seemed more likely that the 8259 in the nvidia > > > > southbridge was transporting legacy mode interrupts over the ht bus than > > > > directly to cpu0 via an actual wire, I wrote the attached patch to add > > > > a quirk > > > > for nvidia chipsets, which scanned for hypertransport controllers, and > > > > ensured > > > > that that broadcast bit was set. Test results indicate that this > > > > solves the > > > > problem, and kdump kernels boot just fine on the affected system. > > > > > > > > > > Hi Neil, > > > > > > Should we disable this broadcasting feature once we are through? Otherwise > > > in normal systems it might mean extra traffic on hypertransport. There > > > is no need for every interrupt to be broadcasted in normal systems? > > > > > > Thanks > > > Vivek > > > > No, I don't think thats necessecary. Once the apics are enabled, interrupts > > shouldn't travel accross the hypertransport bus anyway, opting instead to > > use > > the dedicated apic bus (at least thats my understanding). > > I think all interrupt message travel on hypertransport. Even after APICS > have been enabled. > > Look at the following document. > > http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24674.pdf > > Have a look at figure 1, figure 2 and section 3.4.2.2 and 3.4.2.3 > > That's a different thing that once IOAPIC has formed the vectored message, > Hypertransport might not touch the destination field. > Ok, that might be the case then. > Having said that, I am wondering what will happen if a system continues > to operate the timer through IOAPIC in ExtInt mode. Will hypertransport > keep on broadcasting that interrupt to every cpu? And every cpu will > process that interrupt. > I don't think so. IIRC once the other cpus are started they all disable the timer interrupt, except for one cpu, opting instead to get the timer tick via ipi, So while they all might see the interrupt packet on the ht bus, only one cpu will process it. > Hence, I feel it is safe to restore the broadcast bit back to BIOS value once > we are through calibrate_delay(). > I disagree. Looking at what Yinghai said, the default setting for the broadcast bit isn't actually to unicast the interrupt, its just to set the broadcast mask to 0xF, or to 0xFF. Its use is actually to allow cpus with an extended 8 bit apic id see interrupts. So its not so much to direct interrupts to cpu0, but rather to the first 16 cpus rather than to all 255 available cpus. From what I've seen in my testing, systems that 'work' already have this bit set by bios, and my quirk patch above does nothing to them. Disabling this bit after calibrate_dealy is going to introduce more uncertainty in systems that have been proven to work. We should leave well enough alone, and just enable the bit if its off, and we see that we are using extended apic ids via bit 18 of the same register, as Yinghai pointed out. By enabling the quirk that way, all we are really doing is bringing into alignment two bits that should arguably be set/cleared in unison anyway. Regards Neil > Thanks > Vivek
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 06, 2007 at 07:10:23PM -0500, Neil Horman wrote: > On Thu, Dec 06, 2007 at 05:11:43PM -0500, Vivek Goyal wrote: > > On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: > > > On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: > > > > On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: > > > > > > > > > > > Thats what I'm doing at the moment. I'm working on a RHEL5 patch at > > > > the moment > > > > (since thats whats on the production system thats failing), and will > > > > forward > > > > port it once its working > > > > > > > > And not to split hairs, but techically thats not our _only_ choice. We > > > > could > > > > force kdump boots on cpu0 as well ;) > > > > > > > > Thanks > > > > Neil > > > > > > > > > Thanks > > > > > Vivek > > > > > > > > > > > > > Sorry to have been quiet on this issue for a few days. Interesting news to > > > report, though. So I was working on a patch to do early apic enabling on > > > x86_64, and had something working for the old 2.6.18 kernel that we were > > > origionally testing on. Unfortunately while it worked on 2.6.18 it failed > > > miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report > > > that the > > > timer interrupt wasn't getting received (even though we could > > > successfully run > > > calibrate_delay). Vivek and I were digging into this, when I ran accross > > > the > > > description of the hypertransport configuration register in the opteron > > > specification. It contains a bit that, suprise, configures the ht bus to > > > either > > > unicast interrupts delivered accross the ht bus to a single cpu, or to > > > broadcast > > > it to all cpus. Since it seemed more likely that the 8259 in the nvidia > > > southbridge was transporting legacy mode interrupts over the ht bus than > > > directly to cpu0 via an actual wire, I wrote the attached patch to add a > > > quirk > > > for nvidia chipsets, which scanned for hypertransport controllers, and > > > ensured > > > that that broadcast bit was set. Test results indicate that this solves > > > the > > > problem, and kdump kernels boot just fine on the affected system. > > > > > > > Hi Neil, > > > > Should we disable this broadcasting feature once we are through? Otherwise > > in normal systems it might mean extra traffic on hypertransport. There > > is no need for every interrupt to be broadcasted in normal systems? > > > > Thanks > > Vivek > > No, I don't think thats necessecary. Once the apics are enabled, interrupts > shouldn't travel accross the hypertransport bus anyway, opting instead to use > the dedicated apic bus (at least thats my understanding). I think all interrupt message travel on hypertransport. Even after APICS have been enabled. Look at the following document. http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24674.pdf Have a look at figure 1, figure 2 and section 3.4.2.2 and 3.4.2.3 That's a different thing that once IOAPIC has formed the vectored message, Hypertransport might not touch the destination field. Having said that, I am wondering what will happen if a system continues to operate the timer through IOAPIC in ExtInt mode. Will hypertransport keep on broadcasting that interrupt to every cpu? And every cpu will process that interrupt. Hence, I feel it is safe to restore the broadcast bit back to BIOS value once we are through calibrate_delay(). Thanks Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 01:22:04AM -0800, Yinghai Lu wrote: > On Dec 7, 2007 12:50 AM, Yinghai Lu <[EMAIL PROTECTED]> wrote: > > > > On Dec 6, 2007 4:33 PM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > ... > > > > > > My feel is that if it is for legacy interrupts only it should not be a > > > problem. > > > Let's investigate and see if we can unconditionally enable this quirk > > > for all opteron systems. > > > > i checked that bit > > > > http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596=markup > > > > static void enable_apic_ext_id(u8 node) > > { > > #if ENABLE_APIC_EXT_ID==1 > > #warning "FIXME Is the right place to enable apic ext id here?" > > > > u32 val; > > > > val = pci_read_config32(NODE_HT(node), 0x68); > > val |= (HTTC_APIC_EXT_SPUR | HTTC_APIC_EXT_ID | > > HTTC_APIC_EXT_BRD_CST); > > pci_write_config32(NODE_HT(node), 0x68, val); > > #endif > > } > > > > that bit only be should be set when apic id is lifted and cpu apid is > > using 8 bits and that mean broadcast is 0xff instead 0x0f. > > for example 8 socket dual core system or 4 socket quad core > > system,that you should make BSP start from 0x04, so cpus apic id will > > be [0x04, 0x13) > > > > > > So if you want to enable that in early_quirk, you need to > > make sure apic id is using 8 bits by check if the bit 16 (HTTC_APIC_ID) is > > set. > > it should be bit 18 (HTTC_APIC_EXT_ID) > > > YH this seems reasonable, I can reroll the patch for this. As I think about it I'm also going to update the patch to make this check occur for any pci class 0600 device from vendor AMD, since its possible that more than just nvidia chipsets can be affected. I'll repost as soon as I've tested, thanks! Neil -- /*** *Neil Horman *Software Engineer *Red Hat, Inc. [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 7, 2007 12:50 AM, Yinghai Lu <[EMAIL PROTECTED]> wrote: > > On Dec 6, 2007 4:33 PM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: ... > > > > My feel is that if it is for legacy interrupts only it should not be a > > problem. > > Let's investigate and see if we can unconditionally enable this quirk > > for all opteron systems. > > i checked that bit > > http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596=markup > > static void enable_apic_ext_id(u8 node) > { > #if ENABLE_APIC_EXT_ID==1 > #warning "FIXME Is the right place to enable apic ext id here?" > > u32 val; > > val = pci_read_config32(NODE_HT(node), 0x68); > val |= (HTTC_APIC_EXT_SPUR | HTTC_APIC_EXT_ID | > HTTC_APIC_EXT_BRD_CST); > pci_write_config32(NODE_HT(node), 0x68, val); > #endif > } > > that bit only be should be set when apic id is lifted and cpu apid is > using 8 bits and that mean broadcast is 0xff instead 0x0f. > for example 8 socket dual core system or 4 socket quad core > system,that you should make BSP start from 0x04, so cpus apic id will > be [0x04, 0x13) > > > So if you want to enable that in early_quirk, you need to > make sure apic id is using 8 bits by check if the bit 16 (HTTC_APIC_ID) is > set. it should be bit 18 (HTTC_APIC_EXT_ID) YH -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 6, 2007 4:33 PM, Eric W. Biederman <[EMAIL PROTECTED]> wrote: > Vivek Goyal <[EMAIL PROTECTED]> writes: > > > > On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: > >> On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: > >> > On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: > >> > >> > > >> > Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the > > moment > >> > (since thats whats on the production system thats failing), and will > >> > forward > >> > port it once its working > >> > > >> > And not to split hairs, but techically thats not our _only_ choice. We > > could > >> > force kdump boots on cpu0 as well ;) > >> > > >> > Thanks > >> > Neil > >> > > >> > > Thanks > >> > > Vivek > >> > > >> > >> > >> Sorry to have been quiet on this issue for a few days. Interesting news to > >> report, though. So I was working on a patch to do early apic enabling on > >> x86_64, and had something working for the old 2.6.18 kernel that we were > >> origionally testing on. Unfortunately while it worked on 2.6.18 it failed > >> miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report > >> that > > the > >> timer interrupt wasn't getting received (even though we could successfully > >> run > >> calibrate_delay). Vivek and I were digging into this, when I ran accross > >> the > >> description of the hypertransport configuration register in the opteron > >> specification. It contains a bit that, suprise, configures the ht bus to > > either > >> unicast interrupts delivered accross the ht bus to a single cpu, or to > > broadcast > >> it to all cpus. Since it seemed more likely that the 8259 in the nvidia > >> southbridge was transporting legacy mode interrupts over the ht bus than > >> directly to cpu0 via an actual wire, I wrote the attached patch to add a > >> quirk > >> for nvidia chipsets, which scanned for hypertransport controllers, and > >> ensured > >> that that broadcast bit was set. Test results indicate that this solves > >> the > >> problem, and kdump kernels boot just fine on the affected system. > >> > > > > Hi Neil, > > > > Should we disable this broadcasting feature once we are through? Otherwise > > in normal systems it might mean extra traffic on hypertransport. There > > is no need for every interrupt to be broadcasted in normal systems? > > My feel is that if it is for legacy interrupts only it should not be a > problem. > Let's investigate and see if we can unconditionally enable this quirk > for all opteron systems. i checked that bit http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596=markup static void enable_apic_ext_id(u8 node) { #if ENABLE_APIC_EXT_ID==1 #warning "FIXME Is the right place to enable apic ext id here?" u32 val; val = pci_read_config32(NODE_HT(node), 0x68); val |= (HTTC_APIC_EXT_SPUR | HTTC_APIC_EXT_ID | HTTC_APIC_EXT_BRD_CST); pci_write_config32(NODE_HT(node), 0x68, val); #endif } that bit only be should be set when apic id is lifted and cpu apid is using 8 bits and that mean broadcast is 0xff instead 0x0f. for example 8 socket dual core system or 4 socket quad core system,that you should make BSP start from 0x04, so cpus apic id will be [0x04, 0x13) So if you want to enable that in early_quirk, you need to make sure apic id is using 8 bits by check if the bit 16 (HTTC_APIC_ID) is set. most BIOS already did that. You may ask Supermicro fix their broken BIOS instead. YH -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 09:39:44AM -0500, Vivek Goyal wrote: On Thu, Dec 06, 2007 at 07:10:23PM -0500, Neil Horman wrote: On Thu, Dec 06, 2007 at 05:11:43PM -0500, Vivek Goyal wrote: On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: snip Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the moment (since thats whats on the production system thats failing), and will forward port it once its working And not to split hairs, but techically thats not our _only_ choice. We could force kdump boots on cpu0 as well ;) Thanks Neil Thanks Vivek Sorry to have been quiet on this issue for a few days. Interesting news to report, though. So I was working on a patch to do early apic enabling on x86_64, and had something working for the old 2.6.18 kernel that we were origionally testing on. Unfortunately while it worked on 2.6.18 it failed miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that the timer interrupt wasn't getting received (even though we could successfully run calibrate_delay). Vivek and I were digging into this, when I ran accross the description of the hypertransport configuration register in the opteron specification. It contains a bit that, suprise, configures the ht bus to either unicast interrupts delivered accross the ht bus to a single cpu, or to broadcast it to all cpus. Since it seemed more likely that the 8259 in the nvidia southbridge was transporting legacy mode interrupts over the ht bus than directly to cpu0 via an actual wire, I wrote the attached patch to add a quirk for nvidia chipsets, which scanned for hypertransport controllers, and ensured that that broadcast bit was set. Test results indicate that this solves the problem, and kdump kernels boot just fine on the affected system. Hi Neil, Should we disable this broadcasting feature once we are through? Otherwise in normal systems it might mean extra traffic on hypertransport. There is no need for every interrupt to be broadcasted in normal systems? Thanks Vivek No, I don't think thats necessecary. Once the apics are enabled, interrupts shouldn't travel accross the hypertransport bus anyway, opting instead to use the dedicated apic bus (at least thats my understanding). I think all interrupt message travel on hypertransport. Even after APICS have been enabled. Look at the following document. http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24674.pdf Have a look at figure 1, figure 2 and section 3.4.2.2 and 3.4.2.3 That's a different thing that once IOAPIC has formed the vectored message, Hypertransport might not touch the destination field. Ok, that might be the case then. Having said that, I am wondering what will happen if a system continues to operate the timer through IOAPIC in ExtInt mode. Will hypertransport keep on broadcasting that interrupt to every cpu? And every cpu will process that interrupt. I don't think so. IIRC once the other cpus are started they all disable the timer interrupt, except for one cpu, opting instead to get the timer tick via ipi, So while they all might see the interrupt packet on the ht bus, only one cpu will process it. Hence, I feel it is safe to restore the broadcast bit back to BIOS value once we are through calibrate_delay(). I disagree. Looking at what Yinghai said, the default setting for the broadcast bit isn't actually to unicast the interrupt, its just to set the broadcast mask to 0xF, or to 0xFF. Its use is actually to allow cpus with an extended 8 bit apic id see interrupts. So its not so much to direct interrupts to cpu0, but rather to the first 16 cpus rather than to all 255 available cpus. From what I've seen in my testing, systems that 'work' already have this bit set by bios, and my quirk patch above does nothing to them. Disabling this bit after calibrate_dealy is going to introduce more uncertainty in systems that have been proven to work. We should leave well enough alone, and just enable the bit if its off, and we see that we are using extended apic ids via bit 18 of the same register, as Yinghai pointed out. By enabling the quirk that way, all we are really doing is bringing into alignment two bits that should arguably be set/cleared in unison anyway. Regards Neil Thanks Vivek -- /*** *Neil Horman *Software Engineer *Red Hat, Inc. [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 06, 2007 at 07:10:23PM -0500, Neil Horman wrote: On Thu, Dec 06, 2007 at 05:11:43PM -0500, Vivek Goyal wrote: On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: snip Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the moment (since thats whats on the production system thats failing), and will forward port it once its working And not to split hairs, but techically thats not our _only_ choice. We could force kdump boots on cpu0 as well ;) Thanks Neil Thanks Vivek Sorry to have been quiet on this issue for a few days. Interesting news to report, though. So I was working on a patch to do early apic enabling on x86_64, and had something working for the old 2.6.18 kernel that we were origionally testing on. Unfortunately while it worked on 2.6.18 it failed miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that the timer interrupt wasn't getting received (even though we could successfully run calibrate_delay). Vivek and I were digging into this, when I ran accross the description of the hypertransport configuration register in the opteron specification. It contains a bit that, suprise, configures the ht bus to either unicast interrupts delivered accross the ht bus to a single cpu, or to broadcast it to all cpus. Since it seemed more likely that the 8259 in the nvidia southbridge was transporting legacy mode interrupts over the ht bus than directly to cpu0 via an actual wire, I wrote the attached patch to add a quirk for nvidia chipsets, which scanned for hypertransport controllers, and ensured that that broadcast bit was set. Test results indicate that this solves the problem, and kdump kernels boot just fine on the affected system. Hi Neil, Should we disable this broadcasting feature once we are through? Otherwise in normal systems it might mean extra traffic on hypertransport. There is no need for every interrupt to be broadcasted in normal systems? Thanks Vivek No, I don't think thats necessecary. Once the apics are enabled, interrupts shouldn't travel accross the hypertransport bus anyway, opting instead to use the dedicated apic bus (at least thats my understanding). I think all interrupt message travel on hypertransport. Even after APICS have been enabled. Look at the following document. http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24674.pdf Have a look at figure 1, figure 2 and section 3.4.2.2 and 3.4.2.3 That's a different thing that once IOAPIC has formed the vectored message, Hypertransport might not touch the destination field. Having said that, I am wondering what will happen if a system continues to operate the timer through IOAPIC in ExtInt mode. Will hypertransport keep on broadcasting that interrupt to every cpu? And every cpu will process that interrupt. Hence, I feel it is safe to restore the broadcast bit back to BIOS value once we are through calibrate_delay(). Thanks Vivek -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 01:22:04AM -0800, Yinghai Lu wrote: On Dec 7, 2007 12:50 AM, Yinghai Lu [EMAIL PROTECTED] wrote: On Dec 6, 2007 4:33 PM, Eric W. Biederman [EMAIL PROTECTED] wrote: ... My feel is that if it is for legacy interrupts only it should not be a problem. Let's investigate and see if we can unconditionally enable this quirk for all opteron systems. i checked that bit http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596view=markup static void enable_apic_ext_id(u8 node) { #if ENABLE_APIC_EXT_ID==1 #warning FIXME Is the right place to enable apic ext id here? u32 val; val = pci_read_config32(NODE_HT(node), 0x68); val |= (HTTC_APIC_EXT_SPUR | HTTC_APIC_EXT_ID | HTTC_APIC_EXT_BRD_CST); pci_write_config32(NODE_HT(node), 0x68, val); #endif } that bit only be should be set when apic id is lifted and cpu apid is using 8 bits and that mean broadcast is 0xff instead 0x0f. for example 8 socket dual core system or 4 socket quad core system,that you should make BSP start from 0x04, so cpus apic id will be [0x04, 0x13) So if you want to enable that in early_quirk, you need to make sure apic id is using 8 bits by check if the bit 16 (HTTC_APIC_ID) is set. it should be bit 18 (HTTC_APIC_EXT_ID) YH this seems reasonable, I can reroll the patch for this. As I think about it I'm also going to update the patch to make this check occur for any pci class 0600 device from vendor AMD, since its possible that more than just nvidia chipsets can be affected. I'll repost as soon as I've tested, thanks! Neil -- /*** *Neil Horman *Software Engineer *Red Hat, Inc. [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 10:16:23AM -0500, Vivek Goyal wrote: On Fri, Dec 07, 2007 at 09:53:15AM -0500, Neil Horman wrote: On Fri, Dec 07, 2007 at 09:39:44AM -0500, Vivek Goyal wrote: On Thu, Dec 06, 2007 at 07:10:23PM -0500, Neil Horman wrote: On Thu, Dec 06, 2007 at 05:11:43PM -0500, Vivek Goyal wrote: On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: snip Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the moment (since thats whats on the production system thats failing), and will forward port it once its working And not to split hairs, but techically thats not our _only_ choice. We could force kdump boots on cpu0 as well ;) Thanks Neil Thanks Vivek Sorry to have been quiet on this issue for a few days. Interesting news to report, though. So I was working on a patch to do early apic enabling on x86_64, and had something working for the old 2.6.18 kernel that we were origionally testing on. Unfortunately while it worked on 2.6.18 it failed miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that the timer interrupt wasn't getting received (even though we could successfully run calibrate_delay). Vivek and I were digging into this, when I ran accross the description of the hypertransport configuration register in the opteron specification. It contains a bit that, suprise, configures the ht bus to either unicast interrupts delivered accross the ht bus to a single cpu, or to broadcast it to all cpus. Since it seemed more likely that the 8259 in the nvidia southbridge was transporting legacy mode interrupts over the ht bus than directly to cpu0 via an actual wire, I wrote the attached patch to add a quirk for nvidia chipsets, which scanned for hypertransport controllers, and ensured that that broadcast bit was set. Test results indicate that this solves the problem, and kdump kernels boot just fine on the affected system. Hi Neil, Should we disable this broadcasting feature once we are through? Otherwise in normal systems it might mean extra traffic on hypertransport. There is no need for every interrupt to be broadcasted in normal systems? Thanks Vivek No, I don't think thats necessecary. Once the apics are enabled, interrupts shouldn't travel accross the hypertransport bus anyway, opting instead to use the dedicated apic bus (at least thats my understanding). I think all interrupt message travel on hypertransport. Even after APICS have been enabled. Look at the following document. http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24674.pdf Have a look at figure 1, figure 2 and section 3.4.2.2 and 3.4.2.3 That's a different thing that once IOAPIC has formed the vectored message, Hypertransport might not touch the destination field. Ok, that might be the case then. Having said that, I am wondering what will happen if a system continues to operate the timer through IOAPIC in ExtInt mode. Will hypertransport keep on broadcasting that interrupt to every cpu? And every cpu will process that interrupt. I don't think so. IIRC once the other cpus are started they all disable the timer interrupt, except for one cpu, opting instead to get the timer tick via ipi, So while they all might see the interrupt packet on the ht bus, only one cpu will process it. Does LAPIC allow to disable a specific vector and not accept interrupts? I don't think so. If a timer interrupt is broadcasted to every cpu I think everybody will accept it (like broadcast IPI). That's why intelligence is built into IOAPIC and direct interrupts to a cpu or group of cpu. See disable_APIC_timer(). It seems to set the mask bit in the APIC_LVTT entry. I am just trying to understand the functionality better. Can somebody help me understand how do we make sure that same timer interrupt is not processed by all cpus (assuming hypertransport is broadcasting it)? I understand your desire, but clearly, something prevents it. Note our earlier conversation, this bit doesn't actually force a unicast of an interrupt packet, but simply masks the destination field. When set to zero, it simply means that the ht interrupt packet destination field is restricted to 4 bits rather than 8. So its not like when its set to zero we are guaranteed that it is forced to a single processor anyway. All setting this bit does
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 7, 2007 12:50 AM, Yinghai Lu [EMAIL PROTECTED] wrote: On Dec 6, 2007 4:33 PM, Eric W. Biederman [EMAIL PROTECTED] wrote: ... My feel is that if it is for legacy interrupts only it should not be a problem. Let's investigate and see if we can unconditionally enable this quirk for all opteron systems. i checked that bit http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596view=markup static void enable_apic_ext_id(u8 node) { #if ENABLE_APIC_EXT_ID==1 #warning FIXME Is the right place to enable apic ext id here? u32 val; val = pci_read_config32(NODE_HT(node), 0x68); val |= (HTTC_APIC_EXT_SPUR | HTTC_APIC_EXT_ID | HTTC_APIC_EXT_BRD_CST); pci_write_config32(NODE_HT(node), 0x68, val); #endif } that bit only be should be set when apic id is lifted and cpu apid is using 8 bits and that mean broadcast is 0xff instead 0x0f. for example 8 socket dual core system or 4 socket quad core system,that you should make BSP start from 0x04, so cpus apic id will be [0x04, 0x13) So if you want to enable that in early_quirk, you need to make sure apic id is using 8 bits by check if the bit 16 (HTTC_APIC_ID) is set. it should be bit 18 (HTTC_APIC_EXT_ID) YH -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 09:53:15AM -0500, Neil Horman wrote: On Fri, Dec 07, 2007 at 09:39:44AM -0500, Vivek Goyal wrote: On Thu, Dec 06, 2007 at 07:10:23PM -0500, Neil Horman wrote: On Thu, Dec 06, 2007 at 05:11:43PM -0500, Vivek Goyal wrote: On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: snip Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the moment (since thats whats on the production system thats failing), and will forward port it once its working And not to split hairs, but techically thats not our _only_ choice. We could force kdump boots on cpu0 as well ;) Thanks Neil Thanks Vivek Sorry to have been quiet on this issue for a few days. Interesting news to report, though. So I was working on a patch to do early apic enabling on x86_64, and had something working for the old 2.6.18 kernel that we were origionally testing on. Unfortunately while it worked on 2.6.18 it failed miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that the timer interrupt wasn't getting received (even though we could successfully run calibrate_delay). Vivek and I were digging into this, when I ran accross the description of the hypertransport configuration register in the opteron specification. It contains a bit that, suprise, configures the ht bus to either unicast interrupts delivered accross the ht bus to a single cpu, or to broadcast it to all cpus. Since it seemed more likely that the 8259 in the nvidia southbridge was transporting legacy mode interrupts over the ht bus than directly to cpu0 via an actual wire, I wrote the attached patch to add a quirk for nvidia chipsets, which scanned for hypertransport controllers, and ensured that that broadcast bit was set. Test results indicate that this solves the problem, and kdump kernels boot just fine on the affected system. Hi Neil, Should we disable this broadcasting feature once we are through? Otherwise in normal systems it might mean extra traffic on hypertransport. There is no need for every interrupt to be broadcasted in normal systems? Thanks Vivek No, I don't think thats necessecary. Once the apics are enabled, interrupts shouldn't travel accross the hypertransport bus anyway, opting instead to use the dedicated apic bus (at least thats my understanding). I think all interrupt message travel on hypertransport. Even after APICS have been enabled. Look at the following document. http://www.amd.com/us-en/assets/content_type/white_papers_and_tech_docs/24674.pdf Have a look at figure 1, figure 2 and section 3.4.2.2 and 3.4.2.3 That's a different thing that once IOAPIC has formed the vectored message, Hypertransport might not touch the destination field. Ok, that might be the case then. Having said that, I am wondering what will happen if a system continues to operate the timer through IOAPIC in ExtInt mode. Will hypertransport keep on broadcasting that interrupt to every cpu? And every cpu will process that interrupt. I don't think so. IIRC once the other cpus are started they all disable the timer interrupt, except for one cpu, opting instead to get the timer tick via ipi, So while they all might see the interrupt packet on the ht bus, only one cpu will process it. Does LAPIC allow to disable a specific vector and not accept interrupts? I don't think so. If a timer interrupt is broadcasted to every cpu I think everybody will accept it (like broadcast IPI). That's why intelligence is built into IOAPIC and direct interrupts to a cpu or group of cpu. I am just trying to understand the functionality better. Can somebody help me understand how do we make sure that same timer interrupt is not processed by all cpus (assuming hypertransport is broadcasting it)? Hence, I feel it is safe to restore the broadcast bit back to BIOS value once we are through calibrate_delay(). I disagree. Looking at what Yinghai said, the default setting for the broadcast bit isn't actually to unicast the interrupt, its just to set the broadcast mask to 0xF, or to 0xFF. Its use is actually to allow cpus with an extended 8 bit apic id see interrupts. So its not so much to direct interrupts to cpu0, but rather to the first 16 cpus rather than to all 255 available cpus. From what I've seen in my testing, systems that 'work' already have this bit set by bios, and my quirk patch above does nothing to them. Disabling this bit after calibrate_dealy is going
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 6, 2007 4:33 PM, Eric W. Biederman [EMAIL PROTECTED] wrote: Vivek Goyal [EMAIL PROTECTED] writes: On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: snip Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the moment (since thats whats on the production system thats failing), and will forward port it once its working And not to split hairs, but techically thats not our _only_ choice. We could force kdump boots on cpu0 as well ;) Thanks Neil Thanks Vivek Sorry to have been quiet on this issue for a few days. Interesting news to report, though. So I was working on a patch to do early apic enabling on x86_64, and had something working for the old 2.6.18 kernel that we were origionally testing on. Unfortunately while it worked on 2.6.18 it failed miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that the timer interrupt wasn't getting received (even though we could successfully run calibrate_delay). Vivek and I were digging into this, when I ran accross the description of the hypertransport configuration register in the opteron specification. It contains a bit that, suprise, configures the ht bus to either unicast interrupts delivered accross the ht bus to a single cpu, or to broadcast it to all cpus. Since it seemed more likely that the 8259 in the nvidia southbridge was transporting legacy mode interrupts over the ht bus than directly to cpu0 via an actual wire, I wrote the attached patch to add a quirk for nvidia chipsets, which scanned for hypertransport controllers, and ensured that that broadcast bit was set. Test results indicate that this solves the problem, and kdump kernels boot just fine on the affected system. Hi Neil, Should we disable this broadcasting feature once we are through? Otherwise in normal systems it might mean extra traffic on hypertransport. There is no need for every interrupt to be broadcasted in normal systems? My feel is that if it is for legacy interrupts only it should not be a problem. Let's investigate and see if we can unconditionally enable this quirk for all opteron systems. i checked that bit http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596view=markup static void enable_apic_ext_id(u8 node) { #if ENABLE_APIC_EXT_ID==1 #warning FIXME Is the right place to enable apic ext id here? u32 val; val = pci_read_config32(NODE_HT(node), 0x68); val |= (HTTC_APIC_EXT_SPUR | HTTC_APIC_EXT_ID | HTTC_APIC_EXT_BRD_CST); pci_write_config32(NODE_HT(node), 0x68, val); #endif } that bit only be should be set when apic id is lifted and cpu apid is using 8 bits and that mean broadcast is 0xff instead 0x0f. for example 8 socket dual core system or 4 socket quad core system,that you should make BSP start from 0x04, so cpus apic id will be [0x04, 0x13) So if you want to enable that in early_quirk, you need to make sure apic id is using 8 bits by check if the bit 16 (HTTC_APIC_ID) is set. most BIOS already did that. You may ask Supermicro fix their broken BIOS instead. YH -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 09:21:44AM -0500, Neil Horman wrote: On Fri, Dec 07, 2007 at 01:22:04AM -0800, Yinghai Lu wrote: On Dec 7, 2007 12:50 AM, Yinghai Lu [EMAIL PROTECTED] wrote: On Dec 6, 2007 4:33 PM, Eric W. Biederman [EMAIL PROTECTED] wrote: ... My feel is that if it is for legacy interrupts only it should not be a problem. Let's investigate and see if we can unconditionally enable this quirk for all opteron systems. i checked that bit http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596view=markup snip it should be bit 18 (HTTC_APIC_EXT_ID) YH this seems reasonable, I can reroll the patch for this. As I think about it I'm also going to update the patch to make this check occur for any pci class 0600 device from vendor AMD, since its possible that more than just nvidia chipsets can be affected. I'll repost as soon as I've tested, thanks! Neil Ok, New patch attached. It preforms the same function as previously described, but is more restricted in its application. As Yinghai pointed out, the broadcast mask bit (bit 17 in the htcfg register) should only be enabled, if the extened apic id bit (bit 18 in the same register) is also set. So this patch now check for that bit to be turned on first. Also, this patch now adds an independent quirk check for all AMD hypertransport host controllers, since its possible for this misconfiguration to be present in systems other than nvidias. The net effect of these changes is, that its now applicable to all AMD systems containing hypertransport busses, and is only activated if extended apic ids are in use, meaning that this quirk guarantees that all processors in a system are elligible to receive interrupts from the ioapic, even if their apicid extends beyond the nominal 4 bit limitation. Tested successfully by me. Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 83 - 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..d5a7b30 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if ((htcfg (1 18)) == 1) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num 32; num++) { + for (slot = 0; slot 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor = 0x; + device = 16; + if ((vendor == PCI_VENDOR_ID_AMD) + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,6 +127,12 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} + struct chipset { u16 vendor; void (*f)(void); @@ -95,9 +145,16 @@ static struct chipset early_qrk[] __initdata = { {} }; +static struct chipset early_host_qrk[] __initdata = { + { PCI_VENDOR_ID_AMD, amd_host_bugs}, + {} +}; + void __init early_quirks(void) { int num, slot, func; + u8 found_bridge = 0; + u8 found_host = 0; if (!early_pci_allowed()) return; @@ -115,18 +172,30 @@ void __init early_quirks(void) if (class == 0x) break; - if ((class 16) != PCI_CLASS_BRIDGE_PCI) +
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman [EMAIL PROTECTED] writes: this seems reasonable, I can reroll the patch for this. As I think about it I'm also going to update the patch to make this check occur for any pci class 0600 device from vendor AMD, since its possible that more than just nvidia chipsets can be affected. I'll repost as soon as I've tested, thanks! Thanks. Neil in your testing please confirm the preconditions for setting the Apic Extended Broadcast flag (bit 17) are present. If that is the case it makes sense to always set that bit on conforming systems but we will also want to print a message noting that the BIOS has a bug, and we are working around it. Thanks, Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 11:36:58AM -0700, Eric W. Biederman wrote: Neil Horman [EMAIL PROTECTED] writes: this seems reasonable, I can reroll the patch for this. As I think about it I'm also going to update the patch to make this check occur for any pci class 0600 device from vendor AMD, since its possible that more than just nvidia chipsets can be affected. I'll repost as soon as I've tested, thanks! Thanks. Neil in your testing please confirm the preconditions for setting the Apic Extended Broadcast flag (bit 17) are present. The systems that I have here do _not_ in fact have that precondition, but the systems from Ben, who originoally reported the problem do have that precondition, and he has reported that this fixes the hang in the kdump boot. If that is the case it makes sense to always set that bit on conforming systems but we will also want to print a message noting that the BIOS has a bug, and we are working around it. I've got two printk's in this patch, one that indicates that Extended APIC ID's are in use, and a second that indicates that there is a mismatch between the use of extended APIC ids (bit 18) and the lack of an extended APIC id dest mask for interrupt packets (bit 17). Not sure if that meets you're requirements, but I think its sufficient. If you disagree, let me know and we can enhance them. Thanks Neil -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Neil Horman [EMAIL PROTECTED] writes: Does LAPIC allow to disable a specific vector and not accept interrupts? I don't think so. If a timer interrupt is broadcasted to every cpu I think everybody will accept it (like broadcast IPI). That's why intelligence is built into IOAPIC and direct interrupts to a cpu or group of cpu. See disable_APIC_timer(). It seems to set the mask bit in the APIC_LVTT entry. Yes. The local allows us to choose to accept ExtInt interrupts or not. We can't do it per vector but we can do by interrupt delivery path. External Interrupt. External NMI. Logical Apic Bus (Although I don't know if we can disable this one). I am just trying to understand the functionality better. Can somebody help me understand how do we make sure that same timer interrupt is not processed by all cpus (assuming hypertransport is broadcasting it)? I understand your desire, but clearly, something prevents it. Note our earlier conversation, this bit doesn't actually force a unicast of an interrupt packet, but simply masks the destination field. When set to zero, it simply means that the ht interrupt packet destination field is restricted to 4 bits rather than 8. So its not like when its set to zero we are guaranteed that it is forced to a single processor anyway. All setting this bit does is ensure that if any apics out on a system are addresed using an extended apic id, that interrupts can reach them. Thats why it was suggested that this bit only be forcibly set if bit 18 is also set. We should only have a single cpus local apic configured to accept the interrupt. Further it would not surprise me if there was some first come first served logic with accepting the interrupt in there somewhere. Again for my understanding, I got few questions. - Why does nvidia choose not to broadcast the interrupts and still works fine? Does that mean nvidia chipse will not work the extended cpu apic ids? It doesn't! When booting normally getting interrupts to apics that use 4 bit apic ids is sufficient since cpu0 is in that set, but if we crash on a cpu with an extended id, we hang. Yes. This sounds like a BIOS bug at present. A chipset feature residing in the CPUs was not enabled properly. - Why do I need to broadcast the interrupts and not target specific cpus? Its not a forced broadcast, its a mask on the apic id. The IOAPIC still addresses specific cpus. This is a broadcast for legacy mode interrupts which don't have a cpu destination field. Once we are running the timer through the ioapic we have a destination field and the broadcast logic should not matter. - If I am broadcasting interrupts, how do I make sure only one cpu picks it up. The IOAPIC handles that. Well the local apics. If you are in ioapic mode and give the cpus a choice of which cpu to deliver to it is a hardware anycast irq transmission. That is the hardware magically picks one cpu of the set of allow cpus to deliver the irq to. How that happens is implementation specific. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Dec 07, 2007 at 11:19:10AM -0800, yhlu wrote: On Dec 7, 2007 9:58 AM, Neil Horman [EMAIL PROTECTED] wrote: On Fri, Dec 07, 2007 at 09:21:44AM -0500, Neil Horman wrote: On Fri, Dec 07, 2007 at 01:22:04AM -0800, Yinghai Lu wrote: On Dec 7, 2007 12:50 AM, Yinghai Lu [EMAIL PROTECTED] wrote: On Dec 6, 2007 4:33 PM, Eric W. Biederman [EMAIL PROTECTED] wrote: ... My feel is that if it is for legacy interrupts only it should not be a problem. Let's investigate and see if we can unconditionally enable this quirk for all opteron systems. i checked that bit http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596view=markup snip it should be bit 18 (HTTC_APIC_EXT_ID) YH this seems reasonable, I can reroll the patch for this. As I think about it I'm also going to update the patch to make this check occur for any pci class 0600 device from vendor AMD, since its possible that more than just nvidia chipsets can be affected. I'll repost as soon as I've tested, thanks! Neil Ok, New patch attached. It preforms the same function as previously described, but is more restricted in its application. As Yinghai pointed out, the broadcast mask bit (bit 17 in the htcfg register) should only be enabled, if the extened apic id bit (bit 18 in the same register) is also set. So this patch now check for that bit to be turned on first. Also, this patch now adds an independent quirk check for all AMD hypertransport host controllers, since its possible for this misconfiguration to be present in systems other than nvidias. The net effect of these changes is, that its now applicable to all AMD systems containing hypertransport busses, and is only activated if extended apic ids are in use, meaning that this quirk guarantees that all processors in a system are elligible to receive interrupts from the ioapic, even if their apicid extends beyond the nominal 4 bit limitation. Tested successfully by me. Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 83 - 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..d5a7b30 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if ((htcfg (1 18)) == 1) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num 32; num++) { + for (slot = 0; slot 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor = 0x; + device = 16; + if ((vendor == PCI_VENDOR_ID_AMD) + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,6 +127,12 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} + struct chipset { u16 vendor; void (*f)(void); @@ -95,9 +145,16 @@ static struct chipset early_qrk[] __initdata = { {} }; +static struct chipset early_host_qrk[] __initdata = { + { PCI_VENDOR_ID_AMD, amd_host_bugs}, + {} +}; + void __init
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Dec 7, 2007 9:58 AM, Neil Horman [EMAIL PROTECTED] wrote: On Fri, Dec 07, 2007 at 09:21:44AM -0500, Neil Horman wrote: On Fri, Dec 07, 2007 at 01:22:04AM -0800, Yinghai Lu wrote: On Dec 7, 2007 12:50 AM, Yinghai Lu [EMAIL PROTECTED] wrote: On Dec 6, 2007 4:33 PM, Eric W. Biederman [EMAIL PROTECTED] wrote: ... My feel is that if it is for legacy interrupts only it should not be a problem. Let's investigate and see if we can unconditionally enable this quirk for all opteron systems. i checked that bit http://www.openbios.org/viewvc/trunk/LinuxBIOSv2/src/northbridge/amd/amdk8/coherent_ht.c?revision=2596view=markup snip it should be bit 18 (HTTC_APIC_EXT_ID) YH this seems reasonable, I can reroll the patch for this. As I think about it I'm also going to update the patch to make this check occur for any pci class 0600 device from vendor AMD, since its possible that more than just nvidia chipsets can be affected. I'll repost as soon as I've tested, thanks! Neil Ok, New patch attached. It preforms the same function as previously described, but is more restricted in its application. As Yinghai pointed out, the broadcast mask bit (bit 17 in the htcfg register) should only be enabled, if the extened apic id bit (bit 18 in the same register) is also set. So this patch now check for that bit to be turned on first. Also, this patch now adds an independent quirk check for all AMD hypertransport host controllers, since its possible for this misconfiguration to be present in systems other than nvidias. The net effect of these changes is, that its now applicable to all AMD systems containing hypertransport busses, and is only activated if extended apic ids are in use, meaning that this quirk guarantees that all processors in a system are elligible to receive interrupts from the ioapic, even if their apicid extends beyond the nominal 4 bit limitation. Tested successfully by me. Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 83 - 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..d5a7b30 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,50 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*if we're using extended apic ids +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if ((htcfg (1 18)) == 1) { + printk(KERN_INFO Detected use of extended apic ids on hypertransport bus\n); + if ((htcfg (1 17)) == 0) { + printk(KERN_INFO Enabling hypertransport extended apic interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + } + } + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num 32; num++) { + for (slot = 0; slot 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor = 0x; + device = 16; + if ((vendor == PCI_VENDOR_ID_AMD) + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -83,6 +127,12 @@ static void __init ati_bugs(void) #endif } +static void __init amd_host_bugs(void) +{ + printk(KERN_CRIT IN AMD_HOST_BUGS\n); + check_hypertransport_config(); +} + struct chipset { u16 vendor; void (*f)(void); @@ -95,9 +145,16 @@ static struct chipset early_qrk[] __initdata = { {} }; +static struct chipset early_host_qrk[] __initdata = { + { PCI_VENDOR_ID_AMD, amd_host_bugs}, + {} +}; + void __init early_quirks(void) { int num, slot, func; + u8 found_bridge = 0; + u8 found_host = 0; if (!early_pci_allowed()) return; @@ -115,18 +172,30 @@ void __init early_quirks(void)
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 06, 2007 at 05:33:31PM -0700, Eric W. Biederman wrote: > Vivek Goyal <[EMAIL PROTECTED]> writes: > > > On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: > >> On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: > >> > On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: > >> > >> > > >> > Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the > > moment > >> > (since thats whats on the production system thats failing), and will > >> > forward > >> > port it once its working > >> > > >> > And not to split hairs, but techically thats not our _only_ choice. We > > could > >> > force kdump boots on cpu0 as well ;) > >> > > >> > Thanks > >> > Neil > >> > > >> > > Thanks > >> > > Vivek > >> > > >> > >> > >> Sorry to have been quiet on this issue for a few days. Interesting news to > >> report, though. So I was working on a patch to do early apic enabling on > >> x86_64, and had something working for the old 2.6.18 kernel that we were > >> origionally testing on. Unfortunately while it worked on 2.6.18 it failed > >> miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report > >> that > > the > >> timer interrupt wasn't getting received (even though we could successfully > >> run > >> calibrate_delay). Vivek and I were digging into this, when I ran accross > >> the > >> description of the hypertransport configuration register in the opteron > >> specification. It contains a bit that, suprise, configures the ht bus to > > either > >> unicast interrupts delivered accross the ht bus to a single cpu, or to > > broadcast > >> it to all cpus. Since it seemed more likely that the 8259 in the nvidia > >> southbridge was transporting legacy mode interrupts over the ht bus than > >> directly to cpu0 via an actual wire, I wrote the attached patch to add a > >> quirk > >> for nvidia chipsets, which scanned for hypertransport controllers, and > >> ensured > >> that that broadcast bit was set. Test results indicate that this solves > >> the > >> problem, and kdump kernels boot just fine on the affected system. > >> > > > > Hi Neil, > > > > Should we disable this broadcasting feature once we are through? Otherwise > > in normal systems it might mean extra traffic on hypertransport. There > > is no need for every interrupt to be broadcasted in normal systems? > > My feel is that if it is for legacy interrupts only it should not be a > problem. > Let's investigate and see if we can unconditionally enable this quirk > for all opteron systems. > > Eric Copy that. Thus far, I've tested it on a pure AMD engineering sample, an intel x86_64 box, and the affected system, a quad socket dual core AMD system with an nvidia chipset. That last system is currently the only system that this patch will check/enable the broadcast flag on. I did try a variant of the patch on the AMD engineering sample where it enabled the bit unconditionally. Interestingly enough, the bit was already turned on on that system. I'm wondering if most systems don't already have this bit turned on. You should be able to universally enable this bit, by moving the call to check_hypertransport_config to the top of early_quirks() Regards Neil -- /*** *Neil Horman *Software Engineer *Red Hat, Inc. [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Vivek Goyal <[EMAIL PROTECTED]> writes: > On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: >> On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: >> > On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: >> >> > >> > Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the > moment >> > (since thats whats on the production system thats failing), and will >> > forward >> > port it once its working >> > >> > And not to split hairs, but techically thats not our _only_ choice. We > could >> > force kdump boots on cpu0 as well ;) >> > >> > Thanks >> > Neil >> > >> > > Thanks >> > > Vivek >> > >> >> >> Sorry to have been quiet on this issue for a few days. Interesting news to >> report, though. So I was working on a patch to do early apic enabling on >> x86_64, and had something working for the old 2.6.18 kernel that we were >> origionally testing on. Unfortunately while it worked on 2.6.18 it failed >> miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that > the >> timer interrupt wasn't getting received (even though we could successfully >> run >> calibrate_delay). Vivek and I were digging into this, when I ran accross the >> description of the hypertransport configuration register in the opteron >> specification. It contains a bit that, suprise, configures the ht bus to > either >> unicast interrupts delivered accross the ht bus to a single cpu, or to > broadcast >> it to all cpus. Since it seemed more likely that the 8259 in the nvidia >> southbridge was transporting legacy mode interrupts over the ht bus than >> directly to cpu0 via an actual wire, I wrote the attached patch to add a >> quirk >> for nvidia chipsets, which scanned for hypertransport controllers, and >> ensured >> that that broadcast bit was set. Test results indicate that this solves the >> problem, and kdump kernels boot just fine on the affected system. >> > > Hi Neil, > > Should we disable this broadcasting feature once we are through? Otherwise > in normal systems it might mean extra traffic on hypertransport. There > is no need for every interrupt to be broadcasted in normal systems? My feel is that if it is for legacy interrupts only it should not be a problem. Let's investigate and see if we can unconditionally enable this quirk for all opteron systems. Eric -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 06, 2007 at 05:11:43PM -0500, Vivek Goyal wrote: > On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: > > On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: > > > On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: > > > > > > > > Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the > > > moment > > > (since thats whats on the production system thats failing), and will > > > forward > > > port it once its working > > > > > > And not to split hairs, but techically thats not our _only_ choice. We > > > could > > > force kdump boots on cpu0 as well ;) > > > > > > Thanks > > > Neil > > > > > > > Thanks > > > > Vivek > > > > > > > > > Sorry to have been quiet on this issue for a few days. Interesting news to > > report, though. So I was working on a patch to do early apic enabling on > > x86_64, and had something working for the old 2.6.18 kernel that we were > > origionally testing on. Unfortunately while it worked on 2.6.18 it failed > > miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report > > that the > > timer interrupt wasn't getting received (even though we could successfully > > run > > calibrate_delay). Vivek and I were digging into this, when I ran accross > > the > > description of the hypertransport configuration register in the opteron > > specification. It contains a bit that, suprise, configures the ht bus to > > either > > unicast interrupts delivered accross the ht bus to a single cpu, or to > > broadcast > > it to all cpus. Since it seemed more likely that the 8259 in the nvidia > > southbridge was transporting legacy mode interrupts over the ht bus than > > directly to cpu0 via an actual wire, I wrote the attached patch to add a > > quirk > > for nvidia chipsets, which scanned for hypertransport controllers, and > > ensured > > that that broadcast bit was set. Test results indicate that this solves the > > problem, and kdump kernels boot just fine on the affected system. > > > > Hi Neil, > > Should we disable this broadcasting feature once we are through? Otherwise > in normal systems it might mean extra traffic on hypertransport. There > is no need for every interrupt to be broadcasted in normal systems? > > Thanks > Vivek No, I don't think thats necessecary. Once the apics are enabled, interrupts shouldn't travel accross the hypertransport bus anyway, opting instead to use the dedicated apic bus (at least thats my understanding). The only systems what you are suggesting would help with are systems that have no apic at all, which I can only imagine on 64 bit systems is rare, to say the least. The affected domain is further reduced by the fact that this quirk is only currently being applied to systems with nvidia PCI bridges, since those are the only systems that this problem has manifested on. That seems like a rather small subset, if it exists at all. I suppose we could only optionally enable the quirk if we are booting a kdump kernel (implying that we would need to do something like detect the reset_devices command line option), but I think given the limited affect this patch, its not really needed. Regards Neil -- /*** *Neil Horman *Software Engineer *Red Hat, Inc. [EMAIL PROTECTED] *gpg keyid: 1024D / 0x92A74FA1 *http://pgp.mit.edu ***/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: > On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: > > On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: > > > > > Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the > > moment > > (since thats whats on the production system thats failing), and will forward > > port it once its working > > > > And not to split hairs, but techically thats not our _only_ choice. We > > could > > force kdump boots on cpu0 as well ;) > > > > Thanks > > Neil > > > > > Thanks > > > Vivek > > > > > Sorry to have been quiet on this issue for a few days. Interesting news to > report, though. So I was working on a patch to do early apic enabling on > x86_64, and had something working for the old 2.6.18 kernel that we were > origionally testing on. Unfortunately while it worked on 2.6.18 it failed > miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that > the > timer interrupt wasn't getting received (even though we could successfully run > calibrate_delay). Vivek and I were digging into this, when I ran accross the > description of the hypertransport configuration register in the opteron > specification. It contains a bit that, suprise, configures the ht bus to > either > unicast interrupts delivered accross the ht bus to a single cpu, or to > broadcast > it to all cpus. Since it seemed more likely that the 8259 in the nvidia > southbridge was transporting legacy mode interrupts over the ht bus than > directly to cpu0 via an actual wire, I wrote the attached patch to add a quirk > for nvidia chipsets, which scanned for hypertransport controllers, and ensured > that that broadcast bit was set. Test results indicate that this solves the > problem, and kdump kernels boot just fine on the affected system. > Hi Neil, Should we disable this broadcasting feature once we are through? Otherwise in normal systems it might mean extra traffic on hypertransport. There is no need for every interrupt to be broadcasted in normal systems? Thanks Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: > On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: > > Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the > moment > (since thats whats on the production system thats failing), and will forward > port it once its working > > And not to split hairs, but techically thats not our _only_ choice. We could > force kdump boots on cpu0 as well ;) > > Thanks > Neil > > > Thanks > > Vivek > Sorry to have been quiet on this issue for a few days. Interesting news to report, though. So I was working on a patch to do early apic enabling on x86_64, and had something working for the old 2.6.18 kernel that we were origionally testing on. Unfortunately while it worked on 2.6.18 it failed miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that the timer interrupt wasn't getting received (even though we could successfully run calibrate_delay). Vivek and I were digging into this, when I ran accross the description of the hypertransport configuration register in the opteron specification. It contains a bit that, suprise, configures the ht bus to either unicast interrupts delivered accross the ht bus to a single cpu, or to broadcast it to all cpus. Since it seemed more likely that the 8259 in the nvidia southbridge was transporting legacy mode interrupts over the ht bus than directly to cpu0 via an actual wire, I wrote the attached patch to add a quirk for nvidia chipsets, which scanned for hypertransport controllers, and ensured that that broadcast bit was set. Test results indicate that this solves the problem, and kdump kernels boot just fine on the affected system. I understand that enabling the apic early in the boot process is a nice general solution, but given the wide range of apic configurations possible, and the need for large amounts of testing of such a change, this approach seems like it might be the better way to go, as it corrects a bad behavior only in systems that would be affected by this problem. It introduces no further complexity in the kdump shutdown path, and creates no additional instability in systems that would otherwise be unaffected by this bug. I think this is the best way for us to go forward. Attached patch applies cleanly against 2.6.24-rc3-mm2 and works for me on serveral systems unaffected by the kdump crash problem I origionally reported and fixes the bug on the affected system. Thanks & Regards Neil Signed-off-by: Neil Horman <[EMAIL PROTECTED]> early-quirks.c | 41 - 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..ea16b53 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,45 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if ((htcfg & (1 << 17)) == 0) + printk(KERN_INFO "Enabling hypertransport interrupt broadcast\n"); + htcfg |= (1 << 17); + write_pci_config(num, slot, func, 0x68, htcfg); + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num < 32; num++) { + for (slot = 0; slot < 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor &= 0x; + device >>= 16; + if ((vendor == PCI_VENDOR_ID_AMD) && + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -69,7 +108,7 @@ static void __init nvidia_bugs(void) #endif #endif /* RED-PEN skip them on mptables too? */ - + check_hypertransport_config(); } static void __init ati_bugs(void) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: snip Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the moment (since thats whats on the production system thats failing), and will forward port it once its working And not to split hairs, but techically thats not our _only_ choice. We could force kdump boots on cpu0 as well ;) Thanks Neil Thanks Vivek Sorry to have been quiet on this issue for a few days. Interesting news to report, though. So I was working on a patch to do early apic enabling on x86_64, and had something working for the old 2.6.18 kernel that we were origionally testing on. Unfortunately while it worked on 2.6.18 it failed miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that the timer interrupt wasn't getting received (even though we could successfully run calibrate_delay). Vivek and I were digging into this, when I ran accross the description of the hypertransport configuration register in the opteron specification. It contains a bit that, suprise, configures the ht bus to either unicast interrupts delivered accross the ht bus to a single cpu, or to broadcast it to all cpus. Since it seemed more likely that the 8259 in the nvidia southbridge was transporting legacy mode interrupts over the ht bus than directly to cpu0 via an actual wire, I wrote the attached patch to add a quirk for nvidia chipsets, which scanned for hypertransport controllers, and ensured that that broadcast bit was set. Test results indicate that this solves the problem, and kdump kernels boot just fine on the affected system. I understand that enabling the apic early in the boot process is a nice general solution, but given the wide range of apic configurations possible, and the need for large amounts of testing of such a change, this approach seems like it might be the better way to go, as it corrects a bad behavior only in systems that would be affected by this problem. It introduces no further complexity in the kdump shutdown path, and creates no additional instability in systems that would otherwise be unaffected by this bug. I think this is the best way for us to go forward. Attached patch applies cleanly against 2.6.24-rc3-mm2 and works for me on serveral systems unaffected by the kdump crash problem I origionally reported and fixes the bug on the affected system. Thanks Regards Neil Signed-off-by: Neil Horman [EMAIL PROTECTED] early-quirks.c | 41 - 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/early-quirks.c b/arch/x86/kernel/early-quirks.c index 88bb83e..ea16b53 100644 --- a/arch/x86/kernel/early-quirks.c +++ b/arch/x86/kernel/early-quirks.c @@ -44,6 +44,45 @@ static int __init nvidia_hpet_check(struct acpi_table_header *header) #endif /* CONFIG_X86_IO_APIC */ #endif /* CONFIG_ACPI */ +static void __init fix_hypertransport_config(int num, int slot, int func) +{ + u32 htcfg; + /* +*we found a hypertransport bus +*make sure that are broadcasting +*interrupts to all cpus on the ht bus +*/ + htcfg = read_pci_config(num, slot, func, 0x68); + if ((htcfg (1 17)) == 0) + printk(KERN_INFO Enabling hypertransport interrupt broadcast\n); + htcfg |= (1 17); + write_pci_config(num, slot, func, 0x68, htcfg); + +} + +static void __init check_hypertransport_config() +{ + int num, slot, func; + u32 device, vendor; + func = 0; + for (num = 0; num 32; num++) { + for (slot = 0; slot 32; slot++) { + vendor = read_pci_config(num,slot,func, + PCI_VENDOR_ID); + device = read_pci_config(num,slot,func, + PCI_DEVICE_ID); + vendor = 0x; + device = 16; + if ((vendor == PCI_VENDOR_ID_AMD) + (device == PCI_DEVICE_ID_AMD_K8_NB)) + fix_hypertransport_config(num,slot,func); + } + } + + return; + +} + static void __init nvidia_bugs(void) { #ifdef CONFIG_ACPI @@ -69,7 +108,7 @@ static void __init nvidia_bugs(void) #endif #endif /* RED-PEN skip them on mptables too? */ - + check_hypertransport_config(); } static void __init ati_bugs(void) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: snip Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the moment (since thats whats on the production system thats failing), and will forward port it once its working And not to split hairs, but techically thats not our _only_ choice. We could force kdump boots on cpu0 as well ;) Thanks Neil Thanks Vivek Sorry to have been quiet on this issue for a few days. Interesting news to report, though. So I was working on a patch to do early apic enabling on x86_64, and had something working for the old 2.6.18 kernel that we were origionally testing on. Unfortunately while it worked on 2.6.18 it failed miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that the timer interrupt wasn't getting received (even though we could successfully run calibrate_delay). Vivek and I were digging into this, when I ran accross the description of the hypertransport configuration register in the opteron specification. It contains a bit that, suprise, configures the ht bus to either unicast interrupts delivered accross the ht bus to a single cpu, or to broadcast it to all cpus. Since it seemed more likely that the 8259 in the nvidia southbridge was transporting legacy mode interrupts over the ht bus than directly to cpu0 via an actual wire, I wrote the attached patch to add a quirk for nvidia chipsets, which scanned for hypertransport controllers, and ensured that that broadcast bit was set. Test results indicate that this solves the problem, and kdump kernels boot just fine on the affected system. Hi Neil, Should we disable this broadcasting feature once we are through? Otherwise in normal systems it might mean extra traffic on hypertransport. There is no need for every interrupt to be broadcasted in normal systems? Thanks Vivek -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu
Vivek Goyal [EMAIL PROTECTED] writes: On Thu, Dec 06, 2007 at 04:39:51PM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:51:31AM -0500, Neil Horman wrote: On Fri, Nov 30, 2007 at 09:42:50AM -0500, Vivek Goyal wrote: snip Thats what I'm doing at the moment. I'm working on a RHEL5 patch at the moment (since thats whats on the production system thats failing), and will forward port it once its working And not to split hairs, but techically thats not our _only_ choice. We could force kdump boots on cpu0 as well ;) Thanks Neil Thanks Vivek Sorry to have been quiet on this issue for a few days. Interesting news to report, though. So I was working on a patch to do early apic enabling on x86_64, and had something working for the old 2.6.18 kernel that we were origionally testing on. Unfortunately while it worked on 2.6.18 it failed miserably on 2.6.24-rc3-mm2, causing check_timer to consistently report that the timer interrupt wasn't getting received (even though we could successfully run calibrate_delay). Vivek and I were digging into this, when I ran accross the description of the hypertransport configuration register in the opteron specification. It contains a bit that, suprise, configures the ht bus to either unicast interrupts delivered accross the ht bus to a single cpu, or to broadcast it to all cpus. Since it seemed more likely that the 8259 in the nvidia southbridge was transporting legacy mode interrupts over the ht bus than directly to cpu0 via an actual wire, I wrote the attached patch to add a quirk for nvidia chipsets, which scanned for hypertransport controllers, and ensured that that broadcast bit was set. Test results indicate that this solves the problem, and kdump kernels boot just fine on the affected system. Hi Neil, Should we disable this broadcasting feature once we are through? Otherwise in normal systems it might mean extra traffic on hypertransport. There is no need for every interrupt to be broadcasted in normal systems? My feel is that if it is for legacy interrupts only it should not be a problem. Let's investigate and see if we can unconditionally enable this quirk for all opteron systems. Eric -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/