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 _______________________________________________ fastboot mailing list [email protected] https://lists.osdl.org/mailman/listinfo/fastboot
