On Mon, 2008-10-20 at 15:04 +0530, Mohan Kumar M wrote:
> Michael Ellerman wrote:
> >>> #ifdef CONFIG_CRASH_DUMP
> >>> +#ifdef CONFIG_RELOCATABLE
> >>> +
> >>> +static inline void reserve_kdump_trampoline(void) { ; }
> >>> +static inline void setup_kdump_trampoline(void) { ; }
> >>> +
> >>> +#else
> >>>
> >>> extern void reserve_kdump_trampoline(void);
> >>> extern void setup_kdump_trampoline(void);
> >>>
> >>> +#endif /* CONFIG_RELOCATABLE */
> >
> > You've disabled the else case with your Kconfig changes, so you should
> > just rip all that code out.
>
> I made Kconfig changes only to the 64 bit powerpc path and still the 32
> bit powerpc code uses the legacy kdump code. So we need to retain some
> of legacy kdump code.
Does it? I see CONFIG_CRASH_DUMP depending on PPC64, so there is no
32-bit kdump possible. Or is someone working on it out-of-tree?
> >>> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> >>> index e409338..5b12b10 100644
> >>> --- a/arch/powerpc/kernel/head_64.S
> >>> +++ b/arch/powerpc/kernel/head_64.S
> >>> @@ -97,6 +97,14 @@ __secondary_hold_spinloop:
> >>> __secondary_hold_acknowledge:
> >>> .llong 0x0
> >>>
> >>> + /* This flag is set only for kdump kernels so that */
> >>> + /* it will be relocatable. Purgatory code user space kexec-tools */
> >>> + /* sets this flag. Do not move this variable as purgatory code */
> >>> + /* relies on the position of this variables */
> >>> + .globl __kdump_flag
> >>> +__kdump_flag:
> >>> + .llong 0x0
> >
> > I guess the __ matches the other flags here, it's not the prettiest
> > though. For client code (like in iommu.c) it'd be nice to have static
> > inline, perhaps is_kdump_kernel() that hides this.
> >
> Do you expect a function to do the checking in iommu.c?
You'd use the function in iommu.c, but it should be defined in some
header.
> >>> #ifdef CONFIG_PPC_ISERIES
> >>> /*
> >>> * At offset 0x20, there is a pointer to iSeries LPAR data.
> >>> @@ -1384,8 +1392,13 @@ _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
> >>> +#ifdef CONFIG_CRASH_DUMP
> >>> + ld r7,__kdump_flag-_stext(r26)
> >>> + cmpldi cr0,r7,1 /* relocatable kernel ? */
> >
> > You don't use the signature here?
>
> kexec-tools check the signature and based on the signature it sets
> __kdump_flag to 1 (or 0). So kernel code just checks whether its set or not.
OK. Does old purgatory ensure that the register is 0? Otherwise I think
it's possible that a new kernel could get confused by cruft left in that
register by an old purgatory - causing the 2nd kernel to think it's a
kdump kernel when it shouldn't be.
> >>> @@ -1401,9 +1414,21 @@ _STATIC(__after_prom_start)
> >>> beq 9f /* have already put us at zero */
> >>> li r6,0x100 /* Start offset, the first 0x100 */
> >>> /* bytes were copied earlier. */
> >>> -#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-_stext(r26)
> >>> + cmpldi cr0,r7,1
> >>> + bne regular
> >>> +
> >>> li r5,__end_interrupts - _stext /* just copy interrupts */
> >>> -#else
> >>> + b 5f
> >>> +regular:
> >>> +#endif
> >>> lis r5,(copy_to_here - _stext)@ha
> >>> addi r5,r5,(copy_to_here - _stext)@l /* # bytes of memory to copy */
> >
> > I'm jet lagged to hell, so I'm not sure I can trust my parsing of this.
> > But I think this definitely breaks CONFIG_RELOCATABLE without
> > CRASH_DUMP, and I'm not sure it's right otherwise.
> >
> Hmmm, I compiled and tried the kernel with 3 config option combinations:
> 1. CONFIG_RELOCATABLE and CONFIG_CRASH_DUMP 2. CONFIG_RELOCATABLE 3.
> Without CONFIG_RELOCATABLE (without CONFIG_CRASH_DUMP)
>
> All of the above 3 combinations worked. This patch relies on Pauls'
> patch5 in the relocatable kernel patcheset.
OK if you've tested.
cheers
--
Michael Ellerman
OzLabs, IBM Australia Development Lab
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.infradead.org/pipermail/kexec/attachments/20081021/7d3a1373/attachment.bin
