2015-11-03 09:40:23

by Vitaly Kuznetsov

[permalink] [raw]
Subject: [PATCH v2] x86/irq: Probe for PIC presence before allocating descs for legacy IRQs

Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain
interfaces") brought a regression for Hyper-V Gen2 instances. These
instances don't have i8259 legacy PIC but they use legacy IRQs for serial
port, rtc, and acpi. With this commit included we end up with these IRQs
not initialized. Earlier, there was a special workaround for legacy IRQs
in mp_map_pin_to_irq() doing mp_irqdomain_map() without looking at
nr_legacy_irqs() and now we fail in __irq_domain_alloc_irqs() when
irq_domain_alloc_descs() returns -EEXIST.

The essence of the issue seems to be that early_irq_init() calls
arch_probe_nr_irqs() to figure out the number of legacy IRQs before
we probe for i8259 and gets 16. Later when init_8259A() is called we switch
to NULL legacy PIC and nr_legacy_irqs() starts to return 0 but we already
have 16 descs allocated.

Solve the issue by separating i8259 probe from init and calling it in
arch_probe_nr_irqs() before we actually use nr_legacy_irqs() information.

Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
Signed-off-by: Vitaly Kuznetsov <[email protected]>
---
Changes since v1:
- Make probe() return the number of legacy irqs [Thomas Gleixner]. I'm not
particularly sure it's gonna be obvious for future readers to realize
that probe() is not just a binary answer to the question if PIC is
present but introducing other entities here looks like an overkill.
---
arch/x86/include/asm/i8259.h | 1 +
arch/x86/kernel/apic/vector.c | 6 +++++-
arch/x86/kernel/i8259.c | 29 +++++++++++++++++++++--------
3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/i8259.h b/arch/x86/include/asm/i8259.h
index ccffa53..39bcefc 100644
--- a/arch/x86/include/asm/i8259.h
+++ b/arch/x86/include/asm/i8259.h
@@ -60,6 +60,7 @@ struct legacy_pic {
void (*mask_all)(void);
void (*restore_mask)(void);
void (*init)(int auto_eoi);
+ int (*probe)(void);
int (*irq_pending)(unsigned int irq);
void (*make_irq)(unsigned int irq);
};
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 836d11b..861bc59 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -361,7 +361,11 @@ int __init arch_probe_nr_irqs(void)
if (nr < nr_irqs)
nr_irqs = nr;

- return nr_legacy_irqs();
+ /*
+ * We don't know if PIC is present at this point so we need to do
+ * probe() to get the right number of legacy IRQs.
+ */
+ return legacy_pic->probe();
}

#ifdef CONFIG_X86_IO_APIC
diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index 16cb827..be22f5a 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -295,16 +295,11 @@ static void unmask_8259A(void)
raw_spin_unlock_irqrestore(&i8259A_lock, flags);
}

