[RFC] Fix for interrupt distribution

[RFC] Fix for interrupt distribution

[Mohan Kumar M]

Hello,

When kdump kernel is booted with the parameter "maxcpus=1" on a threaded
CPU, we faced some interrupt routing problems.

In the xics initialization code, "reg" property in each cpu node
(device-tree/cpus/PowerPC,POWER5@x) is used to match the current boot
cpu id and based on that "default_server" and "default_distrib_server"
are calculated. This condition will always meet when OF chooses CPU0 as
boot cpu or crash happenes on any cpu whose id is any physical cpu id.

The "reg" property in cpu node gives the id of the cpu and this cpu node
is created only for physical cpus (not for logical/threaded cpus). The
code compares the "reg" value to the current boot cpu id and if it
matches then only it reads "ibm,ppc-interrupt-gserver#s" and assigns the
last value of it (which is usually 0xff) to default_distrib_server. So
when a crash occurs on CPU 3, it will not be able to match the condition
and thus default_distrib_server is left as zero only. This makes all
interrupts routed to cpu 0 but cpu 0 is not up because of "maxcpus=1"
parameter.

To overcome this, I have just added one more condition to check the
above condition. I have attached the patch also. Patch is generated over
2.6.19-rc3.

One more idea will be instead of using "reg" property in each cpu node,
can we use "ibm,ppc-interrupt-gserver#s" to determine the distribution
server? "ibm,ppc-interrupt-gserver#s" format is (please correct
if I am wrong) 
	phys_cpu_id distrib_server logical_cpu_id distrib_server

In a Dual core SMT enabled system, "ibm,ppc-interrupt-gserver#s" will
be: 

	00000002 000000ff 00000003 000000ff
	 ^ phys cpu id
        	  ^ distribution server
                	      ^ logical cpu id
                        	       ^ distribution server

Tested on POWER5 box.

Since POWER4 does not have SMT, crash can happen on any CPU and kdump
kernel can boot with "maxcpus=1" without any problem.



Allow any cpu to become boot cpu.

Signed-off-by: Mohan Kumar M <mohan@in.ibm.com>
---

