[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: |
Chen Gang |
Subject: |
Re: [Qemu-devel] [PATCH 06/12 v9] target-tilegx: Add cpu basic features for linux-user |
Date: |
Sat, 11 Apr 2015 05:04:06 +0800 |
User-agent: |
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 |
On 4/10/15 05:55, Peter Maydell wrote:
> 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.
>
>
OK, thanks.
>> +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...
>
>
OK, thanks. I shall try.
[...]
>> +
>> +#include "exec/cpu-defs.h"
>> +#include "fpu/softfloat.h"
>
> What's the softfloat include for?
>
OK, thanks. I shall remove it.
[...]
>> +
>> +/* 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.
>
OK, thanks.
>> +#define MMU_USER_IDX 0 /* independent from both qemu and architecture */
>
> Not sure what you mean with this comment?
>
OK, thanks. I shall remove the comment.
Thanks.
--
Chen Gang
Open, share, and attitude like air, water, and life which God blessed