Re: [PATCH] x86: don't special case first IO-APIC

[Keir Fraser <keir.xen@gmail.com>]

On 24/10/2012 00:53, "Jan Beulich" <JBeulich@suse.com> wrote:

> It has always been puzzling me why the first IO-APIC gets special cased
> in two places, and finally Xen got run on a system where this breaks:
> 
> (XEN) ACPI: IOAPIC (id[0x10] address[0xfecff000] gsi_base[0])
> (XEN) IOAPIC[0]: apic_id 16, version 17, address 0xfecff000, GSI 0-2
> (XEN) ACPI: IOAPIC (id[0x0f] address[0xfec00000] gsi_base[3])
> (XEN) IOAPIC[1]: apic_id 15, version 17, address 0xfec00000, GSI 3-38
> (XEN) ACPI: IOAPIC (id[0x0e] address[0xfec01000] gsi_base[39])
> (XEN) IOAPIC[2]: apic_id 14, version 17, address 0xfec01000, GSI 39-74
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 1 global_irq 4 dfl dfl)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 5 dfl dfl)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 3 global_irq 6 dfl dfl)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 4 global_irq 7 dfl dfl)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 6 global_irq 9 dfl dfl)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 7 global_irq 10 dfl dfl)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 8 global_irq 11 low edge)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 12 dfl dfl)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 12 global_irq 15 dfl dfl)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 13 global_irq 16 dfl dfl)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 14 global_irq 17 low edge)
> (XEN) ACPI: INT_SRC_OVR (bus 0 bus_irq 15 global_irq 18 dfl dfl)
> 
> i.e. all legacy IRQs (apart from the timer one, but the firmware passed
> data doesn't look right for that case anyway, as both Xen and native
> Linux are falling back to use the virtual wire setup for IRQ0,
> apparently rather using pin 2 of the first IO-APIC) are being handled
> by the second IO-APIC.
> 
> This at once eliminates the possibility of an unmasked RTE getting
> written without having got a vector put in place (in
> setup_IO_APIC_irqs()).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Keir Fraser <keir@xen.org>

> --- a/xen/arch/x86/io_apic.c
> +++ b/xen/arch/x86/io_apic.c
> @@ -990,18 +990,17 @@ static void __init setup_IO_APIC_irqs(vo
>              else
>                  add_pin_to_irq(irq, apic, pin);
>  
> -            if (!apic && !IO_APIC_IRQ(irq))
> +            if (!IO_APIC_IRQ(irq))
>                  continue;
>  
> -            if (IO_APIC_IRQ(irq)) {
> -                vector = assign_irq_vector(irq, NULL);
> -                BUG_ON(vector < 0);
> -                entry.vector = vector;
> -                ioapic_register_intr(irq, IOAPIC_AUTO);
> +            vector = assign_irq_vector(irq, NULL);
> +            BUG_ON(vector < 0);
> +            entry.vector = vector;
> +            ioapic_register_intr(irq, IOAPIC_AUTO);
> +
> +            if (platform_legacy_irq(irq))
> +                disable_8259A_irq(irq_to_desc(irq));
>  
> -                if (!apic && platform_legacy_irq(irq))
> -                    disable_8259A_irq(irq_to_desc(irq));
> -            }
>              desc = irq_to_desc(irq);
>              SET_DEST(entry.dest.dest32, entry.dest.logical.logical_dest,
>                       cpu_mask_to_apicid(desc->arch.cpu_mask));
> @@ -2245,18 +2244,15 @@ unsigned apic_gsi_base(int apic);
>  
>  static int apic_pin_2_gsi_irq(int apic, int pin)
>  {
> -    int idx, irq;
> +    int idx;
>  
>      if (apic < 0)
>         return -EINVAL;
>  
> -    irq = apic_gsi_base(apic) + pin;
> -    if (apic == 0) {
> -        idx = find_irq_entry(apic, pin, mp_INT);
> -        if (idx >= 0)
> -            irq = pin_2_irq(idx, apic, pin);
> -    }
> -    return irq;
> +    idx = find_irq_entry(apic, pin, mp_INT);
> +
> +    return idx >= 0 ? pin_2_irq(idx, apic, pin)
> +                    : apic_gsi_base(apic) + pin;
>  }
>  
>  int ioapic_guest_read(unsigned long physbase, unsigned int reg, u32 *pval)
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel