Messages in this thread | | | Subject | Re: Xen regression, Was: [PATCH] x86/irq: Probe for PIC presence before allocating descs for legacy IRQs | From | Boris Ostrovsky <> | Date | Thu, 14 Apr 2016 10:46:08 -0400 |
| |
On 04/14/2016 09:45 AM, David Vrabel wrote: > 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 <sstabellini@kernel.org> >> >> 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.
s/PCI/PIC
> */ > > Does this make sense? > > David
OK. (I first thought that we also don't have RTC and yet we claim that dom0 has it. But then I realized that we do emulate it)
-boris
| |