Hi all,
Unfortunately this patch (now commit
8c058b0b9c34d8c8d7912880956543769323e2d8) causes a regression on Xen
when running on top of QEMU: the number of PIT irqs get set to 0 by
probe_8259A but actually there are 16.
Any suggestions on how to fix this?
1) we could revert 8c058b0b9c34d8c8d7912880956543769323e2d8
2) we could introduce an 'if (!xen_domain())' in probe_8259A
3) suggestions welcome
On Mon, 2 Nov 2015, Vitaly Kuznetsov wrote:
> Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain
> interfaces") brought a regression for Hyper-V Gen2 instances. These
> instances don't have i8259 legacy PIC but they use legacy IRQs for serial
> port, rtc, and acpi. With this commit included we end up with these IRQs
> not initialized. Earlier, there was a special workaround for legacy IRQs
> in mp_map_pin_to_irq() doing mp_irqdomain_map() without looking at
> nr_legacy_irqs() and now we fail in __irq_domain_alloc_irqs() when
> irq_domain_alloc_descs() returns -EEXIST.
>
> The essence of the issue seems to be that early_irq_init() calls
> arch_probe_nr_irqs() to figure out the number of legacy IRQs before
> we probe for i8259 and gets 16. Later when init_8259A() is called we switch
> to NULL legacy PIC and nr_legacy_irqs() starts to return 0 but we already
> have 16 descs allocated.
>
> Solve the issue by separating i8259 probe from init and calling it in
> arch_probe_nr_irqs() before we actually use nr_legacy_irqs() information.
>
> Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain
> interfaces")
> Signed-off-by: Vitaly Kuznetsov <[email protected]>
> ---
> arch/x86/include/asm/i8259.h | 1 +
> arch/x86/kernel/apic/vector.c | 3 +++
> arch/x86/kernel/i8259.c | 24 ++++++++++++++++--------
> 3 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/asm/i8259.h b/arch/x86/include/asm/i8259.h
> index ccffa53..bd55a77 100644
> --- a/arch/x86/include/asm/i8259.h
> +++ b/arch/x86/include/asm/i8259.h
> @@ -60,6 +60,7 @@ struct legacy_pic {
> void (*mask_all)(void);
> void (*restore_mask)(void);
> void (*init)(int auto_eoi);
> + void (*probe)(void);
> int (*irq_pending)(unsigned int irq);
> void (*make_irq)(unsigned int irq);
> };
> diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
> index 836d11b..aadd7ae 100644
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -361,6 +361,9 @@ int __init arch_probe_nr_irqs(void)
> if (nr < nr_irqs)
> nr_irqs = nr;
>
> + /* nr_legecy_irqs() depends on the PIC presence */
> + legacy_pic->probe();
> +
> return nr_legacy_irqs();
> }
>
> diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
> index 16cb827..96f1562 100644
> --- a/arch/x86/kernel/i8259.c
> +++ b/arch/x86/kernel/i8259.c
> @@ -295,16 +295,11 @@ static void unmask_8259A(void)
> raw_spin_unlock_irqrestore(&i8259A_lock, flags);
> }
>
> -static void init_8259A(int auto_eoi)
> +static void probe_8259A(void)
> {
> unsigned long flags;
> unsigned char probe_val = ~(1 << PIC_CASCADE_IR);
> unsigned char new_val;
> -
> - i8259A_auto_eoi = auto_eoi;
> -
> - raw_spin_lock_irqsave(&i8259A_lock, flags);
> -
> /*
> * Check to see if we have a PIC.
> * Mask all except the cascade and read
> @@ -312,16 +307,27 @@ static void init_8259A(int auto_eoi)
> * have a PIC, we will read 0xff as opposed to the
> * value we wrote.
> */
> + raw_spin_lock_irqsave(&i8259A_lock, flags);
> +
> outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
> outb(probe_val, PIC_MASTER_IMR);
> new_val = inb(PIC_MASTER_IMR);
> if (new_val != probe_val) {
> printk(KERN_INFO "Using NULL legacy PIC\n");
> legacy_pic = &null_legacy_pic;
> - raw_spin_unlock_irqrestore(&i8259A_lock, flags);
> - return;
> }
>
> + raw_spin_unlock_irqrestore(&i8259A_lock, flags);
> +}
> +
> +static void init_8259A(int auto_eoi)
> +{
> + unsigned long flags;
> +
> + i8259A_auto_eoi = auto_eoi;
> +
> + raw_spin_lock_irqsave(&i8259A_lock, flags);
> +
> outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */
>
> /*
> @@ -388,6 +394,7 @@ struct legacy_pic null_legacy_pic = {
> .mask_all = legacy_pic_noop,
> .restore_mask = legacy_pic_noop,
> .init = legacy_pic_int_noop,
> + .probe = legacy_pic_noop,
> .irq_pending = legacy_pic_irq_pending_noop,
> .make_irq = legacy_pic_uint_noop,
> };
> @@ -400,6 +407,7 @@ struct legacy_pic default_legacy_pic = {
> .mask_all = mask_8259A,
> .restore_mask = unmask_8259A,
> .init = init_8259A,
> + .probe = probe_8259A,
> .irq_pending = i8259A_irq_pending,
> .make_irq = make_8259A_irq,
> };
> --
> 2.4.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
