SN platform needs platform_irq_to_vector?

RFC PATCH In review Kenji Kaneshige 7 participants 14 messages Aug 2, 2007 – Aug 6, 2007

lore

Share & tools

lore

Kenji Kaneshige

author

2007-08-02 04:27 UTC #0 lore raw

cover SN platform needs platform_irq_to_vector?

Hi Jes-san, Russ-san, Christoph-san and SN platform developers,

I glanced over the SN code, and I found SN platform doesn't have SN specific irq_to_vector() function, though it has SN specific local_vector_to_irq() function. I guess it was OK because maybe SN platform depends on irq = vector and old generic irq_to_vector() was just returning the same value as irq.

The irq_to_vector() function, however, has been changed by vector domain support, and currently it can return the different value from irq, even when VECTOR_DOMAIN_PERCPU is not enabled. This can happen when an interrupt thorough I/O SAPIC is unregistered and another interupt is registered. So if my guess (SN platform depends on irq = vector) is true, something problem might happen on SN platform when PCI hotplugging or driver reloading.

If it is true that SN platform depends on irq = vector, we need to implement SN specific irq_to_vector() that returns the same value as irq, I think. I made sample patches to implement platform specific irq_to_vector() as follows. Note that [PATCH 2/2] is not tested yet because I don't have any SN platform unfortunately.

[PATCH 1/2] Add base support for platform_irq_to_vector() [PATCH 2/2] Implement platform_irq_to_vector() for SN

Could you give me comments?

Thanks,
Kenji Kaneshige

Jes Sorensen

2007-08-02 07:27 UTC #1 lore raw

Hi Kenji-san

I believe we need this property, but I am not 100% sure, so I am going to forward your patch to our team who maintain the IRQ code for Altix.

Mike, Aaron or John, would one of you have time to take a look at this?

Thanks,
Jes

Quoted 30 lines · Kenji Kaneshige

Hi Jes-san, Russ-san, Christoph-san and SN platform developers,
I glanced over the SN code, and I found SN platform doesn't have SN specific irq_to_vector() function, though it has SN specific local_vector_to_irq() function. I guess it was OK because maybe SN platform depends on irq = vector and old generic irq_to_vector() was just returning the same value as irq.
The irq_to_vector() function, however, has been changed by vector domain support, and currently it can return the different value from irq, even when VECTOR_DOMAIN_PERCPU is not enabled. This can happen when an interrupt thorough I/O SAPIC is unregistered and another interupt is registered. So if my guess (SN platform depends on irq = vector) is true, something problem might happen on SN platform when PCI hotplugging or driver reloading.
If it is true that SN platform depends on irq = vector, we need to implement SN specific irq_to_vector() that returns the same value as irq, I think. I made sample patches to implement platform specific irq_to_vector() as follows. Note that [PATCH 2/2] is not tested yet because I don't have any SN platform unfortunately.
[PATCH 1/2] Add base support for platform_irq_to_vector() [PATCH 2/2] Implement platform_irq_to_vector() for SN
Could you give me comments?
Thanks,
Kenji Kaneshige

John Keller

2007-08-02 13:26 UTC #2 lore raw

In reply to Kenji Kaneshige

Yes, we'll take a look at it.

John

Quoted 47 lines

Hi Kenji-san
I believe we need this property, but I am not 100% sure, so I am going to forward your patch to our team who maintain the IRQ code for Altix.
Mike, Aaron or John, would one of you have time to take a look at this?
Thanks,
Jes
Quoted 30 lines · Kenji Kaneshige
Hi Jes-san, Russ-san, Christoph-san and SN platform developers,
I glanced over the SN code, and I found SN platform doesn't have SN specific irq_to_vector() function, though it has SN specific local_vector_to_irq() function. I guess it was OK because maybe SN platform depends on irq = vector and old generic irq_to_vector() was just returning the same value as irq.
The irq_to_vector() function, however, has been changed by vector domain support, and currently it can return the different value from irq, even when VECTOR_DOMAIN_PERCPU is not enabled. This can happen when an interrupt thorough I/O SAPIC is unregistered and another interupt is registered. So if my guess (SN platform depends on irq = vector) is true, something problem might happen on SN platform when PCI hotplugging or driver reloading.
If it is true that SN platform depends on irq = vector, we need to implement SN specific irq_to_vector() that returns the same value as irq, I think. I made sample patches to implement platform specific irq_to_vector() as follows. Note that [PATCH 2/2] is not tested yet because I don't have any SN platform unfortunately.
[PATCH 1/2] Add base support for platform_irq_to_vector() [PATCH 2/2] Implement platform_irq_to_vector() for SN
Could you give me comments?
Thanks,
Kenji Kaneshige

Kenji Kaneshige

author

2007-08-02 14:04 UTC #3 lore raw

In reply to Kenji Kaneshige

Hi,

This is FYI.

If VECTOR_DOMAIN_PERCPU is NOT enabled:

