Messages in this thread | | | Subject | Re: [Xen-devel] [PATCH] xen/x86: actually allocate legacy interrupts on PV guests | From | Boris Ostrovsky <> | Date | Wed, 27 Apr 2016 09:38:42 -0400 |
| |
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
| |