[PATCH 0/3] xics mask_irq fix and other patches

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/3] xics mask_irq fix and other patches
@ 2011-03-21 18:12 Milton Miller
  2011-03-21 18:12 ` [PATCH] powerpc: pseries/smp: query-cpu-stopped-state support won't change Milton Miller
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Milton Miller @ 2011-03-21 18:12 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev

Hi Ben.

I've submitted the fix for the xics mask real irq I mentioned.
During testing I realized current code is passing rtas a linux irq,
which means no interrupt (at least those above NR_IRQs) is being disabled.

I also created a patch to rename irq to hwirq in xics to try to make
the distiction more clear.  For your consideration, I don't care if you
don't merge it especially with the upcoming rewrite.

Also, I am sending a printk to printk_once patch that affects BML.

Thanks,
milton

MAIL FROM: <miltonm@bga.com>
RCPT TO: <miltonm@bga.com>
RCPT TO: <linuxppc-dev@lists.ozlabs.org>
RCPT TO: <buytenh@secretlab.ca>
DATA
From: Milton Miller <miltonm@bga.com>
Subject: [PATCH 1/2] powerpc: xics: fix numberspace mismatch from irq_desc conversion
To: linuxppc-dev@lists.ozlabs.org
Cc: Lennert Buytenhek <buytenh@secretlab.ca>
Message-id: <xics-mask-irq-desc-fix@mdm.bga.com>
In-Reply-To: <patches-for-39-rc1@mdm.bga.com>
References: <patches-for-39-rc1@mdm.bga.com>

commit 79f26c268ebad29bd75d078cfc09d3d82b30ccbd (powerpc:
platforms/pseries irq_data conversion) pushed irq_desc down into many
functions, dererencing the descriptor irq field as late as possible.

But it incorrectly passed a linix virtural irq number to RTAS,
resulting in the interrupt not being disabled and possibly
other bad things, such as another interrupt being disabled and/or
a checkstop.  

In addition this missed the point of xics_mask_unknown_vec and
the seperation of xics_mask_real_irq from xics_mask_irq.  When
xics_mask_unknown_vec is called it's because the hardware delivered an
irq source for which we have no linux irq allocated, and thefore we can
not have an irq_desc allocated.

Revert xics_mask_real_irq to its prior version, naming the argument
hwirq to highlight the difference.

Signed-off-by: Milton Miller <miltonm@bga.com>

--

Lennert can you please review the other patches for similar problems?

Any reference to irq_map[x].hwirq is in a different number domain.

I initially saw the problem pushing irq_desc through unknown_vec
found this due to my knowledge of the split (I added that function)
and only realized the wrong number space when preparing to test the fix.
In fact I tried without the patch and my firmware checkstops the machine
(hardware stops executing instructions)!

diff --git a/arch/powerpc/platforms/pseries/xics.c b/arch/powerpc/platforms/pseries/xics.c
index 01fea46..5686db9 100644
--- a/arch/powerpc/platforms/pseries/xics.c
+++ b/arch/powerpc/platforms/pseries/xics.c
@@ -250,26 +250,26 @@ static unsigned int xics_startup(struct irq_data *d)
 	return 0;
 }

-static void xics_mask_real_irq(struct irq_data *d)
+static void xics_mask_real_irq(unsigned int hwirq)
 {
 	int call_status;

-	if (d->irq == XICS_IPI)
+	if (hwirq == XICS_IPI)
 		return;

-	call_status = rtas_call(ibm_int_off, 1, 1, NULL, d->irq);
+	call_status = rtas_call(ibm_int_off, 1, 1, NULL, hwirq);
 	if (call_status != 0) {
 		printk(KERN_ERR "%s: ibm_int_off irq=%u returned %d\n",
-			__func__, d->irq, call_status);
+			__func__, hwirq, call_status);
 		return;
 	}

 	/* Have to set XIVE to 0xff to be able to remove a slot */
-	call_status = rtas_call(ibm_set_xive, 3, 1, NULL, d->irq,
+	call_status = rtas_call(ibm_set_xive, 3, 1, NULL, hwirq,
 				default_server, 0xff);
 	if (call_status != 0) {
 		printk(KERN_ERR "%s: ibm_set_xive(0xff) irq=%u returned %d\n",
-			__func__, d->irq, call_status);
+			__func__, hwirq, call_status);
 		return;
 	}
 }
@@ -283,13 +283,13 @@ static void xics_mask_irq(struct irq_data *d)
 	irq = (unsigned int)irq_map[d->irq].hwirq;
 	if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
 		return;
-	xics_mask_real_irq(d);
+	xics_mask_real_irq(irq);
 }

 static void xics_mask_unknown_vec(unsigned int vec)
 {
 	printk(KERN_ERR "Interrupt %u (real) is invalid, disabling it.\n", vec);
-	xics_mask_real_irq(irq_get_irq_data(vec));
+	xics_mask_real_irq(vec);
 }

 static inline unsigned int xics_xirr_vector(unsigned int xirr)

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] powerpc: pseries/smp: query-cpu-stopped-state support won't change
  2011-03-21 18:12 [PATCH 0/3] xics mask_irq fix and other patches Milton Miller
@ 2011-03-21 18:12 ` Milton Miller
  2011-03-21 18:12 ` [PATCH 2/2] powerpc: xics: use hwirq for xics domain irq number Milton Miller
  2011-03-21 21:38 ` [PATCH 1/2] powerpc: xics: fix numberspace mismatch from irq_desc conversion Milton Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Milton Miller @ 2011-03-21 18:12 UTC (permalink / raw)
  To: linuxppc-dev

