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,PAGE_OFFSET at highest /* compute virtual base of kernel */ > sldi r25,r25,32 > - mr r3,r25 > +#ifdef CONFIG_CRASH_DUMP > + ld r7,__kdump_flag at got(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,__kdump_flag at got(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.
