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,[EMAIL PROTECTED] /* 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
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