If a given firmware doesn't have a token to support query-cpu-stopped-state,
its not likely to change during the lifetime of the kernel.   

Only print this information once, not once per secondary thread.

While here, make the line wrap grep friendly.

Signed-off-by: Milton Miller <miltonm@bga.com>

Index: work.git/arch/powerpc/platforms/pseries/smp.c
===================================================================
--- work.git.orig/arch/powerpc/platforms/pseries/smp.c	2011-03-21 11:27:54.000000000 -0500
+++ work.git/arch/powerpc/platforms/pseries/smp.c	2011-03-21 11:29:51.000000000 -0500
@@ -64,8 +64,8 @@ int smp_query_cpu_stopped(unsigned int p
 	int qcss_tok = rtas_token("query-cpu-stopped-state");
 
 	if (qcss_tok == RTAS_UNKNOWN_SERVICE) {
-		printk(KERN_INFO "Firmware doesn't support "
-				"query-cpu-stopped-state\n");
+		printk_once(KERN_INFO
+			"Firmware doesn't support query-cpu-stopped-state\n");
 		return QCSS_HARDWARE_ERROR;
 	}
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] powerpc: xics: use hwirq for xics domain irq number
  2011-03-21 18:12 [PATCH 0/3] xics mask_irq fix and other patches Milton Miller
  2011-03-21 18:12 ` [PATCH] powerpc: pseries/smp: query-cpu-stopped-state support won't change Milton Miller
@ 2011-03-21 18:12 ` Milton Miller
  2011-03-21 21:38 ` [PATCH 1/2] powerpc: xics: fix numberspace mismatch from irq_desc conversion Milton Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Milton Miller @ 2011-03-21 18:12 UTC (permalink / raw)
  To: linuxppc-dev

To try to avoid future confusion, rename irq to hwirq when it refers
to a xics domain number instead of a linux irq number.

Signed-off-by: Milton Miller <miltonm@bga.com>

Index: next.git/arch/powerpc/platforms/pseries/xics.c
===================================================================
--- next.git.orig/arch/powerpc/platforms/pseries/xics.c	2011-03-19 23:21:21.775085058 -0500
+++ next.git/arch/powerpc/platforms/pseries/xics.c	2011-03-20 00:57:33.144560092 -0500
@@ -204,33 +204,33 @@ static int get_irq_server(unsigned int v
 
 static void xics_unmask_irq(struct irq_data *d)
 {
-	unsigned int irq;
+	unsigned int hwirq;
 	int call_status;
 	int server;
 
 	pr_devel("xics: unmask virq %d\n", d->irq);
 
-	irq = (unsigned int)irq_map[d->irq].hwirq;
-	pr_devel(" -> map to hwirq 0x%x\n", irq);
-	if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
+	hwirq = (unsigned int)irq_map[d->irq].hwirq;
+	pr_devel(" -> map to hwirq 0x%x\n", hwirq);
+	if (hwirq == XICS_IPI || hwirq == XICS_IRQ_SPURIOUS)
 		return;
 
 	server = get_irq_server(d->irq, d->affinity, 0);
 
-	call_status = rtas_call(ibm_set_xive, 3, 1, NULL, irq, server,
+	call_status = rtas_call(ibm_set_xive, 3, 1, NULL, hwirq, server,
 				DEFAULT_PRIORITY);
 	if (call_status != 0) {
 		printk(KERN_ERR
 			"%s: ibm_set_xive irq %u server %x returned %d\n",
-			__func__, irq, server, call_status);
+			__func__, hwirq, server, call_status);
 		return;
 	}
 
 	/* Now unmask the interrupt (often a no-op) */
-	call_status = rtas_call(ibm_int_on, 1, 1, NULL, irq);
+	call_status = rtas_call(ibm_int_on, 1, 1, NULL, hwirq);
 	if (call_status != 0) {
 		printk(KERN_ERR "%s: ibm_int_on irq=%u returned %d\n",
-			__func__, irq, call_status);
+			__func__, hwirq, call_status);
 		return;
 	}
 }
@@ -276,14 +276,14 @@ static void xics_mask_real_irq(unsigned 
 
 static void xics_mask_irq(struct irq_data *d)
 {
-	unsigned int irq;
+	unsigned int hwirq;
 
 	pr_devel("xics: mask virq %d\n", d->irq);
 
-	irq = (unsigned int)irq_map[d->irq].hwirq;
-	if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
+	hwirq = (unsigned int)irq_map[d->irq].hwirq;
+	if (hwirq == XICS_IPI || hwirq == XICS_IRQ_SPURIOUS)
 		return;
-	xics_mask_real_irq(irq);
+	xics_mask_real_irq(hwirq);
 }
 
 static void xics_mask_unknown_vec(unsigned int vec)
@@ -373,37 +373,37 @@ static unsigned char pop_cppr(void)
 
 static void xics_eoi_direct(struct irq_data *d)
 {
-	unsigned int irq = (unsigned int)irq_map[d->irq].hwirq;
+	unsigned int hwirq = (unsigned int)irq_map[d->irq].hwirq;
 
 	iosync();
-	direct_xirr_info_set((pop_cppr() << 24) | irq);
+	direct_xirr_info_set((pop_cppr() << 24) | hwirq);
 }
 
 static void xics_eoi_lpar(struct irq_data *d)
 {
-	unsigned int irq = (unsigned int)irq_map[d->irq].hwirq;
+	unsigned int hwirq = (unsigned int)irq_map[d->irq].hwirq;
 
 	iosync();
-	lpar_xirr_info_set((pop_cppr() << 24) | irq);
+	lpar_xirr_info_set((pop_cppr() << 24) | hwirq);
 }
 
 static int
 xics_set_affinity(struct irq_data *d, const struct cpumask *cpumask, bool force)
 {
-	unsigned int irq;
+	unsigned int hwirq;
 	int status;
 	int xics_status[2];
 	int irq_server;
 
-	irq = (unsigned int)irq_map[d->irq].hwirq;
-	if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
+	hwirq = (unsigned int)irq_map[d->irq].hwirq;
+	if (hwirq == XICS_IPI || hwirq == XICS_IRQ_SPURIOUS)
 		return -1;
 
-	status = rtas_call(ibm_get_xive, 1, 3, xics_status, irq);
+	status = rtas_call(ibm_get_xive, 1, 3, xics_status, hwirq);
 
 	if (status) {
 		printk(KERN_ERR "%s: ibm,get-xive irq=%u returns %d\n",
-			__func__, irq, status);
+			__func__, hwirq, status);
 		return -1;
 	}
 
@@ -418,11 +418,11 @@ xics_set_affinity(struct irq_data *d, co
 	}
 
 	status = rtas_call(ibm_set_xive, 3, 1, NULL,
-				irq, irq_server, xics_status[1]);
+				hwirq, irq_server, xics_status[1]);
 
 	if (status) {
 		printk(KERN_ERR "%s: ibm,set-xive irq=%u returns %d\n",
-			__func__, irq, status);
+			__func__, hwirq, status);
 		return -1;
 	}
 
@@ -874,7 +874,7 @@ void xics_kexec_teardown_cpu(int seconda
 void xics_migrate_irqs_away(void)
 {
 	int cpu = smp_processor_id(), hw_cpu = hard_smp_processor_id();
-	unsigned int irq, virq;
+	int virq;
 
 	/* If we used to be the default server, move to the new "boot_cpuid" */
 	if (hw_cpu == default_server)
@@ -892,6 +892,7 @@ void xics_migrate_irqs_away(void)
 	for_each_irq(virq) {
 		struct irq_desc *desc;
 		struct irq_chip *chip;
+		unsigned int hwirq;
 		int xics_status[2];
 		int status;
 		unsigned long flags;
@@ -901,9 +902,9 @@ void xics_migrate_irqs_away(void)
 			continue;
 		if (irq_map[virq].host != xics_host)
 			continue;
-		irq = (unsigned int)irq_map[virq].hwirq;
+		hwirq = (unsigned int)irq_map[virq].hwirq;
 		/* We need to get IPIs still. */
-		if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
+		if (hwirq == XICS_IPI || hwirq == XICS_IRQ_SPURIOUS)
 			continue;
 
 		desc = irq_to_desc(virq);
@@ -918,10 +919,10 @@ void xics_migrate_irqs_away(void)
 
 		raw_spin_lock_irqsave(&desc->lock, flags);
 
-		status = rtas_call(ibm_get_xive, 1, 3, xics_status, irq);
+		status = rtas_call(ibm_get_xive, 1, 3, xics_status, hwirq);
 		if (status) {
 			printk(KERN_ERR "%s: ibm,get-xive irq=%u returns %d\n",
-					__func__, irq, status);
+					__func__, hwirq, status);
 			goto unlock;
 		}
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] powerpc: xics: fix numberspace mismatch from irq_desc conversion
  2011-03-21 18:12 [PATCH 0/3] xics mask_irq fix and other patches Milton Miller
  2011-03-21 18:12 ` [PATCH] powerpc: pseries/smp: query-cpu-stopped-state support won't change Milton Miller
  2011-03-21 18:12 ` [PATCH 2/2] powerpc: xics: use hwirq for xics domain irq number Milton Miller
@ 2011-03-21 21:38 ` Milton Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Milton Miller @ 2011-03-21 21:38 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Lennert Buytenhek