-static void init_8259A(int auto_eoi)
+static int probe_8259A(void)
{
unsigned long flags;
unsigned char probe_val = ~(1 << PIC_CASCADE_IR);
unsigned char new_val;
-
- i8259A_auto_eoi = auto_eoi;
-
- raw_spin_lock_irqsave(&i8259A_lock, flags);
-
/*
* Check to see if we have a PIC.
* Mask all except the cascade and read
@@ -312,16 +307,28 @@ static void init_8259A(int auto_eoi)
* have a PIC, we will read 0xff as opposed to the
* value we wrote.
*/
+ raw_spin_lock_irqsave(&i8259A_lock, flags);
+
outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
outb(probe_val, PIC_MASTER_IMR);
new_val = inb(PIC_MASTER_IMR);
if (new_val != probe_val) {
printk(KERN_INFO "Using NULL legacy PIC\n");
legacy_pic = &null_legacy_pic;
- raw_spin_unlock_irqrestore(&i8259A_lock, flags);
- return;
}

+ raw_spin_unlock_irqrestore(&i8259A_lock, flags);
+ return nr_legacy_irqs();
+}
+
+static void init_8259A(int auto_eoi)
+{
+ unsigned long flags;
+
+ i8259A_auto_eoi = auto_eoi;
+
+ raw_spin_lock_irqsave(&i8259A_lock, flags);
+
outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */

/*
@@ -379,6 +386,10 @@ static int legacy_pic_irq_pending_noop(unsigned int irq)
{
return 0;
}
+static int legacy_pic_probe(void)
+{
+ return 0;
+}

struct legacy_pic null_legacy_pic = {
.nr_legacy_irqs = 0,
@@ -388,6 +399,7 @@ struct legacy_pic null_legacy_pic = {
.mask_all = legacy_pic_noop,
.restore_mask = legacy_pic_noop,
.init = legacy_pic_int_noop,
+ .probe = legacy_pic_probe,
.irq_pending = legacy_pic_irq_pending_noop,
.make_irq = legacy_pic_uint_noop,
};
@@ -400,6 +412,7 @@ struct legacy_pic default_legacy_pic = {
.mask_all = mask_8259A,
.restore_mask = unmask_8259A,
.init = init_8259A,
+ .probe = probe_8259A,
.irq_pending = i8259A_irq_pending,
.make_irq = make_8259A_irq,
};
--
2.4.3


Subject: [tip:x86/urgent] x86/irq: Probe for PIC presence before allocating descs for legacy IRQs

Commit-ID: 2518594c0481dbf81edd255753e5fdfee1756a7c
Gitweb: http://git.kernel.org/tip/2518594c0481dbf81edd255753e5fdfee1756a7c
Author: Vitaly Kuznetsov <[email protected]>
AuthorDate: Tue, 3 Nov 2015 10:40:14 +0100
Committer: Thomas Gleixner <[email protected]>
CommitDate: Wed, 4 Nov 2015 11:48:47 +0100

x86/irq: Probe for PIC presence before allocating descs for legacy IRQs

Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain
interfaces") brought a regression for Hyper-V Gen2 instances. These
instances don't have i8259 legacy PIC but they use legacy IRQs for serial
port, rtc, and acpi. With this commit included we end up with these IRQs
not initialized. Earlier, there was a special workaround for legacy IRQs
in mp_map_pin_to_irq() doing mp_irqdomain_map() without looking at
nr_legacy_irqs() and now we fail in __irq_domain_alloc_irqs() when
irq_domain_alloc_descs() returns -EEXIST.

The essence of the issue seems to be that early_irq_init() calls
arch_probe_nr_irqs() to figure out the number of legacy IRQs before
we probe for i8259 and gets 16. Later when init_8259A() is called we switch
to NULL legacy PIC and nr_legacy_irqs() starts to return 0 but we already
have 16 descs allocated.

Solve the issue by separating i8259 probe from init and calling it in
arch_probe_nr_irqs() before we actually use nr_legacy_irqs() information.

Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
Signed-off-by: Vitaly Kuznetsov <[email protected]>
Cc: Jiang Liu <[email protected]>
Cc: K. Y. Srinivasan <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/include/asm/i8259.h | 1 +
arch/x86/kernel/apic/vector.c | 6 +++++-
arch/x86/kernel/i8259.c | 29 +++++++++++++++++++++--------
3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/i8259.h b/arch/x86/include/asm/i8259.h
index ccffa53..39bcefc 100644
--- a/arch/x86/include/asm/i8259.h
+++ b/arch/x86/include/asm/i8259.h
@@ -60,6 +60,7 @@ struct legacy_pic {
void (*mask_all)(void);
void (*restore_mask)(void);
void (*init)(int auto_eoi);
+ int (*probe)(void);
int (*irq_pending)(unsigned int irq);
void (*make_irq)(unsigned int irq);
};
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 836d11b..861bc59 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -361,7 +361,11 @@ int __init arch_probe_nr_irqs(void)
if (nr < nr_irqs)
nr_irqs = nr;

- return nr_legacy_irqs();
+ /*
+ * We don't know if PIC is present at this point so we need to do
+ * probe() to get the right number of legacy IRQs.
+ */
+ return legacy_pic->probe();
}

#ifdef CONFIG_X86_IO_APIC
diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index 16cb827..be22f5a 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -295,16 +295,11 @@ static void unmask_8259A(void)
raw_spin_unlock_irqrestore(&i8259A_lock, flags);
}