Index: test/linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
===================================================================
--- test.orig/linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
+++ test/linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
@@ -687,7 +687,8 @@ void __init xics_init_IRQ(void)
 	     np;
 	     np = of_find_node_by_type(np, "cpu")) {
 		ireg = get_property(np, "reg", &ilen);
-		if (ireg && ireg[0] == get_hard_smp_processor_id(boot_cpuid)) {
+		if (ireg && ((ireg[0] == get_hard_smp_processor_id(boot_cpuid))
+		  || (ireg[0] == get_hard_smp_processor_id(boot_cpuid) - 1))) {
 			ireg = get_property(np,
 					"ibm,ppc-interrupt-gserver#s", &ilen);
 			i = ilen / sizeof(int);

Re: [RFC] Fix for interrupt distribution

[Nathan Lynch]

Mohan Kumar M wrote:
> Hello,
> 
> When kdump kernel is booted with the parameter "maxcpus=1" on a threaded
> CPU, we faced some interrupt routing problems.
> 
> In the xics initialization code, "reg" property in each cpu node
> (device-tree/cpus/PowerPC,POWER5@x) is used to match the current boot
> cpu id and based on that "default_server" and "default_distrib_server"
> are calculated. This condition will always meet when OF chooses CPU0 as
> boot cpu or crash happenes on any cpu whose id is any physical cpu id.
> 
> The "reg" property in cpu node gives the id of the cpu and this cpu node
> is created only for physical cpus (not for logical/threaded cpus). The
> code compares the "reg" value to the current boot cpu id and if it
> matches then only it reads "ibm,ppc-interrupt-gserver#s" and assigns the
> last value of it (which is usually 0xff) to default_distrib_server. So
> when a crash occurs on CPU 3, it will not be able to match the condition
> and thus default_distrib_server is left as zero only. This makes all
> interrupts routed to cpu 0 but cpu 0 is not up because of "maxcpus=1"
> parameter.
> 
> To overcome this, I have just added one more condition to check the
> above condition. I have attached the patch also. Patch is generated over
> 2.6.19-rc3.
> 
> One more idea will be instead of using "reg" property in each cpu node,
> can we use "ibm,ppc-interrupt-gserver#s" to determine the distribution
> server? "ibm,ppc-interrupt-gserver#s" format is (please correct
> if I am wrong) 
> 	phys_cpu_id distrib_server logical_cpu_id distrib_server

Firmware has no notion of Linux's logical cpu numbering.


> In a Dual core SMT enabled system, "ibm,ppc-interrupt-gserver#s" will
> be: 
> 
> 	00000002 000000ff 00000003 000000ff
> 	 ^ phys cpu id
>         	  ^ distribution server
>                 	      ^ logical cpu id
>                         	       ^ distribution server
> 
> Tested on POWER5 box.
> 
> Since POWER4 does not have SMT, crash can happen on any CPU and kdump
> kernel can boot with "maxcpus=1" without any problem.
> 
> 
> 
> Allow any cpu to become boot cpu.
> 
> Signed-off-by: Mohan Kumar M <mohan@in.ibm.com>
> ---
> 
> Index: test/linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
> ===================================================================
> --- test.orig/linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
> +++ test/linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
> @@ -687,7 +687,8 @@ void __init xics_init_IRQ(void)
>  	     np;
>  	     np = of_find_node_by_type(np, "cpu")) {
>  		ireg = get_property(np, "reg", &ilen);
> -		if (ireg && ireg[0] == get_hard_smp_processor_id(boot_cpuid)) {
> +		if (ireg && ((ireg[0] == get_hard_smp_processor_id(boot_cpuid))
> +		  || (ireg[0] == get_hard_smp_processor_id(boot_cpuid) - 1))) {
>  			ireg = get_property(np,
>  					"ibm,ppc-interrupt-gserver#s", &ilen);
>  			i = ilen / sizeof(int);

NAK

We can't assume any arithmetic relationship between the
"hard"/platform thread ids; it's completely unspecified.  I bet this
patch happens to work on your system but will fail on others.

Re: [RFC] Fix for interrupt distribution

[Mohan Kumar M]

On Mon, Oct 30, 2006 at 12:17:51PM -0600, Nathan Lynch wrote:
> Mohan Kumar M wrote:
> > Hello,
> > 
> > When kdump kernel is booted with the parameter "maxcpus=1" on a threaded
> > CPU, we faced some interrupt routing problems.
> > 
> > In the xics initialization code, "reg" property in each cpu node
> > (device-tree/cpus/PowerPC,POWER5@x) is used to match the current boot
> > cpu id and based on that "default_server" and "default_distrib_server"
> > are calculated. This condition will always meet when OF chooses CPU0 as
> > boot cpu or crash happenes on any cpu whose id is any physical cpu id.
> > 
> > The "reg" property in cpu node gives the id of the cpu and this cpu node
> > is created only for physical cpus (not for logical/threaded cpus). The
> > code compares the "reg" value to the current boot cpu id and if it
> > matches then only it reads "ibm,ppc-interrupt-gserver#s" and assigns the
> > last value of it (which is usually 0xff) to default_distrib_server. So
> > when a crash occurs on CPU 3, it will not be able to match the condition
> > and thus default_distrib_server is left as zero only. This makes all
> > interrupts routed to cpu 0 but cpu 0 is not up because of "maxcpus=1"
> > parameter.
> > 
> > To overcome this, I have just added one more condition to check the
> > above condition. I have attached the patch also. Patch is generated over
> > 2.6.19-rc3.
> > 
> > One more idea will be instead of using "reg" property in each cpu node,
> > can we use "ibm,ppc-interrupt-gserver#s" to determine the distribution
> > server? "ibm,ppc-interrupt-gserver#s" format is (please correct
> > if I am wrong) 
> > 	phys_cpu_id distrib_server logical_cpu_id distrib_server
> 
> Firmware has no notion of Linux's logical cpu numbering.
> 
>

I have created a patch based on "ibm,ppc-interrupt-gserver#s" property.

Attached patch uses "ibm,ppc-interrupt-gserver#s" property to match the
boot cpu id with available cpu ids. The property
"ibm,ppc-interrupt-gserver#" is encoded as a list of server# and
gserver#s pairs. The first integer specifies a single processor server#
as presented in "ibm,ppc-interrupt-server#s" property and followed by
distribution server. So if a match is found, corresponding server# is
taken as default server and the corresponding gserver# is taken as
default distribution server.

In a Dual core SMT enabled system, "ibm,ppc-interrupt-gserver#s" for
PowerPC,POWER5@2 will be:

        00000002 000000ff 00000003 000000ff
         ^ cpu id
                  ^ distribution server
                              ^ cpu id
                                       ^ distribution server

Tested on a POWER5 box.

Patch is generated over 2.6.19-rc3


Allow any cpu to become boot cpu.

Signed-off-by: Mohan Kumar M <mohan@in.ibm.com>
---
 arch/powerpc/platforms/pseries/xics.c |   30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Index: linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
===================================================================
--- linux-2.6.19-rc3.orig/arch/powerpc/platforms/pseries/xics.c
+++ linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
@@ -658,7 +658,7 @@ static void __init xics_setup_8259_casca
 
 void __init xics_init_IRQ(void)
 {
-	int i;
+	int i, j;
 	struct device_node *np;
 	u32 ilen, indx = 0;
 	const u32 *ireg;
@@ -686,23 +686,23 @@ void __init xics_init_IRQ(void)
 	for (np = of_find_node_by_type(NULL, "cpu");
 	     np;
 	     np = of_find_node_by_type(np, "cpu")) {
-		ireg = get_property(np, "reg", &ilen);
-		if (ireg && ireg[0] == get_hard_smp_processor_id(boot_cpuid)) {
-			ireg = get_property(np,
-					"ibm,ppc-interrupt-gserver#s", &ilen);
-			i = ilen / sizeof(int);
-			if (ireg && i > 0) {
-				default_server = ireg[0];
-				/* take last element */
-				default_distrib_server = ireg[i-1];
-			}
-			ireg = get_property(np,
+		ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
+		i = ilen / sizeof(int);
+		for (j = 0; ireg[j] && j < i; j+=2) {
+			if (ireg[j] == get_hard_smp_processor_id(boot_cpuid)) {
+				if ( j + 1 < i) {
+					default_server = ireg[j];
+					default_distrib_server = ireg[j+1];
+				}
+				ireg = get_property(np,
 					"ibm,interrupt-server#-size", NULL);
-			if (ireg)
-				interrupt_server_size = *ireg;
-			break;
+				if (ireg)
+					interrupt_server_size = *ireg;
+				goto out;
+			}
 		}
 	}
+out:
 	of_node_put(np);
 
 	if (firmware_has_feature(FW_FEATURE_LPAR))

Re: [RFC] Fix for interrupt distribution

[Nathan Lynch]

Mohan Kumar M wrote:
>
> Index: linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
> ===================================================================
> --- linux-2.6.19-rc3.orig/arch/powerpc/platforms/pseries/xics.c
> +++ linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
> @@ -658,7 +658,7 @@ static void __init xics_setup_8259_casca
>  
>  void __init xics_init_IRQ(void)
>  {
> -	int i;
> +	int i, j;
>  	struct device_node *np;
>  	u32 ilen, indx = 0;
>  	const u32 *ireg;
> @@ -686,23 +686,23 @@ void __init xics_init_IRQ(void)
>  	for (np = of_find_node_by_type(NULL, "cpu");
>  	     np;
>  	     np = of_find_node_by_type(np, "cpu")) {
> -		ireg = get_property(np, "reg", &ilen);
> -		if (ireg && ireg[0] == get_hard_smp_processor_id(boot_cpuid)) {
> -			ireg = get_property(np,
> -					"ibm,ppc-interrupt-gserver#s", &ilen);
> -			i = ilen / sizeof(int);
> -			if (ireg && i > 0) {
> -				default_server = ireg[0];
> -				/* take last element */
> -				default_distrib_server = ireg[i-1];
> -			}
> -			ireg = get_property(np,
> +		ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
> +		i = ilen / sizeof(int);
> +		for (j = 0; ireg[j] && j < i; j+=2) {
                            ^^^^^^
I don't understand this condition in the loop -- zero is a valid (and
common) value for interrupt servers:

# xxd /proc/device-tree/cpus/PowerPC,POWER5@0/ibm,ppc-interrupt-server#s
0000000: 0000 0000 0000 0001

# xxd /proc/device-tree/cpus/PowerPC,POWER5@0/ibm,ppc-interrupt-gserver#s
0000000: 0000 0000 0000 00ff 0000 0001 0000 00ff

So I think this patch wouldn't always work.

> +			if (ireg[j] == get_hard_smp_processor_id(boot_cpuid)) {
> +				if ( j + 1 < i) {
> +					default_server = ireg[j];
> +					default_distrib_server = ireg[j+1];
> +				}
> +				ireg = get_property(np,
>  					"ibm,interrupt-server#-size", NULL);
> -			if (ireg)
> -				interrupt_server_size = *ireg;
> -			break;
> +				if (ireg)
> +					interrupt_server_size = *ireg;
> +				goto out;
> +			}
>  		}
>  	}
> +out:
>  	of_node_put(np);


I think we need a helper function or two to make this easier to do
correctly and readably, e.g.

/* Returns the device node (with refcount incremented)
 * corresponding to the given logical cpu id */
struct device_node * cpuid_to_of_node(int cpu) {
	struct device_node *np;
	u32 hcpuid = get_hard_smp_processor_id(cpu);

	for_each_node_by_type(np, "cpu") {
		int len, i;
		u32 *intserv = get_property(np,
				"ibm,ppc-interrupt-server#s", &len);

		if (!intserv)
			intserv = get_property(np, "reg", &len);

		i = len / sizeof(u32);

		while (i--)
			if (intserv[i] == hcpuid)
				return np;
	}
	return NULL;
}


And then the loop in xics_init_IRQ can be replaced with something like:

np = cpuid_to_of_node(boot_cpuid);
BUG_ON(!np);
ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
hcpuid = get_hard_smp_processor_id(boot_cpuid);

if (ireg[0] == hcpuid)
	default_distrib_server = ireg[1];
else
	default_distrib_server = ireg[3];

default_server = hcpuid;

...

Re: [RFC] Fix for interrupt distribution

[Mohan Kumar M]

On Mon, Nov 06, 2006 at 04:46:03PM -0600, Nathan Lynch wrote:
> Mohan Kumar M wrote:
> >
> > Index: linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
> > ===================================================================
> > --- linux-2.6.19-rc3.orig/arch/powerpc/platforms/pseries/xics.c
> > +++ linux-2.6.19-rc3/arch/powerpc/platforms/pseries/xics.c
> > @@ -658,7 +658,7 @@ static void __init xics_setup_8259_casca
> >  
> >  void __init xics_init_IRQ(void)
> >  {
> > -	int i;
> > +	int i, j;
> >  	struct device_node *np;
> >  	u32 ilen, indx = 0;
> >  	const u32 *ireg;
> > @@ -686,23 +686,23 @@ void __init xics_init_IRQ(void)
> >  	for (np = of_find_node_by_type(NULL, "cpu");
> >  	     np;
> >  	     np = of_find_node_by_type(np, "cpu")) {
> > -		ireg = get_property(np, "reg", &ilen);
> > -		if (ireg && ireg[0] == get_hard_smp_processor_id(boot_cpuid)) {
> > -			ireg = get_property(np,
> > -					"ibm,ppc-interrupt-gserver#s", &ilen);
> > -			i = ilen / sizeof(int);
> > -			if (ireg && i > 0) {
> > -				default_server = ireg[0];
> > -				/* take last element */
> > -				default_distrib_server = ireg[i-1];
> > -			}
> > -			ireg = get_property(np,
> > +		ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
> > +		i = ilen / sizeof(int);
> > +		for (j = 0; ireg[j] && j < i; j+=2) {
>                             ^^^^^^
> I don't understand this condition in the loop -- zero is a valid (and
> common) value for interrupt servers:
> 
> # xxd /proc/device-tree/cpus/PowerPC,POWER5@0/ibm,ppc-interrupt-server#s
> 0000000: 0000 0000 0000 0001
> 
> # xxd /proc/device-tree/cpus/PowerPC,POWER5@0/ibm,ppc-interrupt-gserver#s
> 0000000: 0000 0000 0000 00ff 0000 0001 0000 00ff
> 
> So I think this patch wouldn't always work.
> 
> > +			if (ireg[j] == get_hard_smp_processor_id(boot_cpuid)) {
> > +				if ( j + 1 < i) {
> > +					default_server = ireg[j];
> > +					default_distrib_server = ireg[j+1];
> > +				}
> > +				ireg = get_property(np,
> >  					"ibm,interrupt-server#-size", NULL);
> > -			if (ireg)
> > -				interrupt_server_size = *ireg;
> > -			break;
> > +				if (ireg)
> > +					interrupt_server_size = *ireg;
> > +				goto out;
> > +			}
> >  		}
> >  	}
> > +out:
> >  	of_node_put(np);
> 
> 
> I think we need a helper function or two to make this easier to do
> correctly and readably, e.g.
> 
> /* Returns the device node (with refcount incremented)
>  * corresponding to the given logical cpu id */
> struct device_node * cpuid_to_of_node(int cpu) {
> 	struct device_node *np;
> 	u32 hcpuid = get_hard_smp_processor_id(cpu);
> 
> 	for_each_node_by_type(np, "cpu") {
> 		int len, i;
> 		u32 *intserv = get_property(np,
> 				"ibm,ppc-interrupt-server#s", &len);
> 
> 		if (!intserv)
> 			intserv = get_property(np, "reg", &len);
> 
> 		i = len / sizeof(u32);
> 
> 		while (i--)
> 			if (intserv[i] == hcpuid)
> 				return np;
> 	}
> 	return NULL;
> }
> 
> 
> And then the loop in xics_init_IRQ can be replaced with something like:
> 
> np = cpuid_to_of_node(boot_cpuid);
> BUG_ON(!np);
> ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
> hcpuid = get_hard_smp_processor_id(boot_cpuid);
> 
> if (ireg[0] == hcpuid)
> 	default_distrib_server = ireg[1];
> else
> 	default_distrib_server = ireg[3];
> 
> default_server = hcpuid;
> 
> ...

Back to work after a long vacation.

Nathan,

Thanks for your suggestions. Based on your suggestions I have rewritten
the patch to use ibm,ppc-interrupt-server#s to check for both primary
and secondary CPU ids. Accordingly default distribution server value is
initialized from "ibm,ppc-interrupt-gserver#s" property.

Patch created over 2.6.19-rc5.

o The following patch allows any secondary CPU thread also to become
  boot cpu for POWER5. The patch is required to solve kdump boot issue
  when the kdump kernel is booted with parameter "maxcpus=1". XICS init
  code tries to match the current boot cpu id with "reg" property in
  each CPU node in the device tree. But CPU node is created only for
  primary thread CPU ids and "reg" property only reflects primary CPU
  ids. So when a kernel is booted on a secondary cpu thread above
  condition will never meet and the default distribution server is left
  as zero. This leads to route the interrupts to CPU 0, but which is not
  online at this time.

o The patch uses ibm,ppc-interrupt-server#s to check for both primary
  and secondary CPU ids. Accordingly default distribution server value
  is initialized from "ibm,ppc-interrupt-gserver#s" property.

Signed-off-by: Mohan Kumar M <mohan@in.ibm.com>
---
 arch/powerpc/platforms/pseries/xics.c |   57 +++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 21 deletions(-)

Index: linux-2.6.19-rc5/arch/powerpc/platforms/pseries/xics.c
===================================================================
--- linux-2.6.19-rc5.orig/arch/powerpc/platforms/pseries/xics.c
+++ linux-2.6.19-rc5/arch/powerpc/platforms/pseries/xics.c
@@ -656,13 +656,37 @@ static void __init xics_setup_8259_casca
 	set_irq_chained_handler(cascade, pseries_8259_cascade);
 }
 
+static struct device_node *cpuid_to_of_node(int cpu)
+{
+	struct device_node *np;
+	u32 hcpuid = get_hard_smp_processor_id(cpu);
+
+	for_each_node_by_type(np, "cpu") {
+		int i, len;
+		const u32 *intserv;
+
+		intserv = get_property(np, "ibm,ppc-interrupt-server#s", &len);
+
+		if (!intserv)
+			intserv = get_property(np, "reg", &len);
+
+		i = len / sizeof(u32);
+
+		while (i--)
+			if (intserv[i] == hcpuid)
+				return np;
+	}
+
+	return NULL;
+}
+
 void __init xics_init_IRQ(void)
 {
-	int i;
 	struct device_node *np;
 	u32 ilen, indx = 0;
 	const u32 *ireg;
 	int found = 0;
+	int hcpuid;
 
 	ppc64_boot_msg(0x20, "XICS Init");
 
@@ -683,26 +707,17 @@ void __init xics_init_IRQ(void)
 	xics_init_host();
 
 	/* Find the server numbers for the boot cpu. */
-	for (np = of_find_node_by_type(NULL, "cpu");
-	     np;
-	     np = of_find_node_by_type(np, "cpu")) {
-		ireg = get_property(np, "reg", &ilen);
-		if (ireg && ireg[0] == get_hard_smp_processor_id(boot_cpuid)) {
-			ireg = get_property(np,
-					"ibm,ppc-interrupt-gserver#s", &ilen);
-			i = ilen / sizeof(int);
-			if (ireg && i > 0) {
-				default_server = ireg[0];
-				/* take last element */
-				default_distrib_server = ireg[i-1];
-			}
-			ireg = get_property(np,
-					"ibm,interrupt-server#-size", NULL);
-			if (ireg)
-				interrupt_server_size = *ireg;
-			break;
-		}
-	}
+	np = cpuid_to_of_node(boot_cpuid);
+	BUG_ON(!np);
+	ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
+	hcpuid = get_hard_smp_processor_id(boot_cpuid);
+
+	if (ireg[0] == hcpuid)
+		default_distrib_server = ireg[1];
+	else
+		default_distrib_server = ireg[3];
+
+	default_server = hcpuid;
 	of_node_put(np);
 
 	if (firmware_has_feature(FW_FEATURE_LPAR))

Re: [RFC] Fix for interrupt distribution

[Anton Blanchard]

 
Hi,

Thanks for fixing this problem. One thing I noticed in the patch:

> +	np = cpuid_to_of_node(boot_cpuid);
> +	BUG_ON(!np);
> +	ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
> +	hcpuid = get_hard_smp_processor_id(boot_cpuid);
> +
> +	if (ireg[0] == hcpuid)
> +		default_distrib_server = ireg[1];
> +	else
> +		default_distrib_server = ireg[3];

While this will work on current firmware it would be more robust to loop
through interrupt-gserver#s until you find your cpuid. For example we
set up multiple interrupt-gserver#s entries in the lab so we can have
chip, node local and global targets:

ibm,ppc-interrupt-server#s
	00000002 00000003

ibm,ppc-interrupt-gserver#s
	00000002 000000ff 00000003 000000ff 00000002 000000f7
	00000003 000000f7 00000002 000000d7
	00000003 000000d7

We always put the global target at the end (0xd7 in this case) because
the current code in Linux just grabs the last entry in the array to get
the global server. So the question is, do we look for the first or the
last entry that matches your cpuid to find the global server?

It might be worth getting something added to the architecture (PAPR) to
clear it up. To match what current Linux does, we would want servers to
go from including the least to the most cpus, so the global one is at
the end.

Anton

Re: [RFC] Fix for interrupt distribution

[Mohan Kumar M]

On Fri, Nov 17, 2006 at 02:36:16AM +1100, Anton Blanchard wrote:
> 
> Hi,
> 
> Thanks for fixing this problem. One thing I noticed in the patch:
> 
> > +	np = cpuid_to_of_node(boot_cpuid);
> > +	BUG_ON(!np);
> > +	ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
> > +	hcpuid = get_hard_smp_processor_id(boot_cpuid);
> > +
> > +	if (ireg[0] == hcpuid)
> > +		default_distrib_server = ireg[1];
> > +	else
> > +		default_distrib_server = ireg[3];
> 
> While this will work on current firmware it would be more robust to loop
> through interrupt-gserver#s until you find your cpuid. For example we
> set up multiple interrupt-gserver#s entries in the lab so we can have
> chip, node local and global targets:
> 
> ibm,ppc-interrupt-server#s
> 	00000002 00000003
> 
> ibm,ppc-interrupt-gserver#s
> 	00000002 000000ff 00000003 000000ff 00000002 000000f7
> 	00000003 000000f7 00000002 000000d7
> 	00000003 000000d7
> 
> We always put the global target at the end (0xd7 in this case) because
> the current code in Linux just grabs the last entry in the array to get
> the global server. So the question is, do we look for the first or the
> last entry that matches your cpuid to find the global server?
>

Hi Anton,

Thanks for your input.

But I am also having the same question, which entry will be appropriate
to find the global server? First or the last cpu id entry? The existing
code always uses the last entry regardless of the cpu id to determine
the global server. 
 
> It might be worth getting something added to the architecture (PAPR) to
> clear it up. To match what current Linux does, we would want servers to
> go from including the least to the most cpus, so the global one is at
> the end.
> 
> Anton

Re: [RFC] Fix for interrupt distribution

[Mohan Kumar M]

On Fri, Nov 17, 2006 at 02:36:16AM +1100, Anton Blanchard wrote:
> 
> Hi,
> 
> Thanks for fixing this problem. One thing I noticed in the patch:
> 
> > +	np = cpuid_to_of_node(boot_cpuid);
> > +	BUG_ON(!np);
> > +	ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
> > +	hcpuid = get_hard_smp_processor_id(boot_cpuid);
> > +
> > +	if (ireg[0] == hcpuid)
> > +		default_distrib_server = ireg[1];
> > +	else
> > +		default_distrib_server = ireg[3];
> 
> While this will work on current firmware it would be more robust to loop
> through interrupt-gserver#s until you find your cpuid. For example we
> set up multiple interrupt-gserver#s entries in the lab so we can have
> chip, node local and global targets:
> 
> ibm,ppc-interrupt-server#s
> 	00000002 00000003
> 
> ibm,ppc-interrupt-gserver#s
> 	00000002 000000ff 00000003 000000ff 00000002 000000f7
> 	00000003 000000f7 00000002 000000d7
> 	00000003 000000d7
> 
> We always put the global target at the end (0xd7 in this case) because
> the current code in Linux just grabs the last entry in the array to get
> the global server. So the question is, do we look for the first or the
> last entry that matches your cpuid to find the global server?
> 
> It might be worth getting something added to the architecture (PAPR) to
> clear it up. To match what current Linux does, we would want servers to
> go from including the least to the most cpus, so the global one is at
> the end.
> 

Thanks Anton,

I have modified the patch to loop through ibm,ppc-interrupt-gserver#s
property to find the global distribution server from the last entry
that matches with boot cpuid. So this patch will work with above
situation also.

o The following patch allows any secondary CPU thread also to become
  boot cpu for POWER5. The patch is required to solve kdump boot issue
  when the kdump kernel is booted with parameter "maxcpus=1". XICS init
  code tries to match the current boot cpu id with "reg" property in
  each CPU node in the device tree. But CPU node is created only for
  primary thread CPU ids and "reg" property only reflects primary CPU
  ids. So when a kernel is booted on a secondary cpu thread above
  condition will never meet and the default distribution server is left
  as zero. This leads to route the interrupts to CPU 0, but which is not
  online at this time.

o The patch uses ibm,ppc-interrupt-server#s to check for both primary
  and secondary CPU ids. Accordingly default distribution server value
  is initialized from "ibm,ppc-interrupt-gserver#s" property.

Signed-off-by: Mohan Kumar M <mohan@in.ibm.com>
---
 arch/powerpc/platforms/pseries/xics.c |   68 ++++++++++++++++++++++++----------
 1 file changed, 49 insertions(+), 19 deletions(-)

Index: linux-2.6.19-rc5/arch/powerpc/platforms/pseries/xics.c
===================================================================
--- linux-2.6.19-rc5.orig/arch/powerpc/platforms/pseries/xics.c
+++ linux-2.6.19-rc5/arch/powerpc/platforms/pseries/xics.c
@@ -656,13 +656,38 @@ static void __init xics_setup_8259_casca
 	set_irq_chained_handler(cascade, pseries_8259_cascade);
 }
 
+static struct device_node *cpuid_to_of_node(int cpu)
+{
+	struct device_node *np;
+	u32 hcpuid = get_hard_smp_processor_id(cpu);
+
+	for_each_node_by_type(np, "cpu") {
+		int i, len;
+		const u32 *intserv;
+
+		intserv = get_property(np, "ibm,ppc-interrupt-server#s", &len);
+
+		if (!intserv)
+			intserv = get_property(np, "reg", &len);
+
+		i = len / sizeof(u32);
+
+		while (i--)
+			if (intserv[i] == hcpuid)
+				return np;
+	}
+
+	return NULL;
+}
+
 void __init xics_init_IRQ(void)
 {
-	int i;
+	int i, j;
 	struct device_node *np;
 	u32 ilen, indx = 0;
-	const u32 *ireg;
+	const u32 *ireg, *isize;
 	int found = 0;
+	u32 hcpuid;
 
 	ppc64_boot_msg(0x20, "XICS Init");
 
@@ -683,26 +708,31 @@ void __init xics_init_IRQ(void)
 	xics_init_host();
 
 	/* Find the server numbers for the boot cpu. */
-	for (np = of_find_node_by_type(NULL, "cpu");
-	     np;
-	     np = of_find_node_by_type(np, "cpu")) {
-		ireg = get_property(np, "reg", &ilen);
-		if (ireg && ireg[0] == get_hard_smp_processor_id(boot_cpuid)) {
-			ireg = get_property(np,
-					"ibm,ppc-interrupt-gserver#s", &ilen);
-			i = ilen / sizeof(int);
-			if (ireg && i > 0) {
-				default_server = ireg[0];
-				/* take last element */
-				default_distrib_server = ireg[i-1];
-			}
-			ireg = get_property(np,
+	np = cpuid_to_of_node(boot_cpuid);
+	BUG_ON(!np);
+	ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
+	if (!ireg)
+		goto skip_gserver_check;
+	i = ilen / sizeof(int);
+	hcpuid = get_hard_smp_processor_id(boot_cpuid);
+
+	/* Global interrupt distribution server is specified in the last
+	 * entry of "ibm,ppc-interrupt-gserver#s" property. Get the last
+	 * entry fom this property for current boot cpu id and use it as
+	 * default distribution server
+	 */
+	for (j = 0; j < i; j += 2) {
+		if (ireg[j] == hcpuid) {
+			default_server = hcpuid;
+			default_distrib_server = ireg[j+1];
+
+			isize = get_property(np,
 					"ibm,interrupt-server#-size", NULL);
-			if (ireg)
-				interrupt_server_size = *ireg;
-			break;
+			if (isize)
+				interrupt_server_size = *isize;
 		}
 	}
+skip_gserver_check:
 	of_node_put(np);
 
 	if (firmware_has_feature(FW_FEATURE_LPAR))

[PATCH] Fix for interrupt distribution

[Mohan Kumar M]

On Fri, Nov 17, 2006 at 05:42:24PM +0530, Mohan Kumar M wrote:
> On Fri, Nov 17, 2006 at 02:36:16AM +1100, Anton Blanchard wrote:
> > 
> > Hi,
> > 
> > Thanks for fixing this problem. One thing I noticed in the patch:
> > 
> > > +	np = cpuid_to_of_node(boot_cpuid);
> > > +	BUG_ON(!np);
> > > +	ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
> > > +	hcpuid = get_hard_smp_processor_id(boot_cpuid);
> > > +
> > > +	if (ireg[0] == hcpuid)
> > > +		default_distrib_server = ireg[1];
> > > +	else
> > > +		default_distrib_server = ireg[3];
> > 
> > While this will work on current firmware it would be more robust to loop
> > through interrupt-gserver#s until you find your cpuid. For example we
> > set up multiple interrupt-gserver#s entries in the lab so we can have
> > chip, node local and global targets:
> > 
> > ibm,ppc-interrupt-server#s
> > 	00000002 00000003
> > 
> > ibm,ppc-interrupt-gserver#s
> > 	00000002 000000ff 00000003 000000ff 00000002 000000f7
> > 	00000003 000000f7 00000002 000000d7
> > 	00000003 000000d7
> > 
> > We always put the global target at the end (0xd7 in this case) because
> > the current code in Linux just grabs the last entry in the array to get
> > the global server. So the question is, do we look for the first or the
> > last entry that matches your cpuid to find the global server?
> > 
> > It might be worth getting something added to the architecture (PAPR) to
> > clear it up. To match what current Linux does, we would want servers to
> > go from including the least to the most cpus, so the global one is at
> > the end.
> >

Hi Paul,

I updated the patch as per Anton's suggestion. Can you please consider
this patch to be merged in the powerpc git tree?

Regards,
Mohan.
 
> 
> Thanks Anton,
> 
> I have modified the patch to loop through ibm,ppc-interrupt-gserver#s
> property to find the global distribution server from the last entry
> that matches with boot cpuid. So this patch will work with above
> situation also.
> 
> o The following patch allows any secondary CPU thread also to become
>   boot cpu for POWER5. The patch is required to solve kdump boot issue
>   when the kdump kernel is booted with parameter "maxcpus=1". XICS init
>   code tries to match the current boot cpu id with "reg" property in
>   each CPU node in the device tree. But CPU node is created only for
>   primary thread CPU ids and "reg" property only reflects primary CPU
>   ids. So when a kernel is booted on a secondary cpu thread above
>   condition will never meet and the default distribution server is left
>   as zero. This leads to route the interrupts to CPU 0, but which is not
>   online at this time.
> 
> o The patch uses ibm,ppc-interrupt-server#s to check for both primary
>   and secondary CPU ids. Accordingly default distribution server value
>   is initialized from "ibm,ppc-interrupt-gserver#s" property.
> 
> Signed-off-by: Mohan Kumar M <mohan@in.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/xics.c |   68 ++++++++++++++++++++++++----------
>  1 file changed, 49 insertions(+), 19 deletions(-)
> 
> Index: linux-2.6.19-rc5/arch/powerpc/platforms/pseries/xics.c
> ===================================================================
> --- linux-2.6.19-rc5.orig/arch/powerpc/platforms/pseries/xics.c
> +++ linux-2.6.19-rc5/arch/powerpc/platforms/pseries/xics.c
> @@ -656,13 +656,38 @@ static void __init xics_setup_8259_casca
>  	set_irq_chained_handler(cascade, pseries_8259_cascade);
>  }
> 
> +static struct device_node *cpuid_to_of_node(int cpu)
> +{
> +	struct device_node *np;
> +	u32 hcpuid = get_hard_smp_processor_id(cpu);
> +
> +	for_each_node_by_type(np, "cpu") {
> +		int i, len;
> +		const u32 *intserv;
> +
> +		intserv = get_property(np, "ibm,ppc-interrupt-server#s", &len);
> +
> +		if (!intserv)
> +			intserv = get_property(np, "reg", &len);
> +
> +		i = len / sizeof(u32);
> +
> +		while (i--)
> +			if (intserv[i] == hcpuid)
> +				return np;
> +	}
> +
> +	return NULL;
> +}
> +
>  void __init xics_init_IRQ(void)
>  {
> -	int i;
> +	int i, j;
>  	struct device_node *np;
>  	u32 ilen, indx = 0;
> -	const u32 *ireg;
> +	const u32 *ireg, *isize;
>  	int found = 0;
> +	u32 hcpuid;
> 
>  	ppc64_boot_msg(0x20, "XICS Init");
> 
> @@ -683,26 +708,31 @@ void __init xics_init_IRQ(void)
>  	xics_init_host();
> 
>  	/* Find the server numbers for the boot cpu. */
> -	for (np = of_find_node_by_type(NULL, "cpu");
> -	     np;
> -	     np = of_find_node_by_type(np, "cpu")) {
> -		ireg = get_property(np, "reg", &ilen);
> -		if (ireg && ireg[0] == get_hard_smp_processor_id(boot_cpuid)) {
> -			ireg = get_property(np,
> -					"ibm,ppc-interrupt-gserver#s", &ilen);
> -			i = ilen / sizeof(int);
> -			if (ireg && i > 0) {
> -				default_server = ireg[0];
> -				/* take last element */
> -				default_distrib_server = ireg[i-1];
> -			}
> -			ireg = get_property(np,
> +	np = cpuid_to_of_node(boot_cpuid);
> +	BUG_ON(!np);
> +	ireg = get_property(np, "ibm,ppc-interrupt-gserver#s", &ilen);
> +	if (!ireg)
> +		goto skip_gserver_check;
> +	i = ilen / sizeof(int);
> +	hcpuid = get_hard_smp_processor_id(boot_cpuid);
> +
> +	/* Global interrupt distribution server is specified in the last
> +	 * entry of "ibm,ppc-interrupt-gserver#s" property. Get the last
> +	 * entry fom this property for current boot cpu id and use it as
> +	 * default distribution server
> +	 */
> +	for (j = 0; j < i; j += 2) {
> +		if (ireg[j] == hcpuid) {
> +			default_server = hcpuid;
> +			default_distrib_server = ireg[j+1];
> +
> +			isize = get_property(np,
>  					"ibm,interrupt-server#-size", NULL);
> -			if (ireg)
> -				interrupt_server_size = *ireg;
> -			break;
> +			if (isize)
> +				interrupt_server_size = *isize;
>  		}
>  	}
> +skip_gserver_check:
>  	of_node_put(np);
> 
>  	if (firmware_has_feature(FW_FEATURE_LPAR))
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev