Hi,
In ia64 kernel, IOSAPIC's RTEs for PCI interrupts are unmasked at the
boot time before installing device drivers. I think it is very dangerous.
If some PCI devices without device driver generate interrupts, interrupts
are generated repeatedly because these interrupt requests are never
cleared. I think RTEs for PCI interrupts should be unmasked by device
driver.
A following patch fixes this issue.
Regards,
Kenji Kaneshige
diff -Naur linux-2.6.4-rc2/arch/ia64/kernel/iosapic.c
linux-2.6.4-rc2-changed/arch/ia64/kernel/iosapic.c
--- linux-2.6.4-rc2/arch/ia64/kernel/iosapic.c 2004-03-05
15:13:53.155237277 +0900
+++ linux-2.6.4-rc2-changed/arch/ia64/kernel/iosapic.c 2004-03-05
16:48:31.856142526 +0900
@@ -170,7 +170,7 @@
}
static void
-set_rte (unsigned int vector, unsigned int dest)
+set_rte (unsigned int vector, unsigned int dest, int mask)
{
unsigned long pol, trigger, dmode;
u32 low32, high32;
@@ -205,6 +205,7 @@
low32 = ((pol << IOSAPIC_POLARITY_SHIFT) |
(trigger << IOSAPIC_TRIGGER_SHIFT) |
(dmode << IOSAPIC_DELIVERY_SHIFT) |
+ ((mask ? 1 : 0) << IOSAPIC_MASK_SHIFT) |
vector);
/* dest contains both id and eid */
@@ -509,7 +510,7 @@
(trigger == IOSAPIC_EDGE ? "edge" : "level"), dest, vector);
/* program the IOSAPIC routing table */
- set_rte(vector, dest);
+ set_rte(vector, dest, 0);
return vector;
}
@@ -557,7 +558,7 @@
(trigger == IOSAPIC_EDGE ? "edge" : "level"), dest, vector);
/* program the IOSAPIC routing table */
- set_rte(vector, dest);
+ set_rte(vector, dest, 0);
return vector;
}
@@ -583,7 +584,7 @@
trigger == IOSAPIC_EDGE ? "edge" : "level", dest, vector);
/* program the IOSAPIC routing table */
- set_rte(vector, dest);
+ set_rte(vector, dest, 0);
}
void __init
@@ -669,7 +670,7 @@
/* direct the interrupt vector to the running cpu id */
dest = (ia64_getreg(_IA64_REG_CR_LID) >> 16) & 0xffff;
#endif
- set_rte(vector, dest);
+ set_rte(vector, dest, 1);
printk(KERN_INFO "IOSAPIC: vector %d -> CPU 0x%04x, enabled\n",
vector, dest);
On Mon, Mar 08, 2004 at 11:49:10AM +0900, Kenji Kaneshige wrote:
> In ia64 kernel, IOSAPIC's RTEs for PCI interrupts are unmasked at the
> boot time before installing device drivers. I think it is very dangerous.
Hi Kenji,
I think this behavior exists to support "legacy" IRQ probing.
I'm wondering if it would be sufficient to wrap the patch in
"#ifndef CONFIG_ISA" or something like that.
grant
Hi, Kenji,
I am a little bit confused with the patch:
Mask bit of RDL's low word indicates the interrupt is enabled when it is zero, disabled when it is 1. But from the patch below, it seems that Kenji enable interrupts in iosapic_register_intr(), iosapic_register_platform_intr(), iosapic_override_isa_irq(), and disable the interrupt in iosapic_enable_intr(). Is it converse?
Thanks,
Pingping (Benjamin) Liu
Intel China Software Center
>-----Original Message-----
>From: [email protected]
>[mailto:[email protected]] On Behalf Of Kenji Kaneshige
>Sent: 2004??3??8?? 10:49
>To: [email protected]
>Cc: [email protected]
>Subject: [PATCH] fix PCI interrupt setting for ia64
>
>
>Hi,
>
>In ia64 kernel, IOSAPIC's RTEs for PCI interrupts are unmasked at the
>boot time before installing device drivers. I think it is very
>dangerous.
>If some PCI devices without device driver generate interrupts,
>interrupts
>are generated repeatedly because these interrupt requests are never
>cleared. I think RTEs for PCI interrupts should be unmasked by device
>driver.
>
>A following patch fixes this issue.
>
>Regards,
>Kenji Kaneshige
>
>
>diff -Naur linux-2.6.4-rc2/arch/ia64/kernel/iosapic.c
>linux-2.6.4-rc2-changed/arch/ia64/kernel/iosapic.c
>--- linux-2.6.4-rc2/arch/ia64/kernel/iosapic.c 2004-03-05
>15:13:53.155237277 +0900
>+++ linux-2.6.4-rc2-changed/arch/ia64/kernel/iosapic.c 2004-03-05
>16:48:31.856142526 +0900
>@@ -170,7 +170,7 @@
> }
>
> static void
>-set_rte (unsigned int vector, unsigned int dest)
>+set_rte (unsigned int vector, unsigned int dest, int mask)
> {
> unsigned long pol, trigger, dmode;
> u32 low32, high32;
>@@ -205,6 +205,7 @@
> low32 = ((pol << IOSAPIC_POLARITY_SHIFT) |
> (trigger << IOSAPIC_TRIGGER_SHIFT) |
> (dmode << IOSAPIC_DELIVERY_SHIFT) |
>+ ((mask ? 1 : 0) << IOSAPIC_MASK_SHIFT) |
> vector);
>
> /* dest contains both id and eid */
>@@ -509,7 +510,7 @@
> (trigger == IOSAPIC_EDGE ? "edge" : "level"),
>dest, vector);
>
> /* program the IOSAPIC routing table */
>- set_rte(vector, dest);
>+ set_rte(vector, dest, 0);
> return vector;
> }
>
>@@ -557,7 +558,7 @@
> (trigger == IOSAPIC_EDGE ? "edge" : "level"),
>dest, vector);
>
> /* program the IOSAPIC routing table */
>- set_rte(vector, dest);
>+ set_rte(vector, dest, 0);
> return vector;
> }
>
>@@ -583,7 +584,7 @@
> trigger == IOSAPIC_EDGE ? "edge" : "level", dest, vector);
>
> /* program the IOSAPIC routing table */
>- set_rte(vector, dest);
>+ set_rte(vector, dest, 0);
> }
>
> void __init
>@@ -669,7 +670,7 @@
> /* direct the interrupt vector to the running cpu id */
> dest = (ia64_getreg(_IA64_REG_CR_LID) >> 16) & 0xffff;
> #endif
>- set_rte(vector, dest);
>+ set_rte(vector, dest, 1);
>
> printk(KERN_INFO "IOSAPIC: vector %d -> CPU 0x%04x, enabled\n",
> vector, dest);
>
>-
>To unsubscribe from this list: send the line "unsubscribe
>linux-ia64" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Grant,
Both ISA and PCI device drivers would call arch/ia64/kernel/irq.c:request_irq()-->arch/ia64/kernel/irq.c:setup_irq() -->arch/ia64/kernel/iosapic.c:iosapic_startup_level_irq() or arch/ia64/kernel/iosapic.c:iosapic_startup_edge_irq() function to unmask the IRQ. I believe the ISA can be handled gracefully, if any.
ISA is legacy to IA64. The configuration script of 2.4.23 has CONFIG_ISA off explicitly for IA64, 2.6.2 doesn't have this option for IA64. I just wonder whether the legacy probing method still exists on IA64.
Thanks,
Pingping (Benjamin) Liu
Intel China Software Center
>-----Original Message-----
>From: [email protected]
>[mailto:[email protected]] On Behalf Of Grant Grundler
>Sent: 2004??3??8?? 14:31
>To: Kenji Kaneshige
>Cc: [email protected]; [email protected]
>Subject: Re: [PATCH] fix PCI interrupt setting for ia64
>
>
>On Mon, Mar 08, 2004 at 11:49:10AM +0900, Kenji Kaneshige wrote:
>> In ia64 kernel, IOSAPIC's RTEs for PCI interrupts are unmasked at the
>> boot time before installing device drivers. I think it is
>very dangerous.
>
>Hi Kenji,
>I think this behavior exists to support "legacy" IRQ probing.
>I'm wondering if it would be sufficient to wrap the patch in
>"#ifndef CONFIG_ISA" or something like that.
>
>grant
>-
>To unsubscribe from this list: send the line "unsubscribe
>linux-ia64" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Hi Liu,
I think it is not converse.
What I wanted to do was to mask (disable) IRQ in iosapic_enable_intr().
I think PCI IRQ should be unmasked (enabled) by device driver, not by
iosapic_enable_intr() at the boot time.
Regards,
Kenji Kaneshige
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]]On Behalf Of Liu, Benjamin
> Sent: Monday, March 08, 2004 4:34 PM
> To: Kenji Kaneshige; [email protected]
> Cc: [email protected]
> Subject: RE: [PATCH] fix PCI interrupt setting for ia64
>
>
> Hi, Kenji,
>
> I am a little bit confused with the patch:
> Mask bit of RDL's low word indicates the interrupt is enabled
> when it is zero, disabled when it is 1. But from the patch below,
> it seems that Kenji enable interrupts in iosapic_register_intr(),
> iosapic_register_platform_intr(), iosapic_override_isa_irq(), and
> disable the interrupt in iosapic_enable_intr(). Is it converse?
>
> Thanks,
> Pingping (Benjamin) Liu
> Intel China Software Center
>
>
> >-----Original Message-----
> >From: [email protected]
> >[mailto:[email protected]] On Behalf Of Kenji Kaneshige
> >Sent: 2004??3??8?? 10:49
> >To: [email protected]
> >Cc: [email protected]
> >Subject: [PATCH] fix PCI interrupt setting for ia64
> >
> >
> >Hi,
> >
> >In ia64 kernel, IOSAPIC's RTEs for PCI interrupts are unmasked at the
> >boot time before installing device drivers. I think it is very
> >dangerous.
> >If some PCI devices without device driver generate interrupts,
> >interrupts
> >are generated repeatedly because these interrupt requests are never
> >cleared. I think RTEs for PCI interrupts should be unmasked by device
> >driver.
> >
> >A following patch fixes this issue.
> >
> >Regards,
> >Kenji Kaneshige
> >
> >
> >diff -Naur linux-2.6.4-rc2/arch/ia64/kernel/iosapic.c
> >linux-2.6.4-rc2-changed/arch/ia64/kernel/iosapic.c
> >--- linux-2.6.4-rc2/arch/ia64/kernel/iosapic.c 2004-03-05
> >15:13:53.155237277 +0900
> >+++ linux-2.6.4-rc2-changed/arch/ia64/kernel/iosapic.c 2004-03-05
> >16:48:31.856142526 +0900
> >@@ -170,7 +170,7 @@
> > }
> >
> > static void
> >-set_rte (unsigned int vector, unsigned int dest)
> >+set_rte (unsigned int vector, unsigned int dest, int mask)
> > {
> > unsigned long pol, trigger, dmode;
> > u32 low32, high32;
> >@@ -205,6 +205,7 @@
> > low32 = ((pol << IOSAPIC_POLARITY_SHIFT) |
> > (trigger << IOSAPIC_TRIGGER_SHIFT) |
> > (dmode << IOSAPIC_DELIVERY_SHIFT) |
> >+ ((mask ? 1 : 0) << IOSAPIC_MASK_SHIFT) |
> > vector);
> >
> > /* dest contains both id and eid */
> >@@ -509,7 +510,7 @@
> > (trigger == IOSAPIC_EDGE ? "edge" : "level"),
> >dest, vector);
> >
> > /* program the IOSAPIC routing table */
> >- set_rte(vector, dest);
> >+ set_rte(vector, dest, 0);
> > return vector;
> > }
> >
> >@@ -557,7 +558,7 @@
> > (trigger == IOSAPIC_EDGE ? "edge" : "level"),
> >dest, vector);
> >
> > /* program the IOSAPIC routing table */
> >- set_rte(vector, dest);
> >+ set_rte(vector, dest, 0);
> > return vector;
> > }
> >
> >@@ -583,7 +584,7 @@
> > trigger == IOSAPIC_EDGE ? "edge" : "level", dest, vector);
> >
> > /* program the IOSAPIC routing table */
> >- set_rte(vector, dest);
> >+ set_rte(vector, dest, 0);
> > }
> >
> > void __init
> >@@ -669,7 +670,7 @@
> > /* direct the interrupt vector to the running cpu id */
> > dest = (ia64_getreg(_IA64_REG_CR_LID) >> 16) & 0xffff;
> > #endif
> >- set_rte(vector, dest);
> >+ set_rte(vector, dest, 1);
> >
> > printk(KERN_INFO "IOSAPIC: vector %d -> CPU 0x%04x, enabled\n",
> > vector, dest);
> >
> >-
> >To unsubscribe from this list: send the line "unsubscribe
> >linux-ia64" in
> >the body of a message to [email protected]
> >More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Thank you for the information, Kenji. But is there any reason to leave it unmasked in iosapic_register_intr(), iosapic_register_platform_intr(), iosapic_override_isa_irq(), given the fact that they would be unmasked finally in individual device drivers?
Thanks,
Pingping (Benjamin) Liu
Intel China Software Center
>-----Original Message-----
>From: Kenji Kaneshige [mailto:[email protected]]
>Sent: 2004??3??8?? 16:31
>To: Liu, Benjamin; Kenji Kaneshige; [email protected]
>Cc: [email protected]
>Subject: RE: [PATCH] fix PCI interrupt setting for ia64
>
>
>Hi Liu,
>
>I think it is not converse.
>What I wanted to do was to mask (disable) IRQ in iosapic_enable_intr().
>I think PCI IRQ should be unmasked (enabled) by device driver, not by
>iosapic_enable_intr() at the boot time.
>
>Regards,
>Kenji Kaneshige
>
>> -----Original Message-----
>> From: [email protected]
>> [mailto:[email protected]]On Behalf Of Liu, Benjamin
>> Sent: Monday, March 08, 2004 4:34 PM
>> To: Kenji Kaneshige; [email protected]
>> Cc: [email protected]
>> Subject: RE: [PATCH] fix PCI interrupt setting for ia64
>>
>>
>> Hi, Kenji,
>>
>> I am a little bit confused with the patch:
>> Mask bit of RDL's low word indicates the interrupt is enabled
>> when it is zero, disabled when it is 1. But from the patch below,
>> it seems that Kenji enable interrupts in iosapic_register_intr(),
>> iosapic_register_platform_intr(), iosapic_override_isa_irq(), and
>> disable the interrupt in iosapic_enable_intr(). Is it converse?
>>
>> Thanks,
>> Pingping (Benjamin) Liu
>> Intel China Software Center
>>
>>
>> >-----Original Message-----
>> >From: [email protected]
>> >[mailto:[email protected]] On Behalf Of
>Kenji Kaneshige
>> >Sent: 2004??3??8?? 10:49
>> >To: [email protected]
>> >Cc: [email protected]
>> >Subject: [PATCH] fix PCI interrupt setting for ia64
>> >
>> >
>> >Hi,
>> >
>> >In ia64 kernel, IOSAPIC's RTEs for PCI interrupts are
>unmasked at the
>> >boot time before installing device drivers. I think it is very
>> >dangerous.
>> >If some PCI devices without device driver generate interrupts,
>> >interrupts
>> >are generated repeatedly because these interrupt requests are never
>> >cleared. I think RTEs for PCI interrupts should be unmasked
>by device
>> >driver.
>> >
>> >A following patch fixes this issue.
>> >
>> >Regards,
>> >Kenji Kaneshige
>> >
>> >
>> >diff -Naur linux-2.6.4-rc2/arch/ia64/kernel/iosapic.c
>> >linux-2.6.4-rc2-changed/arch/ia64/kernel/iosapic.c
>> >--- linux-2.6.4-rc2/arch/ia64/kernel/iosapic.c 2004-03-05
>> >15:13:53.155237277 +0900
>> >+++ linux-2.6.4-rc2-changed/arch/ia64/kernel/iosapic.c 2004-03-05
>> >16:48:31.856142526 +0900
>> >@@ -170,7 +170,7 @@
>> > }
>> >
>> > static void
>> >-set_rte (unsigned int vector, unsigned int dest)
>> >+set_rte (unsigned int vector, unsigned int dest, int mask)
>> > {
>> > unsigned long pol, trigger, dmode;
>> > u32 low32, high32;
>> >@@ -205,6 +205,7 @@
>> > low32 = ((pol << IOSAPIC_POLARITY_SHIFT) |
>> > (trigger << IOSAPIC_TRIGGER_SHIFT) |
>> > (dmode << IOSAPIC_DELIVERY_SHIFT) |
>> >+ ((mask ? 1 : 0) << IOSAPIC_MASK_SHIFT) |
>> > vector);
>> >
>> > /* dest contains both id and eid */
>> >@@ -509,7 +510,7 @@
>> > (trigger == IOSAPIC_EDGE ? "edge" : "level"),
>> >dest, vector);
>> >
>> > /* program the IOSAPIC routing table */
>> >- set_rte(vector, dest);
>> >+ set_rte(vector, dest, 0);
>> > return vector;
>> > }
>> >
>> >@@ -557,7 +558,7 @@
>> > (trigger == IOSAPIC_EDGE ? "edge" : "level"),
>> >dest, vector);
>> >
>> > /* program the IOSAPIC routing table */
>> >- set_rte(vector, dest);
>> >+ set_rte(vector, dest, 0);
>> > return vector;
>> > }
>> >
>> >@@ -583,7 +584,7 @@
>> > trigger == IOSAPIC_EDGE ? "edge" : "level",
>dest, vector);
>> >
>> > /* program the IOSAPIC routing table */
>> >- set_rte(vector, dest);
>> >+ set_rte(vector, dest, 0);
>> > }
>> >
>> > void __init
>> >@@ -669,7 +670,7 @@
>> > /* direct the interrupt vector to the running cpu id */
>> > dest = (ia64_getreg(_IA64_REG_CR_LID) >> 16) & 0xffff;
>> > #endif
>> >- set_rte(vector, dest);
>> >+ set_rte(vector, dest, 1);
>> >
>> > printk(KERN_INFO "IOSAPIC: vector %d -> CPU 0x%04x,
>enabled\n",
>> > vector, dest);
>> >
>> >-
>> >To unsubscribe from this list: send the line "unsubscribe
>> >linux-ia64" in
>> >the body of a message to [email protected]
>> >More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >
>> -
>> To unsubscribe from this list: send the line "unsubscribe
>linux-ia64" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
Hi,
From: "Liu, Benjamin" <[email protected]>
Subject: RE: [PATCH] fix PCI interrupt setting for ia64
Date: Mon, 8 Mar 2004 15:44:16 +0800
> ISA is legacy to IA64. The configuration script of 2.4.23 has
> CONFIG_ISA off explicitly for IA64, 2.6.2 doesn't have this
> option for IA64. I just wonder whether the legacy probing
> method still exists on IA64.
I think that's still true for IDE / serial port drivers.
Kaneshige-san, could you confirm your changes are compatible
with probe_irq_on()?
Itanium-generation machines (such as BigSur) depends on
probe_irq_on() for finding serial port IRQ.
---
Takayoshi Kochi <[email protected]>
Hi,
As a matter of fact, I don't have special reason to leave RTEs unmasked in
iosapic_register_intr(), iosapic_register_platform_intr(),
iosapic_override_isa_irq().
I think it is better that interrupts are unmasked by individual device
drivers, but
there are some exceptions. For example, PMI and INIT don't need device
drivers.
So I think more investigation is needed about them.
Regards,
Kenji Kaneshige
> -----Original Message-----
> From: Liu, Benjamin [mailto:[email protected]]
> Sent: Monday, March 08, 2004 6:15 PM
> To: Kenji Kaneshige; [email protected]
> Cc: [email protected]
> Subject: RE: [PATCH] fix PCI interrupt setting for ia64
>
>
> Thank you for the information, Kenji. But is there any reason to
> leave it unmasked in iosapic_register_intr(),
> iosapic_register_platform_intr(), iosapic_override_isa_irq(),
> given the fact that they would be unmasked finally in individual
> device drivers?
Hi,
> Kaneshige-san, could you confirm your changes are compatible
> with probe_irq_on()?
OK. I'll confirm.
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]]On Behalf Of Takayoshi Kochi
> Sent: Monday, March 08, 2004 6:26 PM
> To: [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH] fix PCI interrupt setting for ia64
>
>
> Hi,
>
> From: "Liu, Benjamin" <[email protected]>
> Subject: RE: [PATCH] fix PCI interrupt setting for ia64
> Date: Mon, 8 Mar 2004 15:44:16 +0800
>
> > ISA is legacy to IA64. The configuration script of 2.4.23 has
> > CONFIG_ISA off explicitly for IA64, 2.6.2 doesn't have this
> > option for IA64. I just wonder whether the legacy probing
> > method still exists on IA64.
>
> I think that's still true for IDE / serial port drivers.
> Kaneshige-san, could you confirm your changes are compatible
> with probe_irq_on()?
>
> Itanium-generation machines (such as BigSur) depends on
> probe_irq_on() for finding serial port IRQ.
>
> ---
> Takayoshi Kochi <[email protected]>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Monday 08 March 2004 2:25 am, Takayoshi Kochi wrote:
> I think that's still true for IDE / serial port drivers.
> Kaneshige-san, could you confirm your changes are compatible
> with probe_irq_on()?
>
> Itanium-generation machines (such as BigSur) depends on
> probe_irq_on() for finding serial port IRQ.
Strictly speaking, since ACPI tells us about IRQs, we shouldn't need
probe_irq_on() on ia64, should we?
I don't see any ACPI smarts in the IDE driver, but I think the
serial driver needs only the attached patch to make it avoid
the use of probe_irq_on(). I tested this on i2k and various
HP zx1 boxes, and it works fine.
Russell, if you agree, would you mind applying this?
ACPI and HCDP tell us what IRQ the serial port uses, so there's
no need to have the driver probe for the IRQ.
===== drivers/serial/8250_acpi.c 1.7 vs edited =====
--- 1.7/drivers/serial/8250_acpi.c Fri Jan 16 15:01:45 2004
+++ edited/drivers/serial/8250_acpi.c Mon Mar 8 11:14:51 2004
@@ -134,8 +134,7 @@
}
serial_req.baud_base = BASE_BAUD;
- serial_req.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF |
- UPF_AUTO_IRQ | UPF_RESOURCES;
+ serial_req.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF | UPF_RESOURCES;
priv->line = register_serial(&serial_req);
if (priv->line < 0) {
===== drivers/serial/8250_hcdp.c 1.2 vs edited =====
--- 1.2/drivers/serial/8250_hcdp.c Sun Jan 11 16:27:13 2004
+++ edited/drivers/serial/8250_hcdp.c Mon Mar 8 11:28:27 2004
@@ -186,8 +186,6 @@
port.irq = gsi;
#endif
port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF | UPF_RESOURCES;
- if (gsi)
- port.flags |= ASYNC_AUTO_IRQ;
/*
* Note: the above memset() initializes port.line to 0,
On Monday 08 March 2004 2:25 am, Takayoshi Kochi wrote:
> I think that's still true for IDE / serial port drivers.
> Kaneshige-san, could you confirm your changes are compatible
> with probe_irq_on()?
>
> Itanium-generation machines (such as BigSur) depends on
> probe_irq_on() for finding serial port IRQ.
Strictly speaking, since ACPI tells us about IRQs, we shouldn't need
probe_irq_on() on ia64, should we?
I don't see any ACPI smarts in the IDE driver, but I think the
serial driver needs only the attached patch to make it avoid
the use of probe_irq_on(). I tested this on i2k and various
HP zx1 boxes, and it works fine.
Russell, if you agree, would you mind applying this?
ACPI and HCDP tell us what IRQ the serial port uses, so there's
no need to have the driver probe for the IRQ.
===== drivers/serial/8250_acpi.c 1.7 vs edited =====
--- 1.7/drivers/serial/8250_acpi.c Fri Jan 16 15:01:45 2004
+++ edited/drivers/serial/8250_acpi.c Mon Mar 8 11:14:51 2004
@@ -134,8 +134,7 @@
}
serial_req.baud_base = BASE_BAUD;
- serial_req.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF |
- UPF_AUTO_IRQ | UPF_RESOURCES;
+ serial_req.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF | UPF_RESOURCES;
priv->line = register_serial(&serial_req);
if (priv->line < 0) {
===== drivers/serial/8250_hcdp.c 1.2 vs edited =====
--- 1.2/drivers/serial/8250_hcdp.c Sun Jan 11 16:27:13 2004
+++ edited/drivers/serial/8250_hcdp.c Mon Mar 8 11:28:27 2004
@@ -186,8 +186,6 @@
port.irq = gsi;
#endif
port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF | UPF_RESOURCES;
- if (gsi)
- port.flags |= ASYNC_AUTO_IRQ;
/*
* Note: the above memset() initializes port.line to 0,
>>>>> On Sun, 7 Mar 2004 22:30:44 -0800, Grant Grundler <[email protected]> said:
Grant> Hi Kenji, I think this behavior exists to support "legacy"
Grant> IRQ probing.
I don't think so. probe_irq_on() calls the "startup" callback which
should do an unmask_irq() for I/O SAPIC. Can somebody confirm that a
Merced box (without Bjorn's patch) will boot fine with Kenji
Kaneshige's patch applied?
--david
>>>>> On Mon, 8 Mar 2004 12:13:22 -0700, Bjorn Helgaas <[email protected]> said:
Bjorn> Strictly speaking, since ACPI tells us about IRQs, we
Bjorn> shouldn't need probe_irq_on() on ia64, should we?
Bjorn> I don't see any ACPI smarts in the IDE driver, but I think
Bjorn> the serial driver needs only the attached patch to make it
Bjorn> avoid the use of probe_irq_on(). I tested this on i2k and
Bjorn> various HP zx1 boxes, and it works fine.
Bjorn> Russell, if you agree, would you mind applying this?
Bjorn> ACPI and HCDP tell us what IRQ the serial port uses, so
Bjorn> there's no need to have the driver probe for the IRQ.
The patch looks good to me and I certainly agree that probe_irq_on()
should be avoided. However, as long as this interface is part of the
Linux kernel, we should support it on ia64 since we can't know for
sure when there are no drivers left using that interface. (And there
is no reason this interface is limited to ISA, though that certainly
is the bus that generally had to use it, for lack of explicit irq
info.)
--david
On Mon, Mar 08, 2004 at 01:44:05PM -0800, David Mosberger wrote:
> >>>>> On Mon, 8 Mar 2004 12:13:22 -0700, Bjorn Helgaas <[email protected]> said:
>
> Bjorn> Strictly speaking, since ACPI tells us about IRQs, we
> Bjorn> shouldn't need probe_irq_on() on ia64, should we?
>
> Bjorn> I don't see any ACPI smarts in the IDE driver, but I think
> Bjorn> the serial driver needs only the attached patch to make it
> Bjorn> avoid the use of probe_irq_on(). I tested this on i2k and
> Bjorn> various HP zx1 boxes, and it works fine.
>
> Bjorn> Russell, if you agree, would you mind applying this?
>
> Bjorn> ACPI and HCDP tell us what IRQ the serial port uses, so
> Bjorn> there's no need to have the driver probe for the IRQ.
>
> The patch looks good to me and I certainly agree that probe_irq_on()
> should be avoided. However, as long as this interface is part of the
> Linux kernel, we should support it on ia64 since we can't know for
> sure when there are no drivers left using that interface. (And there
> is no reason this interface is limited to ISA, though that certainly
> is the bus that generally had to use it, for lack of explicit irq
> info.)
OTOH, if you know "this port is absolutely definitely using this IRQ"
then you don't want to use IRQ probing.
As long as ACPI positively knows the correct IRQ, so we really shouldn't
be using IRQ probing here. The only thing which causes me concern is...
what about ACPI table/BIOS bugs?
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
On Monday 08 March 2004 2:54 pm, Russell King wrote:
> OTOH, if you know "this port is absolutely definitely using this IRQ"
> then you don't want to use IRQ probing.
>
> As long as ACPI positively knows the correct IRQ, so we really shouldn't
> be using IRQ probing here. The only thing which causes me concern is...
> what about ACPI table/BIOS bugs?
I suspect (not being expert in the serial innards) that with the old
code things would "just work", and the ACPI table bug would never
be noticed.
With the new code, my guess is that the device just wouldn't work
(we think it has an interrupt, but it's not hooked up correctly).
My inclination is that it's better to help find ACPI bugs, and
if broken tables turn out to be a problem, we can add some kind
of command-line switch or blacklist to deal with it. But I
guess we should really get David's opinion, since this is a
potential issue for 2.6 distributions.
Bjorn
>>>>> On Mon, 8 Mar 2004 15:05:21 -0700, Bjorn Helgaas <[email protected]> said:
Bjorn> My inclination is that it's better to help find ACPI bugs,
Bjorn> and if broken tables turn out to be a problem, we can add
Bjorn> some kind of command-line switch or blacklist to deal with
Bjorn> it. But I guess we should really get David's opinion, since
Bjorn> this is a potential issue for 2.6 distributions.
I agree with Bjorn's reasoning, but think that the patch should be
tested first on a Big Sur machine (with the latest official firmware).
If something breaks with old firmware, we can then at least ask the
affected people to upgrade their firmware (or come up with a kernel
workaround).
--david
On Monday 08 March 2004 3:10 pm, David Mosberger wrote:
> I agree with Bjorn's reasoning, but think that the patch should be
> tested first on a Big Sur machine (with the latest official firmware).
> If something breaks with old firmware, we can then at least ask the
> affected people to upgrade their firmware (or come up with a kernel
> workaround).
I've tested it on my i2000, and it seems to work fine. This is with
firmware version
W460GXBS2.86E.0117C.P09.200108091154
This is the only BIOS version I found mentioned on http://www.hp.com,
so I assume it's the latest official version.
>>>>> On Mon, 8 Mar 2004 15:41:35 -0700, Bjorn Helgaas <[email protected]> said:
Bjorn> I've tested it on my i2000, and it seems to work fine. This
Bjorn> is with firmware version
Bjorn> W460GXBS2.86E.0117C.P09.200108091154
Bjorn> This is the only BIOS version I found mentioned on
Bjorn> http://www.hp.com, so I assume it's the latest official version.
I'm all for the patch then.
--david
Kenji,
Sorry, I lost track of the status of this patch. Has it been checked
out OK with respect to interrupt probing?
--david
>>>>> On Mon, 08 Mar 2004 11:49:10 +0900, Kenji Kaneshige <[email protected]> said:
Kenji> Hi, In ia64 kernel, IOSAPIC's RTEs for PCI interrupts are
Kenji> unmasked at the boot time before installing device drivers. I
Kenji> think it is very dangerous. If some PCI devices without
Kenji> device driver generate interrupts, interrupts are generated
Kenji> repeatedly because these interrupt requests are never
Kenji> cleared. I think RTEs for PCI interrupts should be unmasked
Kenji> by device driver.
Hi,
I'm sorry that the report falls behind. I wanted to check out by using
real device driver which uses a probe_irq_on(), but I don't have appropriate
environment now.
Though I didn't check out on a real machine yet, I believe my patch doesn't
have any influence on probe_irq_on() because current probe_irq_on() calls
startup callback to unmask the RTEs as you said before.
Regards,
Kenji Kaneshige
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]]On Behalf Of David Mosberger
> Sent: Thursday, March 11, 2004 5:10 AM
> To: Kenji Kaneshige
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH] fix PCI interrupt setting for ia64
>
>
> Kenji,
>
> Sorry, I lost track of the status of this patch. Has it been checked
> out OK with respect to interrupt probing?
>
> --david
>
> >>>>> On Mon, 08 Mar 2004 11:49:10 +0900, Kenji Kaneshige
> <[email protected]> said:
>
> Kenji> Hi, In ia64 kernel, IOSAPIC's RTEs for PCI interrupts are
> Kenji> unmasked at the boot time before installing device drivers. I
> Kenji> think it is very dangerous. If some PCI devices without
> Kenji> device driver generate interrupts, interrupts are generated
> Kenji> repeatedly because these interrupt requests are never
> Kenji> cleared. I think RTEs for PCI interrupts should be unmasked
> Kenji> by device driver.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi,
From: Kenji Kaneshige <[email protected]>
Subject: RE: [PATCH] fix PCI interrupt setting for ia64
Date: Thu, 11 Mar 2004 09:34:06 +0900
> Hi,
>
> I'm sorry that the report falls behind. I wanted to check out by using
> real device driver which uses a probe_irq_on(), but I don't have appropriate
> environment now.
>
> Though I didn't check out on a real machine yet, I believe my patch doesn't
> have any influence on probe_irq_on() because current probe_irq_on() calls
> startup callback to unmask the RTEs as you said before.
My concern was that if there's a buggy PCI device that raises
interrupts all the time until it's initialized by some device driver,
probe_irq_on() would not work as expected regardless of whether
your patch is applied or not. I thought masking the interrupt line
doesn't work around this case.
---
Takayoshi Kochi <[email protected]>
Hi,
Sorry, I misunderstood your concern.
I think you are right. Probe_irq_on() still have another problems even if my
patch
is applied. For example, if buggy PCI device generates interrupts during its
device
driver calls probe_irq_on(), these interrupts might be considered as
spurious and
IRQ probing will fail. I think this is another problem than I pointed out.
In addition to this, if probe_irq_on() is used for PCI interrupts (level
triggered),
interrupts are generated repeatedly because there are no handlers which
clears
these interrupt request. But I think this is not a problem, because these
interrupts
will be masked by probe_irq_on() or probe_irq_off() soon. If this is a
problem, I think
probe_irq_on() should never be used for PCI (level triggered) interrupt
probing.
Regards,
Kenji Kaneshige
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]]On Behalf Of Takayoshi Kochi
> Sent: Thursday, March 11, 2004 10:35 AM
> To: [email protected]
> Cc: [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH] fix PCI interrupt setting for ia64
>
>
> Hi,
>
> From: Kenji Kaneshige <[email protected]>
> Subject: RE: [PATCH] fix PCI interrupt setting for ia64
> Date: Thu, 11 Mar 2004 09:34:06 +0900
>
> > Hi,
> >
> > I'm sorry that the report falls behind. I wanted to check out by using
> > real device driver which uses a probe_irq_on(), but I don't
> have appropriate
> > environment now.
> >
> > Though I didn't check out on a real machine yet, I believe my
> patch doesn't
> > have any influence on probe_irq_on() because current
> probe_irq_on() calls
> > startup callback to unmask the RTEs as you said before.
>
> My concern was that if there's a buggy PCI device that raises
> interrupts all the time until it's initialized by some device driver,
> probe_irq_on() would not work as expected regardless of whether
> your patch is applied or not. I thought masking the interrupt line
> doesn't work around this case.
>
> ---
> Takayoshi Kochi <[email protected]>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>> On Thu, 11 Mar 2004 09:34:06 +0900, Kenji Kaneshige <[email protected]> said:
Kenji> Hi, I'm sorry that the report falls behind. I wanted to check
Kenji> out by using real device driver which uses a probe_irq_on(),
Kenji> but I don't have appropriate environment now.
Kenji> Though I didn't check out on a real machine yet, I believe my
Kenji> patch doesn't have any influence on probe_irq_on() because
Kenji> current probe_irq_on() calls startup callback to unmask the
Kenji> RTEs as you said before.
OK, I agree.
Thanks,
--david
Kenji,
Your patch failed to apply apparently because all tabs got replaced by
blanks. I fixed it by hand for now, but in the future, I'd appreciate
it if you could either fix your mailer config so it doesn't muck with
whitespace (or don't paste & cut patches if that's how the patch got
mangled). As a last resort, send patches as a MIME attachment.
>>>>> On Thu, 11 Mar 2004 14:29:59 +0900, Kenji Kaneshige <[email protected]> said:
Kenji> I think you are right. Probe_irq_on() still have another
Kenji> problems even if my patch is applied. For example, if buggy
Kenji> PCI device generates interrupts during its device driver
Kenji> calls probe_irq_on(), these interrupts might be considered as
Kenji> spurious and IRQ probing will fail. I think this is another
Kenji> problem than I pointed out.
That should be OK. The probe_irq_on() interface never was guaranteed
to work under all circumstances.
Kenji> In addition to this, if probe_irq_on() is used for PCI
Kenji> interrupts (level triggered), interrupts are generated
Kenji> repeatedly because there are no handlers which clears these
Kenji> interrupt request. But I think this is not a problem, because
Kenji> these interrupts will be masked by probe_irq_on() or
Kenji> probe_irq_off() soon. If this is a problem, I think
Kenji> probe_irq_on() should never be used for PCI (level triggered)
Kenji> interrupt probing.
That's a good point but I doubt it's going to cause trouble because of
the history behind this interface (it's mostly needed for ISA
devices). If it did, it wouldn't be hard to fix, but I'm not sure
it's worth bothering. Perhaps it would be a good idea to get rid of
the interface altogether in the 2.7 timeframe (for ia64 and other
ISA-free platforms at least; I don't think it can be gotten rid of for
x86).
--david