Re: [patch 08/18] PS3: Kexec support

Michael Ellerman wrote:

On Tue, 2007-06-05 at 20:00 -0700, Geoff Levand wrote:

quoted

Fixup the core platform parts needed for kexec to work on the PS3.
 - Setup ps3_hpte_clear correctly.
 - Mask interrupts on irq removal.
 - Release all hypervisor resources.

The irq changes might be kexec related, but it's a mess to review. You
seem to moving a bunch of code around in the patch as well.

Yes, I need to move the static chip_mask routines up so they would be
defined before the irq setup/destroy routines.

quoted

Signed-off-by: Geoff Levand <redacted>
---
 arch/powerpc/platforms/ps3/htab.c      |   14 +-
 arch/powerpc/platforms/ps3/interrupt.c |  199 ++++++++++++++++++++-------------
 arch/powerpc/platforms/ps3/setup.c     |   29 ++--
 3 files changed, 147 insertions(+), 95 deletions(-)

--- a/arch/powerpc/platforms/ps3/htab.c
+++ b/arch/powerpc/platforms/ps3/htab.c

@@ -234,10 +234,18 @@ static void ps3_hpte_invalidate(unsigned
 
 static void ps3_hpte_clear(void)
 {
-	/* Make sure to clean up the frame buffer device first */
-	ps3fb_cleanup();
+	int result;
 
-	lv1_unmap_htab(htab_addr);
+	DBG(" -> %s:%d\n", __func__, __LINE__);
+
+	result = lv1_unmap_htab(htab_addr);
+	BUG_ON(result);
+
+	ps3_mm_shutdown();
+
+	ps3_mm_vas_destroy();
+
+	DBG(" <- %s:%d\n", __func__, __LINE__);
 }

Do you really want to be calling DBG() here? Hmm, it looks like it
doesn't actually do anything?

Sure, it uses udbg_printf, and works OK.

quoted

+static void ps3_chip_mask(unsigned int virq)
+{
+	struct ps3_private *pd = get_irq_chip_data(virq);
+	u64 bit = 0x8000000000000000UL >> virq;
+	u64 *p = &pd->bmp.mask;
+	u64 old;
+	unsigned long flags;
+
+	pr_debug("%s:%d: cpu %u, virq %d\n", __func__, __LINE__, pd->cpu, virq);
+
+	local_irq_save(flags);
+	asm volatile(
+		     "1:	ldarx %0,0,%3\n"
+		     "andc	%0,%0,%2\n"
+		     "stdcx.	%0,0,%3\n"
+		     "bne-	1b"
+		     : "=&r" (old), "+m" (*p)
+		     : "r" (bit), "r" (p)
+		     : "cc" );
+
+	lv1_did_update_interrupt_mask(pd->node, pd->cpu);
+	local_irq_restore(flags);

How is this different from set_bit() ?  (asm-powerpc/bitops.h)

ps. now that I see you're just moving this code around someone's
probably already asked that question.

This was contributed by Ben H as the fastest way.  I think the
reason was that we could minimize the time between local_irq_save
and local_irq_restore?

quoted

 static void ps3_machine_kexec(struct kimage *image)
 {
-	unsigned long ppe_id;
-
 	DBG(" -> %s:%d\n", __func__, __LINE__);
 
-	lv1_get_logical_ppe_id(&ppe_id);
-	lv1_configure_irq_state_bitmap(ppe_id, 0, 0);
-	ps3_mm_shutdown();
-	ps3_mm_vas_destroy();
-
-	default_machine_kexec(image);
+	default_machine_kexec(image); // needs ipi, never returns.

Just get rid of ps3_machine_kexec() and hook default_machine_kexec()
directly into your ppc_md.

Right, just a debugging leftover...


-Geoff