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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/2/06, Eric W. Biederman <[email protected]> wrote:
"Magnus Damm" <[email protected]> writes:

> On 5/2/06, Eric W. Biederman <[email protected]> wrote:
>>
>> 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.

Well I think it would be fairly simple to have a structure:
struct control_page {
       type     variabe;
       ...
       code[0];
};

Or something like that we can work with.

The big reason for doing this is that I believe control pages
have additional protection that struct kimage does, being allocated
in areas where the kernel never sets up DMA transfers.  Possibly
that needs to be fixed, but this is something we need to be very
careful with.

I suppose you mean that control pages have additional protection that
struct kimage does _not_ have. Protection provided by
kimage_alloc_control_pages(), right?

I agree with you that this protection is good. But I do not see how
that applies to my patch, because the page_table_a[] pages pointed out
by struct kimage are read out by machine_kexec() and passed as
arguments to the assembly code. So the assembly code itself never
tries to access struct kimage. All data accessed by the assembly code
is allocated with kimage_alloc_control_pages(), isn't that good
enough? Or maybe I'm misunderstanding?

On top of that I fear that my patch likes the fact that the assembly
code in the control page is page aligned. But it is probably no biggie
to change. I'm just lazy. =)

For a page table all we need to store is the physical address of the
first page.  Storing and working with a struct page entry is the wrong
thing to do.  I would prefer to stomp the kernel data structures than
to add an extra dependencies on the original panic'd kernel.  At first
glance I am afraid that you current code introduces extra
dependencies.

I used struct page *[] because the control page was a struct page *. I
never use the contents of what the struct page points to, I only use
them to convert to physical/virtual addresses or pfns. So I would say
that no extra dependencies are introduced at all actually. But I may
be wrong.

I would be happy to change the struct page *[] to unsigned long [] or
something else, but I must say I like the typing that struct page
provides.

Regarding storing the just root page or all pages - I stored all pages
because I need to pass them to the Xen hypervisor which will fill in
new values in page_table_a. page_table_b OTOH never gets modified by
the hypervisor, which is why page_table_a is an array of pointers and
page_table_b is just a root pointer.

You don't need two x86_64 page tables as you can easily map
all of the kernel virtual address, and the identity mapped physical
address until the x86_64 kernel stops using an 8TB/8TB split.

Sure. I just thought that one page table with a mix of 4K pages and
huge pages would result in difficult code. The current page_table_a
code is actually more or less the same on x86 and x86_64, with the
exception of some macro magic.

Thanks for the detailed reply!

/ magnus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux