| Message ID | 4F333CEC.7080007@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On 2012-02-09 04:26, Wen Congyang wrote: > Dump info contains: endian, class and architecture. The next > patch will use these information to create vmcore. > > Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> > --- > cpu-all.h | 3 +++ > dump.h | 10 ++++++++++ > target-i386/arch-dump.c | 34 ++++++++++++++++++++++++++++++++++ > 3 files changed, 47 insertions(+), 0 deletions(-) > create mode 100644 dump.h > > diff --git a/cpu-all.h b/cpu-all.h > index 290c43a..268d1f6 100644 > --- a/cpu-all.h > +++ b/cpu-all.h > @@ -23,6 +23,7 @@ > #include "qemu-tls.h" > #include "cpu-common.h" > #include "memory_mapping.h" > +#include "dump.h" > > /* some important defines: > * > @@ -531,11 +532,13 @@ int cpu_write_elf64_note(int fd, CPUState *env, int cpuid, > int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, > target_phys_addr_t *offset); > int cpu_add_extra_memory_mapping(MemoryMappingList *list); > +int cpu_get_dump_info(ArchDumpInfo *info); > #else > #define cpu_get_memory_mapping(list, env) > #define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; }) > #define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; }) > #define cpu_add_extra_memory_mapping(list) ({ 0; }) > +#define cpu_get_dump_info(info) ({ -1; }) Please use static inlines where possible (applies to earlier patches as well). > #endif > > #endif /* CPU_ALL_H */ > diff --git a/dump.h b/dump.h > new file mode 100644 > index 0000000..a36468b > --- /dev/null > +++ b/dump.h > @@ -0,0 +1,10 @@ License header missing. > +#ifndef DUMP_H > +#define DUMP_H > + > +typedef struct ArchDumpInfo { > + int d_machine; /* Architecture */ > + int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */ > + int d_class; /* ELFCLASS32 or ELFCLASS64 */ > +} ArchDumpInfo; > + > +#endif > diff --git a/target-i386/arch-dump.c b/target-i386/arch-dump.c > index d96f6ae..92a53bc 100644 > --- a/target-i386/arch-dump.c > +++ b/target-i386/arch-dump.c > @@ -15,6 +15,7 @@ > > #include "cpu.h" > #include "cpu-all.h" > +#include "dump.h" > #include "monitor.h" > > /* PAE Paging or IA-32e Paging */ > @@ -538,3 +539,36 @@ int cpu_add_extra_memory_mapping(MemoryMappingList *list) > #endif > return 0; > } > + > +int cpu_get_dump_info(ArchDumpInfo *info) > +{ > + bool lma = false; > + RAMBlock *block; > + > +#ifdef TARGET_X86_64 > + lma = !!(first_cpu->hflags & HF_LMA_MASK); > +#endif > + > + if (lma) { > + info->d_machine = EM_X86_64; > + } else { > + info->d_machine = EM_386; > + } > + info->d_endian = ELFDATA2LSB; > + > + if (lma) { > + info->d_class = ELFCLASS64; > + } else { > + info->d_class = ELFCLASS32; > + } > + > + QLIST_FOREACH(block, &ram_list.blocks, next) { > + if (!lma && (block->offset + block->length > UINT_MAX)) { > + /* The memory size is greater than 4G */ > + info->d_class = ELFCLASS32; Is that correct, or did you rather mean ELFCLASS64? > + break; > + } > + } > + > + return 0; > +} Jan
At 02/15/2012 01:39 AM, Jan Kiszka Wrote: > On 2012-02-09 04:26, Wen Congyang wrote: >> Dump info contains: endian, class and architecture. The next >> patch will use these information to create vmcore. >> >> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> >> --- >> cpu-all.h | 3 +++ >> dump.h | 10 ++++++++++ >> target-i386/arch-dump.c | 34 ++++++++++++++++++++++++++++++++++ >> 3 files changed, 47 insertions(+), 0 deletions(-) >> create mode 100644 dump.h >> >> diff --git a/cpu-all.h b/cpu-all.h >> index 290c43a..268d1f6 100644 >> --- a/cpu-all.h >> +++ b/cpu-all.h >> @@ -23,6 +23,7 @@ >> #include "qemu-tls.h" >> #include "cpu-common.h" >> #include "memory_mapping.h" >> +#include "dump.h" >> >> /* some important defines: >> * >> @@ -531,11 +532,13 @@ int cpu_write_elf64_note(int fd, CPUState *env, int cpuid, >> int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, >> target_phys_addr_t *offset); >> int cpu_add_extra_memory_mapping(MemoryMappingList *list); >> +int cpu_get_dump_info(ArchDumpInfo *info); >> #else >> #define cpu_get_memory_mapping(list, env) >> #define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; }) >> #define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; }) >> #define cpu_add_extra_memory_mapping(list) ({ 0; }) >> +#define cpu_get_dump_info(info) ({ -1; }) > > Please use static inlines where possible (applies to earlier patches as > well). OK > >> #endif >> >> #endif /* CPU_ALL_H */ >> diff --git a/dump.h b/dump.h >> new file mode 100644 >> index 0000000..a36468b >> --- /dev/null >> +++ b/dump.h >> @@ -0,0 +1,10 @@ > > License header missing. There is no license in other header files. > >> +#ifndef DUMP_H >> +#define DUMP_H >> + >> +typedef struct ArchDumpInfo { >> + int d_machine; /* Architecture */ >> + int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */ >> + int d_class; /* ELFCLASS32 or ELFCLASS64 */ >> +} ArchDumpInfo; >> + >> +#endif >> diff --git a/target-i386/arch-dump.c b/target-i386/arch-dump.c >> index d96f6ae..92a53bc 100644 >> --- a/target-i386/arch-dump.c >> +++ b/target-i386/arch-dump.c >> @@ -15,6 +15,7 @@ >> >> #include "cpu.h" >> #include "cpu-all.h" >> +#include "dump.h" >> #include "monitor.h" >> >> /* PAE Paging or IA-32e Paging */ >> @@ -538,3 +539,36 @@ int cpu_add_extra_memory_mapping(MemoryMappingList *list) >> #endif >> return 0; >> } >> + >> +int cpu_get_dump_info(ArchDumpInfo *info) >> +{ >> + bool lma = false; >> + RAMBlock *block; >> + >> +#ifdef TARGET_X86_64 >> + lma = !!(first_cpu->hflags & HF_LMA_MASK); >> +#endif >> + >> + if (lma) { >> + info->d_machine = EM_X86_64; >> + } else { >> + info->d_machine = EM_386; >> + } >> + info->d_endian = ELFDATA2LSB; >> + >> + if (lma) { >> + info->d_class = ELFCLASS64; >> + } else { >> + info->d_class = ELFCLASS32; >> + } >> + >> + QLIST_FOREACH(block, &ram_list.blocks, next) { >> + if (!lma && (block->offset + block->length > UINT_MAX)) { >> + /* The memory size is greater than 4G */ >> + info->d_class = ELFCLASS32; > > Is that correct, or did you rather mean ELFCLASS64? Yes, it should be ELFCLASS64. Thanks Wen Congyang > >> + break; >> + } >> + } >> + >> + return 0; >> +} > > Jan >
On 2012-02-15 04:30, Wen Congyang wrote: >>> diff --git a/dump.h b/dump.h >>> new file mode 100644 >>> index 0000000..a36468b >>> --- /dev/null >>> +++ b/dump.h >>> @@ -0,0 +1,10 @@ >> >> License header missing. > > There is no license in other header files. But those are preexisting files, no need to repeat the mistake for new one. Please fix. Jan
At 02/15/2012 05:05 PM, Jan Kiszka Wrote: > On 2012-02-15 04:30, Wen Congyang wrote: >>>> diff --git a/dump.h b/dump.h >>>> new file mode 100644 >>>> index 0000000..a36468b >>>> --- /dev/null >>>> +++ b/dump.h >>>> @@ -0,0 +1,10 @@ >>> >>> License header missing. >> >> There is no license in other header files. > > But those are preexisting files, no need to repeat the mistake for new > one. Please fix. OK, I will fix it. Thanks Wen Congyang > > Jan >
On 9 February 2012 03:26, Wen Congyang <wency@cn.fujitsu.com> wrote: > +int cpu_get_dump_info(ArchDumpInfo *info) > +{ > + bool lma = false; > + RAMBlock *block; > + > +#ifdef TARGET_X86_64 > + lma = !!(first_cpu->hflags & HF_LMA_MASK); > +#endif > + > + if (lma) { > + info->d_machine = EM_X86_64; > + } else { > + info->d_machine = EM_386; > + } > + info->d_endian = ELFDATA2LSB; > + > + if (lma) { > + info->d_class = ELFCLASS64; > + } else { > + info->d_class = ELFCLASS32; > + } > + > + QLIST_FOREACH(block, &ram_list.blocks, next) { > + if (!lma && (block->offset + block->length > UINT_MAX)) { > + /* The memory size is greater than 4G */ > + info->d_class = ELFCLASS32; > + break; > + } > + } I think it would be cleaner to have a single if (lma) { stuff; } else { stuff; } rather than checking it three times, especially for the loop, where if lma is true we'll walk the ram_list without ever doing anything. -- PMM
At 02/15/2012 05:12 PM, Peter Maydell Wrote: > On 9 February 2012 03:26, Wen Congyang <wency@cn.fujitsu.com> wrote: >> +int cpu_get_dump_info(ArchDumpInfo *info) >> +{ >> + bool lma = false; >> + RAMBlock *block; >> + >> +#ifdef TARGET_X86_64 >> + lma = !!(first_cpu->hflags & HF_LMA_MASK); >> +#endif >> + >> + if (lma) { >> + info->d_machine = EM_X86_64; >> + } else { >> + info->d_machine = EM_386; >> + } >> + info->d_endian = ELFDATA2LSB; >> + >> + if (lma) { >> + info->d_class = ELFCLASS64; >> + } else { >> + info->d_class = ELFCLASS32; >> + } >> + >> + QLIST_FOREACH(block, &ram_list.blocks, next) { >> + if (!lma && (block->offset + block->length > UINT_MAX)) { >> + /* The memory size is greater than 4G */ >> + info->d_class = ELFCLASS32; >> + break; >> + } >> + } > > I think it would be cleaner to have a single > if (lma) { > stuff; > } else { > stuff; > } > > rather than checking it three times, especially for > the loop, where if lma is true we'll walk the ram_list > without ever doing anything. Nice. I will change it. Thanks Wen Congyang > > -- PMM >
diff --git a/cpu-all.h b/cpu-all.h index 290c43a..268d1f6 100644 --- a/cpu-all.h +++ b/cpu-all.h @@ -23,6 +23,7 @@ #include "qemu-tls.h" #include "cpu-common.h" #include "memory_mapping.h" +#include "dump.h" /* some important defines: * @@ -531,11 +532,13 @@ int cpu_write_elf64_note(int fd, CPUState *env, int cpuid, int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, target_phys_addr_t *offset); int cpu_add_extra_memory_mapping(MemoryMappingList *list); +int cpu_get_dump_info(ArchDumpInfo *info); #else #define cpu_get_memory_mapping(list, env) #define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; }) #define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; }) #define cpu_add_extra_memory_mapping(list) ({ 0; }) +#define cpu_get_dump_info(info) ({ -1; }) #endif #endif /* CPU_ALL_H */ diff --git a/dump.h b/dump.h new file mode 100644 index 0000000..a36468b --- /dev/null +++ b/dump.h @@ -0,0 +1,10 @@ +#ifndef DUMP_H +#define DUMP_H + +typedef struct ArchDumpInfo { + int d_machine; /* Architecture */ + int d_endian; /* ELFDATA2LSB or ELFDATA2MSB */ + int d_class; /* ELFCLASS32 or ELFCLASS64 */ +} ArchDumpInfo; + +#endif diff --git a/target-i386/arch-dump.c b/target-i386/arch-dump.c index d96f6ae..92a53bc 100644 --- a/target-i386/arch-dump.c +++ b/target-i386/arch-dump.c @@ -15,6 +15,7 @@ #include "cpu.h" #include "cpu-all.h" +#include "dump.h" #include "monitor.h" /* PAE Paging or IA-32e Paging */ @@ -538,3 +539,36 @@ int cpu_add_extra_memory_mapping(MemoryMappingList *list) #endif return 0; } + +int cpu_get_dump_info(ArchDumpInfo *info) +{ + bool lma = false; + RAMBlock *block; + +#ifdef TARGET_X86_64 + lma = !!(first_cpu->hflags & HF_LMA_MASK); +#endif + + if (lma) { + info->d_machine = EM_X86_64; + } else { + info->d_machine = EM_386; + } + info->d_endian = ELFDATA2LSB; + + if (lma) { + info->d_class = ELFCLASS64; + } else { + info->d_class = ELFCLASS32; + } + + QLIST_FOREACH(block, &ram_list.blocks, next) { + if (!lma && (block->offset + block->length > UINT_MAX)) { + /* The memory size is greater than 4G */ + info->d_class = ELFCLASS32; + break; + } + } + + return 0; +}
Dump info contains: endian, class and architecture. The next patch will use these information to create vmcore. Signed-off-by: Wen Congyang <wency@cn.fujitsu.com> --- cpu-all.h | 3 +++ dump.h | 10 ++++++++++ target-i386/arch-dump.c | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 0 deletions(-) create mode 100644 dump.h