[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/12 v9] target-tilegx: Add cpu basic features
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PATCH 06/12 v9] target-tilegx: Add cpu basic features for linux-user |
Date: |
Thu, 9 Apr 2015 22:55:14 +0100 |
On 27 March 2015 at 10:54, Chen Gang <address@hidden> wrote:
> It implements minimized cpu features for linux-user.
>
> Signed-off-by: Chen Gang <address@hidden>
> ---
> target-tilegx/cpu-qom.h | 73 ++++++++++++++++++++++++
> target-tilegx/cpu.c | 149
> ++++++++++++++++++++++++++++++++++++++++++++++++
> target-tilegx/cpu.h | 94 ++++++++++++++++++++++++++++++
You don't really need a separate cpu-qom.h and cpu.h -- that's
just the way we've ended up with for the older targets which
got converted to QOM for legacy reasons. See target-moxie/
for an example which combines the two headers.
> +static const VMStateDescription vmstate_tilegx_cpu = {
> + .name = "cpu",
> + .unmigratable = 1,
> +};
I'd prefer to see a correct VMState from the start -- it's
not very difficult. Migration/snapshotting is much easier
to enforce at the point where we let code in to the tree
than if we let in non-migratable devices and CPUs and then
have to fix them up later...
> +++ b/target-tilegx/cpu.h
> @@ -0,0 +1,94 @@
> +/*
> + * TILE-Gx virtual CPU header
> + *
> + * Copyright (c) 2015 Chen Gang
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see
> <http://www.gnu.org/licenses/>.
> + */
> +#ifndef CPU_TILEGX_H
> +#define CPU_TILEGX_H
> +
> +#include "config.h"
> +#include "qemu-common.h"
> +
> +#define TARGET_LONG_BITS 64
> +
> +#define CPUArchState struct CPUTLGState
> +
> +#include "exec/cpu-defs.h"
> +#include "fpu/softfloat.h"
What's the softfloat include for?
> +
> +/* TILE-Gx register alias */
> +#define TILEGX_R_RE 0 /* 0 register, for function/syscall return value
> */
> +#define TILEGX_R_NR 10 /* 10 register, for syscall number */
> +#define TILEGX_R_BP 52 /* 52 register, optional frame pointer */
> +#define TILEGX_R_TP 53 /* TP register, thread local storage data */
> +#define TILEGX_R_SP 54 /* SP register, stack pointer */
> +#define TILEGX_R_LR 55 /* LR register, may save pc, but it is not pc */
> +#define TILEGX_R_ZERO 63 /* Zero register, always zero */
> +#define TILEGX_R_COUNT 56 /* Only 56 registers are really useful */
> +#define TILEGX_R_NOREG 255 /* Invalid register value */
> +
> +
> +typedef struct CPUTLGState {
> + uint64_t regs[TILEGX_R_COUNT]; /* Common used registers by outside */
> + uint64_t pc; /* Current pc */
> +
> + CPU_COMMON
> +} CPUTLGState;
> +
> +#include "cpu-qom.h"
> +
> +/* TILE-Gx memory attributes */
> +#define TARGET_PAGE_BITS 16 /* TILE-Gx uses 64KB page size */
> +#define MMAP_SHIFT TARGET_PAGE_BITS
> +#define TARGET_PHYS_ADDR_SPACE_BITS 42 /* TILE-Gx is 42 bit physical address
> */
> +#define TARGET_VIRT_ADDR_SPACE_BITS 64 /* TILE-Gx has 64 bit virtual address
> */
nitpick: "has [...] addresses" is the correct grammar in both these comments.
> +#define MMU_USER_IDX 0 /* independent from both qemu and architecture */
Not sure what you mean with this comment?
thanks
-- PMM
- Re: [Qemu-devel] [PATCH 06/12 v9] target-tilegx: Add cpu basic features for linux-user,
Peter Maydell <=