[PATCH 6/14] ps3: smp interrupt fixes

STALE7131d

5 messages, 3 authors, 2007-01-26 · open the first message on its own page

[PATCH 6/14] ps3: smp interrupt fixes

From: Geoff Levand <hidden>
Date: 2007-01-25 02:40:10

PS3 fixups for interrups on SMP.

Fixes the alignment of the interrupt status bitmap, changes the hypervisor
interrupt calls to the '_ext' versions that take an explicit processor
thread ID, and sets the cpu_features flag CPU_FTR_SMT.


Signed-off-by: Geoff Levand <redacted>

---
 arch/powerpc/platforms/ps3/interrupt.c |   64 +++++++++++++++------------------
 arch/powerpc/platforms/ps3/setup.c     |    1 
 2 files changed, 32 insertions(+), 33 deletions(-)
--- ps3-linux-dev.orig/arch/powerpc/platforms/ps3/interrupt.c
+++ ps3-linux-dev/arch/powerpc/platforms/ps3/interrupt.c
@@ -317,22 +317,23 @@ struct ps3_bmp {
 		unsigned long unused_1[3];
 		unsigned long mask;
 		unsigned long unused_2[3];
-	} __attribute__ ((packed));
+	} __attribute__ ((aligned (64)));
+
 	spinlock_t lock;
 	unsigned long ipi_debug_brk_mask;
 };
 
 /**
  * struct ps3_private - a per cpu data structure
- * @node: HV node id
- * @cpu: HV thread id
- * @bmp: an HV ps3_bmp structure
+ * @bmp: ps3_bmp structure
+ * @node: HV logical_ppe_id
+ * @cpu: HV thread_id
  */
 
 struct ps3_private {
+	struct ps3_bmp bmp;
 	unsigned long node;
 	unsigned int cpu;
-	struct ps3_bmp bmp;
 };
 
 #if defined(DEBUG)
@@ -389,9 +390,8 @@ static void ps3_chip_mask(unsigned int v
 
 	spin_lock_irqsave(&pd->bmp.lock, flags);
 	pd->bmp.mask &= ~(0x8000000000000000UL >> virq);
-	spin_unlock_irqrestore(&pd->bmp.lock, flags);
-
 	lv1_did_update_interrupt_mask(pd->node, pd->cpu);
+	spin_unlock_irqrestore(&pd->bmp.lock, flags);
 }
 
 static void ps3_chip_unmask(unsigned int virq)
