[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] translate-all.c: memory walker initial address
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH] translate-all.c: memory walker initial address miscalculation |
Date: |
Tue, 30 Sep 2014 17:16:50 +0300 |
On Mon, Sep 08, 2014 at 10:51:22AM +0400, address@hidden wrote:
> From: Mikhail Ilyin <address@hidden>
>
> 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 <address@hidden>
I'd prefer for someone who knows about linux-user to
apply this, but it has been too long since this was
posted.
This seems to make sense superficially.
Acked-by: Michael S. Tsirkin <address@hidden>
Riku?
> ---
> include/exec/cpu-all.h | 4 ++--
> linux-user/elfload.c | 18 +++++++++---------
> translate-all.c | 27 +++++++++++++--------------
> 3 files changed, 24 insertions(+), 25 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..885a4ec 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -1660,12 +1660,12 @@ 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) {
> int rc = data->fn(data->priv, data->start, end, data->prot);
> @@ -1681,9 +1681,9 @@ static int walk_memory_regions_end(struct
> walk_memory_regions_data *data,
> }
>
> 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) {
> @@ -1731,9 +1731,8 @@ int walk_memory_regions(void *priv,
> walk_memory_regions_fn fn)
> 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) {
> --
> 1.9.1