-static void init_8259A(int auto_eoi)
+static int probe_8259A(void)
{
unsigned long flags;
unsigned char probe_val = ~(1 << PIC_CASCADE_IR);
unsigned char new_val;
-
- i8259A_auto_eoi = auto_eoi;
-
- raw_spin_lock_irqsave(&i8259A_lock, flags);
-
/*
* Check to see if we have a PIC.
* Mask all except the cascade and read
@@ -312,16 +307,28 @@ static void init_8259A(int auto_eoi)
* have a PIC, we will read 0xff as opposed to the
* value we wrote.
*/
+ raw_spin_lock_irqsave(&i8259A_lock, flags);
+
outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
outb(probe_val, PIC_MASTER_IMR);
new_val = inb(PIC_MASTER_IMR);
if (new_val != probe_val) {
printk(KERN_INFO "Using NULL legacy PIC\n");
legacy_pic = &null_legacy_pic;
- raw_spin_unlock_irqrestore(&i8259A_lock, flags);
- return;
}

+ raw_spin_unlock_irqrestore(&i8259A_lock, flags);
+ return nr_legacy_irqs();
+}
+
+static void init_8259A(int auto_eoi)
+{
+ unsigned long flags;
+
+ i8259A_auto_eoi = auto_eoi;
+
+ raw_spin_lock_irqsave(&i8259A_lock, flags);
+
outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */

/*
@@ -379,6 +386,10 @@ static int legacy_pic_irq_pending_noop(unsigned int irq)
{
return 0;
}
+static int legacy_pic_probe(void)
+{
+ return 0;
+}

struct legacy_pic null_legacy_pic = {
.nr_legacy_irqs = 0,
@@ -388,6 +399,7 @@ struct legacy_pic null_legacy_pic = {
.mask_all = legacy_pic_noop,
.restore_mask = legacy_pic_noop,
.init = legacy_pic_int_noop,
+ .probe = legacy_pic_probe,
.irq_pending = legacy_pic_irq_pending_noop,
.make_irq = legacy_pic_uint_noop,
};
@@ -400,6 +412,7 @@ struct legacy_pic default_legacy_pic = {
.mask_all = mask_8259A,
.restore_mask = unmask_8259A,
.init = init_8259A,
+ .probe = probe_8259A,
.irq_pending = i8259A_irq_pending,
.make_irq = make_8259A_irq,
};

Subject: [tip:x86/urgent] x86/irq: Probe for PIC presence before allocating descs for legacy IRQs

Commit-ID: 8c058b0b9c34d8c8d7912880956543769323e2d8
Gitweb: http://git.kernel.org/tip/8c058b0b9c34d8c8d7912880956543769323e2d8
Author: Vitaly Kuznetsov <[email protected]>
AuthorDate: Tue, 3 Nov 2015 10:40:14 +0100
Committer: Thomas Gleixner <[email protected]>
CommitDate: Sat, 7 Nov 2015 10:37:37 +0100

x86/irq: Probe for PIC presence before allocating descs for legacy IRQs

Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain
interfaces") brought a regression for Hyper-V Gen2 instances. These
instances don't have i8259 legacy PIC but they use legacy IRQs for serial
port, rtc, and acpi. With this commit included we end up with these IRQs
not initialized. Earlier, there was a special workaround for legacy IRQs
in mp_map_pin_to_irq() doing mp_irqdomain_map() without looking at
nr_legacy_irqs() and now we fail in __irq_domain_alloc_irqs() when
irq_domain_alloc_descs() returns -EEXIST.

The essence of the issue seems to be that early_irq_init() calls
arch_probe_nr_irqs() to figure out the number of legacy IRQs before
we probe for i8259 and gets 16. Later when init_8259A() is called we switch
to NULL legacy PIC and nr_legacy_irqs() starts to return 0 but we already
have 16 descs allocated.

Solve the issue by separating i8259 probe from init and calling it in
arch_probe_nr_irqs() before we actually use nr_legacy_irqs() information.

Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
Signed-off-by: Vitaly Kuznetsov <[email protected]>
Cc: Jiang Liu <[email protected]>
Cc: K. Y. Srinivasan <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/include/asm/i8259.h | 1 +
arch/x86/kernel/apic/vector.c | 6 +++++-
arch/x86/kernel/i8259.c | 29 +++++++++++++++++++++--------
3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/i8259.h b/arch/x86/include/asm/i8259.h
index ccffa53..39bcefc 100644
--- a/arch/x86/include/asm/i8259.h
+++ b/arch/x86/include/asm/i8259.h
@@ -60,6 +60,7 @@ struct legacy_pic {
void (*mask_all)(void);
void (*restore_mask)(void);
void (*init)(int auto_eoi);
+ int (*probe)(void);
int (*irq_pending)(unsigned int irq);
void (*make_irq)(unsigned int irq);
};
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 836d11b..861bc59 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -361,7 +361,11 @@ int __init arch_probe_nr_irqs(void)
if (nr < nr_irqs)
nr_irqs = nr;

- return nr_legacy_irqs();
+ /*
+ * We don't know if PIC is present at this point so we need to do
+ * probe() to get the right number of legacy IRQs.
+ */
+ return legacy_pic->probe();
}

