|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 4 of 4] KEXEC: Introduce new crashtables interface
>>> On 09.03.12 at 15:42, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -82,6 +85,21 @@ paddr_t crashinfo_maxaddr_bits = 64;
> /* Pointers to keep track of the crash heap region. */
> static void *crash_heap_current = NULL, *crash_heap_end = NULL;
>
> +char * StringTab = NULL;
> +size_t StringTabSize = 0;
static? Naming convention?
> +
> +static val64tab_t ValTab[] =
const? Naming convention?
> +{
> + { XEN_VALTAB_TOP_OF_MEM, 0 },
> + { XEN_VALTAB_CONRING_SIZE, 0 }
> +};
> +
> +static sym64tab_t SymTab[] =
const? Naming convention?
> +{
> + { XEN_SYMTAB_CONRING, 0 }
> +};
> +
> +
> /*
> * Parse command lines in the format
> *
> @@ -262,12 +280,116 @@ void kexec_crash_save_cpu(void)
> elf_core_save_regs(&prstatus->pr_reg, xencore);
> }
>
> +/* Round val up to a machine word alignment */
> +size_t round_up(size_t val)
> +{
> + return ((val + (sizeof(unsigned long)-1)) &
> + ~((sizeof (unsigned long))-1));
> +}
> +
> +
> +extern xen_commandline_t saved_cmdline;
> +static int setup_stringtable(void)
> +{
> + char * ptr;
> + size_t sl = sizeof(unsigned long);
> +
> +#define STRING(x) #x
> +#define INT2STR(x) STRING(x)
> +
> +#define XEN_STR_VERSION INT2STR(XEN_VERSION) "." INT2STR(XEN_SUBVERSION) \
> + XEN_EXTRAVERSION
> +#define XEN_STR_COMPILED_BY XEN_COMPILE_BY "@" XEN_COMPILE_HOST "." \
> + XEN_COMPILE_DOMAIN
> +
> + size_t size =
> + sl + round_up(sizeof(XEN_STR_VERSION)) +
> + sl + round_up(sizeof(XEN_CHANGESET)) +
> + sl + round_up(sizeof(XEN_COMPILE_DATE)) +
> + sl + round_up(sizeof(XEN_STR_COMPILED_BY)) +
> + sl + round_up(sizeof(XEN_COMPILER)) +
> + sl + round_up(strlen(saved_cmdline)+1);
> +
> + StringTab = xmalloc_bytes(size);
> + if ( ! StringTab )
> + return -ENOMEM;
> +
> + StringTabSize = size;
> + memset(StringTab, 0, size);
> +
> + ptr = StringTab;
> +
> +#define WRITE_STRTAB_ENTRY_CONST(v, s) \
> + *(unsigned long*)ptr = (v); ptr += sl; \
> + memcpy(ptr, s, sizeof(s)); ptr += round_up(sizeof(s))
> +
> +#define WRITE_STRTAB_ENTRY_VAR(v, s) \
> + *(unsigned long*)ptr = (v); ptr += sl; \
> + memcpy(ptr, s, strlen(s)); ptr += round_up(strlen(s)+1)
> +
> + WRITE_STRTAB_ENTRY_CONST(XEN_STRINGTAB_VERSION, XEN_STR_VERSION);
> + WRITE_STRTAB_ENTRY_CONST(XEN_STRINGTAB_CSET, XEN_CHANGESET);
> + WRITE_STRTAB_ENTRY_CONST(XEN_STRINGTAB_COMPILE_DATE, XEN_COMPILE_DATE);
> + WRITE_STRTAB_ENTRY_CONST(XEN_STRINGTAB_COMPILED_BY, XEN_STR_COMPILED_BY);
> + WRITE_STRTAB_ENTRY_CONST(XEN_STRINGTAB_COMPILER, XEN_COMPILER);
> + WRITE_STRTAB_ENTRY_VAR(XEN_STRINGTAB_CMDLINE, saved_cmdline);
> +
> +#undef WRITE_STRTAB_ENTRY_VAR
> +#undef WRITE_STRTAB_ENTRY_CONST
> +#undef XEN_STR_COMPILED_BY
> +#undef XEN_STR_VERSION
> +#undef INT2STR
> +#undef STRING
> +
> + return 0;
> +}
> +
> +static void write_val64tab(void)
> +{
> + int i;
> + for ( i=0; i<ARRAY_SIZE(ValTab); ++i )
> + {
> + switch( ValTab[i].id )
> + {
> + case XEN_VALTAB_TOP_OF_MEM:
> + ValTab[i].val = (uint64_t)top_of_mem;
> + break;
> + case XEN_VALTAB_CONRING_SIZE:
> + ValTab[i].val = (uint64_t)conring_size;
> + break;
> + default:
> + printk(XENLOG_WARNING "Unrecognised ValTab ID 0x%016"PRIx64"\n",
> + ValTab[i].id);
Sorry, but this is a BUG() or an ASSERT(), but not a warning.
> + break;
> + }
> + }
> +}
> +
> +static void write_sym64tab(void)
> +{
> + int i;
> + for ( i=0; i<ARRAY_SIZE(SymTab); ++i )
> + {
> + switch( SymTab[i].id )
> + {
> + case XEN_SYMTAB_CONRING:
> + SymTab[i].addr = (uint64_t)__pa(conring);
> + break;
> + default:
> + printk(XENLOG_WARNING "Unrecognised SymTab ID 0x%016"PRIx64"\n",
> + SymTab[i].id);
Same here.
> + break;
> + }
> + }
> +}
> +
> /* Set up the single Xen-specific-info crash note. */
> crash_xen_info_t *kexec_crash_save_info(void)
> {
> int cpu = smp_processor_id();
> crash_xen_info_t info;
> crash_xen_info_t *out = (crash_xen_info_t *)ELFNOTE_DESC(xen_crash_note);
> + char * details;
>
> BUG_ON(!cpumask_test_and_set_cpu(cpu, &crash_saved_cpus));
>
> @@ -284,6 +406,17 @@ crash_xen_info_t *kexec_crash_save_info(
> /* Copy from guaranteed-aligned local copy to possibly-unaligned dest. */
> memcpy(out, &info, sizeof(info));
>
> + details = ELFNOTE_DESC(xen_stringtab);
> + memcpy(details, StringTab, StringTabSize);
> +
> + details = ELFNOTE_DESC(xen_valtab);
> + write_val64tab();
> + memcpy(details, ValTab, sizeof(ValTab));
> +
> + details = ELFNOTE_DESC(xen_symtab);
> + write_sym64tab();
> + memcpy(details, SymTab, sizeof(SymTab));
> +
> return out;
> }
>
> @@ -365,7 +498,10 @@ static size_t sizeof_cpu_notes(const uns
> /* CPU0 also presents the crash_xen_info note. */
> if ( ! cpu )
> bytes = bytes +
> - sizeof_note("Xen", sizeof(crash_xen_info_t));
> + sizeof_note("Xen", sizeof(crash_xen_info_t)) +
> + sizeof_note("Xen", StringTabSize) +
> + sizeof_note("Xen", sizeof(ValTab)) +
> + sizeof_note("Xen", sizeof(SymTab));
>
> return bytes;
> }
> @@ -449,6 +585,21 @@ static int kexec_init_cpu_notes(const un
> xen_crash_note = note = ELFNOTE_NEXT(note);
> setup_note(note, "Xen", XEN_ELFNOTE_CRASH_INFO,
> sizeof(crash_xen_info_t));
> +
> + /* Set up Xen StringTab note. */
> + xen_stringtab = note = ELFNOTE_NEXT(note);
> + setup_note(note, "Xen", XEN_CRASHTABLE_STRINGTAB,
> + StringTabSize);
> +
> + /* Set up Xen ValTab note. */
> + xen_valtab = note = ELFNOTE_NEXT(note);
> + setup_note(note, "Xen", XEN_CRASHTABLE_VAL64TAB,
> + sizeof(ValTab));
> +
> + /* Set up Xen SymTab note. */
> + xen_symtab = note = ELFNOTE_NEXT(note);
> + setup_note(note, "Xen", XEN_CRASHTABLE_SYM64TAB,
> + sizeof(SymTab));
> }
> }
> }
> @@ -505,6 +656,9 @@ static int __init kexec_init(void)
>
> register_keyhandler('C', &crashdump_trigger_keyhandler);
>
> + if ( setup_stringtable() )
> + return -ENOMEM;
Given that this is an extension, I don't think failing to set this up
should be fatal in any way. Just don't write those new notes when
you can't set them up.
> +
> if ( low_crashinfo_mode > LOW_CRASHINFO_NONE )
> {
> size_t crash_heap_size;
> --- a/xen/drivers/char/console.c
> +++ b/xen/drivers/char/console.c
> @@ -59,8 +59,8 @@ size_param("conring_size", opt_conring_s
> #define _CONRING_SIZE 16384
> #define CONRING_IDX_MASK(i) ((i)&(conring_size-1))
> static char __initdata _conring[_CONRING_SIZE];
> -static char *__read_mostly conring = _conring;
> -static uint32_t __read_mostly conring_size = _CONRING_SIZE;
> +char *__read_mostly conring = _conring;
> +uint32_t __read_mostly conring_size = _CONRING_SIZE;
Please don't. If anything, add a call into this file to have the note
fields filled.
> static uint32_t conringc, conringp;
>
> static int __read_mostly sercon_handle = -1;
> --- /dev/null
> +++ b/xen/include/public/crashtables.h
> @@ -0,0 +1,77 @@
> +/******************************************************************************
> + * crashtables.h
> + *
> + * Definitions used for the Xen ELF crash notes.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> + * of this software and associated documentation files (the "Software"), to
> + * deal in the Software without restriction, including without limitation the
> + * rights to use, copy, modify, merge, publish, distribute, sublicense,
> and/or
> + * sell copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Copyright (c) 2011,2012 Andrew Cooper, Citrix Ltd.
> + */
> +
> +#ifndef __XEN_PUBLIC_CRASHTABLES_H__
> +#define __XEN_PUBLIC_CRASHTABLES_H__
> +
> +#include "xen.h"
> +
> +/*
> + * Xen string crash table.
> + * Note is a list in the form of:
> + * Machine word (32/64bit) String ID.
> + * Null terminating string, 0-extended to word alignment.
> + */
> +#define XEN_CRASHTABLE_STRINGTAB 0x3000000
The number looks as if it wasn't picked arbitrarily, so a comment should
say what it was derived from. Or if it is truly (and compatibly) arbitrary,
imo this should be stated here too.
> +
> +#define XEN_STRINGTAB_VERSION 0
> +#define XEN_STRINGTAB_CSET 1
> +#define XEN_STRINGTAB_COMPILE_DATE 2
> +#define XEN_STRINGTAB_COMPILED_BY 3
> +#define XEN_STRINGTAB_COMPILER 4
> +#define XEN_STRINGTAB_CMDLINE 5
> +
> +/*
> + * Xen value crash table.
> + * Note is a list in the form of:
> + * Machine word (32/64bit) Value ID.
> + * 64bit value.
> + */
> +#define XEN_CRASHTABLE_VAL64TAB 0x3000001
> +
> +typedef struct { unsigned long id; uint64_t val; } val64tab_t;
You're saying you're doing this to improve 32-/64-bit interaction. How
is using 'unsigned long' here (and below) fitting into this picture?
> +
> +#define XEN_VALTAB_TOP_OF_MEM 0
> +#define XEN_VALTAB_CONRING_SIZE 1
> +
> +/*
> + * Xen symbol crash table
> + * Note is a list in the form of:
> + * Machine word (32/64bit) Symbol ID.
> + * Symbol Address (64bit pointers).
> + */
> +#define XEN_CRASHTABLE_SYM64TAB 0x3000002
> +
> +typedef struct { unsigned long id; uint64_t addr; } sym64tab_t;
> +
> +#define XEN_SYMTAB_CONRING 0
> +
> +
> +#endif /* __XEN_PUBLIC_CRASHTABLES_H__ */
> +
> +/*
> +TODO: reintroduce local vars here. emacs doesn't get on well with patch
> files claiming to be C files
> + */
???
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |