Re: [PATCH -mm 2/2] kexec/i386: kexec page table code clean up - page table setup in C

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2008-01-10 at 16:21 +0900, Simon Horman wrote: 
> [ CCing Ian Campbell who handles much of the maintenance of kexec in Xen ]
> 
> On Thu, Jan 10, 2008 at 10:08:09AM +0800, Huang, Ying wrote:
> > On Wed, 2008-01-09 at 20:05 -0500, Vivek Goyal wrote:
> > > On Wed, Jan 09, 2008 at 10:57:50AM +0800, Huang, Ying wrote:
> > > > This patch transforms the kexec page tables setup code from asseumbler
> > > > code to iC code in machine_kexec_prepare. This improves readability and
> > > > reduces code line number.
> > > > 
> > > 
> > > I think this will create issues for Xen. Initially page table setup
> > > was in C but Xen Guests could not modify the page tables. I think Xen
> > > folks implemented a hypercall where they passed all the page table pages
> > > and the control pages and then hypervisor executed the control page(which
> > > in turn setup the page tables). I think that's why page table setup
> > > code is on the control page in assembly.
> > > 
> > > You might want to go through Xen kexec implementation and dig through
> > > kexec mailing list archive.
> > 
> > OK, I will check the Xen kexec implementation.
> > 
> > > CCing Magnus and Horms. They had done the page tables related changes
> > > for Xen.
> 
> I think that potentially there is a problem here, though weather or
> not it manifests is another question.
> 
> In machine_kexec() a PAGE_SIZE blob of code starting at relocate_kernel
> gets saved. I think that previously x86 Linux excuded this saved blob,
> though the current code seems to execute relocate_kernel() directly -
> then again perhaps I'm confusing things with ia64 which still executes
> the saved blob.
> 
> In any case, in the case of xen, the hypervisor will execute this saved
> blob. So I think that the crux of the issue is that the blob contains
> all the instructions required to run relocate_kernel, then it should
> work from inside xen, if and if it doesn't, then i'll blow up.
> 
> The code that you have seems to satisfy this requirement,
> so relocate_kernel itself shouldn't be a problem. And as
> long as the preparation work does exacltly what the removed
> portions of relocate_kernel used to do, which it seems to,
> things should be fine.
> 
> That said, this certainly warrants testing :-)

In the absence of domain 0 or Xen kexec in the upstream kernel testing
might be quite hard.

> I'm all in favour of this kind of consolodation,
> so hopefully we can bend Xen's will if it doesn't work as is.

Indeed, patch seem reasonable to me. Whatever issues this causes for Xen
will be stuff that can be fixed once someone does the kexec for Xen work
in the upstream kernel, I don't think it paints us into an inescapable
corner.

Ian.