Hi Mike,

> you should scan the code and make sure it's doing func (foo) everywhere.
> i noticed a few places like this where it's not.

Done.

>> +sim_fetch_register (SIM_DESC sd, int regno, unsigned char *buf, int length)
>> +sim_store_register (SIM_DESC sd, int regno, unsigned char *buf, int length)
>
> please convert to CPU_REG_FETCH/CPU_REG_STORE instead of defining these.

Done.

>> +SIM_OBJS = \
>> ...
>> +	sim-reason.o \
>> +	sim-stop.o
>
> you can delete these two from your Makefile as the common core adds them.

Thanks.

>> +#define TRACE_MEM_WRITES (1 << 0)
>> +#define TRACE_REG_WRITES (1 << 1)
>> +#define TRACE_FUNCTIONS  (1 << 2)
>> +#define TRACE_MISC       (1 << 3)
>> +#define TRACE_ALL       ((1 << 4) - 1)
>
> unused -> delete

Done.

>> +#define TST( _flag )   (aarch64_test_CPSR_bit (cpu, _flag))
>> +#define IS_SET( _X )   ( TST (( _X )))
>> +#define IS_CLEAR( _X ) (!TST (( _X )))
>
> drop the weird spacing here

Done.

>> +report_and_die (sim_cpu *cpu, int exitCode)
>> +{
>> +  TRACE_EVENTS (cpu, "Exiting simulator with exit code %d", exitCode);
>> +  exit (exitCode);
>> +}
>
> pretty sure you can use one of the existing abort funcs instead
> of defining your own.  like sim_engine_abort.

Done.

Revised patch attached.  OK to apply ?

Cheers
   Nick