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

[Vivek Goyal <vgoyal@redhat.com>]

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 <nhorman@tuxdriver.com>
> > 
> > 
> >  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 &= 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,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 == 0xffffffff)
> >  					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 &= 0xffff;
> > -
> > -				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++)
> > +						if (early_qrk[i].vendor == vendor) {
> > +							early_qrk[i].f();
> > +							found_bridge = 1;;
> > +						}
> > +				} else if (!found_host) {
> > +					for (i = 0; early_host_qrk[i].f; i++)
> > +						if (early_host_qrk[i].vendor == vendor) {
> > +							early_host_qrk[i].f();
> > +							found_host = 1;
> > +						}
> > +				}
> > +
> > +				if (found_bridge && found_host)
> > +					return;
> >  
> >  				type = read_pci_config_byte(num, slot, func,
> >  							    PCI_HEADER_TYPE);
> > 
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec
> 
> Sorry to reply to myself, but do we have consensus on this patch?  I'd like to
> figure out its disposition if possible.  
> 

I agree with the approach taken. Somebody needs to review the changes done
for applying early_quirks. I am not well versed with it.

Thanks
Vivek