@@ -406,14 +406,14 @@ static void ps3_chip_unmask(unsigned int
 
 	spin_lock_irqsave(&pd->bmp.lock, flags);
 	pd->bmp.mask |= (0x8000000000000000UL >> virq);
-	spin_unlock_irqrestore(&pd->bmp.lock, flags);
-
 	lv1_did_update_interrupt_mask(pd->node, pd->cpu);
+	spin_unlock_irqrestore(&pd->bmp.lock, flags);
 }
 
 static void ps3_chip_eoi(unsigned int virq)
 {
-	lv1_end_of_interrupt(virq);
+	const struct ps3_private *pd = get_irq_chip_data(virq);
+	lv1_end_of_interrupt_ext(pd->node, pd->cpu, virq);
 }
 
 static struct irq_chip irq_chip = {
@@ -426,10 +426,12 @@ static struct irq_chip irq_chip = {
 static void ps3_host_unmap(struct irq_host *h, unsigned int virq)
 {
 	int result;
+	const struct ps3_private *pd = get_irq_chip_data(virq);
 
-	pr_debug("%s:%d: virq %d\n", __func__, __LINE__, virq);
+	pr_debug("%s:%d: node %lu, cpu %d, virq %u\n", __func__, __LINE__,
+		pd->node, pd->cpu, virq);
 
-	lv1_disconnect_irq_plug(virq);
+	lv1_disconnect_irq_plug_ext(pd->node, pd->cpu, virq);
 
 	result = set_irq_chip_data(virq, NULL);
 	BUG_ON(result);
@@ -441,31 +443,26 @@ static int ps3_host_map(struct irq_host 
 	irq_hw_number_t hwirq)
 {
 	int result;
-	unsigned int cpu;
+	struct ps3_private *pd = &__get_cpu_var(ps3_private);
+
+	pr_debug("%s:%d: node %lu, cpu %d, hwirq %lu => virq %u\n", __func__,
+		__LINE__, pd->node, pd->cpu, hwirq, virq);
 
-	pr_debug(" -> %s:%d\n", __func__, __LINE__);
-	pr_debug("%s:%d: hwirq %lu => virq %u\n", __func__, __LINE__, hwirq,
-		virq);
-
-	/* bind this virq to a cpu */
-
-	preempt_disable();
-	cpu = smp_processor_id();
-	result = lv1_connect_irq_plug(virq, hwirq);
-	preempt_enable();
+	/* Binds this virq to pd->cpu (current cpu) */
+
+	result = lv1_connect_irq_plug_ext(pd->node, pd->cpu, virq, hwirq, 0);
 
 	if (result) {
-		pr_info("%s:%d: lv1_connect_irq_plug failed:"
+		pr_info("%s:%d: lv1_connect_irq_plug_ext failed:"
 			" %s\n", __func__, __LINE__, ps3_result(result));
 		return -EPERM;
 	}
 
-	result = set_irq_chip_data(virq, &per_cpu(ps3_private, cpu));
+	result = set_irq_chip_data(virq, pd);
 	BUG_ON(result);
 
 	set_irq_chip_and_handler(virq, &irq_chip, handle_fasteoi_irq);
 
-	pr_debug(" <- %s:%d\n", __func__, __LINE__);
 	return result;
 }
 
@@ -544,12 +541,9 @@ unsigned int ps3_get_irq(void)
 void __init ps3_init_IRQ(void)
 {
 	int result;
-	unsigned long node;
 	unsigned cpu;
 	struct irq_host *host;
 
-	lv1_get_logical_ppe_id(&node);
-
 	host = irq_alloc_host(IRQ_HOST_MAP_NOMAP, 0, &ps3_host_ops,
 		PS3_INVALID_OUTLET);
 	irq_set_default_host(host);
@@ -558,12 +552,16 @@ void __init ps3_init_IRQ(void)
 	for_each_possible_cpu(cpu) {
 		struct ps3_private *pd = &per_cpu(ps3_private, cpu);
 
-		pd->node = node;
-		pd->cpu = cpu;
+		lv1_get_logical_ppe_id(&pd->node);
+		pd->cpu = get_hard_smp_processor_id(cpu);
 		spin_lock_init(&pd->bmp.lock);
 
-		result = lv1_configure_irq_state_bitmap(node, cpu,
-			ps3_mm_phys_to_lpar(__pa(&pd->bmp.status)));
+		pr_debug("%s:%d: node %lu, cpu %d, bmp %lxh\n", __func__,
+			__LINE__, pd->node, pd->cpu,
+			ps3_mm_phys_to_lpar(__pa(&pd->bmp)));
+
+		result = lv1_configure_irq_state_bitmap(pd->node, pd->cpu,
+			ps3_mm_phys_to_lpar(__pa(&pd->bmp)));
 
 		if (result)
 			pr_debug("%s:%d: lv1_configure_irq_state_bitmap failed:"
--- ps3-linux-dev.orig/arch/powerpc/platforms/ps3/setup.c
+++ ps3-linux-dev/arch/powerpc/platforms/ps3/setup.c
@@ -109,6 +109,7 @@ static int __init ps3_probe(void)
 		return 0;
 
 	powerpc_firmware_features |= FW_FEATURE_PS3_POSSIBLE;
+	cur_cpu_spec->cpu_features |= CPU_FTR_SMT;
 
 	ps3_os_area_init();
 	ps3_mm_init();

Re: [PATCH 6/14] ps3: smp interrupt fixes

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: 2007-01-25 03:51:48

On Wed, 2007-01-24 at 18:40 -0800, Geoff Levand wrote:
PS3 fixups for interrups on SMP.

Fixes the alignment of the interrupt status bitmap, changes the hypervisor
interrupt calls to the '_ext' versions that take an explicit processor
thread ID, and sets the cpu_features flag CPU_FTR_SMT.


Signed-off-by: Geoff Levand <redacted>
Looks good.

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
quoted hunk
---
 arch/powerpc/platforms/ps3/interrupt.c |   64 +++++++++++++++------------------
 arch/powerpc/platforms/ps3/setup.c     |    1 
 2 files changed, 32 insertions(+), 33 deletions(-)
--- ps3-linux-dev.orig/arch/powerpc/platforms/ps3/interrupt.c
+++ ps3-linux-dev/arch/powerpc/platforms/ps3/interrupt.c
@@ -317,22 +317,23 @@ struct ps3_bmp {
 		unsigned long unused_1[3];
 		unsigned long mask;
 		unsigned long unused_2[3];
-	} __attribute__ ((packed));
+	} __attribute__ ((aligned (64)));
+
 	spinlock_t lock;
 	unsigned long ipi_debug_brk_mask;
 };
 
 /**
  * struct ps3_private - a per cpu data structure
- * @node: HV node id
- * @cpu: HV thread id
- * @bmp: an HV ps3_bmp structure
+ * @bmp: ps3_bmp structure
+ * @node: HV logical_ppe_id
+ * @cpu: HV thread_id
  */
 
 struct ps3_private {
+	struct ps3_bmp bmp;
 	unsigned long node;
 	unsigned int cpu;
-	struct ps3_bmp bmp;
 };
 
 #if defined(DEBUG)
@@ -389,9 +390,8 @@ static void ps3_chip_mask(unsigned int v
 
 	spin_lock_irqsave(&pd->bmp.lock, flags);
 	pd->bmp.mask &= ~(0x8000000000000000UL >> virq);
-	spin_unlock_irqrestore(&pd->bmp.lock, flags);
-
 	lv1_did_update_interrupt_mask(pd->node, pd->cpu);
+	spin_unlock_irqrestore(&pd->bmp.lock, flags);
 }
 
 static void ps3_chip_unmask(unsigned int virq)
@@ -406,14 +406,14 @@ static void ps3_chip_unmask(unsigned int
 
 	spin_lock_irqsave(&pd->bmp.lock, flags);
 	pd->bmp.mask |= (0x8000000000000000UL >> virq);
-	spin_unlock_irqrestore(&pd->bmp.lock, flags);
-
 	lv1_did_update_interrupt_mask(pd->node, pd->cpu);
+	spin_unlock_irqrestore(&pd->bmp.lock, flags);
 }
 
 static void ps3_chip_eoi(unsigned int virq)
 {
-	lv1_end_of_interrupt(virq);
+	const struct ps3_private *pd = get_irq_chip_data(virq);
+	lv1_end_of_interrupt_ext(pd->node, pd->cpu, virq);
 }
 
 static struct irq_chip irq_chip = {
@@ -426,10 +426,12 @@ static struct irq_chip irq_chip = {
 static void ps3_host_unmap(struct irq_host *h, unsigned int virq)
 {
 	int result;
+	const struct ps3_private *pd = get_irq_chip_data(virq);
 
-	pr_debug("%s:%d: virq %d\n", __func__, __LINE__, virq);
+	pr_debug("%s:%d: node %lu, cpu %d, virq %u\n", __func__, __LINE__,
+		pd->node, pd->cpu, virq);
 
-	lv1_disconnect_irq_plug(virq);
+	lv1_disconnect_irq_plug_ext(pd->node, pd->cpu, virq);
 
 	result = set_irq_chip_data(virq, NULL);
 	BUG_ON(result);
@@ -441,31 +443,26 @@ static int ps3_host_map(struct irq_host 
 	irq_hw_number_t hwirq)
 {
 	int result;
-	unsigned int cpu;
+	struct ps3_private *pd = &__get_cpu_var(ps3_private);
+
+	pr_debug("%s:%d: node %lu, cpu %d, hwirq %lu => virq %u\n", __func__,
+		__LINE__, pd->node, pd->cpu, hwirq, virq);
 
-	pr_debug(" -> %s:%d\n", __func__, __LINE__);
-	pr_debug("%s:%d: hwirq %lu => virq %u\n", __func__, __LINE__, hwirq,
-		virq);
-
-	/* bind this virq to a cpu */
-
-	preempt_disable();
-	cpu = smp_processor_id();
-	result = lv1_connect_irq_plug(virq, hwirq);
-	preempt_enable();
+	/* Binds this virq to pd->cpu (current cpu) */
+
+	result = lv1_connect_irq_plug_ext(pd->node, pd->cpu, virq, hwirq, 0);
 
 	if (result) {
-		pr_info("%s:%d: lv1_connect_irq_plug failed:"
+		pr_info("%s:%d: lv1_connect_irq_plug_ext failed:"
 			" %s\n", __func__, __LINE__, ps3_result(result));
 		return -EPERM;
 	}
 
-	result = set_irq_chip_data(virq, &per_cpu(ps3_private, cpu));
+	result = set_irq_chip_data(virq, pd);
 	BUG_ON(result);
 
 	set_irq_chip_and_handler(virq, &irq_chip, handle_fasteoi_irq);
 
-	pr_debug(" <- %s:%d\n", __func__, __LINE__);
 	return result;
 }
 
@@ -544,12 +541,9 @@ unsigned int ps3_get_irq(void)
 void __init ps3_init_IRQ(void)
 {
 	int result;
-	unsigned long node;
 	unsigned cpu;
 	struct irq_host *host;
 
-	lv1_get_logical_ppe_id(&node);
-
 	host = irq_alloc_host(IRQ_HOST_MAP_NOMAP, 0, &ps3_host_ops,
 		PS3_INVALID_OUTLET);
 	irq_set_default_host(host);
@@ -558,12 +552,16 @@ void __init ps3_init_IRQ(void)
 	for_each_possible_cpu(cpu) {
 		struct ps3_private *pd = &per_cpu(ps3_private, cpu);
 
-		pd->node = node;
-		pd->cpu = cpu;
+		lv1_get_logical_ppe_id(&pd->node);
+		pd->cpu = get_hard_smp_processor_id(cpu);
 		spin_lock_init(&pd->bmp.lock);
 
-		result = lv1_configure_irq_state_bitmap(node, cpu,
-			ps3_mm_phys_to_lpar(__pa(&pd->bmp.status)));
+		pr_debug("%s:%d: node %lu, cpu %d, bmp %lxh\n", __func__,
+			__LINE__, pd->node, pd->cpu,
+			ps3_mm_phys_to_lpar(__pa(&pd->bmp)));
+
+		result = lv1_configure_irq_state_bitmap(pd->node, pd->cpu,
+			ps3_mm_phys_to_lpar(__pa(&pd->bmp)));
 
 		if (result)
 			pr_debug("%s:%d: lv1_configure_irq_state_bitmap failed:"
--- ps3-linux-dev.orig/arch/powerpc/platforms/ps3/setup.c
+++ ps3-linux-dev/arch/powerpc/platforms/ps3/setup.c
@@ -109,6 +109,7 @@ static int __init ps3_probe(void)
 		return 0;
 
 	powerpc_firmware_features |= FW_FEATURE_PS3_POSSIBLE;
+	cur_cpu_spec->cpu_features |= CPU_FTR_SMT;
 
 	ps3_os_area_init();
 	ps3_mm_init();
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 6/14] ps3: smp interrupt fixes

From: Arnd Bergmann <arnd@arndb.de>
Date: 2007-01-25 06:07:15

On Thursday 25 January 2007 03:40, Geoff Levand wrote:
quoted hunk
--- ps3-linux-dev.orig/arch/powerpc/platforms/ps3/interrupt.c
+++ ps3-linux-dev/arch/powerpc/platforms/ps3/interrupt.c
@@ -317,22 +317,23 @@ struct ps3_bmp {
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0unsigned long unused_1[3];
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0unsigned long mask;
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0unsigned long unused_2[3];
-=A0=A0=A0=A0=A0=A0=A0} __attribute__ ((packed));
+=A0=A0=A0=A0=A0=A0=A0} __attribute__ ((aligned (64)));
Is that meant to be a 64 _byte_ alignment? By default, it is
already aligned to 64 bit.
If 64 byte is some magic requirement by the hypervisor, you
might want to make that explicit by using a constant definition
for it.
quoted hunk
--- ps3-linux-dev.orig/arch/powerpc/platforms/ps3/setup.c
+++ ps3-linux-dev/arch/powerpc/platforms/ps3/setup.c
@@ -109,6 +109,7 @@ static int __init ps3_probe(void)
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return 0;
=A0
=A0=A0=A0=A0=A0=A0=A0=A0powerpc_firmware_features |=3D FW_FEATURE_PS3_POS=
SIBLE;
+=A0=A0=A0=A0=A0=A0=A0cur_cpu_spec->cpu_features |=3D CPU_FTR_SMT;
=A0
=A0=A0=A0=A0=A0=A0=A0=A0ps3_os_area_init();
=A0=A0=A0=A0=A0=A0=A0=A0ps3_mm_init();
=20
CPU_FTRS_CELL already contains CPU_FTR_SMT, why do you need to enable
it explicitly?

	Arnd <><

Re: [PATCH 6/14] ps3: smp interrupt fixes

From: Geoff Levand <hidden>
Date: 2007-01-26 02:47:43

Arnd Bergmann wrote:
On Thursday 25 January 2007 03:40, Geoff Levand wrote:
quoted
--- ps3-linux-dev.orig/arch/powerpc/platforms/ps3/interrupt.c
+++ ps3-linux-dev/arch/powerpc/platforms/ps3/interrupt.c
@@ -317,22 +317,23 @@ struct ps3_bmp {
=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=
=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BDu=
nsigned long unused_1[3];
quoted
=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=
=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BDu=
nsigned long mask;
quoted
=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=
=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BDu=
nsigned long unused_2[3];
quoted
-=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD} __at=
tribute__ ((packed));
quoted
+=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD} __at=
tribute__ ((aligned (64)));
=20
Is that meant to be a 64 _byte_ alignment? By default, it is
already aligned to 64 bit.
If 64 byte is some magic requirement by the hypervisor, you
might want to make that explicit by using a constant definition
for it.

The status + mask (512 bits) can't cross a page boundary, as required
by the HV.  That is the intent of the aligned (64) attribute.

Sorry, I don't quite understand what you mean by 'using a constant defini=
tion
for it'.  Could you elaborate?

As you suggested I have already changed this to:

struct ps3_bmp {
	struct {
		u64 status;
		u64 unused_1[3];
		u64 mask;
		u64 unused_2[3];
	};
	u64 ipi_debug_brk_mask;
	spinlock_t lock;
};

struct ps3_private {
	struct ps3_bmp bmp __attribute__ ((aligned (64)));
	u64 node;
	unsigned int cpu;
};

quoted
+=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BD=EF=BF=BDcur_cp=
u_spec->cpu_features |=3D CPU_FTR_SMT;
quoted
=20
CPU_FTRS_CELL already contains CPU_FTR_SMT, why do you need to enable
it explicitly?

Yes, I questioned whether that was needed or not.  I'll remove it.

-Geoff

Re: [PATCH 6/14] ps3: smp interrupt fixes

From: Arnd Bergmann <arnd@arndb.de>
Date: 2007-01-26 04:28:34

T24gRnJpZGF5IDI2IEphbnVhcnkgMjAwNyAwMzo0NywgR2VvZmYgTGV2YW5kIHdyb3RlOgo+IFNv
cnJ5LCBJIGRvbid0IHF1aXRlIHVuZGVyc3RhbmQgd2hhdCB5b3UgbWVhbiBieSAndXNpbmcgYSBj
b25zdGFudCBkZWZpbml0aW9uCj4gZm9yIGl0Jy4gwqBDb3VsZCB5b3UgZWxhYm9yYXRlPwoKSSBh
c3N1bWVkIHRoYXQgJzY0JyBoYWQgYSBzcGVjaWFsIG1lYW5pbmcgaW4gdGhpcywgd2hpY2ggd291
bGQgYmUgaWRlYWxseQpleHByZXNzZWQgYnkgc29tZXRoaW5nIGxpa2UKCiNkZWZpbmUgUFMzX0JJ
VE1BUF9NSU5BTElHTiA2NAoKd2hpY2ggaXQgb25seSBoYXMgaW5kaXJlY3RseSwgaW5zdGVhZAoK
PiBBcyB5b3Ugc3VnZ2VzdGVkIEkgaGF2ZSBhbHJlYWR5IGNoYW5nZWQgdGhpcyB0bzoKPiAKPiBz
dHJ1Y3QgcHMzX2JtcCB7Cj4gwqDCoMKgwqDCoMKgwqDCoHN0cnVjdCB7Cj4gwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqB1NjQgc3RhdHVzOwo+IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgdTY0IHVudXNlZF8xWzNdOwo+IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgdTY0IG1hc2s7Cj4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB1NjQgdW51c2Vk
XzJbM107Cj4gwqDCoMKgwqDCoMKgwqDCoH07Cj4gwqDCoMKgwqDCoMKgwqDCoHU2NCBpcGlfZGVi
dWdfYnJrX21hc2s7Cj4gwqDCoMKgwqDCoMKgwqDCoHNwaW5sb2NrX3QgbG9jazsKPiB9Owo+IAo+
IHN0cnVjdCBwczNfcHJpdmF0ZSB7Cj4gwqDCoMKgwqDCoMKgwqDCoHN0cnVjdCBwczNfYm1wIGJt
cCBfX2F0dHJpYnV0ZV9fICgoYWxpZ25lZCAoNjQpKSk7Cj4gwqDCoMKgwqDCoMKgwqDCoHU2NCBu
b2RlOwo+IMKgwqDCoMKgwqDCoMKgwqB1bnNpZ25lZCBpbnQgY3B1Owo+IH07CgppdCB3b3VsZCBi
ZSBldmVuIGJldHRlciB0byBhZGQgYSBjb21tZW50IHRvIHRoaXMgc3RydWN0IGxpa2UKCi8qIGFs
aWduZWQoNjQpIGJlY2F1c2UgcHMzX2JtcCBtdXN0IG5vdCBjcm9zcyBwYWdlIGJvdW5kYXJpZXMg
Ki8KCglBcm5kIDw+PAo=
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help