Re: [Qemu-devel] [PATCH] translate-all.c: memory walker initial address miscalculation

[Mikhail Ilin <m.ilin@samsung.com>]

ping

http://patchwork.ozlabs.org/patch/386918/

On 22.09.2014 13:04, Mikhail Ilin wrote:
> ping
>
> http://patchwork.ozlabs.org/patch/386918/
>
> On 08.09.2014 17:28, m.ilin@samsung.com wrote:
>> From: Mikhail Ilyin <m.ilin@samsung.com>
>>
>> The initial base address is miscalculated in walk_memory_regions().
>> It has to be shifted TARGET_PAGE_BITS more. Holder variables are
>> extended to target_ulong size otherwise they don't fit for MIPS N32
>> (a 32-bit ABI with a 64-bit address space) and qemu won't compile.
>> The issue led to incorrect debug output of memory maps and a
>> mis-formed coredumped file.
>>
>> Signed-off-by: Mikhail Ilyin <m.ilin@samsung.com>
>> ---
>> Add a small fix to build with a 64-bit compiler.
>>
>>   include/exec/cpu-all.h |  4 ++--
>>   linux-user/elfload.c   | 18 +++++++++---------
>>   translate-all.c        | 33 ++++++++++++++++-----------------
>>   3 files changed, 27 insertions(+), 28 deletions(-)
>>
>> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
>> index f9d132f..c085804 100644
>> --- a/include/exec/cpu-all.h
>> +++ b/include/exec/cpu-all.h
>> @@ -232,8 +232,8 @@ extern uintptr_t qemu_host_page_mask;
>>   #if defined(CONFIG_USER_ONLY)
>>   void page_dump(FILE *f);
>> -typedef int (*walk_memory_regions_fn)(void *, abi_ulong,
>> -                                      abi_ulong, unsigned long);
>> +typedef int (*walk_memory_regions_fn)(void *, target_ulong,
>> +                                      target_ulong, unsigned long);
>>   int walk_memory_regions(void *, walk_memory_regions_fn);
>>   int page_get_flags(target_ulong address);
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index bea803b..1c04fcf 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -2355,9 +2355,9 @@ struct elf_note_info {
>>   };
>>   struct vm_area_struct {
>> -    abi_ulong   vma_start;  /* start vaddr of memory region */
>> -    abi_ulong   vma_end;    /* end vaddr of memory region */
>> -    abi_ulong   vma_flags;  /* protection etc. flags for the region */
>> +    target_ulong   vma_start;  /* start vaddr of memory region */
>> +    target_ulong   vma_end;    /* end vaddr of memory region */
>> +    abi_ulong      vma_flags;  /* protection etc. flags for the
>> region */
>>       QTAILQ_ENTRY(vm_area_struct) vma_link;
>>   };
>> @@ -2368,13 +2368,13 @@ struct mm_struct {
>>   static struct mm_struct *vma_init(void);
>>   static void vma_delete(struct mm_struct *);
>> -static int vma_add_mapping(struct mm_struct *, abi_ulong,
>> -                           abi_ulong, abi_ulong);
>> +static int vma_add_mapping(struct mm_struct *, target_ulong,
>> +                           target_ulong, abi_ulong);
>>   static int vma_get_mapping_count(const struct mm_struct *);
>>   static struct vm_area_struct *vma_first(const struct mm_struct *);
>>   static struct vm_area_struct *vma_next(struct vm_area_struct *);
>>   static abi_ulong vma_dump_size(const struct vm_area_struct *);
>> -static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
>> +static int vma_walker(void *priv, target_ulong start, target_ulong end,
>>                         unsigned long flags);
>>   static void fill_elf_header(struct elfhdr *, int, uint16_t, uint32_t);
>> @@ -2466,8 +2466,8 @@ static void vma_delete(struct mm_struct *mm)
>>       g_free(mm);
>>   }
>> -static int vma_add_mapping(struct mm_struct *mm, abi_ulong start,
>> -                           abi_ulong end, abi_ulong flags)
>> +static int vma_add_mapping(struct mm_struct *mm, target_ulong start,
>> +                           target_ulong end, abi_ulong flags)
>>   {
>>       struct vm_area_struct *vma;
>> @@ -2535,7 +2535,7 @@ static abi_ulong vma_dump_size(const struct
>> vm_area_struct *vma)
>>       return (vma->vma_end - vma->vma_start);
>>   }
>> -static int vma_walker(void *priv, abi_ulong start, abi_ulong end,
>> +static int vma_walker(void *priv, target_ulong start, target_ulong end,
>>                         unsigned long flags)
>>   {
>>       struct mm_struct *mm = (struct mm_struct *)priv;
>> diff --git a/translate-all.c b/translate-all.c
>> index 2e0265a..ba5c840 100644
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -1660,30 +1660,30 @@ void cpu_interrupt(CPUState *cpu, int mask)
>>   struct walk_memory_regions_data {
>>       walk_memory_regions_fn fn;
>>       void *priv;
>> -    uintptr_t start;
>> +    target_ulong start;
>>       int prot;
>>   };
>>   static int walk_memory_regions_end(struct walk_memory_regions_data
>> *data,
>> -                                   abi_ulong end, int new_prot)
>> +                                   target_ulong end, int new_prot)
>>   {
>> -    if (data->start != -1ul) {
>> +    if (data->start != -1u) {
>>           int rc = data->fn(data->priv, data->start, end, data->prot);
>>           if (rc != 0) {
>>               return rc;
>>           }
>>       }
>> -    data->start = (new_prot ? end : -1ul);
>> +    data->start = (new_prot ? end : -1u);
>>       data->prot = new_prot;
>>       return 0;
>>   }
>>   static int walk_memory_regions_1(struct walk_memory_regions_data *data,
>> -                                 abi_ulong base, int level, void **lp)
>> +                                 target_ulong base, int level, void
>> **lp)
>>   {
>> -    abi_ulong pa;
>> +    target_ulong pa;
>>       int i, rc;
>>       if (*lp == NULL) {
>> @@ -1708,7 +1708,7 @@ static int walk_memory_regions_1(struct
>> walk_memory_regions_data *data,
>>           void **pp = *lp;
>>           for (i = 0; i < V_L2_SIZE; ++i) {
>> -            pa = base | ((abi_ulong)i <<
>> +            pa = base | ((target_ulong)i <<
>>                   (TARGET_PAGE_BITS + V_L2_BITS * level));
>>               rc = walk_memory_regions_1(data, pa, level - 1, pp + i);
>>               if (rc != 0) {
>> @@ -1727,13 +1727,12 @@ int walk_memory_regions(void *priv,
>> walk_memory_regions_fn fn)
>>       data.fn = fn;
>>       data.priv = priv;
>> -    data.start = -1ul;
>> +    data.start = -1u;
>>       data.prot = 0;
>>       for (i = 0; i < V_L1_SIZE; i++) {
>> -        int rc = walk_memory_regions_1(&data, (abi_ulong)i <<
>> V_L1_SHIFT,
>> +        int rc = walk_memory_regions_1(&data, (target_ulong)i <<
>> (V_L1_SHIFT + TARGET_PAGE_BITS),
>>                                          V_L1_SHIFT / V_L2_BITS - 1,
>> l1_map + i);
>> -
>>           if (rc != 0) {
>>               return rc;
>>           }
>> @@ -1742,13 +1741,13 @@ int walk_memory_regions(void *priv,
>> walk_memory_regions_fn fn)
>>       return walk_memory_regions_end(&data, 0, 0);
>>   }
>> -static int dump_region(void *priv, abi_ulong start,
>> -    abi_ulong end, unsigned long prot)
>> +static int dump_region(void *priv, target_ulong start,
>> +    target_ulong end, unsigned long prot)
>>   {
>>       FILE *f = (FILE *)priv;
>> -    (void) fprintf(f, TARGET_ABI_FMT_lx"-"TARGET_ABI_FMT_lx
>> -        " "TARGET_ABI_FMT_lx" %c%c%c\n",
>> +    (void) fprintf(f, TARGET_FMT_lx"-"TARGET_FMT_lx
>> +        " "TARGET_FMT_lx" %c%c%c\n",
>>           start, end, end - start,
>>           ((prot & PAGE_READ) ? 'r' : '-'),
>>           ((prot & PAGE_WRITE) ? 'w' : '-'),
>> @@ -1760,7 +1759,7 @@ static int dump_region(void *priv, abi_ulong start,
>>   /* dump memory mappings */
>>   void page_dump(FILE *f)
>>   {
>> -    const int length = sizeof(abi_ulong) * 2;
>> +    const int length = sizeof(target_ulong) * 2;
>>       (void) fprintf(f, "%-*s %-*s %-*s %s\n",
>>               length, "start", length, "end", length, "size", "prot");
>>       walk_memory_regions(f, dump_region);
>> @@ -1788,7 +1787,7 @@ void page_set_flags(target_ulong start,
>> target_ulong end, int flags)
>>          guest address space.  If this assert fires, it probably
>> indicates
>>          a missing call to h2g_valid.  */
>>   #if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS
>> -    assert(end < ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
>> +    assert(end < ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
>>   #endif
>>       assert(start < end);
>> @@ -1825,7 +1824,7 @@ int page_check_range(target_ulong start,
>> target_ulong len, int flags)
>>          guest address space.  If this assert fires, it probably
>> indicates
>>          a missing call to h2g_valid.  */
>>   #if TARGET_ABI_BITS > L1_MAP_ADDR_SPACE_BITS
>> -    assert(start < ((abi_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
>> +    assert(start < ((target_ulong)1 << L1_MAP_ADDR_SPACE_BITS));
>>   #endif
>>       if (len == 0) {
>
>
>