linux-kernel - Re: [Xen-devel] [PATCH] xen/x86: actually allocate legacy interrupts on PV guests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <572087E0.4030708@citrix.com>
Date:	Wed, 27 Apr 2016 10:35:28 +0100
From:	David Vrabel <david.vrabel@...rix.com>
To:	Juergen Gross <jgross@...e.com>,
	Stefano Stabellini <sstabellini@...nel.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	<xen-devel@...ts.xenproject.org>, <boris.ostrovsky@...cle.com>,
	<linux-kernel@...r.kernel.org>, <david.vrabel@...rix.com>
Subject: Re: [Xen-devel] [PATCH] xen/x86: actually allocate legacy interrupts
 on PV guests

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