Re: [Fastboot] [PATCH] kexec: Avoid overwriting the current pgd (i386)

["Magnus Damm" <magnus.damm@gmail.com>]

On 5/2/06, Eric W. Biederman <ebiederm@xmission.com> wrote:
> Vivek Goyal <vgoyal@in.ibm.com> writes:
> > On Mon, May 01, 2006 at 06:49:16PM +0900, Magnus Damm wrote:
> >> --- 0001/include/linux/kexec.h
> >> +++ work/include/linux/kexec.h       2006-05-01 11:13:14.000000000 +0900
> >> @@ -69,6 +69,17 @@ struct kimage {
> >>      unsigned long start;
> >>      struct page *control_code_page;
> >>
> >> +    /* page_table_a[] holds enough pages to create a new page table
> >> +     * that maps the control page twice..
> >> +     */
> >> +
> >> +#if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE)
> >> +    struct page *page_table_a[3]; /* (2 * pte) + pgd */
> >> +#endif
> >> +#if defined(CONFIG_X86_32) && defined(CONFIG_X86_PAE)
> >> +    struct page *page_table_a[5]; /* (2 * pte) + (2 * pmd) + pgd */
> >> +#endif
> >> +
> >
> > Would it make a cleaner interface if these array of pointers are maintained
> > in arch dependent code as global variables instead of putting in arch
> > independent code. Existing code does something simlar. This becomes further
> > ugly when another array comes into the picture for x86_64 in next patch.
> > (page_table_b)
>
> Well global variables don't quite work in the normal case.
>
> However it probably makes most sense to maintain the needed variables
> in a structure on the control page.  Which will keep them out of harms way,
> and won't require patches to the generic code.

I agree with both of you that the #ifdefs in struct kimage should be
avoided, but I wonder if adding variables in a structure on the
control page is the easiest and cleanest way.

I think that defining a structure for each architecture in
include/asm/kexec.h that is included in struct kimage is the best way
to go. Then each architecture can have whatever data it wants there,
and we both avoid #ifdefs in struct kimage _and_ we stay away from
overly complicated code that just allocates, frees and parses
architecture-dependent data.

Thanks for the input,

/ magnus