i've only got some small stylistic gripes: > +void __init enable_IR_x2apic(void) > +{ > + unsigned long flags; > + struct IO_APIC_route_entry **ioapic_entries = NULL; > + int ret, x2apic_enabled = 0; > + > ioapic_entries = alloc_ioapic_entries(); > if (!ioapic_entries) { > - pr_info("Allocate ioapic_entries failed: %d\n", ret); > - goto end; > + pr_info("Allocate ioapic_entries failed\n"); > + goto out; That's probably a fatal condition so perhaps a WARN() or pr_err() would be more appropriate. > + ret = enable_IR(); > + if (!ret) { > + /* IR is required if x2apic is enabled by BIOS even > + * when running in kvm since this indicates present > + * of APIC ID > 255 */ please use the customary (multi-line) comment style: /* * Comment ..... * ...... goes here. */ specified in Documentation/CodingStyle. > + if (max_physical_apicid > 255 || !kvm_para_available()) > + goto nox2apic; > + /* without IR all CPUs can be addressed by IOAPIC/MSI > + * only in physical mode */ ditto. > + x2apic_phys = 1; > + } > > - pr_info("Enabled Interrupt-remapping\n"); > + x2apic_enabled = 1; > > if (x2apic_supported() && !x2apic_mode) { > x2apic_mode = 1; > @@ -1414,41 +1437,30 @@ void __init enable_IR_x2apic(void) > pr_info("Enabled x2apic\n"); > } > > -end_restore: > - if (ret) > - /* > - * IR enabling failed > - */ > +nox2apic: > + if (!ret) /* IR enabling failed */ > restore_IO_APIC_setup(ioapic_entries); > - > unmask_8259A(); > local_irq_restore(flags); > > -end: > +out: > if (ioapic_entries) > free_ioapic_entries(ioapic_entries); > > - if (!ret) > + if (x2apic_enabled) > return; > > -ir_failed: > - if (x2apic_preenabled) > + if (x2apic_preenabled) { > +#ifdef CONFIG_INTR_REMAP > panic("x2apic enabled by bios. But IR enabling failed"); > - else if (cpu_has_x2apic) > - pr_info("Not enabling x2apic,Intr-remapping\n"); > #else > - if (!cpu_has_x2apic) > - return; > - > - if (x2apic_preenabled) > panic("x2apic enabled prior OS handover," > - " enable CONFIG_X86_X2APIC, CONFIG_INTR_REMAP"); > + " enable CONFIG_X86_X2APIC, CONFIG_INTR_REMAP"); > #endif can this #ifdef be avoided? > - > - return; > + } else if (cpu_has_x2apic) > + pr_info("Not enabling x2apic,Intr-remapping\n"); please put a space after commas. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html