Re: [Xen-devel] [PATCH v9 04/19] amd64: introduce hook for custom preload metadata parsers
On 03/01/14 21:59, Konrad Rzeszutek Wilk wrote:
> 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
>> #include
>> #include
>> +#include
>> #ifdef PERFMON
>> #include
>> #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.
It's for alignment, looks strange now because there's only one hook.
>> +};
>> +
>> /*
>> * 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.
Yes, it's just code movement.
>
>> -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
>> +
>> +/*
>> + * 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
>> #include
>>
>> +#include
>> +
>> #include
>> #include
>>
>> @@ -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
Re: [Xen-devel] [PATCH v9 04/19] amd64: introduce hook for custom preload metadata parsers
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
> #include
> #include
> +#include
> #ifdef PERFMON
> #include
> #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
> +
> +/*
> + * 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
> #include
>
> +#include
> +
> #include
> #include
>
> @@ -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}
>
