Re: Kexec and libxenctlr.so

[Julien Grall <julien@xxxxxxx>]

Hi Wei,

On 26/06/2020 12:08, Wei Liu wrote:
> On Thu, Jun 11, 2020 at 03:57:37PM +0100, Julien Grall wrote:
> > Hi all,
> >
> > kexec-tools has an option to load dynamically libxenctlr.so (not .so.4.x)
> > (see [1]).
> >
> > Given that the library has never been considered stable, it is probably a
> > disaster that is waiting to happen.
> >
> > Looking at the tree kexec is using the follow libxc functions:
> >     - xc_kexec_get_range()
> >     - xc_kexec_load()
> >     - xc_kexec_unload()
> >     - xc_kexec_status()
> >     - xc_kexec_exec()
> >     - xc_version()
> >     - xc_interface_open()
> >     - xc_interface_close()
> >     - xc_get_max_cpus()
> >     - xc_get_machine_memory_map()
> >
> > I think it is uncontroversial that we want a new stable library for all the
> > xc_kexec_* functions (maybe libxenexec)?
> That sounds fine to me.
>
> Looking at the list of functions, all the xc_kexec_* ones are probably
> already rather stable.
That's my understanding as well.

Although, we may want to rethink some of the hypercalls (such as 
KEXEC_cmd_kexec_get_range) in the future as they have different layout 
between 32-bit and 64-bit. Thanksfully this wasn't exposed outside of 
libxc, so it shouldn't be an issue to have a stable library.
> For xc_interface_open / close, they are perhaps used only to obtain an
> xc handle such that it can be used to make hypercalls. You new kexec
> library is going to expose its own handle with a xencall handle wrapped
> inside, so you can do away with an xc handle.
I have already a PoC for the new library. I had to tweak a bit the list 
of helpers as some use hypercalls arguments directly. Below, the 
proposed interface:
/* Callers who don't care don't need to #include <xentoollog.h> */
struct xentoollog_logger;

typedef struct xenkexec_handle xenkexec_handle;

typedef struct xenkexec_segments xenkexec_segments;

xenkexec_handle *xenkexec_open(struct xentoollog_logger *logger,
                               unsigned int open_flags);
int xenkexec_close(xenkexec_handle *khdl);

int xenkexec_exec(xenkexec_handle *khdl, int type);
int xenkexec_get_range(xenkexec_handle *khdl, int range, int nr,
                       uint64_t *size, uint64_t *start);
int xenkexec_load(xenkexec_handle *khdl, uint8_t type, uint16_t arch,
                  uint64_t entry_maddr, uint32_t nr_segments,
                  xenkexec_segments *segments);
int xenkexec_unload(xenkexec_handle *khdl, int type);
int xenkexec_status(xenkexec_handle *khdl, int type);

xenkexec_segments *xenkexec_allocate_segments(xenkexec_handle *khdl,
                                              unsigned int nr);
void xenkexec_free_segments(xenkexec_handle *khdl, xenkexec_segments *segs);

int xenkexec_update_segment(xenkexec_handle *khdl, xenkexec_segments *segs,
                            unsigned int idx, void *buffer, size_t 
buffer_size,
                            uint64_t dest_maddr, size_t dest_size);


> > However I am not entirely sure where to put the others.
> >
> > I am thinking to introduce libxensysctl for xc_get_max_cpus() as it is a
> > XEN_SYSCTL. We could possibly include xc_get_machine_memory_map() (despite
> > it is a XENMEM_).
> Introducing an libxensysctl before we stabilise sysctl interface seems
> wrong to me. We can bury the call inside libxenkexec itself for the time
> being.
That would work for me.

> > For xc_version(), I am thinking to extend libxentoolcore to also include
> > "stable xen API".
> If you can do without an xc handle, do you still need to call
> xc_version?
Looking at kexec, xc_version() is used by crashdump to determine which 
architecture is used by Xen (in this case 32-bit x86 vs 64-bit x86).
The was introcuded by commit:

commit cdbc9b011fe43407908632d842e3a39e495e48d9
Author: Ian Campbell <ian.campbell@xxxxxxxxxxxxx>
Date:   Fri Mar 16 10:10:24 2007 +0000

    Set crash dump ELF header e_machine field based on underlying 
hypervisor architecture.
    This is necessary when running Xen with a 64 bit hypervisor and 32 bit
    domain 0 since the CPU crash notes will be 64 bit.

    Detecting the hypervisor archiecture requires libxenctrl and 
therefore this
    support is optional and disabled by default.

    Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxxxxx>
    Acked-by: Magnus Damm <magnus@xxxxxxxxxxxxx>
    Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>

As we drop support for 32-bit Xen quite a long time ago, we may be able 
to remove the call to xc_version().
Cheers,

--
Julien Grall