commit 79f26c268ebad29bd75d078cfc09d3d82b30ccbd (powerpc:
platforms/pseries irq_data conversion) pushed irq_desc down into many
functions, dererencing the descriptor irq field as late as possible.

But it incorrectly passed a linix virtural irq number to RTAS,
resulting in the interrupt not being disabled and possibly
other bad things, such as another interrupt being disabled and/or
a checkstop.  

In addition this missed the point of xics_mask_unknown_vec and
the seperation of xics_mask_real_irq from xics_mask_irq.  When
xics_mask_unknown_vec is called it's because the hardware delivered an
irq source for which we have no linux irq allocated, and thefore we can
not have an irq_desc allocated.

Revert xics_mask_real_irq to its prior version, naming the argument
hwirq to highlight the difference.

Signed-off-by: Milton Miller <miltonm@bga.com>

--

Lennert can you please review the other patches for similar problems?

Any reference to irq_map[x].hwirq is in a different number domain.

I initially saw the problem pushing irq_desc through unknown_vec
found this due to my knowledge of the split (I added that function)
and only realized the wrong number space when preparing to test the fix.
In fact I tried without the patch and my firmware checkstops the machine
(hardware stops executing instructions)!

diff --git a/arch/powerpc/platforms/pseries/xics.c b/arch/powerpc/platforms/pseries/xics.c
index 01fea46..5686db9 100644
--- a/arch/powerpc/platforms/pseries/xics.c
+++ b/arch/powerpc/platforms/pseries/xics.c
@@ -250,26 +250,26 @@ static unsigned int xics_startup(struct irq_data *d)
 	return 0;
 }

