Patchwork [RFC,08/16,v6] target-i386: add API to get dump info

login
register
mail settings
Submitter Wen Congyang
Date Feb. 9, 2012, 3:26 a.m.
Message ID <4F333CEC.7080007@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/140298/
State New
Headers show

Comments

Wen Congyang - Feb. 9, 2012, 3:26 a.m.
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
Jan Kiszka - Feb. 14, 2012, 5:39 p.m.
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
Wen Congyang - Feb. 15, 2012, 3:30 a.m.
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
>
Jan Kiszka - Feb. 15, 2012, 9:05 a.m.
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
Wen Congyang - Feb. 15, 2012, 9:10 a.m.
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
>
Peter Maydell - Feb. 15, 2012, 9:12 a.m.
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
Wen Congyang - Feb. 15, 2012, 9:19 a.m.
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
>

Patch

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;
+}