Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753703AbcD0Jfe (ORCPT ); Wed, 27 Apr 2016 05:35:34 -0400 Received: from smtp02.citrix.com ([66.165.176.63]:60778 "EHLO SMTP02.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753680AbcD0Jfc (ORCPT ); Wed, 27 Apr 2016 05:35:32 -0400 X-IronPort-AV: E=Sophos;i="5.24,540,1454976000"; d="scan'208";a="356718259" Subject: Re: [Xen-devel] [PATCH] xen/x86: actually allocate legacy interrupts on PV guests To: Juergen Gross , Stefano Stabellini , Konrad Rzeszutek Wilk References: <57189880.6030102@suse.com> <572047D3.1000002@suse.com> CC: , , , From: David Vrabel Message-ID: <572087E0.4030708@citrix.com> Date: Wed, 27 Apr 2016 10:35:28 +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: <572047D3.1000002@suse.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-DLP: MIA2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1766 Lines: 43 On 27/04/16 06:02, Juergen Gross wrote: > On 21/04/16 11:30, Stefano Stabellini wrote: >> On Thu, 21 Apr 2016, Juergen Gross wrote: >>> On 20/04/16 15:15, Stefano Stabellini wrote: >>>> 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. >>> >>> Would you mind describing the resulting problem? >> >> This is a good question. The symptom is: >> >> ata_piix: probe of 0000:00:01.1 failed with error -22 >> >> >>> With this commit message I'm absolutely not capable to decide whether >>> e.g. the other use of nr_legacy_irqs() in pci_xen_initial_domain() is >>> correct or not. >> >> I looked at it but I couldn't really test that code because if I try to >> change the number of ioapics in the system using the "noapic" command >> line option (which actually changes the number if ioapics, not lapics), >> I get an error from Linux saying that noapic is not supported when >> running on Xen. >> >> In my opinion having nr_legacy_irqs() calls in Xen code, which returns >> 0, is like playing with fire. I think it would be safer/saner to replace >> them all with NR_IRQS_LEGACY, simply because reading the code one would >> not expect that all those loops don't actually have any iterations. > > I'm quite sure you should change both uses of nr_legacy_irqs() in > pci_xen_initial_domain(). > > Looking at xen_pcifront_enable_irq() I'm not really sure what is the > correct thing to do. > > Adding Konrad as he might have a better insight. I wonder if it would be helpful to have a xen-specific #define like XEN_NR_LEGACY_PIRQS or something, and document carefully what this means and why it is != nr_legacy_irqs(). David