Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753090AbcD0Njr (ORCPT ); Wed, 27 Apr 2016 09:39:47 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:51499 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752610AbcD0Njp (ORCPT ); Wed, 27 Apr 2016 09:39:45 -0400 Subject: Re: [Xen-devel] [PATCH] xen/x86: actually allocate legacy interrupts on PV guests To: David Vrabel , Juergen Gross , Stefano Stabellini , Konrad Rzeszutek Wilk References: <57189880.6030102@suse.com> <572047D3.1000002@suse.com> <572087E0.4030708@citrix.com> Cc: xen-devel@lists.xenproject.org, linux-kernel@vger.kernel.org From: Boris Ostrovsky Message-ID: <5720C0E2.8080904@oracle.com> Date: Wed, 27 Apr 2016 09:38:42 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <572087E0.4030708@citrix.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Source-IP: userv0021.oracle.com [156.151.31.71] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2008 Lines: 51 On 04/27/2016 05:35 AM, David Vrabel wrote: > 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(). int xen_nr_legacy_irqs() { if (xen_hvm_domain()) return nr_legacy_irqs(); if (xen_initial_domain()) return NR_IRQS_LEGACY; return 0; } ? -boris