| 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
| ||
|
Message-ID: <alpine.DEB.2.10.1604121440480.23347@sstabellini-ThinkPad-X260>
Date: Tue, 12 Apr 2016 14:56:22 -0700 (PDT)
From: Stefano Stabellini <sstabellini@...nel.org>
To: Boris Ostrovsky <boris.ostrovsky@...cle.com>
cc: Stefano Stabellini <sstabellini@...nel.org>,
Vitaly Kuznetsov <vkuznets@...hat.com>, x86@...nel.org,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>,
Jiang Liu <jiang.liu@...ux.intel.com>,
"K. Y. Srinivasan" <kys@...rosoft.com>,
linux-kernel@...r.kernel.org, konrad.wilk@...cle.com,
david.vrabel@...rix.com, jgross@...e.com
Subject: Re: Xen regression, Was: [PATCH] x86/irq: Probe for PIC presence
before allocating descs for legacy IRQs
On Tue, 12 Apr 2016, Boris Ostrovsky wrote:
> On 04/12/2016 05:14 PM, Stefano Stabellini wrote:
> > On Tue, 12 Apr 2016, Boris Ostrovsky wrote:
> > > On 04/12/2016 02:06 PM, 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?
> > > Right.
> > >
> > > > If so, it doesn't work because it
> > > > doesn't get called for irq 14:
> > > So how has it worked until now then?
> > It didn't for me :-)
> > Maybe it was working for DomUs, but not for Dom0?
>
> I most certainly run this as both dom0 and domU on variety of machines.
>
> So perhaps there is some other problem?
I am not sure, maybe you didn't have CONFIG_SPARSE_IRQ?
But I am certain that 4.6-rc2, with the attached config, fails as Dom0
on QEMU with the following sequence of calls:
> > > > 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@...nel.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)
> > >
> > > Won't we need the same change in the 'if (0 == nr_ioapics)' clause?
> > That's not a problem: there is 1 ioapic in my system and is detected
> > correctly.
>
> True, but if a system has no ioapics then we will again fail to manage the
> pirq, no?
Right, possibly. I guess there is no harm in doing that, given that if
xen_bind_pirq_gsi_to_irq has already been called for the irq, then it
simply returns.
---
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@...nel.org>
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index beac4df..5668aca 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)
@@ -503,7 +503,7 @@ int __init pci_xen_initial_domain(void)
true /* Map GSI to PIRQ */);
}
if (0 == nr_ioapics) {
- for (irq = 0; irq < nr_legacy_irqs(); irq++)
+ for (irq = 0; irq < NR_IRQS_LEGACY; irq++)
xen_bind_pirq_gsi_to_irq(irq, irq, 0, "xt-pic");
}
return 0;
View attachment "config" of type "TEXT/PLAIN" (120874 bytes)
Powered by blists - more mailing lists