Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755651AbcDNNp1 (ORCPT ); Thu, 14 Apr 2016 09:45:27 -0400 Received: from smtp.citrix.com ([66.165.176.89]:39177 "EHLO SMTP.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754896AbcDNNp0 (ORCPT ); Thu, 14 Apr 2016 09:45:26 -0400 X-IronPort-AV: E=Sophos;i="5.24,484,1454976000"; d="scan'208";a="347085186" Subject: Re: Xen regression, Was: [PATCH] x86/irq: Probe for PIC presence before allocating descs for legacy IRQs To: Stefano Stabellini , Boris Ostrovsky References: <1446470676-1877-1-git-send-email-vkuznets@redhat.com> <570CF69A.1020701@oracle.com> CC: Vitaly Kuznetsov , , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , Jiang Liu , "K. Y. Srinivasan" , , , From: David Vrabel X-Enigmail-Draft-Status: N1110 Message-ID: <570F9EF2.1030604@citrix.com> Date: Thu, 14 Apr 2016 14:45:22 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-DLP: MIA1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2802 Lines: 80 On 12/04/16 19:06, Stefano Stabellini wrote: > On Tue, 12 Apr 2016, Boris Ostrovsky wrote: >> On 04/11/2016 10:08 PM, Stefano Stabellini wrote: >>> 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 >> >> Stefano, do you have b4ff8389ed14b849354b59ce9b360bdefcdbf99c ? >> >> It was supposed to fix this problem for Xen. However, I just noticed that >> arch/arm64/include/asm/irq.h makes nr_legacy_irqs() return 0 (unlike >> arch/arm/include/asm/irq.h). Could that be the problem? > > I have b4ff8389ed14b849354b59ce9b360bdefcdbf99c but it doesn't fix the > issue for me. > > Is the idea of your patch that xen_allocate_irq_gsi will allocate the > descriptor dynamically instead? If so, it doesn't work because it > doesn't get called for irq 14: > > piix_init_one -> ata_pci_sff_activate_host -> devm_request_irq -> > devm_request_threaded_irq-> request_threaded_irq -> irq_to_desc(14) -> > -EVAIL > > If you look at pci_xen_initial_domain, the loop: > > for (irq = 0; irq < nr_legacy_irqs(); irq++) { > > won't work anymore because by the time is called, nr_legacy_irqs() > already returns 0, because it has been changed by probe_8259A(). > > We also need the following patch: > > --- > > xen/x86: actually allocate legacy interrupts on PV guests > > b4ff8389ed14 is incomplete: relies on nr_legacy_irqs() to get the number > of legacy interrupts when actually nr_legacy_irqs() returns 0 after > probe_8259A(). Use NR_IRQS_LEGACY instead. > > Signed-off-by: Stefano Stabellini > > diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c > index beac4df..6db0060 100644 > --- a/arch/x86/pci/xen.c > +++ b/arch/x86/pci/xen.c > @@ -492,7 +492,7 @@ int __init pci_xen_initial_domain(void) > __acpi_register_gsi = acpi_register_gsi_xen; > __acpi_unregister_gsi = NULL; > /* Pre-allocate legacy irqs */ > - for (irq = 0; irq < nr_legacy_irqs(); irq++) { > + for (irq = 0; irq < NR_IRQS_LEGACY; irq++) { > int trigger, polarity; > > if (acpi_get_override_irq(irq, &trigger, &polarity) == -1) I think I prefer this fix because PV guests don't have a legacy PIC and don't need any legacy irqs allocated so fiddling with nr_legacy_irqs() seems wrong. But we do need a comment here saying something like: /* * Pre-allocate the legacy IRQs. Use NR_LEGACY_IRQS here * because we don't have a PCI and thus nr_legacy_irqs() is zero. */ Does this make sense? David