Re: [PATCH] kexec: force x86_64 arches to boot kdump kernels on boot cpu

Neil Horman
2007-12-11T03:45:43Z

On Mon, Dec 10, 2007 at 06:08:03PM -0700, Eric W. Biederman wrote:
> Neil Horman <nhorman@tuxdriver.com> 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 <nhorman@tuxdriver.com>

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 &= 0x0000ffff;
+ 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 +162,35 @@ void __init early_quirks(void)
for (func = 0; func < 8; func++) {
u32 class;
u32 vendor;
+ u32 device;
u8 type;
int i;
+
class = read_pci_config(num,slot,func,
PCI_CLASS_REVISION);
if (class == 0xffffffff)
break;

- if ((class >> 16) != PCI_CLASS_BRIDGE_PCI)
- continue;
+ class >>= 16;

vendor = read_pci_config(num, slot, func,
PCI_VENDOR_ID);
vendor &= 0xffff;

- for (i = 0; early_qrk[i].f; i++)
- if (early_qrk[i].vendor == vendor) {
- early_qrk[i].f();
- return;
+ device = read_pci_config(num, slot, func,
+ PCI_DEVICE_ID);
+ device >>= 16;
+
+ for(i=0;early_qrk[i].f != NULL;i++) {
+ if (((early_qrk[i].vendor == PCI_ANY_ID) ||
+ (early_qrk[i].vendor == vendor)) &&
+ ((early_qrk[i].device == PCI_ANY_ID) ||
+ (early_qrk[i].device == device)) &&
+ ((early_qrk[i].class ^ class) &
+ early_qrk[i].class_mask)) {
+ early_qrk[i].f();
}
+ }

type = read_pci_config_byte(num, slot, func,
PCI_HEADER_TYPE);