#ifdef CONFIG_X86_IO_APIC
diff --git a/arch/x86/kernel/i8259.c b/arch/x86/kernel/i8259.c
index 16cb827..be22f5a 100644
--- a/arch/x86/kernel/i8259.c
+++ b/arch/x86/kernel/i8259.c
@@ -295,16 +295,11 @@ static void unmask_8259A(void)
raw_spin_unlock_irqrestore(&i8259A_lock, flags);
}

-static void init_8259A(int auto_eoi)
+static int probe_8259A(void)
{
unsigned long flags;
unsigned char probe_val = ~(1 << PIC_CASCADE_IR);
unsigned char new_val;
-
- i8259A_auto_eoi = auto_eoi;
-
- raw_spin_lock_irqsave(&i8259A_lock, flags);
-
/*
* Check to see if we have a PIC.
* Mask all except the cascade and read
@@ -312,16 +307,28 @@ static void init_8259A(int auto_eoi)
* have a PIC, we will read 0xff as opposed to the
* value we wrote.
*/
+ raw_spin_lock_irqsave(&i8259A_lock, flags);
+
outb(0xff, PIC_SLAVE_IMR); /* mask all of 8259A-2 */
outb(probe_val, PIC_MASTER_IMR);
new_val = inb(PIC_MASTER_IMR);
if (new_val != probe_val) {
printk(KERN_INFO "Using NULL legacy PIC\n");
legacy_pic = &null_legacy_pic;
- raw_spin_unlock_irqrestore(&i8259A_lock, flags);
- return;
}

+ raw_spin_unlock_irqrestore(&i8259A_lock, flags);
+ return nr_legacy_irqs();
+}
+
+static void init_8259A(int auto_eoi)
+{
+ unsigned long flags;
+
+ i8259A_auto_eoi = auto_eoi;
+
+ raw_spin_lock_irqsave(&i8259A_lock, flags);
+
outb(0xff, PIC_MASTER_IMR); /* mask all of 8259A-1 */

/*
@@ -379,6 +386,10 @@ static int legacy_pic_irq_pending_noop(unsigned int irq)
{
return 0;
}
+static int legacy_pic_probe(void)
+{
+ return 0;
+}

struct legacy_pic null_legacy_pic = {
.nr_legacy_irqs = 0,
@@ -388,6 +399,7 @@ struct legacy_pic null_legacy_pic = {
.mask_all = legacy_pic_noop,
.restore_mask = legacy_pic_noop,
.init = legacy_pic_int_noop,
+ .probe = legacy_pic_probe,
.irq_pending = legacy_pic_irq_pending_noop,
.make_irq = legacy_pic_uint_noop,
};
@@ -400,6 +412,7 @@ struct legacy_pic default_legacy_pic = {
.mask_all = mask_8259A,
.restore_mask = unmask_8259A,
.init = init_8259A,
+ .probe = probe_8259A,
.irq_pending = i8259A_irq_pending,
.make_irq = make_8259A_irq,
};

2015-11-16 21:00:58

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH v2] x86/irq: Probe for PIC presence before allocating descs for legacy IRQs

On 11/03/2015 04:40 AM, Vitaly Kuznetsov wrote:
> Commit d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain
> interfaces") brought a regression for Hyper-V Gen2 instances. These
> instances don't have i8259 legacy PIC but they use legacy IRQs for serial
> port, rtc, and acpi. With this commit included we end up with these IRQs
> not initialized. Earlier, there was a special workaround for legacy IRQs
> in mp_map_pin_to_irq() doing mp_irqdomain_map() without looking at
> nr_legacy_irqs() and now we fail in __irq_domain_alloc_irqs() when
> irq_domain_alloc_descs() returns -EEXIST.
>
> The essence of the issue seems to be that early_irq_init() calls
> arch_probe_nr_irqs() to figure out the number of legacy IRQs before
> we probe for i8259 and gets 16. Later when init_8259A() is called we switch
> to NULL legacy PIC and nr_legacy_irqs() starts to return 0 but we already
> have 16 descs allocated.
>
> Solve the issue by separating i8259 probe from init and calling it in
> arch_probe_nr_irqs() before we actually use nr_legacy_irqs() information.
>
> Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
> Signed-off-by: Vitaly Kuznetsov <[email protected]>

This breaks PV Xen with

[ 0.000000] NR_IRQS:33024 nr_irqs:440 0

...

[ 6.949434] BUG: unable to handle kernel NULL pointer dereference at (null)
[ 6.957564] IP: [<ffffffff8143d339>] xen_irq_init+0x29/0xe0
[ 6.963385] PGD 0
[ 6.965528] Oops: 0002 [#1] SMP
[ 6.968927] Modules linked in:
[ 6.972152] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.3.0upstream-12231-g3e546cd-dirty #1
[ 6.980842] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./To be filled by O.E.M., BIOS 080014 07/18/2008
[ 6.992123] task: ffff880037992d00 ti: ffff880037994000 task.ti: ffff880037994000
[ 6.999904] RIP: e030:[<ffffffff8143d339>] [<ffffffff8143d339>] xen_irq_init+0x29/0xe0
[ 7.008231] RSP: e02b:ffff880037997c48 EFLAGS: 00010246
[ 7.013778] RAX: 0000000000000001 RBX: 0000000000000009 RCX: 0000000000000058
[ 7.021204] RDX: 0000000000000000 RSI: 00000000024080c0 RDI: ffff880038000d88
[ 7.028634] RBP: ffff880037997c58 R08: 0000000000000000 R09: ffff880037997be8
[ 7.036062] R10: 0000000000000000 R11: ffff880038000df8 R12: 0000000000000009
[ 7.043491] R13: 0000000000000001 R14: 0000000000000009 R15: ffffffff819dfa04
[ 7.050922] FS: 0000000000000000(0000) GS:ffff88003de00000(0000) knlGS:0000000000000000
[ 7.059343] CS: e033 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 7.065330] CR2: 0000000000000000 CR3: 0000000001c0b000 CR4: 0000000000000660
[ 7.072759] Stack:
[ 7.074905] 0000000000000009 0000000000000009 ffff880037997cb8 ffffffff8143e5f1
[ 7.082601] 00ff880037997c88 0000000000000000 ffffffff81832820 0000000000000009
[ 7.090299] ffff880037997da4 0000000000000009 0000000000000000 00000000ffffffff
[ 7.097999] Call Trace:
[ 7.100601] [<ffffffff8143e5f1>] xen_bind_pirq_gsi_to_irq+0x81/0x210
[ 7.107313] [<ffffffff815df37c>] xen_register_pirq.clone.0+0x9c/0xf0
[ 7.114026] [<ffffffff815df431>] acpi_register_gsi_xen+0x61/0xd0
[ 7.120378] [<ffffffff8108bd39>] acpi_gsi_to_irq+0x69/0x80
[ 7.126199] [<ffffffff813f4f14>] ? acpi_ev_remove_all_sci_handlers+0xa5/0xa5
[ 7.133626] [<ffffffff813d7bd7>] acpi_os_install_interrupt_handler+0x47/0xc5
[ 7.141053] [<ffffffff813f260c>] ? acpi_ev_gpe_initialize+0x230/0x240
[ 7.147861] [<ffffffff813f5021>] acpi_ev_install_sci_handler+0x3f/0x6a
[ 7.154752] [<ffffffff813f0f27>] ? acpi_ev_initialize_events+0x138/0x151
[ 7.161823] [<ffffffff813f0d38>] acpi_ev_install_xrupt_handlers+0x54/0x10b
[ 7.169074] [<ffffffff81d53ef9>] acpi_enable_subsystem+0x22d/0x27f
[ 7.175605] [<ffffffff81d522e0>] ? acpi_early_init+0xeb/0xeb
[ 7.181599] [<ffffffff81d52355>] acpi_init+0x75/0x294
[ 7.186974] [<ffffffff81d50d91>] ? video_setup+0x85/0x85
[ 7.192612] [<ffffffff81d522e0>] ? acpi_early_init+0xeb/0xeb
[ 7.198607] [<ffffffff81002081>] do_one_initcall+0x81/0x1b0
[ 7.204516] [<ffffffff81d0fb3b>] kernel_init_freeable+0x171/0x20c
[ 7.210958] [<ffffffff81d0fbd6>] ? kernel_init_freeable+0x20c/0x20c
[ 7.217583] [<ffffffff817522f0>] ? rest_init+0x90/0x90
[ 7.223046] [<ffffffff817522f9>] kernel_init+0x9/0xe0
[ 7.228416] [<ffffffff8175f30f>] ret_from_fork+0x3f/0x70
[ 7.234054] [<ffffffff817522f0>] ? rest_init+0x90/0x90
[ 7.239513] Code: 00 00 55 48 89 e5 41 54 53 89 fb e8 e2 65 cc ff 31 d2 48 85 c0 74 08 48 8b 50 10 48 83 c2 18 48 8b 05 ec 72 3d 00 be c0 80 40 02 <48> 89 02 48 8b 05 e5 72 3d 00 48 89 42 08 48 8b 05 e2 72 3d 00
[ 7.259023] RIP [<ffffffff8143d339>] xen_irq_init+0x29/0xe0
[ 7.264932] RSP <ffff880037997c48>
[ 7.268598] CR2: 0000000000000000
[ 7.272095] ---[ end trace 725f5e1483cadab0 ]---
[ 7.276921] Kernel panic - not syncing: Fatal exception
(XEN) Domain 0 crashed: rebooting machine in 5 seconds.


Xen expects legacy interrupts to be there (pretty much for the same
reason as Hyper-V does) and with this change arch_probe_nr_irqs()
returns zero and no descriptors are allocated.

We can allocate those descriptors as needed in xen_irq_init() (if we
know that IRQs are legacy), although that would look somewhat ugly and
out of place.

-boris

-boris

2015-11-16 21:39:57

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v2] x86/irq: Probe for PIC presence before allocating descs for legacy IRQs

On Mon, 16 Nov 2015, Boris Ostrovsky wrote:
> Xen expects legacy interrupts to be there (pretty much for the same reason as
> Hyper-V does) and with this change arch_probe_nr_irqs() returns zero and no
> descriptors are allocated.

Right, because everything which has a PIT gets them and everything
which does not have a PIT does not.

> We can allocate those descriptors as needed in xen_irq_init() (if we know that
> IRQs are legacy), although that would look somewhat ugly and out of place.

Why preallocating them in xen_irq_init()? You simply can remove the
NR_IRQS_LEGACY checks in xen_allocate_irq_gsi/xen_free_irq(), right?

Thanks,

tglx

2015-11-16 21:46:45

by Boris Ostrovsky

[permalink] [raw]
Subject: Re: [PATCH v2] x86/irq: Probe for PIC presence before allocating descs for legacy IRQs

On 11/16/2015 04:39 PM, Thomas Gleixner wrote:
> On Mon, 16 Nov 2015, Boris Ostrovsky wrote:
>> Xen expects legacy interrupts to be there (pretty much for the same reason as
>> Hyper-V does) and with this change arch_probe_nr_irqs() returns zero and no
>> descriptors are allocated.
> Right, because everything which has a PIT gets them and everything
> which does not have a PIT does not.
>
>> We can allocate those descriptors as needed in xen_irq_init() (if we know that
>> IRQs are legacy), although that would look somewhat ugly and out of place.
> Why preallocating them in xen_irq_init()? You simply can remove the
> NR_IRQS_LEGACY checks in xen_allocate_irq_gsi/xen_free_irq(), right?

Of course. Thanks.

-boris