On 4/10/15 05:55, Peter Maydell wrote:
> On 27 March 2015 at 10:54, Chen Gang <xili_gchen_5...@hotmail.com> wrote:
>> It implements minimized cpu features for linux-user.
>>
>> Signed-off-by: Chen Gang <gang.chen.5...@gmail.com>
>> ---
>>  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