Re: [MINI-OS PATCH 01/12] kexec: add kexec framework

[Jürgen Groß <jgross@xxxxxxxx>]

Jason,

thanks for having a look at the series! I very much appreciate that!

On 14.06.25 18:40, Jason Andryuk wrote:
> On Fri, Mar 21, 2025 at 5:25 AM Juergen Gross <jgross@xxxxxxxx> wrote:
> > Add a new config option CONFIG_KEXEC for support of kexec-ing into a
> > new mini-os kernel. Add a related kexec.c source and a kexec.h header.
> >
> > For now allow CONFIG_KEXEC to be set only for PVH variant of mini-os.
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
>
> > diff --git a/arch/x86/testbuild/all-yes b/arch/x86/testbuild/all-yes
> > index 8ae489a4..99ba75dd 100644
> > --- a/arch/x86/testbuild/all-yes
> > +++ b/arch/x86/testbuild/all-yes
> > @@ -19,3 +19,5 @@ CONFIG_BALLOON = y
> >   CONFIG_USE_XEN_CONSOLE = y
> >   # The following are special: they need support from outside
> >   CONFIG_LWIP = n
> > +# KEXEC only without PARAVIRT
> Maybe: "KEXEC not implemented for PARAVIRT"?
Fine with me.

> > +CONFIG_KEXEC = n
> > diff --git a/kexec.c b/kexec.c
> > new file mode 100644
> > index 00000000..53528169
> > --- /dev/null
> > +++ b/kexec.c
> > @@ -0,0 +1,62 @@
> > +
> > +#include <errno.h>
> > +#include <mini-os/os.h>
> > +#include <mini-os/lib.h>
> > +#include <mini-os/kexec.h>
> > +
> > +/*
> > + * General approach for kexec support (PVH only) is as follows:
> > + *
> > + * - New kernel needs to be in memory in form of a ELF file in a virtual
> "in the form of an ELF binary"
>
> > + *   memory region.
> Maybe just "The new kernel needs to be an ELF binary loaded into the
> Mini-OS address space"?
The "virtual memory region" is quite important, as this allows to handle
conflicts with the target memory layout on a per-page basis.

> > + * - A new start_info structure is constructed in memory with the final
> > + *   memory locations included.
> > + * - All memory areas needed for kexec execution are being finalized.
> > + * - From here on a graceful failure is no longer possible.
> > + * - Grants and event channels are torn down.
> > + * - A temporary set of page tables is constructed at a location where it
> > + *   doesn't conflict with old and new kernel or start_info.
> > + * - The final kexec execution stage is copied to a memory area below 4G which
> > + *   doesn't conflict with the target areas of kernel etc.
> > + * - Cr3 is switched to the new set of page tables.
> > + * - Execution continues in the final execution stage.
> > + * - All data is copied to its final addresses.
> > + * - Processing is switched to 32-bit mode without address translation.
> Maybe "CPU is switched to 32-bit mode with paging disabled."?
Okay.

> Is the following memory layout correct?
>
> [ 0 ... 8MB ] ... [ X ... X + Y ] ... [ Z ...      ]
>   Old stubdom        New stubdom         kexec code
With:
O: old stubdom kernel
P: active page tables
N: new stubdom kernel
Z: kexec code.

The guest physical memory layout is more like:
OPOOONP.NN.N.NNN..ZNN..PP..

The target layout of this example (before the final kexec stage) will be:
O.OOO....N.N.NNNP.ZNNP.PPNN

Note that all conflicting N and P entries have been moved to a position
behind the target position of the new kernel. This includes the page
tables in the old kernel which were pre-populated at boot time.

And before passing control to the new kernel it will be:
NNNNNNNNN.........Z........

> kexec code copies New stubdom to 0 and later jumps to New stubdom @ 0
Kind of. The "0" is not hard wired in the kexec code.

> The temporary page tables are to allow old stubdom and kexec code to
> be called while overwriting the "Old stubdom" range which would
> include the page tables originally used?  Or it can only run the kexec
> code once old stubdom is overwritten, right?
Yes.

I just realized that some of the comments are stale now. The current
implementation doesn't setup a new set of page tables, but is tweaking
the existing one to avoid conflicts.

> I think some comments tweaks would be helpful, but code-wise
> everything is okay, so:
>
> Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>
Thanks,


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature