No point in assigning the interrupt vectors if there is no interrupt
chip installed. Move it to lguest_setup_irq().
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Rusty Russell <[email protected]>
---
arch/x86/lguest/boot.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
Index: tip/arch/x86/lguest/boot.c
===================================================================
--- tip.orig/arch/x86/lguest/boot.c
+++ tip/arch/x86/lguest/boot.c
@@ -855,17 +855,13 @@ static void lguest_disable_irq(struct pc
/*
* This sets up the Interrupt Descriptor Table (IDT) entry for each hardware
- * interrupt (except 128, which is used for system calls), and then tells the
- * Linux infrastructure that each interrupt is controlled by our level-based
- * lguest interrupt controller.
+ * interrupt (except 128, which is used for system calls).
*/
static void __init lguest_init_IRQ(void)
{
unsigned int i;
for (i = FIRST_EXTERNAL_VECTOR; i < FIRST_SYSTEM_VECTOR; i++) {
- /* Some systems map "vectors" to interrupts weirdly. Not us! */
- __this_cpu_write(vector_irq[i], i - FIRST_EXTERNAL_VECTOR);
if (i != IA32_SYSCALL_VECTOR)
set_intr_gate(i, irq_entries_start +
8 * (i - FIRST_EXTERNAL_VECTOR));
@@ -893,8 +889,15 @@ int lguest_setup_irq(unsigned int irq)
if (err < 0 && err != -EEXIST)
return err;
+ /*
+ * Tell the Linux infrastructure that the interrupt is
+ * controlled by our level-based lguest interrupt controller.
+ */
irq_set_chip_and_handler_name(irq, &lguest_irq_controller,
handle_level_irq, "level");
+
+ /* Some systems map "vectors" to interrupts weirdly. Not us! */
+ __this_cpu_write(vector_irq[FIRST_EXTERNAL_VECTOR + irq, irq);
return 0;
}
Thomas Gleixner <[email protected]> writes:
> No point in assigning the interrupt vectors if there is no interrupt
> chip installed. Move it to lguest_setup_irq().
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Rusty Russell <[email protected]>
> ---
> arch/x86/lguest/boot.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> Index: tip/arch/x86/lguest/boot.c
> ===================================================================
> --- tip.orig/arch/x86/lguest/boot.c
> +++ tip/arch/x86/lguest/boot.c
> @@ -855,17 +855,13 @@ static void lguest_disable_irq(struct pc
>
> /*
> * This sets up the Interrupt Descriptor Table (IDT) entry for each hardware
> - * interrupt (except 128, which is used for system calls), and then tells the
> - * Linux infrastructure that each interrupt is controlled by our level-based
> - * lguest interrupt controller.
> + * interrupt (except 128, which is used for system calls).
> */
> static void __init lguest_init_IRQ(void)
> {
> unsigned int i;
>
> for (i = FIRST_EXTERNAL_VECTOR; i < FIRST_SYSTEM_VECTOR; i++) {
> - /* Some systems map "vectors" to interrupts weirdly. Not us! */
> - __this_cpu_write(vector_irq[i], i - FIRST_EXTERNAL_VECTOR);
> if (i != IA32_SYSCALL_VECTOR)
> set_intr_gate(i, irq_entries_start +
> 8 * (i - FIRST_EXTERNAL_VECTOR));
> @@ -893,8 +889,15 @@ int lguest_setup_irq(unsigned int irq)
> if (err < 0 && err != -EEXIST)
> return err;
>
> + /*
> + * Tell the Linux infrastructure that the interrupt is
> + * controlled by our level-based lguest interrupt controller.
> + */
> irq_set_chip_and_handler_name(irq, &lguest_irq_controller,
> handle_level_irq, "level");
> +
> + /* Some systems map "vectors" to interrupts weirdly. Not us! */
> + __this_cpu_write(vector_irq[FIRST_EXTERNAL_VECTOR + irq, irq);
Missing ].
Then it doesn't work:
[ 3.832028] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 3.832028] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 3.832028] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 3.839983] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 3.840026] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 3.840026] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 3.840026] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 3.848349] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 3.848349] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 3.848349] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 4.056027] brd: module loaded
[ 4.156025] loop: module loaded
<hit return>
[ 17.712169] do_IRQ: 4 callbacks suppressed
[ 17.712169] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 17.720462] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 17.720462] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 17.729129] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 17.729129] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 17.736523] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 17.736523] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 17.744288] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 17.744288] do_IRQ: 0.33 No irq handler for vector (irq -1)
[ 17.751889] do_IRQ: 0.33 No irq handler for vector (irq -1)
You broke interrupts :(
Cheers,
Rusty.
On Mon, 3 Aug 2015, Rusty Russell wrote:
> Thomas Gleixner <[email protected]> writes:
> > +
> > + /* Some systems map "vectors" to interrupts weirdly. Not us! */
> > + __this_cpu_write(vector_irq[FIRST_EXTERNAL_VECTOR + irq, irq);
>
> Missing ].
Doh.
> [ 17.751889] do_IRQ: 0.33 No irq handler for vector (irq -1)
>
> You broke interrupts :(
Right, because I missed the other place which fiddles with
interrupts. Does the patch below fix the issue?
Thanks,
tglx
---------------->
Index: tip/arch/x86/lguest/boot.c
===================================================================
--- tip.orig/arch/x86/lguest/boot.c
+++ tip/arch/x86/lguest/boot.c
@@ -841,8 +841,7 @@ static int lguest_enable_irq(struct pci_
/* We literally use the PCI interrupt line as the irq number. */
pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &line);
- irq_set_chip_and_handler_name(line, &lguest_irq_controller,
- handle_level_irq, "level");
+ lguest_setup_irq(line);
dev->irq = line;
return 0;
}
Thomas Gleixner <[email protected]> writes:
> On Mon, 3 Aug 2015, Rusty Russell wrote:
>> Thomas Gleixner <[email protected]> writes:
>> > +
>> > + /* Some systems map "vectors" to interrupts weirdly. Not us! */
>> > + __this_cpu_write(vector_irq[FIRST_EXTERNAL_VECTOR + irq, irq);
>>
>> Missing ].
>
> Doh.
>
>> [ 17.751889] do_IRQ: 0.33 No irq handler for vector (irq -1)
>>
>> You broke interrupts :(
>
> Right, because I missed the other place which fiddles with
> interrupts. Does the patch below fix the issue?
Yep. I added error handling.
I reworked it into two patches: one which staticizes lguest_setup_irq()
and moves it up, the other of which applies your changes.
Will post, you can take them...
Thanks,
Rusty.
We make it static and hoist it higher in the file for the next patch.
We also give a nice panic if it fails during boot.
Signed-off-by: Rusty Russell <[email protected]>
---
arch/x86/lguest/boot.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 433e5a7dd37f..f38b7e8a88d2 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -835,6 +835,26 @@ static struct irq_chip lguest_irq_controller = {
.irq_unmask = enable_lguest_irq,
};
+/*
+ * Interrupt descriptors are allocated as-needed, but low-numbered ones are
+ * reserved by the generic x86 code. So we ignore irq_alloc_desc_at if it
+ * tells us the irq is already used: other errors (ie. ENOMEM) we take
+ * seriously.
+ */
+static int lguest_setup_irq(unsigned int irq)
+{
+ int err;
+
+ /* Returns -ve error or vector number. */
+ err = irq_alloc_desc_at(irq, 0);
+ if (err < 0 && err != -EEXIST)
+ return err;
+
+ irq_set_chip_and_handler_name(irq, &lguest_irq_controller,
+ handle_level_irq, "level");
+ return 0;
+}
+
static int lguest_enable_irq(struct pci_dev *dev)
{
u8 line = 0;
@@ -879,26 +899,6 @@ static void __init lguest_init_IRQ(void)
}
/*
- * Interrupt descriptors are allocated as-needed, but low-numbered ones are
- * reserved by the generic x86 code. So we ignore irq_alloc_desc_at if it
- * tells us the irq is already used: other errors (ie. ENOMEM) we take
- * seriously.
- */
-int lguest_setup_irq(unsigned int irq)
-{
- int err;
-
- /* Returns -ve error or vector number. */
- err = irq_alloc_desc_at(irq, 0);
- if (err < 0 && err != -EEXIST)
- return err;
-
- irq_set_chip_and_handler_name(irq, &lguest_irq_controller,
- handle_level_irq, "level");
- return 0;
-}
-
-/*
* Time.
*
* It would be far better for everyone if the Guest had its own clock, but
@@ -1028,7 +1028,8 @@ static void lguest_time_irq(unsigned int irq, struct irq_desc *desc)
static void lguest_time_init(void)
{
/* Set up the timer interrupt (0) to go to our simple timer routine */
- lguest_setup_irq(0);
+ if (lguest_setup_irq(0) != 0)
+ panic("Could not set up timer irq");
irq_set_handler(0, lguest_time_irq);
clocksource_register_hz(&lguest_clock, NSEC_PER_SEC);
--
2.1.4
From: Thomas Gleixner <[email protected]>
No point in assigning the interrupt vectors if there is no interrupt
chip installed. Move it to lguest_setup_irq().
(And call it from lguest_enable_irq).
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Rusty Russell <[email protected]> (fixed typo)
Signed-off-by: Rusty Russell <[email protected]>
---
arch/x86/lguest/boot.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index f38b7e8a88d2..2566c97c01c8 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -850,21 +850,29 @@ static int lguest_setup_irq(unsigned int irq)
if (err < 0 && err != -EEXIST)
return err;
+ /*
+ * Tell the Linux infrastructure that the interrupt is
+ * controlled by our level-based lguest interrupt controller.
+ */
irq_set_chip_and_handler_name(irq, &lguest_irq_controller,
handle_level_irq, "level");
+
+ /* Some systems map "vectors" to interrupts weirdly. Not us! */
+ __this_cpu_write(vector_irq[FIRST_EXTERNAL_VECTOR + irq], irq);
return 0;
}
static int lguest_enable_irq(struct pci_dev *dev)
{
+ int err;
u8 line = 0;
/* We literally use the PCI interrupt line as the irq number. */
pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &line);
- irq_set_chip_and_handler_name(line, &lguest_irq_controller,
- handle_level_irq, "level");
- dev->irq = line;
- return 0;
+ err = lguest_setup_irq(line);
+ if (!err)
+ dev->irq = line;
+ return err;
}
/* We don't do hotplug PCI, so this shouldn't be called. */
@@ -875,17 +883,13 @@ static void lguest_disable_irq(struct pci_dev *dev)
/*
* This sets up the Interrupt Descriptor Table (IDT) entry for each hardware
- * interrupt (except 128, which is used for system calls), and then tells the
- * Linux infrastructure that each interrupt is controlled by our level-based
- * lguest interrupt controller.
+ * interrupt (except 128, which is used for system calls).
*/
static void __init lguest_init_IRQ(void)
{
unsigned int i;
for (i = FIRST_EXTERNAL_VECTOR; i < FIRST_SYSTEM_VECTOR; i++) {
- /* Some systems map "vectors" to interrupts weirdly. Not us! */
- __this_cpu_write(vector_irq[i], i - FIRST_EXTERNAL_VECTOR);
if (i != IA32_SYSCALL_VECTOR)
set_intr_gate(i, irq_entries_start +
8 * (i - FIRST_EXTERNAL_VECTOR));
--
2.1.4
Commit-ID: 27a6f41c1a20a3339f456647a21e45fca5b82b62
Gitweb: http://git.kernel.org/tip/27a6f41c1a20a3339f456647a21e45fca5b82b62
Author: Rusty Russell <[email protected]>
AuthorDate: Tue, 4 Aug 2015 14:02:55 +0930
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 6 Aug 2015 00:14:58 +0200
x86/lguest: Clean up lguest_setup_irq
We make it static and hoist it higher in the file for the next patch.
We also give a nice panic if it fails during boot.
Signed-off-by: Rusty Russell <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/lguest/boot.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index f2dc08c..d933a11 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -835,6 +835,26 @@ static struct irq_chip lguest_irq_controller = {
.irq_unmask = enable_lguest_irq,
};
+/*
+ * Interrupt descriptors are allocated as-needed, but low-numbered ones are
+ * reserved by the generic x86 code. So we ignore irq_alloc_desc_at if it
+ * tells us the irq is already used: other errors (ie. ENOMEM) we take
+ * seriously.
+ */
+static int lguest_setup_irq(unsigned int irq)
+{
+ int err;
+
+ /* Returns -ve error or vector number. */
+ err = irq_alloc_desc_at(irq, 0);
+ if (err < 0 && err != -EEXIST)
+ return err;
+
+ irq_set_chip_and_handler_name(irq, &lguest_irq_controller,
+ handle_level_irq, "level");
+ return 0;
+}
+
static int lguest_enable_irq(struct pci_dev *dev)
{
u8 line = 0;
@@ -879,26 +899,6 @@ static void __init lguest_init_IRQ(void)
}
/*
- * Interrupt descriptors are allocated as-needed, but low-numbered ones are
- * reserved by the generic x86 code. So we ignore irq_alloc_desc_at if it
- * tells us the irq is already used: other errors (ie. ENOMEM) we take
- * seriously.
- */
-int lguest_setup_irq(unsigned int irq)
-{
- int err;
-
- /* Returns -ve error or vector number. */
- err = irq_alloc_desc_at(irq, 0);
- if (err < 0 && err != -EEXIST)
- return err;
-
- irq_set_chip_and_handler_name(irq, &lguest_irq_controller,
- handle_level_irq, "level");
- return 0;
-}
-
-/*
* Time.
*
* It would be far better for everyone if the Guest had its own clock, but
@@ -1040,7 +1040,8 @@ static void lguest_time_irq(unsigned int irq, struct irq_desc *desc)
static void lguest_time_init(void)
{
/* Set up the timer interrupt (0) to go to our simple timer routine */
- lguest_setup_irq(0);
+ if (lguest_setup_irq(0) != 0)
+ panic("Could not set up timer irq");
irq_set_handler(0, lguest_time_irq);
clocksource_register_hz(&lguest_clock, NSEC_PER_SEC);
Commit-ID: ad3f8d5afe503faa78d61a50a1f6eec3afa7c787
Gitweb: http://git.kernel.org/tip/ad3f8d5afe503faa78d61a50a1f6eec3afa7c787
Author: Thomas Gleixner <[email protected]>
AuthorDate: Tue, 4 Aug 2015 14:02:56 +0930
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 6 Aug 2015 00:14:58 +0200
x86/lguest: Do not setup unused irq vectors
No point in assigning the interrupt vectors if there is no interrupt
chip installed. Move it to lguest_setup_irq() and call it from
lguest_enable_irq.
[ rusty: Typo fix and error handling ]
Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/lguest/boot.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index d933a11..2165f45 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -850,21 +850,29 @@ static int lguest_setup_irq(unsigned int irq)
if (err < 0 && err != -EEXIST)
return err;
+ /*
+ * Tell the Linux infrastructure that the interrupt is
+ * controlled by our level-based lguest interrupt controller.
+ */
irq_set_chip_and_handler_name(irq, &lguest_irq_controller,
handle_level_irq, "level");
+
+ /* Some systems map "vectors" to interrupts weirdly. Not us! */
+ __this_cpu_write(vector_irq[FIRST_EXTERNAL_VECTOR + irq], irq);
return 0;
}
static int lguest_enable_irq(struct pci_dev *dev)
{
+ int err;
u8 line = 0;
/* We literally use the PCI interrupt line as the irq number. */
pci_read_config_byte(dev, PCI_INTERRUPT_LINE, &line);
- irq_set_chip_and_handler_name(line, &lguest_irq_controller,
- handle_level_irq, "level");
- dev->irq = line;
- return 0;
+ err = lguest_setup_irq(line);
+ if (!err)
+ dev->irq = line;
+ return err;
}
/* We don't do hotplug PCI, so this shouldn't be called. */
@@ -875,17 +883,13 @@ static void lguest_disable_irq(struct pci_dev *dev)
/*
* This sets up the Interrupt Descriptor Table (IDT) entry for each hardware
- * interrupt (except 128, which is used for system calls), and then tells the
- * Linux infrastructure that each interrupt is controlled by our level-based
- * lguest interrupt controller.
+ * interrupt (except 128, which is used for system calls).
*/
static void __init lguest_init_IRQ(void)
{
unsigned int i;
for (i = FIRST_EXTERNAL_VECTOR; i < FIRST_SYSTEM_VECTOR; i++) {
- /* Some systems map "vectors" to interrupts weirdly. Not us! */
- __this_cpu_write(vector_irq[i], i - FIRST_EXTERNAL_VECTOR);
if (i != IA32_SYSCALL_VECTOR)
set_intr_gate(i, irq_entries_start +
8 * (i - FIRST_EXTERNAL_VECTOR));