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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