The "irq = vector" is always true in the following cases:
- reserve_irq_vector(), which is used only by SN
- create_irq() used on the system that has NO I/O SAPIC
- assign_irq_vector() with AUTO_ASSIGN specified
- Vectors outside FIRST_DEVICE_VECTOR to LAST_DEVICE_VECTOR

The "irq = vector" is not always true in the folliwng cases:
- create_irq() used on the system that has I/O SAPICs.
- assign_irq_vector() with the value other than AUTO_ASSIGN specified on the system has I/O SAPICs.

And it is not ensured that those are true even in the future.

Yasuaki, could you check if it is correct?

Thanks,
Kenji Kaneshige

2007-08-02 (木) の 08:26 -0500 に John Keller さんは書きました:

Quoted 57 lines

Yes, we'll take a look at it.
John
Quoted 7 lines
Hi Kenji-san
I believe we need this property, but I am not 100% sure, so I am going to forward your patch to our team who maintain the IRQ code for Altix.
Quoted 40 lines
Mike, Aaron or John, would one of you have time to take a look at this?
Thanks,
Jes
Quoted 30 lines · Kenji Kaneshige
Hi Jes-san, Russ-san, Christoph-san and SN platform developers,
I glanced over the SN code, and I found SN platform doesn't have SN specific irq_to_vector() function, though it has SN specific local_vector_to_irq() function. I guess it was OK because maybe SN platform depends on irq = vector and old generic irq_to_vector() was just returning the same value as irq.
The irq_to_vector() function, however, has been changed by vector domain support, and currently it can return the different value from irq, even when VECTOR_DOMAIN_PERCPU is not enabled. This can happen when an interrupt thorough I/O SAPIC is unregistered and another interupt is registered. So if my guess (SN platform depends on irq = vector) is true, something problem might happen on SN platform when PCI hotplugging or driver reloading.
If it is true that SN platform depends on irq = vector, we need to implement SN specific irq_to_vector() that returns the same value as irq, I think. I made sample patches to implement platform specific irq_to_vector() as follows. Note that [PATCH 2/2] is not tested yet because I don't have any SN platform unfortunately.
[PATCH 1/2] Add base support for platform_irq_to_vector() [PATCH 2/2] Implement platform_irq_to_vector() for SN
Could you give me comments?
Thanks,
Kenji Kaneshige
-
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

Russ Anderson

2007-08-02 14:50 UTC #4 lore raw

In reply to Kenji Kaneshige

Quoted 17 lines · Kenji Kaneshige

Hi Jes-san, Russ-san, Christoph-san and SN platform developers,
I glanced over the SN code, and I found SN platform doesn't have SN specific irq_to_vector() function, though it has SN specific local_vector_to_irq() function. I guess it was OK because maybe SN platform depends on irq = vector and old generic irq_to_vector() was just returning the same value as irq.
The irq_to_vector() function, however, has been changed by vector domain support, and currently it can return the different value from irq, even when VECTOR_DOMAIN_PERCPU is not enabled. This can happen when an interrupt thorough I/O SAPIC is unregistered and another interupt is registered. So if my guess (SN platform depends on irq = vector) is true, something problem might happen on SN platform when PCI hotplugging or driver reloading.

Quoted 5 lines

If it is true that SN platform depends on irq = vector, we need to implement SN specific irq_to_vector() that returns the same value as irq, I think. I made sample patches to implement platform specific irq_to_vector() as follows. Note that [PATCH 2/2] is not tested yet because I don't have any SN platform unfortunately.

There seems to be an issue with this for CPEs. The net result is when there is a correctable error, the interrupt does not get handled resulting in this message:

-----------------------------------------------------------------
irq 30, desc: a000000100c37000, depth: 1, count: 0, unhandled: 0 ->handle_irq(): a0000001009a6a50, __end_rodata+0x5010/0x225c0 ->chip(): a000000100dfee80, irq_type_sn+0x0/0x80 ->action(): 0000000000000000
IRQ_DISABLED set
Unexpected irq vector 0x1e on CPU 0!
-----------------------------------------------------------------

