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

[ebiederm@xmission.com (Eric W. Biederman)]

Jeremy Fitzhardinge <jeremy@goop.org> 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@citrix.com>

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@xmission.com>

> 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();