qemu-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]