The CPE handler should be on irq 0x1e (30). See IA64_CPE_VECTOR . In ia64_mca_late_init() the line

           if (irq_to_vector(irq) = cpe_vector) {

is never true, so setup_irq(irq, &mca_cpe_irqaction); never gets called.

Quoted 9 lines

[PATCH 1/2] Add base support for platform_irq_to_vector() [PATCH 2/2] Implement platform_irq_to_vector() for SN
Could you give me comments?
Thanks,
Kenji Kaneshige

Signature · 2 lines

Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          [email protected]

Keith Rich

2007-08-02 17:38 UTC #5 lore raw

In reply to Kenji Kaneshige

Russ, I see this on dorado, but not on A4700.
I don't understand why there's a difference.

Keith

John Keller

2007-08-02 19:32 UTC #6 lore raw

In reply to Kenji Kaneshige

Quoted 10 lines

Hi,
This is FYI.
If VECTOR_DOMAIN_PERCPU is NOT enabled:
The "irq = vector" is always true in the following cases:
- reserve_irq_vector(), which is used only by SN
- create_irq() used on the system that has NO I/O SAPIC

Could you say a bit more about this case. I see create_irq() calling find_unassigned_vector() and find_unassigned_irq(). How is "irq = vector" guaranteed to be always true in this case?

John

Quoted 77 lines

- assign_irq_vector() with AUTO_ASSIGN specified
- Vectors outside FIRST_DEVICE_VECTOR to LAST_DEVICE_VECTOR
The "irq = vector" is not always true in the folliwng cases:
- create_irq() used on the system that has I/O SAPICs.
- assign_irq_vector() with the value other than AUTO_ASSIGN specified on the system has I/O SAPICs.
And it is not ensured that those are true even in the future.
Yasuaki, could you check if it is correct?
Thanks,
Kenji Kaneshige
2007-08-02 (木) の 08:26 -0500 に John Keller さんは書きました:
Quoted 57 lines
Yes, we'll take a look at it.
John
Quoted 7 lines
Hi Kenji-san
I believe we need this property, but I am not 100% sure, so I am going to forward your patch to our team who maintain the IRQ code for Altix.
Quoted 40 lines
Mike, Aaron or John, would one of you have time to take a look at this?
Thanks,
Jes
Quoted 30 lines · Kenji Kaneshige
Hi Jes-san, Russ-san, Christoph-san and SN platform developers,
I glanced over the SN code, and I found SN platform doesn't have SN specific irq_to_vector() function, though it has SN specific local_vector_to_irq() function. I guess it was OK because maybe SN platform depends on irq = vector and old generic irq_to_vector() was just returning the same value as irq.
The irq_to_vector() function, however, has been changed by vector domain support, and currently it can return the different value from irq, even when VECTOR_DOMAIN_PERCPU is not enabled. This can happen when an interrupt thorough I/O SAPIC is unregistered and another interupt is registered. So if my guess (SN platform depends on irq = vector) is true, something problem might happen on SN platform when PCI hotplugging or driver reloading.
If it is true that SN platform depends on irq = vector, we need to implement SN specific irq_to_vector() that returns the same value as irq, I think. I made sample patches to implement platform specific irq_to_vector() as follows. Note that [PATCH 2/2] is not tested yet because I don't have any SN platform unfortunately.
[PATCH 1/2] Add base support for platform_irq_to_vector() [PATCH 2/2] Implement platform_irq_to_vector() for SN
Could you give me comments?
Thanks,
Kenji Kaneshige
-
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

Kenji Kaneshige

author

2007-08-03 05:22 UTC #7 lore raw

In reply to Kenji Kaneshige

2007-08-02 (木) の 09:50 -0500 に Russ Anderson さんは書きました:

Quoted 13 lines

There seems to be an issue with this for CPEs. The net result is when there is a correctable error, the interrupt does not get handled resulting in this message:
-----------------------------------------------------------------
irq 30, desc: a000000100c37000, depth: 1, count: 0, unhandled: 0 ->handle_irq(): a0000001009a6a50, __end_rodata+0x5010/0x225c0 ->chip(): a000000100dfee80, irq_type_sn+0x0/0x80 ->action(): 0000000000000000
IRQ_DISABLED set
Unexpected irq vector 0x1e on CPU 0!
-----------------------------------------------------------------

Quoted 7 lines

The CPE handler should be on irq 0x1e (30). See IA64_CPE_VECTOR . In ia64_mca_late_init() the line
           if (irq_to_vector(irq) = cpe_vector) {
is never true, so setup_irq(irq, &mca_cpe_irqaction); never gets called.

Could you please check if [PATCH 2/2] is applied on your kernel again?

With [PATCH 2/2], irq_to_vector(irq) just returns the same value as vector. So the line

	for (irq = 0; irq < NR_IRQS; ++irq)
		if (irq_to_vector(irq) = cpe_vector) {

should be true if cpe_vector is within 0 to NR_IRQ-1.

I'm sorry but the important thing was missing in my "FYI". I said like folows:

Quoted 3 lines

The "irq = vector" is always true in the following cases:
...
- Vectors outside FIRST_DEVICE_VECTOR to LAST_DEVICE_VECTOR

But if the irq is not initialized by __bind_irq_vector() through bind_irq_vector() or register_percpu_irq(), irq_to_vector(irq) returns IRQ_VECTOR_UNASSIGNED, which is zero.

So if [PATCH 2/2] was not applied, irq_to_vector(IA64_CPE_VECTOR) may returns zero and "irq_to_vector(irq) = cpe_vector" would never be true.

BTW, as you pointed out in the other mail, sn_irq_to_vector() will truncate any IRQ > 255. Because of this, "if (irq_to_vector(irq) = cpe_vector)" check will true several times. But I don't know if this is the cause of CPE problem, becasue CPE is still fine on my dig64 kernel even when the following debug patch applied.

Thanks,
Kenji Kaneshige

---
 arch/ia64/kernel/irq.c    |    6 ++++--
 include/asm-ia64/hw_irq.h |    6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

Index: linux-2.6.23-rc1/arch/ia64/kernel/irq.c =================================--- linux-2.6.23-rc1.orig/arch/ia64/kernel/irq.c +++ linux-2.6.23-rc1/arch/ia64/kernel/irq.c
@@ -35,12 +35,14 @@ void ack_bad_irq(unsigned int irq)
#ifdef CONFIG_IA64_GENERIC
ia64_vector __ia64_irq_to_vector(int irq)
{
- return irq_cfg[irq].vector;
+ return (ia64_vector)irq;
+ //return irq_cfg[irq].vector;
}

unsigned int __ia64_local_vector_to_irq (ia64_vector vec) {
- return __get_cpu_var(vector_irq)[vec];
+ return (unsigned int)vec;
+ //return __get_cpu_var(vector_irq)[vec];
}
#endif

Index: linux-2.6.23-rc1/include/asm-ia64/hw_irq.h =================================--- linux-2.6.23-rc1.orig/include/asm-ia64/hw_irq.h +++ linux-2.6.23-rc1/include/asm-ia64/hw_irq.h
@@ -126,13 +126,15 @@ extern irq_desc_t irq_desc[NR_IRQS];
#ifndef CONFIG_IA64_GENERIC
static inline ia64_vector __ia64_irq_to_vector(int irq)
{
- return irq_cfg[irq].vector;
+ return (ia64_vector)irq;
+ //return irq_cfg[irq].vector;
}

static inline unsigned int
__ia64_local_vector_to_irq (ia64_vector vec)
{
- return __get_cpu_var(vector_irq)[vec];
+ return (unsigned int)vec;
+ //return __get_cpu_var(vector_irq)[vec];
}
#endif

Yasuaki Ishimatsu

2007-08-03 06:04 UTC #8 lore raw

In reply to Kenji Kaneshige

Hi John,

Quoted 6 lines · John Keller

Quoted 6 lines
Hi,
This is FYI.
If VECTOR_DOMAIN_PERCPU is NOT enabled:

Quoted 7 lines

Quoted 3 lines
The "irq = vector" is always true in the following cases:
- reserve_irq_vector(), which is used only by SN
- create_irq() used on the system that has NO I/O SAPIC
Could you say a bit more about this case. I see create_irq() calling find_unassigned_vector() and find_unassigned_irq(). How is "irq = vector" guaranteed to be always true in this case?

It's guaranteed as follows:

- When find_unassigned_vector()/find_unassigned_irq() search available vector and irq, it always start to search from IA64_FIRST_DEVICE_VECTOR.

- clear_irq_vector() releases both vector and irq.

Thanks,
Yasuaki

Quoted 83 lines

John
Quoted 10 lines
- assign_irq_vector() with AUTO_ASSIGN specified
- Vectors outside FIRST_DEVICE_VECTOR to LAST_DEVICE_VECTOR
The "irq = vector" is not always true in the folliwng cases:
- create_irq() used on the system that has I/O SAPICs.
- assign_irq_vector() with the value other than AUTO_ASSIGN specified on the system has I/O SAPICs.
And it is not ensured that those are true even in the future.
Quoted 63 lines
Yasuaki, could you check if it is correct?
Thanks,
Kenji Kaneshige
2007-08-02 (木) の 08:26 -0500 に John Keller さんは書きました:
Quoted 54 lines
Yes, we'll take a look at it.
John
Quoted 6 lines
Hi Kenji-san
I believe we need this property, but I am not 100% sure, so I am going to forward your patch to our team who maintain the IRQ code for Altix.
Quoted 39 lines
Mike, Aaron or John, would one of you have time to take a look at this?
Thanks,
Jes
Quoted 30 lines · Kenji Kaneshige
Hi Jes-san, Russ-san, Christoph-san and SN platform developers,
I glanced over the SN code, and I found SN platform doesn't have SN specific irq_to_vector() function, though it has SN specific local_vector_to_irq() function. I guess it was OK because maybe SN platform depends on irq = vector and old generic irq_to_vector() was just returning the same value as irq.
The irq_to_vector() function, however, has been changed by vector domain support, and currently it can return the different value from irq, even when VECTOR_DOMAIN_PERCPU is not enabled. This can happen when an interrupt thorough I/O SAPIC is unregistered and another interupt is registered. So if my guess (SN platform depends on irq = vector) is true, something problem might happen on SN platform when PCI hotplugging or driver reloading.
If it is true that SN platform depends on irq = vector, we need to implement SN specific irq_to_vector() that returns the same value as irq, I think. I made sample patches to implement platform specific irq_to_vector() as follows. Note that [PATCH 2/2] is not tested yet because I don't have any SN platform unfortunately.
[PATCH 1/2] Add base support for platform_irq_to_vector() [PATCH 2/2] Implement platform_irq_to_vector() for SN
Could you give me comments?
Thanks,
Kenji Kaneshige
-
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

Kenji Kaneshige

author

2007-08-03 06:20 UTC #9 lore raw

In reply to Kenji Kaneshige

2007-08-02 (木) の 14:32 -0500 に John Keller さんは書きました:

Quoted 7 lines

Quoted 7 lines
Hi,
This is FYI.
If VECTOR_DOMAIN_PERCPU is NOT enabled:

Quoted 8 lines

Quoted 3 lines
The "irq = vector" is always true in the following cases:
- reserve_irq_vector(), which is used only by SN
- create_irq() used on the system that has NO I/O SAPIC
Could you say a bit more about this case. I see create_irq() calling find_unassigned_vector() and find_unassigned_irq(). How is "irq = vector" guaranteed to be always true in this case?

Both find_unassigned_vector() and find_unassigned_irq() searchs the number from IA64_FIRST_DEVICE_VECTOR. And all irq and vector numbers from IA64_FIRST_DEVICE_VECTOR to IA64_LAST_DEVICE_VECTOR are not used.

The create_irq() acquires both irq number and vector number using find_unassigned_irq() and find_unassigned_vector() respectively and bind them, with vector_lock held.

The clear_irq_vector(), which is called through destroy_irq() and so on, frees both irq and vector with vector_lock held.

Since oth irq and vector are acquired/freed at a time, "irq = vector" is guaranteed.

But there is a exception destroy_and_reserve_irq(), which is called by IOSAPIC code. It frees both irq and vector, and reserve irq after that. That is, it only frees vector. This is required for fixed mapping between gsi and irq.

Thanks,
Kenji Kaneshige

Quoted 86 lines

John
Quoted 10 lines
- assign_irq_vector() with AUTO_ASSIGN specified
- Vectors outside FIRST_DEVICE_VECTOR to LAST_DEVICE_VECTOR
The "irq = vector" is not always true in the folliwng cases:
- create_irq() used on the system that has I/O SAPICs.
- assign_irq_vector() with the value other than AUTO_ASSIGN specified on the system has I/O SAPICs.
And it is not ensured that those are true even in the future.
Quoted 67 lines
Yasuaki, could you check if it is correct?
Thanks,
Kenji Kaneshige
2007-08-02 (木) の 08:26 -0500 に John Keller さんは書きました:
Quoted 57 lines
Yes, we'll take a look at it.
John
Quoted 7 lines
Hi Kenji-san
I believe we need this property, but I am not 100% sure, so I am going to forward your patch to our team who maintain the IRQ code for Altix.
Quoted 40 lines
Mike, Aaron or John, would one of you have time to take a look at this?
Thanks,
Jes
Quoted 30 lines · Kenji Kaneshige
Hi Jes-san, Russ-san, Christoph-san and SN platform developers,
I glanced over the SN code, and I found SN platform doesn't have SN specific irq_to_vector() function, though it has SN specific local_vector_to_irq() function. I guess it was OK because maybe SN platform depends on irq = vector and old generic irq_to_vector() was just returning the same value as irq.
The irq_to_vector() function, however, has been changed by vector domain support, and currently it can return the different value from irq, even when VECTOR_DOMAIN_PERCPU is not enabled. This can happen when an interrupt thorough I/O SAPIC is unregistered and another interupt is registered. So if my guess (SN platform depends on irq = vector) is true, something problem might happen on SN platform when PCI hotplugging or driver reloading.
If it is true that SN platform depends on irq = vector, we need to implement SN specific irq_to_vector() that returns the same value as irq, I think. I made sample patches to implement platform specific irq_to_vector() as follows. Note that [PATCH 2/2] is not tested yet because I don't have any SN platform unfortunately.
[PATCH 1/2] Add base support for platform_irq_to_vector() [PATCH 2/2] Implement platform_irq_to_vector() for SN
Could you give me comments?
Thanks,
Kenji Kaneshige
-
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

Russ Anderson

2007-08-03 14:58 UTC #10 lore raw

In reply to Kenji Kaneshige

Quoted 16 lines · Kenji Kaneshige

Quoted 16 lines
There seems to be an issue with this for CPEs. The net result is when there is a correctable error, the interrupt does not get handled resulting in this message:
-----------------------------------------------------------------
irq 30, desc: a000000100c37000, depth: 1, count: 0, unhandled: 0 ->handle_irq(): a0000001009a6a50, __end_rodata+0x5010/0x225c0 ->chip(): a000000100dfee80, irq_type_sn+0x0/0x80 ->action(): 0000000000000000
IRQ_DISABLED set
Unexpected irq vector 0x1e on CPU 0!
-----------------------------------------------------------------
The CPE handler should be on irq 0x1e (30). See IA64_CPE_VECTOR . In ia64_mca_late_init() the line

Quoted 6 lines

Quoted 4 lines
           if (irq_to_vector(irq) = cpe_vector) {
is never true, so setup_irq(irq, &mca_cpe_irqaction); never gets called.
Could you please check if [PATCH 2/2] is applied on your kernel again?

Your patch [PATCH 2/2] fixes the problem.

Quoted 15 lines

With [PATCH 2/2], irq_to_vector(irq) just returns the same value as vector. So the line
	for (irq = 0; irq < NR_IRQS; ++irq)
		if (irq_to_vector(irq) = cpe_vector) {
should be true if cpe_vector is within 0 to NR_IRQ-1.
I'm sorry but the important thing was missing in my "FYI". I said like folows:
Quoted 3 lines
The "irq = vector" is always true in the following cases:
...
- Vectors outside FIRST_DEVICE_VECTOR to LAST_DEVICE_VECTOR

Quoted 7 lines

But if the irq is not initialized by __bind_irq_vector() through bind_irq_vector() or register_percpu_irq(), irq_to_vector(irq) returns IRQ_VECTOR_UNASSIGNED, which is zero.
So if [PATCH 2/2] was not applied, irq_to_vector(IA64_CPE_VECTOR) may returns zero and "irq_to_vector(irq) = cpe_vector" would never be true.

Correct.

Quoted 5 lines

BTW, as you pointed out in the other mail, sn_irq_to_vector() will truncate any IRQ > 255. Because of this, "if (irq_to_vector(irq) = cpe_vector)" check will true several times. But I don't know if this is the cause of CPE problem, becasue CPE is still fine on my dig64 kernel even when the following debug patch applied.

That caused a different problem of the CPE handler getting registered on multiple irqs.

From /proc/interrupts:

 30:          1          0          0          0          SN hub  cpe_hndlr
286:          0          0          0          0          SN hub  cpe_hndlr
542:          0          0          0          0          SN hub  cpe_hndlr
798:          0          0          0          0          SN hub  cpe_hndlr

I have a patch in the works to clean up ia64_mca_late_init() so the handler only gets registered for irq 30. I will post it shortly.

Signature · 2 lines

Russ Anderson, OS RAS/Partitioning Project Lead  
SGI - Silicon Graphics Inc          [email protected]

John Keller

2007-08-03 18:03 UTC #11 lore raw

In reply to Kenji Kaneshige

Quoted 32 lines

2007-08-02 (木) の 14:32 -0500 に John Keller さんは書きました:
Quoted 7 lines
Quoted 7 lines
Hi,
This is FYI.
If VECTOR_DOMAIN_PERCPU is NOT enabled:
Quoted 8 lines
Quoted 3 lines
The "irq = vector" is always true in the following cases:
- reserve_irq_vector(), which is used only by SN
- create_irq() used on the system that has NO I/O SAPIC
Could you say a bit more about this case. I see create_irq() calling find_unassigned_vector() and find_unassigned_irq(). How is "irq = vector" guaranteed to be always true in this case?
Both find_unassigned_vector() and find_unassigned_irq() searchs the number from IA64_FIRST_DEVICE_VECTOR. And all irq and vector numbers from IA64_FIRST_DEVICE_VECTOR to IA64_LAST_DEVICE_VECTOR are not used.
The create_irq() acquires both irq number and vector number using find_unassigned_irq() and find_unassigned_vector() respectively and bind them, with vector_lock held.
The clear_irq_vector(), which is called through destroy_irq() and so on, frees both irq and vector with vector_lock held.
Since oth irq and vector are acquired/freed at a time, "irq = vector" is guaranteed.

Quoted 7 lines

But there is a exception destroy_and_reserve_irq(), which is called by IOSAPIC code. It frees both irq and vector, and reserve irq after that. That is, it only frees vector. This is required for fixed mapping between gsi and irq.
Thanks,
Kenji Kaneshige

Thanks for the info.
Your patch looks good to me, along with the other change to sn_irq_to_vector().

Quoted 10 lines

What about this:
ia64_vector sn_irq_to_vector(int irq)
{
       if (irq >= IA64_NUM_VECTORS)
               return 0;
       return (ia64_vector)irq;
}

John Keller

Quoted 90 lines

Quoted 88 lines
John
Quoted 10 lines
- assign_irq_vector() with AUTO_ASSIGN specified
- Vectors outside FIRST_DEVICE_VECTOR to LAST_DEVICE_VECTOR
The "irq = vector" is not always true in the folliwng cases:
- create_irq() used on the system that has I/O SAPICs.
- assign_irq_vector() with the value other than AUTO_ASSIGN specified on the system has I/O SAPICs.
And it is not ensured that those are true even in the future.
Quoted 67 lines
Yasuaki, could you check if it is correct?
Thanks,
Kenji Kaneshige
2007-08-02 (木) の 08:26 -0500 に John Keller さんは書きました:
Quoted 57 lines
Yes, we'll take a look at it.
John
Quoted 7 lines
Hi Kenji-san
I believe we need this property, but I am not 100% sure, so I am going to forward your patch to our team who maintain the IRQ code for Altix.
Quoted 40 lines
Mike, Aaron or John, would one of you have time to take a look at this?
Thanks,
Jes
Quoted 30 lines · Kenji Kaneshige
Hi Jes-san, Russ-san, Christoph-san and SN platform developers,
I glanced over the SN code, and I found SN platform doesn't have SN specific irq_to_vector() function, though it has SN specific local_vector_to_irq() function. I guess it was OK because maybe SN platform depends on irq = vector and old generic irq_to_vector() was just returning the same value as irq.
The irq_to_vector() function, however, has been changed by vector domain support, and currently it can return the different value from irq, even when VECTOR_DOMAIN_PERCPU is not enabled. This can happen when an interrupt thorough I/O SAPIC is unregistered and another interupt is registered. So if my guess (SN platform depends on irq = vector) is true, something problem might happen on SN platform when PCI hotplugging or driver reloading.
If it is true that SN platform depends on irq = vector, we need to implement SN specific irq_to_vector() that returns the same value as irq, I think. I made sample patches to implement platform specific irq_to_vector() as follows. Note that [PATCH 2/2] is not tested yet because I don't have any SN platform unfortunately.
[PATCH 1/2] Add base support for platform_irq_to_vector() [PATCH 2/2] Implement platform_irq_to_vector() for SN
Could you give me comments?
Thanks,
Kenji Kaneshige
-
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

Luck, Tony

2007-08-03 18:19 UTC #12 lore raw

In reply to Kenji Kaneshige

Quoted 3 lines

Thanks for the info.
Your patch looks good to me, along with the other change to sn_irq_to_vector().

Sounds like an Ack ... can one of you fold the changes into a new patch please.

-Tony

John Keller

Acked-by on the v1 series

2007-08-06 13:59 UTC #13 lore raw

In reply to Kenji Kaneshige

Quoted 9 lines

Quoted 4 lines
Thanks for the info.
Your patch looks good to me, along with the other change to sn_irq_to_vector().
Sounds like an Ack ... can one of you fold the changes into a new patch please.
-Tony

This is the patch as I was running it.

Add base support for implementing platform_irq_to_vector().

Acked-by John Keller <[email protected]>

---
 arch/ia64/kernel/irq.c          |    5 +++++
 arch/ia64/sn/kernel/irq.c       |    7 +++++++
 include/asm-ia64/hw_irq.h       |    7 ++++++-
 include/asm-ia64/machvec.h      |    7 +++++++
 include/asm-ia64/machvec_init.h |    1 +
 include/asm-ia64/machvec_sn2.h  |    2 ++
 6 files changed, 28 insertions(+), 1 deletion(-)

Index: linux-2.6/arch/ia64/kernel/irq.c
=================================--- linux-2.6.orig/arch/ia64/kernel/irq.c 2007-08-04 18:45:15.705972596 -0500
+++ linux-2.6/arch/ia64/kernel/irq.c 2007-08-04 18:45:43.197343168 -0500
@@ -33,6 +33,11 @@ void ack_bad_irq(unsigned int irq)
}

#ifdef CONFIG_IA64_GENERIC
+ia64_vector __ia64_irq_to_vector(int irq)
+{
+ return irq_cfg[irq].vector;
+}
+
unsigned int __ia64_local_vector_to_irq (ia64_vector vec) {
return __get_cpu_var(vector_irq)[vec];
Index: linux-2.6/include/asm-ia64/hw_irq.h
=================================--- linux-2.6.orig/include/asm-ia64/hw_irq.h 2007-08-04 18:45:15.705972596 -0500
+++ linux-2.6/include/asm-ia64/hw_irq.h 2007-08-04 18:45:43.217345621 -0500
@@ -124,6 +124,11 @@ static inline void ia64_resend_irq(unsig
extern irq_desc_t irq_desc[NR_IRQS];

#ifndef CONFIG_IA64_GENERIC
+static inline ia64_vector __ia64_irq_to_vector(int irq) +{
+ return irq_cfg[irq].vector;
+}
+
static inline unsigned int __ia64_local_vector_to_irq (ia64_vector vec)
{
@@ -145,7 +150,7 @@ __ia64_local_vector_to_irq (ia64_vector
static inline ia64_vector
irq_to_vector (int irq)
{
- return irq_cfg[irq].vector;
+ return platform_irq_to_vector(irq);
}

/*
Index: linux-2.6/include/asm-ia64/machvec.h
=================================--- linux-2.6.orig/include/asm-ia64/machvec.h 2007-08-04 18:45:15.709973086 -0500
+++ linux-2.6/include/asm-ia64/machvec.h 2007-08-04 18:45:43.237348073 -0500
@@ -30,6 +30,7 @@ typedef void ia64_mv_send_ipi_t (int, in
typedef void ia64_mv_timer_interrupt_t (int, void *);
typedef void ia64_mv_global_tlb_purge_t (struct mm_struct *, unsigned long, unsigned long, unsigned long); typedef void ia64_mv_tlb_migrate_finish_t (struct mm_struct *);
+typedef u8 ia64_mv_irq_to_vector (int);
typedef unsigned int ia64_mv_local_vector_to_irq (u8);
typedef char *ia64_mv_pci_get_legacy_mem_t (struct pci_bus *);
typedef int ia64_mv_pci_legacy_read_t (struct pci_bus *, u16 port, u32 *val,
@@ -145,6 +146,7 @@ extern void machvec_tlb_migrate_finish (

 #  define platform_dma_sync_sg_for_device ia64_mv.dma_sync_sg_for_device
 #  define platform_dma_mapping_error		ia64_mv.dma_mapping_error
 #  define platform_dma_supported	ia64_mv.dma_supported
+#  define platform_irq_to_vector	ia64_mv.irq_to_vector
 #  define platform_local_vector_to_irq	ia64_mv.local_vector_to_irq
 #  define platform_pci_get_legacy_mem	ia64_mv.pci_get_legacy_mem
 #  define platform_pci_legacy_read	ia64_mv.pci_legacy_read

@@ -198,6 +200,7 @@ struct ia64_machine_vector {
ia64_mv_dma_sync_sg_for_device *dma_sync_sg_for_device; ia64_mv_dma_mapping_error *dma_mapping_error; ia64_mv_dma_supported *dma_supported;
+ ia64_mv_irq_to_vector *irq_to_vector;
ia64_mv_local_vector_to_irq *local_vector_to_irq; ia64_mv_pci_get_legacy_mem_t *pci_get_legacy_mem; ia64_mv_pci_legacy_read_t *pci_legacy_read;
@@ -247,6 +250,7 @@ struct ia64_machine_vector {
platform_dma_sync_sg_for_device, \
platform_dma_mapping_error, \
platform_dma_supported, \
+ platform_irq_to_vector, \
platform_local_vector_to_irq, \
platform_pci_get_legacy_mem, \
platform_pci_legacy_read, \
@@ -366,6 +370,9 @@ extern ia64_mv_dma_supported swiotlb_dm
#ifndef platform_dma_supported

 # define  platform_dma_supported	swiotlb_dma_supported
 #endif
+#ifndef platform_irq_to_vector
+# define platform_irq_to_vector		__ia64_irq_to_vector
+#endif
 #ifndef platform_local_vector_to_irq
 # define platform_local_vector_to_irq	__ia64_local_vector_to_irq
 #endif
Index: linux-2.6/include/asm-ia64/machvec_init.h
=================================--- linux-2.6.orig/include/asm-ia64/machvec_init.h	2007-08-04 18:45:15.709973086 -0500
+++ linux-2.6/include/asm-ia64/machvec_init.h	2007-08-04 18:45:43.253350035 -0500
@@ -2,6 +2,7 @@

extern ia64_mv_send_ipi_t ia64_send_ipi;
extern ia64_mv_global_tlb_purge_t ia64_global_tlb_purge;
+extern ia64_mv_irq_to_vector __ia64_irq_to_vector;
extern ia64_mv_local_vector_to_irq __ia64_local_vector_to_irq; extern ia64_mv_pci_get_legacy_mem_t ia64_pci_get_legacy_mem; extern ia64_mv_pci_legacy_read_t ia64_pci_legacy_read;
Index: linux-2.6/arch/ia64/sn/kernel/irq.c
=================================--- linux-2.6.orig/arch/ia64/sn/kernel/irq.c 2007-08-04 18:45:15.705972596 -0500
+++ linux-2.6/arch/ia64/sn/kernel/irq.c 2007-08-04 18:45:43.273352487 -0500
@@ -256,6 +256,13 @@ struct irq_chip irq_type_sn = {
.set_affinity = sn_set_affinity_irq
};

+ia64_vector sn_irq_to_vector(int irq)
+{
+ if (irq >= IA64_NUM_VECTORS)
+ return 0;
+ return (ia64_vector)irq;
+}
+
unsigned int sn_local_vector_to_irq(u8 vector)
{
return (CPU_VECTOR_TO_IRQ(smp_processor_id(), vector));
Index: linux-2.6/include/asm-ia64/machvec_sn2.h
=================================--- linux-2.6.orig/include/asm-ia64/machvec_sn2.h 2007-08-04 18:45:15.709973086 -0500
+++ linux-2.6/include/asm-ia64/machvec_sn2.h 2007-08-04 18:45:43.289354449 -0500
@@ -35,6 +35,7 @@ extern ia64_mv_send_ipi_t sn2_send_IPI;
extern ia64_mv_timer_interrupt_t sn_timer_interrupt; extern ia64_mv_global_tlb_purge_t sn2_global_tlb_purge;
extern ia64_mv_tlb_migrate_finish_t sn_tlb_migrate_finish;
+extern ia64_mv_irq_to_vector sn_irq_to_vector;
extern ia64_mv_local_vector_to_irq sn_local_vector_to_irq; extern ia64_mv_pci_get_legacy_mem_t sn_pci_get_legacy_mem; extern ia64_mv_pci_legacy_read_t sn_pci_legacy_read;
@@ -104,6 +105,7 @@ extern ia64_mv_pci_fixup_bus_t sn_pci_f
#define platform_readw_relaxed __sn_readw_relaxed
#define platform_readl_relaxed __sn_readl_relaxed
#define platform_readq_relaxed __sn_readq_relaxed
+#define platform_irq_to_vector sn_irq_to_vector
#define platform_local_vector_to_irq sn_local_vector_to_irq
#define platform_pci_get_legacy_mem sn_pci_get_legacy_mem
#define platform_pci_legacy_read sn_pci_legacy_read