[Xen-devel] Re: [PATCH RFC] x86/acpi: don't ignore I/O APICs just because there's no local APIC

[ebiederm@xxxxxxxxxxxx (Eric W. Biederman)]

Jeremy Fitzhardinge <jeremy@xxxxxxxx> writes:

> Parse the ACPI MADT for I/O APIC information, even if the cpu has no
> (apparent) local APIC (ie, the CPU's APIC feature flag is clear).
>
> In principle, the local APIC and the I/O APIC are distinct (but related)
> components, which can be independently present.
>
> In practice this can happen in a Xen system, where the hypervisor has
> full control over the local APICs, and delivers interrupts initiated by
> the I/O APICs via Xen's event channel mechanism.
>
> (This eliminates the need for any explicit if (xen...) tests in
> acpi/boot.c)
>
> Signed-off-by: Jeremy Fitzhardinge<jeremy.fitzhardinge@xxxxxxxxxx>

To be very clear.  We have mechanism and policy mixed in the mptable
and related code today.  While we continue to have that mixed I think
even attempting to reuse it for Xen dom0 is a horrifically bad move.

Nacked-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>

> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 2410469..19d13e5 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -193,9 +193,6 @@ static int __init acpi_parse_madt(struct 
> acpi_table_header *table)
>  {
>       struct acpi_table_madt *madt = NULL;
>
> -     if (!cpu_has_apic)
> -             return -EINVAL;
> -
>       madt = (struct acpi_table_madt *)table;
>       if (!madt) {
>               printk(KERN_WARNING PREFIX "Unable to map MADT\n");
> @@ -1252,9 +1249,6 @@ static int __init acpi_parse_madt_ioapic_entries(void)
>       if (acpi_disabled || acpi_noirq)
>               return -ENODEV;
>
> -     if (!cpu_has_apic)
> -             return -ENODEV;
> -
>       /*
>        * if "noapic" boot option, don't look for IO-APICs
>        */
> @@ -1357,6 +1351,16 @@ static void __init acpi_process_madt(void)
>  #ifdef CONFIG_X86_BIGSMP
>                       generic_bigsmp_probe();
>  #endif
> +             }
> +             if (error == -EINVAL) {
> +                     /*
> +                      * The ACPI tables themselves were malformed.
> +                      * Dell Precision Workstation 410, 610 come here.
> +                      */
> +                     printk(KERN_ERR PREFIX
> +                            "Invalid BIOS MADT, disabling ACPI\n");
> +                     disable_acpi();
> +             } else {
>                       /*
>                        * Parse MADT IO-APIC entries
>                        */
> @@ -1370,14 +1374,6 @@ static void __init acpi_process_madt(void)
>                                       apic->setup_apic_routing();
>                       }
>               }
> -             if (error == -EINVAL) {
> -                     /*
> -                      * Dell Precision Workstation 410, 610 come here.
> -                      */
> -                     printk(KERN_ERR PREFIX
> -                            "Invalid BIOS MADT, disabling ACPI\n");
> -                     disable_acpi();
> -             }
>       } else {
>               /*
>                * ACPI found no MADT, and so ACPI wants UP PIC mode.
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index c6acce2..d5e3f03 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1807,6 +1807,10 @@ __apicdebuginit(void) print_all_local_APICs(void)
>  {
>       int cpu;
>
> +     /* don't print out if apic is not there */
> +     if (!cpu_has_apic)
> +             return;
> +
>       preempt_disable();
>       for_each_online_cpu(cpu)
>               smp_call_function_single(cpu, print_local_APIC, NULL, 1);
> @@ -1849,8 +1853,7 @@ __apicdebuginit(int) print_all_ICs(void)
>  {
>       print_PIC();
>
> -     /* don't print out if apic is not there */
> -     if (!cpu_has_apic || disable_apic)
> +     if (disable_apic)
>               return 0;
>
>       print_all_local_APICs();

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel