Re: [MINI-OS PATCH 12/19] mini-os: kexec: add support for handing over some memory across kexec

[Juergen Gross <jgross@xxxxxxxx>]

On 14.07.25 23:22, Jason Andryuk wrote:
> On 2025-07-11 03:49, Jürgen Groß wrote:
> > On 11.07.25 01:21, Jason Andryuk wrote:
> > > On 2025-07-02 04:12, Juergen Gross wrote:
> > > > Especially for support of Xenstore-stubdom live update some memory must
> > > > be handed over to the new kernel without moving it around: as the
> > > > 9pfs device used for storing and retrieving the state of Xenstore
> > > > needs to be kept operational across kexec (it can't be reopened due to
> > > > Xenstore not being available without access to the device), the ring
> > > > pages need to be accessible via active grants by the backend all the
> > > > time.
> > > >
> > > > Add the basic support for that by reserving a pre-defined number of
> > > > memory pages at the top of the memory. This memory area will be
> > > > handed over to the new kernel via specifying it as a module in
> > > > struct hvm_start_info.
> > > >
> > > > The contents of the memory area are described via a generic table of
> > > > contents in the last page of the memory.
> > > >
> > > > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > > > diff --git a/Config.mk b/Config.mk
> > > > index b9675e61..0e4e86d8 100644
> > > > --- a/Config.mk
> > > > +++ b/Config.mk
> > > > @@ -220,6 +220,8 @@ CONFIG-$(lwip) += CONFIG_LWIP
> > > >   $(foreach i,$(CONFIG-y),$(eval $(i) ?= y))
> > > >   $(foreach i,$(CONFIG-n),$(eval $(i) ?= n))
> > > > +CONFIG-val-$(CONFIG_KEXEC) += CONFIG_KEXEC_MODULE_PAGES
> > > > +
> > > I don't know Makefiles well enough to review the preceding patch. This 
> > > doesn't seem to be used?
> > It is.
> Oh, so setting CONFIG-val-y, through the Makefile magic, creates
> -DCONFIG_KEXEC_MODULE_PAGES=$n
>
> I said I didn't know Makefiles well :)
>
> > > >   $(foreach i,$(CONFIG-val-y),$(eval $(i) ?= 0))
> > > >   CONFIG-x += CONFIG_LIBXS
> > > > +    module_ptr = (void *)(mod + mod_size - (unsigned long)PAGE_SIZE);
> > > > +
> > > > +    /* Check eye catcher. */
> > > > +    if ( memcmp(module_ptr->eye_catcher, KEXECMOD_EYECATCHER,
> > > > +                sizeof(module_ptr->eye_catcher)) )
> > > > +        return NULL;
> > > > +    if ( module_ptr->n_pages != (mod_size >> PAGE_SHIFT) - 1 )
> > > > +        return NULL;
> > > > +
> > > > +    return module_ptr;
> > > > +}
> > > > +#define min(a, b) ((a) < (b) ? (a) : (b))
> > > > +void kexec_module(unsigned long start_pfn, unsigned long max_pfn)
> > > > +{
> > > > +    /* Reuse already existing kexec module. */
> > > > +    mod_ptr = kexec_check_module();
> > > > +    if ( !mod_ptr && CONFIG_KEXEC_MODULE_PAGES )
> > > What if CONFIG_KEXEC_MODULE_PAGES changes between the old and the new stubdom?
> > Right now this wouldn't really matter. The CONFIG value is used
> > only for sizing the module if we are not started via kexec().
> When I wrote this I was thinking of:
> A -- kexec --> B -- kexec --> C
>
> A: CONFIG_KEXEC_MODULE_PAGES=4
> B: CONFIG_KEXEC_MODULE_PAGES=6
>
> When B is running, it has the 4 pages from A, but it expects 6 available for its 
> use....
Depends...

Looking at the first user (xenstore-stubdom), things are a little bit more
complicated.

All the allocated pages (so everything apart from the last page containing the
records) are used for the 9pfs device shared pages. This number will only
change, if the ring buffer size is changed (lets skip the case where another
use case of a kexec module page is coming up). As the device is being kept open
across kexec(), the size of the ring buffer will be kept, so there is no reason
to use more kexec module pages. This is working fine, as all the relevant
parameters of the 9pfs device are taken from the kexec 9pfs record or the
shared pages themselves after kexec(), instead of using the builtin defaults.

This could change only, if we'd add support to reconnect the 9pfs device after
kexec() using a different ring buffer size (and/or maybe multiple rings). In
this case we'd need to implement the capability to resize the kexec module,
which is possible with the current design, but not needed yet.

> > > > diff --git a/include/kexec.h b/include/kexec.h
> > > > index b89c3000..0200005f 100644
> > > > --- a/include/kexec.h
> > > > +++ b/include/kexec.h
> > > > @@ -2,6 +2,48 @@
> > > >   #define _KEXEC_H
> > > >   #include <mini-os/elf.h>
> > > > +/*
> > > > + * Kexec module used to hand over memory across kexec().
> > > > + *
> > > > + * This is an ABI which should be modified only in a compatible way.
> > > > + * struct kexec_module is located at the start of the last page of the module.
> > > Why is kexec_module, which seems like a header, placed in the last page?
> > In case we ever need to grow the module (e.g. when adding new data
> > to it and with that having to handle different CONFIG_KEXEC_MODULE_PAGES
> > values between old and new build), it will be easier, as the module will
> > normally be located at the end of the usable memory, so we wouldn't have
> > to move the struct kexec_module around.
> .. and this avoids that issue since the extra pages can be grabbed without 
> moving struct kexec_module.
Right.

> > > > + *
> > > > + * The module can contain data/pages of multiple users. Each user has an own
> > > > + * record which layout is depending on the user. Records are linked via a 
> > > > table
> > > > + * of record offsets.
> > > > + *
> > > > + * All admin data (struct kexec_module, record offset table and records) must
> > > > + * fit into the last page of the module.
> > > > + */
> > > > +struct kexec_module {
> > > > +    uint8_t eye_catcher[8];
> > > > +#define KEXECMOD_EYECATCHER "KexecMem"
> > > > +    uint16_t n_pages;       /* Number of allocatable pages in the 
> > > > module.    */
> > > > +    uint16_t n_records;     /* Size of record table (max. 
> > > > 255).              */
> > > > +#define KEXECMOD_REC_MAX    255
> > > > +    uint16_t recs_off;      /* Offset to record table from start of 
> > > > page.    */
> > > > +                            /* The record table is an array 
> > > > of               */
> > > > +                            /* struct 
> > > > kexec_module_rec.                      */
> > > > +    uint8_t pg2rec[];       /* One entry per allocatable module page, 
> > > > value  */
> > > > +                            /* is record number (starting from 0) 
> > > > associated */
> > > > +                            /* with it. Free pages have value 
> > > > 255.           */
> > > This reads like it is 1 page per record...
> > No, this means that each used page is associated with a record, but
> > there can be [0 ... n_pages] associated with each record.
> Maybe:
> "Mapping of module pages to associated module record.  Allocated pages are 
> indicated by their record number (starting from 0).  Free pages have value 255."?
Fine with me.

> > > > +#define KEXECMOD_PG_FREE    255
> > > > +};
> > > > +
> > > > +struct kexec_module_rec {
> > > > +    uint16_t offset;        /* Offset to record from start of 
> > > > page.          */
> > > ... but then why would you need an offset?  How do you identify which "page" 
> > > or do you mean from the start of the module?
> > The record itself needs to contain the data for finding the pages
> > associated with it. See patch 18 for the details of the 9pfs record
> > added there.
> >
> > It will use all 17 allocatable pages from the module (1 for the main
> > 9pfs shared interface page plus 16 for the shared rings), while the
> > record itself will hold the grant reference of the 9pfs interface
> > page, which serves as the key for locating the page itself and the
> > ring pages.
> I was missing the overall view.  I think I have it now from reading patch 13.
>
>  From kexec_mod_start, we have:
> (CONFIG_KEXEC_MODULE_PAGES - 1) pages (allocated as pages - referenced by 
> pg2rec)
> struct kexec_module *mod_ptr is the start of the final page.
> pg2rec[n_pages]
> struct kexec_module_rec mod_recs[n_records]
> < extra data for each mod_recs found by .offset and .size >
>
> I didn't realize the mod_recs were separate from their "extra data", and extra 
> data took up the end of the.  If that's correct, I think it would be helpful to 
> describe the overall layout.
I'll change the kexec module description comment.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature