Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756350AbcDLIhv (ORCPT ); Tue, 12 Apr 2016 04:37:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34485 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756169AbcDLIhs (ORCPT ); Tue, 12 Apr 2016 04:37:48 -0400 From: Vitaly Kuznetsov To: Stefano Stabellini Cc: x86@kernel.org, Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Jiang Liu , "K. Y. Srinivasan" , linux-kernel@vger.kernel.org, konrad.wilk@oracle.com, boris.ostrovsky@oracle.com, david.vrabel@citrix.com, jgross@suse.com Subject: Re: Xen regression, Was: [PATCH] x86/irq: Probe for PIC presence before allocating descs for legacy IRQs References: <1446470676-1877-1-git-send-email-vkuznets@redhat.com> Date: Tue, 12 Apr 2016 10:37:43 +0200 In-Reply-To: (Stefano Stabellini's message of "Mon, 11 Apr 2016 19:08:40 -0700 (PDT)") Message-ID: <87inzndxyg.fsf@vitty.brq.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5518 Lines: 162 Stefano Stabellini writes: > 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. > How would one see the regression? 8c058b0b9c34d8c8d7912880956543769323e2d8 is present since 4.4 and HVM guests seem to work. Other than that, why does probe_8259A() lie? > Any suggestions on how to fix this? > > 1) we could revert 8c058b0b9c34d8c8d7912880956543769323e2d8 This would re-introduce the original issue I was fixing. > 2) we could introduce an 'if (!xen_domain())' in probe_8259A > 3) suggestions welcome I'd suggest we make probe_8259A() work. It can only return 0 if PIC probe by outb()/inb() fails. Why does it fail on QEMU? > > 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 >> --- >> 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 majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ >> -- Vitaly