Re: pSeries_mach_cpu_die() question

[Nathan Lynch <ntl@pobox.com>]

Benjamin Herrenschmidt wrote:
> While doing some autumn cleaning of the irq stuff in general and xics
> specifically, I found out that
> the low level pSeriesLP_cppr_info() is exported because
> pSeries_mach_cpu_die() calls it:
> 
> static void pSeries_mach_cpu_die(void)
> {
> 	local_irq_disable();
> 	idle_task_exit();
> 	/* Some hardware requires clearing the CPPR, while other hardware does
> not
> 	 * it is safe either way
> 	 */
> 	pSeriesLP_cppr_info(0, 0);
> 	rtas_stop_self();
> 	/* Should never get here... */
> 	BUG();
> 	for(;;);
> }
> 
> This leads to a few questions:
> 
>  - We always pass "0" as the CPU. Is that right ? I seems not, but maybe
> pHyp doesn't care and always assume the calling CPU ...

The cpu parameter is actually unused by in the lpar case:

void pSeriesLP_cppr_info(int n_cpu, u8 value)
{
	unsigned long lpar_rc;

	lpar_rc = plpar_cppr(value);
	if (lpar_rc != H_SUCCESS)
		panic("bad return code cppr - rc = %lx\n", lpar_rc);
}


>  - xics has a xics_teardown_cpu() now, used by kexec, that does
> something very similar except that it passes the proper CPU number, and
> for secondary CPUs also does an EOI of any pending IPI (just in case). I
> think that could be used instead of the direct call to the low level
> pSeriesLP_* funciton (which I itend to unexport and rename anyway as
> part of my rework). Can whoever knows that code confirm ?

Sounds okay to me.


>  - There is a comment about "some hardware....", what does it mean ? Is
> it ok to do it unconditionally ? I suppose so but heh...

The comment should be changed or removed really.  We got away without
doing plpar_cppr() on the Power4 hypervisor but we found out it was
necessary when testing Power5.  I think it's required by the
architecture regardless, and yes, it's safe on both platforms.