-static void xics_mask_real_irq(struct irq_data *d)
+static void xics_mask_real_irq(unsigned int hwirq)
 {
 	int call_status;

-	if (d->irq == XICS_IPI)
+	if (hwirq == XICS_IPI)
 		return;

-	call_status = rtas_call(ibm_int_off, 1, 1, NULL, d->irq);
+	call_status = rtas_call(ibm_int_off, 1, 1, NULL, hwirq);
 	if (call_status != 0) {
 		printk(KERN_ERR "%s: ibm_int_off irq=%u returned %d\n",
-			__func__, d->irq, call_status);
+			__func__, hwirq, call_status);
 		return;
 	}

 	/* Have to set XIVE to 0xff to be able to remove a slot */
-	call_status = rtas_call(ibm_set_xive, 3, 1, NULL, d->irq,
+	call_status = rtas_call(ibm_set_xive, 3, 1, NULL, hwirq,
 				default_server, 0xff);
 	if (call_status != 0) {
 		printk(KERN_ERR "%s: ibm_set_xive(0xff) irq=%u returned %d\n",
-			__func__, d->irq, call_status);
+			__func__, hwirq, call_status);
 		return;
 	}
 }
@@ -283,13 +283,13 @@ static void xics_mask_irq(struct irq_data *d)
 	irq = (unsigned int)irq_map[d->irq].hwirq;
 	if (irq == XICS_IPI || irq == XICS_IRQ_SPURIOUS)
 		return;
-	xics_mask_real_irq(d);
+	xics_mask_real_irq(irq);
 }

 static void xics_mask_unknown_vec(unsigned int vec)
 {
 	printk(KERN_ERR "Interrupt %u (real) is invalid, disabling it.\n", vec);
-	xics_mask_real_irq(irq_get_irq_data(vec));
+	xics_mask_real_irq(vec);
 }

 static inline unsigned int xics_xirr_vector(unsigned int xirr)

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2011-03-21 21:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-21 18:12 [PATCH 0/3] xics mask_irq fix and other patches Milton Miller
2011-03-21 18:12 ` [PATCH] powerpc: pseries/smp: query-cpu-stopped-state support won't change Milton Miller
2011-03-21 18:12 ` [PATCH 2/2] powerpc: xics: use hwirq for xics domain irq number Milton Miller
2011-03-21 21:38 ` [PATCH 1/2] powerpc: xics: fix numberspace mismatch from irq_desc conversion Milton Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.