Re: [Xen-devel] [PATCH v9 04/19] amd64: introduce hook for custom preload metadata parsers

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Thu, Jan 02, 2014 at 04:43:38PM +0100, Roger Pau Monne wrote:
> ---
>  sys/amd64/amd64/machdep.c   |   41 ++++++++++++++++------
>  sys/amd64/include/sysarch.h |   12 ++++++
>  sys/x86/xen/pv.c            |   82 
> +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 124 insertions(+), 11 deletions(-)
> 
> diff --git a/sys/amd64/amd64/machdep.c b/sys/amd64/amd64/machdep.c
> index eae657b..e073eea 100644
> --- a/sys/amd64/amd64/machdep.c
> +++ b/sys/amd64/amd64/machdep.c
> @@ -126,6 +126,7 @@ __FBSDID("$FreeBSD$");
>  #include <machine/reg.h>
>  #include <machine/sigframe.h>
>  #include <machine/specialreg.h>
> +#include <machine/sysarch.h>
>  #ifdef PERFMON
>  #include <machine/perfmon.h>
>  #endif
> @@ -165,6 +166,14 @@ static int  set_fpcontext(struct thread *td, const 
> mcontext_t *mcp,
>      char *xfpustate, size_t xfpustate_len);
>  SYSINIT(cpu, SI_SUB_CPU, SI_ORDER_FIRST, cpu_startup, NULL);
>  
> +/* Preload data parse function */
> +static caddr_t native_parse_preload_data(u_int64_t);
> +
> +/* Default init_ops implementation. */
> +struct init_ops init_ops = {
> +     .parse_preload_data =   native_parse_preload_data,

Extra space there.

> +};
> +
>  /*
>   * The file "conf/ldscript.amd64" defines the symbol "kernphys".  Its value 
> is
>   * the physical address at which the kernel is loaded.
> @@ -1683,6 +1692,26 @@ do_next:
>       msgbufp = (struct msgbuf *)PHYS_TO_DMAP(phys_avail[pa_indx]);
>  }
>  
> +static caddr_t
> +native_parse_preload_data(u_int64_t modulep)
> +{
> +     caddr_t kmdp;
> +
> +     preload_metadata = (caddr_t)(uintptr_t)(modulep + KERNBASE);

Two casts? Could it be done via one?

> +     preload_bootstrap_relocate(KERNBASE);
> +     kmdp = preload_search_by_type("elf kernel");
> +     if (kmdp == NULL)
> +             kmdp = preload_search_by_type("elf64 kernel");
> +     boothowto = MD_FETCH(kmdp, MODINFOMD_HOWTO, int);
> +     kern_envp = MD_FETCH(kmdp, MODINFOMD_ENVP, char *) + KERNBASE;
> +#ifdef DDB
> +     ksym_start = MD_FETCH(kmdp, MODINFOMD_SSYM, uintptr_t);
> +     ksym_end = MD_FETCH(kmdp, MODINFOMD_ESYM, uintptr_t);
> +#endif
> +
> +     return (kmdp);
> +}
> +
>  u_int64_t
>  hammer_time(u_int64_t modulep, u_int64_t physfree)
>  {
> @@ -1707,17 +1736,7 @@ hammer_time(u_int64_t modulep, u_int64_t physfree)
>        */
>       proc_linkup0(&proc0, &thread0);
>  
> -     preload_metadata = (caddr_t)(uintptr_t)(modulep + KERNBASE);

Oh, you just moved the code - right, lets not modify it in this patch.

