Mohan Kumar M writes:
> Support for relocatable kdump kernel
[snip]
> @@ -1384,7 +1392,15 @@ _STATIC(__after_prom_start)
> /* process relocations for the final address of the kernel */
> lis r25,[EMAIL PROTECTED] /* compute virtual base of kernel */
> sldi r25,r25,32
> - mr r3,r25
> +#ifdef CONFIG_CRASH_DUMP
> + ld r7,[EMAIL PROTECTED](r2)
> + add r7,r7,r26
> + ld r7,0(r7)
I think it is dangerous to use an address from the GOT at this point
when we haven't called relocate() yet. In fact those 3 instructions
can be replaced by one:
ld r7,__kdump_flag-_stext(r26)
since we have our base address (i.e. the address of _stext) in r26 at
this point.
> +#ifdef CONFIG_RELOCATABLE
> +#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 relocatble
> + * kernel, otherwise it will be moved to PHYSICAL_START
> + */
> + ld r7,[EMAIL PROTECTED](r2)
> + ld r7,0(r7)
Here again I would rather you did
ld r7,__kdump_flag-_stext(r26)
since the kernel is relocated for its final location by this point,
but it is not running at the final location yet.
> + cmpldi cr0,r7,1
> + bne regular
> +
> + li r5,__end_interrupts - _stext /* just copy interrupts */
> + b 5f
> +regular:
> +#endif
> + lis r5,(copy_to_here - _stext)@ha
> + addi r5,r5,(copy_to_here - _stext)@l /* # bytes of memory to copy */
>
> bl .copy_and_flush /* copy the first n bytes */
> /* this includes the code being */
> @@ -1411,15 +1443,33 @@ _STATIC(__after_prom_start)
> mtctr r8
> bctr
>
> +p_end: .llong _end - _stext
> +
> 4: /* Now copy the rest of the kernel up to _end */
> addis r5,r26,(p_end - _stext)@ha
> ld r5,(p_end - _stext)@l(r5) /* get _end */
> - bl .copy_and_flush /* copy the rest */
> +#else
> + lis r5,(copy_to_here - _stext)@ha
> + addi r5,r5,(copy_to_here - _stext)@l /* # bytes of memory to copy */
>
> -9: b .start_here_multiplatform
> + bl .copy_and_flush /* copy the first n bytes */
> + /* this includes the code being */
> + /* executed here. */
> + addis r8,r3,(4f - _stext)@ha /* Jump to the copy of this code */
> + addi r8,r8,(4f - _stext)@l /* that we just made */
> + mtctr r8
> + bctr
>
> p_end: .llong _end - _stext
>
> +4: /* Now copy the rest of the kernel up to _end */
> + addis r5,r26,(p_end - _stext)@ha
> + ld r5,(p_end - _stext)@l(r5) /* get _end */
> +#endif
> +5: bl .copy_and_flush /* copy the rest */
> +
> +9: b .start_here_multiplatform
You have ended up with two separate copies of the code here depending
on whether or not we have CONFIG_RELOCATABLE set. I don't think the
code paths should be different to such an extent. Please try to make
the ifdef as small as possible (ideally, nonexistent).
Paul.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
