Re: [PATCH 3/3] powerpc/ppc64/kdump: better flag for running relocatable

[Michael Neuling <mikey@neuling.org>]

In message <kexec-kern-2@bga.com> you wrote:
> The __kdump_flag ABI is overly constraining for future development.  
> 
> As of 2.6.27, the kernel entry point has 4 constraints:  Offset 0 is
> the starting point for the master (boot) cpu (entered with r3 pointing
> to the device tree structure), offset 0x60 is code for the slave cpus
> (entered with r3 set to their device tree physical id), offset 0x20 is
> used by the iseries hypervisor, and secondary cpus must be well behaved
> when the first 256 bytes are copied to address 0.
> 
> Placing the __kdump_flag at 0x18 is bad because:
> 
> - It was taking the last 8 bytes before the iseries hypervisor data.  
> - It was 8 bytes for a boolean flag
> - It had no way of identifying that the flag was present
> - It does leave any room for the master to add any additional code
>   before branching, which hurts debug.
> - It will be unnecessarily hard for 32 bit code to be common (8 bytes)
> 
> Now that we have eliminated the use of __kdump_flag in favor of
> the standard is_kdump_kernel(), this flag only controls run without
> relocating the kernel to PHYSICAL_START (0), so rename it __run_at_load.
> 
> Move the flag to 0x5c, 1 word before the secondary cpu entry point at
> 0x60.  Use the copy at address 0 not the one in the base kernel image to
> make it easier on kexec-tools.  Initialize it with "run0" to say it will
> run at 0 unless it is set to 1.  It only exists if we are relocatable.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>
> ---
> I left it global so it appears that way in System.map, but it would
> not need to be.
> 
> I kept the guards with CONFIG_CRASH_DUMP for now.  They could be relaxed
> to just CONFIG_RELOCATABLE.
> 
> Tested with normal kexec (kernel moved to 0) and a custom boot-loader
> (kernel stayed at loaded 16MB start).
> 
> Index: next.git/arch/powerpc/kernel/head_64.S
> ===================================================================
> --- next.git.orig/arch/powerpc/kernel/head_64.S	2008-10-22 04:30:08.000
000000 -0500
> +++ next.git/arch/powerpc/kernel/head_64.S	2008-10-22 04:59:55.000000000 -
0500
> @@ -97,12 +97,6 @@ __secondary_hold_spinloop:
>  __secondary_hold_acknowledge:
>  	.llong	0x0
>  
> -	/* This flag is set by purgatory if we should be a kdump kernel. */
> -	/* Do not move this variable as purgatory knows about it. */
> -	.globl	__kdump_flag
> -__kdump_flag:
> -	.llong	0x0
> -
>  #ifdef CONFIG_PPC_ISERIES
>  	/*
>  	 * At offset 0x20, there is a pointer to iSeries LPAR data.
> @@ -112,6 +106,20 @@ __kdump_flag:
>  	.llong hvReleaseData-KERNELBASE
>  #endif /* CONFIG_PPC_ISERIES */
>  
> +#ifdef CONFIG_CRASH_DUMP
> +	/* This flag is set to 1 by a loader if the kernel should run
> +	 * at the loaded address instead of the linked address.  This
> +	 * is used by kexec-tools to keep the the kdump kernel in the
> +	 * crash_kernel region.  The loader is responsible for
> +	 * observing the alignment requirement.
> +	 */
> +	/* Do not move this variable as kexec-tools knows about it. */
> +	. = 0x5c
> +	.globl	__run_at_load
> +__run_at_load:
> +	.long	0x72756e30	/* "run0" -- relocate to 0 by default */
> +#endif
> +
>  	. = 0x60
>  /*
>   * The following code is used to hold secondary processors
> @@ -1391,8 +1399,8 @@ _STATIC(__after_prom_start)
>  	lis	r25,PAGE_OFFSET@highest	/* compute virtual base of kernel */
>  	sldi	r25,r25,32
>  #ifdef CONFIG_CRASH_DUMP
> -	ld	r7,__kdump_flag-_stext(r26)
> -	cmpldi	cr0,r7,1	/* kdump kernel ? - stay where we are */
> +	lwz	r7,__run_at_load-_stext(0)
> +	cmplwi	cr0,r7,1	/* kdump kernel ? - stay where we are */

Do we really want the flag to always be at 0x5c not 0x5c + kernel offset?

Also, comment "kdump kernel" needs to be updated to reflect the new
name.  

Other than that, the patch series works for me.

Mikey

>  	bne	1f
>  	add	r25,r25,r26
>  #endif
> @@ -1416,11 +1424,11 @@ _STATIC(__after_prom_start)
>  #ifdef CONFIG_CRASH_DUMP
>  /*
>   * Check if the kernel has to be running as relocatable kernel based on the
> - * variable __kdump_flag, if it is set the kernel is treated as relocatable
> + * variable __run_at_load, if it is set the kernel is treated as relocatable
>   * kernel, otherwise it will be moved to PHYSICAL_START
>   */
> -	ld	r7,__kdump_flag-_stext(r26)
> -	cmpldi	cr0,r7,1
> +	lwz	r7,__run_at_load-_stext(0)
> +	cmplwi	cr0,r7,1
>  	bne	3f
>  
>  	li	r5,__end_interrupts - _stext	/* just copy interrupts */
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>