> -     preload_bootstrap_relocate(KERNBASE);
> -     kmdp = preload_search_by_type("elf kernel");
> -     if (kmdp == NULL)
> -             kmdp = preload_search_by_type("elf64 kernel");
> -     boothowto = MD_FETCH(kmdp, MODINFOMD_HOWTO, int);
> -     kern_envp = MD_FETCH(kmdp, MODINFOMD_ENVP, char *) + KERNBASE;
> -#ifdef DDB
> -     ksym_start = MD_FETCH(kmdp, MODINFOMD_SSYM, uintptr_t);
> -     ksym_end = MD_FETCH(kmdp, MODINFOMD_ESYM, uintptr_t);
> -#endif
> +     kmdp = init_ops.parse_preload_data(modulep);
>  
>       /* Init basic tunables, hz etc */
>       init_param1();
> diff --git a/sys/amd64/include/sysarch.h b/sys/amd64/include/sysarch.h
> index cd380d4..58ac8cd 100644
> --- a/sys/amd64/include/sysarch.h
> +++ b/sys/amd64/include/sysarch.h
> @@ -4,3 +4,15 @@
>  /* $FreeBSD$ */
>  
>  #include <x86/sysarch.h>
> +
> +/*
> + * Struct containing pointers to init functions whose
> + * implementation is run time selectable.  Selection can be made,
> + * for example, based on detection of a BIOS variant or
> + * hypervisor environment.
> + */
> +struct init_ops {
> +     caddr_t (*parse_preload_data)(u_int64_t);
> +};
> +
> +extern struct init_ops init_ops;
> diff --git a/sys/x86/xen/pv.c b/sys/x86/xen/pv.c
> index db3b7a3..908b50b 100644
> --- a/sys/x86/xen/pv.c
> +++ b/sys/x86/xen/pv.c
> @@ -46,6 +46,8 @@ __FBSDID("$FreeBSD$");
>  #include <vm/vm_pager.h>
>  #include <vm/vm_param.h>
>  
> +#include <machine/sysarch.h>
> +
>  #include <xen/xen-os.h>
>  #include <xen/hypervisor.h>
>  
> @@ -54,6 +56,36 @@ extern u_int64_t hammer_time(u_int64_t, u_int64_t);
>  /* Xen initial function */
>  extern u_int64_t hammer_time_xen(start_info_t *, u_int64_t);
>  
> +/*--------------------------- Forward Declarations 
> ---------------------------*/
> +static caddr_t xen_pv_parse_preload_data(u_int64_t);
> +
> +static void xen_pv_set_init_ops(void);
> +
> +/*-------------------------------- Global Data 
> -------------------------------*/
> +/* Xen init_ops implementation. */
> +struct init_ops xen_init_ops = {
> +     .parse_preload_data =   xen_pv_parse_preload_data,
> +};
> +
> +static struct
> +{
> +     const char      *ev;
> +     int             mask;
> +} howto_names[] = {
> +     {"boot_askname",        RB_ASKNAME},
> +     {"boot_single",         RB_SINGLE},
> +     {"boot_nosync",         RB_NOSYNC},
> +     {"boot_halt",           RB_ASKNAME},
> +     {"boot_serial",         RB_SERIAL},
> +     {"boot_cdrom",          RB_CDROM},
> +     {"boot_gdb",            RB_GDB},
> +     {"boot_gdb_pause",      RB_RESERVED1},
> +     {"boot_verbose",        RB_VERBOSE},
> +     {"boot_multicons",      RB_MULTIPLE},
> +     {NULL,  0}
> +};
> +
> +/*-------------------------------- Xen PV init 
> -------------------------------*/
>  /*
>   * First function called by the Xen PVH boot sequence.
>   *
> @@ -118,6 +150,56 @@ hammer_time_xen(start_info_t *si, u_int64_t xenstack)
>       }
>       load_cr3(((u_int64_t)&PT4[0]) - KERNBASE);
>  
> +     /* Set the hooks for early functions that diverge from bare metal */
> +     xen_pv_set_init_ops();
> +
>       /* Now we can jump into the native init function */
>       return (hammer_time(0, physfree));
>  }
> +
> +/*-------------------------------- PV specific 
> -------------------------------*/
> +/*
> + * Functions to convert the "extra" parameters passed by Xen
> + * into FreeBSD boot options (from the i386 Xen port).
> + */
> +static char *
> +xen_setbootenv(char *cmd_line)
> +{
> +     char *cmd_line_next;
> +
> +        /* Skip leading spaces */
> +        for (; *cmd_line == ' '; cmd_line++);

Spaces?

> +
> +     for (cmd_line_next = cmd_line; strsep(&cmd_line_next, ",") != NULL;);
> +     return (cmd_line);
> +}
> +
> +static int
> +xen_boothowto(char *envp)
> +{
> +     int i, howto = 0;
> +
> +     /* get equivalents from the environment */
> +     for (i = 0; howto_names[i].ev != NULL; i++)
> +             if (getenv(howto_names[i].ev) != NULL)
> +                     howto |= howto_names[i].mask;

You don't believe in '{}' do you ? :-)

> +     return (howto);
> +}
> +
> +static caddr_t
> +xen_pv_parse_preload_data(u_int64_t modulep)
> +{
> +     /* Parse the extra boot information given by Xen */
> +     if (HYPERVISOR_start_info->cmd_line)
> +             kern_envp = xen_setbootenv(HYPERVISOR_start_info->cmd_line);
> +     boothowto |= xen_boothowto(kern_envp);
> +
> +     return (NULL);
> +}
> +
> +static void
> +xen_pv_set_init_ops(void)
> +{
> +     /* Init ops for Xen PV */
> +     init_ops = xen_init_ops;
> +}
> -- 
> 1.7.7.5 (Apple Git-26)
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel