On 4/10/15 05:55, Peter Maydell wrote:
> On 27 March 2015 at 10:54, Chen Gang <[email protected]> wrote:
>> It implements minimized cpu features for linux-user.
>>
>> Signed-off-by: Chen Gang <[email protected]>
>> ---
>> 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