[v5,03/43] monitor: Don't access registers through CPUState

Use CPUX86State etc. instead (hand-converted).

Signed-off-by: Andreas Färber <afaerber@suse.de>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
---
 monitor.c |  488 ++++++++++++++++++++++++++++++------------------------------
 1 files changed, 244 insertions(+), 244 deletions(-)

Andreas Färber writes:

> Use CPUX86State etc. instead (hand-converted).

Is there any reason not to move this into CPU${arch}State and instead provide
virtual methods in CPUState?

For example (a CPUState argument is implicit on all methods):

    int reg_get_number(const char *regname);
    const char* reg_get_name(int regnum);

    /* register validity for a specific instantiation must be chekced somehow */
    bool reg_valid(int regnum);

    size_t reg_get_size(int regnum);

    /* multiple sizes must be handled somehow */
    /* precondition: get_reg_validity(regnum) == true */
    void reg_get_value(int regnum, void *buf);
    void reg_set_value(int regnum, void *buf);


This way, target-specific code would be moved where it belongs, partially
merging (for example) code from both monitor.c, gdbstub.c and the target
itself. The downside is that using pointers to set/get the value is pretty
uncomfortable.


Thanks,
   Lluis



> Signed-off-by: Andreas Färber <afaerber@suse.de>
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  monitor.c |  488 ++++++++++++++++++++++++++++++------------------------------
>  1 files changed, 244 insertions(+), 244 deletions(-)

> diff --git a/monitor.c b/monitor.c
> index cbdfbad..e3b72ff 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2696,30 +2696,30 @@ static const MonitorDef monitor_defs[] = {
>  #ifdef TARGET_I386
 
>  #define SEG(name, seg) \
> -    { name, offsetof(CPUState, segs[seg].selector), NULL, MD_I32 },\
> -    { name ".base", offsetof(CPUState, segs[seg].base) },\
> -    { name ".limit", offsetof(CPUState, segs[seg].limit), NULL, MD_I32 },
> +    { name, offsetof(CPUX86State, segs[seg].selector), NULL, MD_I32 },\
> +    { name ".base", offsetof(CPUX86State, segs[seg].base) },\
> +    { name ".limit", offsetof(CPUX86State, segs[seg].limit), NULL, MD_I32 },
 
> -    { "eax", offsetof(CPUState, regs[0]) },
> -    { "ecx", offsetof(CPUState, regs[1]) },
> -    { "edx", offsetof(CPUState, regs[2]) },
> -    { "ebx", offsetof(CPUState, regs[3]) },
> -    { "esp|sp", offsetof(CPUState, regs[4]) },
> -    { "ebp|fp", offsetof(CPUState, regs[5]) },
> -    { "esi", offsetof(CPUState, regs[6]) },
> -    { "edi", offsetof(CPUState, regs[7]) },
> +    { "eax", offsetof(CPUX86State, regs[0]) },
> +    { "ecx", offsetof(CPUX86State, regs[1]) },
> +    { "edx", offsetof(CPUX86State, regs[2]) },
> +    { "ebx", offsetof(CPUX86State, regs[3]) },
> +    { "esp|sp", offsetof(CPUX86State, regs[4]) },
> +    { "ebp|fp", offsetof(CPUX86State, regs[5]) },
> +    { "esi", offsetof(CPUX86State, regs[6]) },
> +    { "edi", offsetof(CPUX86State, regs[7]) },
>  #ifdef TARGET_X86_64
> -    { "r8", offsetof(CPUState, regs[8]) },
> -    { "r9", offsetof(CPUState, regs[9]) },
> -    { "r10", offsetof(CPUState, regs[10]) },
> -    { "r11", offsetof(CPUState, regs[11]) },
> -    { "r12", offsetof(CPUState, regs[12]) },
> -    { "r13", offsetof(CPUState, regs[13]) },
> -    { "r14", offsetof(CPUState, regs[14]) },
> -    { "r15", offsetof(CPUState, regs[15]) },
> +    { "r8", offsetof(CPUX86State, regs[8]) },
> +    { "r9", offsetof(CPUX86State, regs[9]) },
> +    { "r10", offsetof(CPUX86State, regs[10]) },
> +    { "r11", offsetof(CPUX86State, regs[11]) },
> +    { "r12", offsetof(CPUX86State, regs[12]) },
> +    { "r13", offsetof(CPUX86State, regs[13]) },
> +    { "r14", offsetof(CPUX86State, regs[14]) },
> +    { "r15", offsetof(CPUX86State, regs[15]) },
>  #endif
> -    { "eflags", offsetof(CPUState, eflags) },
> -    { "eip", offsetof(CPUState, eip) },
> +    { "eflags", offsetof(CPUX86State, eflags) },
> +    { "eip", offsetof(CPUX86State, eip) },
>      SEG("cs", R_CS)
>      SEG("ds", R_DS)
>      SEG("es", R_ES)
> @@ -2729,76 +2729,76 @@ static const MonitorDef monitor_defs[] = {
>      { "pc", 0, monitor_get_pc, },
>  #elif defined(TARGET_PPC)
>      /* General purpose registers */
> -    { "r0", offsetof(CPUState, gpr[0]) },
> -    { "r1", offsetof(CPUState, gpr[1]) },
> -    { "r2", offsetof(CPUState, gpr[2]) },
> -    { "r3", offsetof(CPUState, gpr[3]) },
> -    { "r4", offsetof(CPUState, gpr[4]) },
> -    { "r5", offsetof(CPUState, gpr[5]) },
> -    { "r6", offsetof(CPUState, gpr[6]) },
> -    { "r7", offsetof(CPUState, gpr[7]) },
> -    { "r8", offsetof(CPUState, gpr[8]) },
> -    { "r9", offsetof(CPUState, gpr[9]) },
> -    { "r10", offsetof(CPUState, gpr[10]) },
> -    { "r11", offsetof(CPUState, gpr[11]) },
> -    { "r12", offsetof(CPUState, gpr[12]) },
> -    { "r13", offsetof(CPUState, gpr[13]) },
> -    { "r14", offsetof(CPUState, gpr[14]) },
> -    { "r15", offsetof(CPUState, gpr[15]) },
> -    { "r16", offsetof(CPUState, gpr[16]) },
> -    { "r17", offsetof(CPUState, gpr[17]) },
> -    { "r18", offsetof(CPUState, gpr[18]) },
> -    { "r19", offsetof(CPUState, gpr[19]) },
> -    { "r20", offsetof(CPUState, gpr[20]) },
> -    { "r21", offsetof(CPUState, gpr[21]) },
> -    { "r22", offsetof(CPUState, gpr[22]) },
> -    { "r23", offsetof(CPUState, gpr[23]) },
> -    { "r24", offsetof(CPUState, gpr[24]) },
> -    { "r25", offsetof(CPUState, gpr[25]) },
> -    { "r26", offsetof(CPUState, gpr[26]) },
> -    { "r27", offsetof(CPUState, gpr[27]) },
> -    { "r28", offsetof(CPUState, gpr[28]) },
> -    { "r29", offsetof(CPUState, gpr[29]) },
> -    { "r30", offsetof(CPUState, gpr[30]) },
> -    { "r31", offsetof(CPUState, gpr[31]) },
> +    { "r0", offsetof(CPUPPCState, gpr[0]) },
> +    { "r1", offsetof(CPUPPCState, gpr[1]) },
> +    { "r2", offsetof(CPUPPCState, gpr[2]) },
> +    { "r3", offsetof(CPUPPCState, gpr[3]) },
> +    { "r4", offsetof(CPUPPCState, gpr[4]) },
> +    { "r5", offsetof(CPUPPCState, gpr[5]) },
> +    { "r6", offsetof(CPUPPCState, gpr[6]) },
> +    { "r7", offsetof(CPUPPCState, gpr[7]) },
> +    { "r8", offsetof(CPUPPCState, gpr[8]) },
> +    { "r9", offsetof(CPUPPCState, gpr[9]) },
> +    { "r10", offsetof(CPUPPCState, gpr[10]) },
> +    { "r11", offsetof(CPUPPCState, gpr[11]) },
> +    { "r12", offsetof(CPUPPCState, gpr[12]) },
> +    { "r13", offsetof(CPUPPCState, gpr[13]) },
> +    { "r14", offsetof(CPUPPCState, gpr[14]) },
> +    { "r15", offsetof(CPUPPCState, gpr[15]) },
> +    { "r16", offsetof(CPUPPCState, gpr[16]) },
> +    { "r17", offsetof(CPUPPCState, gpr[17]) },
> +    { "r18", offsetof(CPUPPCState, gpr[18]) },
> +    { "r19", offsetof(CPUPPCState, gpr[19]) },
> +    { "r20", offsetof(CPUPPCState, gpr[20]) },
> +    { "r21", offsetof(CPUPPCState, gpr[21]) },
> +    { "r22", offsetof(CPUPPCState, gpr[22]) },
> +    { "r23", offsetof(CPUPPCState, gpr[23]) },
> +    { "r24", offsetof(CPUPPCState, gpr[24]) },
> +    { "r25", offsetof(CPUPPCState, gpr[25]) },
> +    { "r26", offsetof(CPUPPCState, gpr[26]) },
> +    { "r27", offsetof(CPUPPCState, gpr[27]) },
> +    { "r28", offsetof(CPUPPCState, gpr[28]) },
> +    { "r29", offsetof(CPUPPCState, gpr[29]) },
> +    { "r30", offsetof(CPUPPCState, gpr[30]) },
> +    { "r31", offsetof(CPUPPCState, gpr[31]) },
>      /* Floating point registers */
> -    { "f0", offsetof(CPUState, fpr[0]) },
> -    { "f1", offsetof(CPUState, fpr[1]) },
> -    { "f2", offsetof(CPUState, fpr[2]) },
> -    { "f3", offsetof(CPUState, fpr[3]) },
> -    { "f4", offsetof(CPUState, fpr[4]) },
> -    { "f5", offsetof(CPUState, fpr[5]) },
> -    { "f6", offsetof(CPUState, fpr[6]) },
> -    { "f7", offsetof(CPUState, fpr[7]) },
> -    { "f8", offsetof(CPUState, fpr[8]) },
> -    { "f9", offsetof(CPUState, fpr[9]) },
> -    { "f10", offsetof(CPUState, fpr[10]) },
> -    { "f11", offsetof(CPUState, fpr[11]) },
> -    { "f12", offsetof(CPUState, fpr[12]) },
> -    { "f13", offsetof(CPUState, fpr[13]) },
> -    { "f14", offsetof(CPUState, fpr[14]) },
> -    { "f15", offsetof(CPUState, fpr[15]) },
> -    { "f16", offsetof(CPUState, fpr[16]) },
> -    { "f17", offsetof(CPUState, fpr[17]) },
> -    { "f18", offsetof(CPUState, fpr[18]) },
> -    { "f19", offsetof(CPUState, fpr[19]) },
> -    { "f20", offsetof(CPUState, fpr[20]) },
> -    { "f21", offsetof(CPUState, fpr[21]) },
> -    { "f22", offsetof(CPUState, fpr[22]) },
> -    { "f23", offsetof(CPUState, fpr[23]) },
> -    { "f24", offsetof(CPUState, fpr[24]) },
> -    { "f25", offsetof(CPUState, fpr[25]) },
> -    { "f26", offsetof(CPUState, fpr[26]) },
> -    { "f27", offsetof(CPUState, fpr[27]) },
> -    { "f28", offsetof(CPUState, fpr[28]) },
> -    { "f29", offsetof(CPUState, fpr[29]) },
> -    { "f30", offsetof(CPUState, fpr[30]) },
> -    { "f31", offsetof(CPUState, fpr[31]) },
> -    { "fpscr", offsetof(CPUState, fpscr) },
> +    { "f0", offsetof(CPUPPCState, fpr[0]) },
> +    { "f1", offsetof(CPUPPCState, fpr[1]) },
> +    { "f2", offsetof(CPUPPCState, fpr[2]) },
> +    { "f3", offsetof(CPUPPCState, fpr[3]) },
> +    { "f4", offsetof(CPUPPCState, fpr[4]) },
> +    { "f5", offsetof(CPUPPCState, fpr[5]) },
> +    { "f6", offsetof(CPUPPCState, fpr[6]) },
> +    { "f7", offsetof(CPUPPCState, fpr[7]) },
> +    { "f8", offsetof(CPUPPCState, fpr[8]) },
> +    { "f9", offsetof(CPUPPCState, fpr[9]) },
> +    { "f10", offsetof(CPUPPCState, fpr[10]) },
> +    { "f11", offsetof(CPUPPCState, fpr[11]) },
> +    { "f12", offsetof(CPUPPCState, fpr[12]) },
> +    { "f13", offsetof(CPUPPCState, fpr[13]) },
> +    { "f14", offsetof(CPUPPCState, fpr[14]) },
> +    { "f15", offsetof(CPUPPCState, fpr[15]) },
> +    { "f16", offsetof(CPUPPCState, fpr[16]) },
> +    { "f17", offsetof(CPUPPCState, fpr[17]) },
> +    { "f18", offsetof(CPUPPCState, fpr[18]) },
> +    { "f19", offsetof(CPUPPCState, fpr[19]) },
> +    { "f20", offsetof(CPUPPCState, fpr[20]) },
> +    { "f21", offsetof(CPUPPCState, fpr[21]) },
> +    { "f22", offsetof(CPUPPCState, fpr[22]) },
> +    { "f23", offsetof(CPUPPCState, fpr[23]) },
> +    { "f24", offsetof(CPUPPCState, fpr[24]) },
> +    { "f25", offsetof(CPUPPCState, fpr[25]) },
> +    { "f26", offsetof(CPUPPCState, fpr[26]) },
> +    { "f27", offsetof(CPUPPCState, fpr[27]) },
> +    { "f28", offsetof(CPUPPCState, fpr[28]) },
> +    { "f29", offsetof(CPUPPCState, fpr[29]) },
> +    { "f30", offsetof(CPUPPCState, fpr[30]) },
> +    { "f31", offsetof(CPUPPCState, fpr[31]) },
> +    { "fpscr", offsetof(CPUPPCState, fpscr) },
>      /* Next instruction pointer */
> -    { "nip|pc", offsetof(CPUState, nip) },
> -    { "lr", offsetof(CPUState, lr) },
> -    { "ctr", offsetof(CPUState, ctr) },
> +    { "nip|pc", offsetof(CPUPPCState, nip) },
> +    { "lr", offsetof(CPUPPCState, lr) },
> +    { "ctr", offsetof(CPUPPCState, ctr) },
>      { "decr", 0, &monitor_get_decr, },
>      { "ccr", 0, &monitor_get_ccr, },
>      /* Machine state register */
> @@ -2808,105 +2808,105 @@ static const MonitorDef monitor_defs[] = {
>      { "tbl", 0, &monitor_get_tbl, },
>  #if defined(TARGET_PPC64)
>      /* Address space register */
> -    { "asr", offsetof(CPUState, asr) },
> +    { "asr", offsetof(CPUPPCState, asr) },
>  #endif
>      /* Segment registers */
> -    { "sdr1", offsetof(CPUState, spr[SPR_SDR1]) },
> -    { "sr0", offsetof(CPUState, sr[0]) },
> -    { "sr1", offsetof(CPUState, sr[1]) },
> -    { "sr2", offsetof(CPUState, sr[2]) },
> -    { "sr3", offsetof(CPUState, sr[3]) },
> -    { "sr4", offsetof(CPUState, sr[4]) },
> -    { "sr5", offsetof(CPUState, sr[5]) },
> -    { "sr6", offsetof(CPUState, sr[6]) },
> -    { "sr7", offsetof(CPUState, sr[7]) },
> -    { "sr8", offsetof(CPUState, sr[8]) },
> -    { "sr9", offsetof(CPUState, sr[9]) },
> -    { "sr10", offsetof(CPUState, sr[10]) },
> -    { "sr11", offsetof(CPUState, sr[11]) },
> -    { "sr12", offsetof(CPUState, sr[12]) },
> -    { "sr13", offsetof(CPUState, sr[13]) },
> -    { "sr14", offsetof(CPUState, sr[14]) },
> -    { "sr15", offsetof(CPUState, sr[15]) },
> +    { "sdr1", offsetof(CPUPPCState, spr[SPR_SDR1]) },
> +    { "sr0", offsetof(CPUPPCState, sr[0]) },
> +    { "sr1", offsetof(CPUPPCState, sr[1]) },
> +    { "sr2", offsetof(CPUPPCState, sr[2]) },
> +    { "sr3", offsetof(CPUPPCState, sr[3]) },
> +    { "sr4", offsetof(CPUPPCState, sr[4]) },
> +    { "sr5", offsetof(CPUPPCState, sr[5]) },
> +    { "sr6", offsetof(CPUPPCState, sr[6]) },
> +    { "sr7", offsetof(CPUPPCState, sr[7]) },
> +    { "sr8", offsetof(CPUPPCState, sr[8]) },
> +    { "sr9", offsetof(CPUPPCState, sr[9]) },
> +    { "sr10", offsetof(CPUPPCState, sr[10]) },
> +    { "sr11", offsetof(CPUPPCState, sr[11]) },
> +    { "sr12", offsetof(CPUPPCState, sr[12]) },
> +    { "sr13", offsetof(CPUPPCState, sr[13]) },
> +    { "sr14", offsetof(CPUPPCState, sr[14]) },
> +    { "sr15", offsetof(CPUPPCState, sr[15]) },
>      /* Too lazy to put BATs... */
> -    { "pvr", offsetof(CPUState, spr[SPR_PVR]) },
> +    { "pvr", offsetof(CPUPPCState, spr[SPR_PVR]) },
 
> -    { "srr0", offsetof(CPUState, spr[SPR_SRR0]) },
> -    { "srr1", offsetof(CPUState, spr[SPR_SRR1]) },
> -    { "sprg0", offsetof(CPUState, spr[SPR_SPRG0]) },
> -    { "sprg1", offsetof(CPUState, spr[SPR_SPRG1]) },
> -    { "sprg2", offsetof(CPUState, spr[SPR_SPRG2]) },
> -    { "sprg3", offsetof(CPUState, spr[SPR_SPRG3]) },
> -    { "sprg4", offsetof(CPUState, spr[SPR_SPRG4]) },
> -    { "sprg5", offsetof(CPUState, spr[SPR_SPRG5]) },
> -    { "sprg6", offsetof(CPUState, spr[SPR_SPRG6]) },
> -    { "sprg7", offsetof(CPUState, spr[SPR_SPRG7]) },
> -    { "pid", offsetof(CPUState, spr[SPR_BOOKE_PID]) },
> -    { "csrr0", offsetof(CPUState, spr[SPR_BOOKE_CSRR0]) },
> -    { "csrr1", offsetof(CPUState, spr[SPR_BOOKE_CSRR1]) },
> -    { "esr", offsetof(CPUState, spr[SPR_BOOKE_ESR]) },
> -    { "dear", offsetof(CPUState, spr[SPR_BOOKE_DEAR]) },
> -    { "mcsr", offsetof(CPUState, spr[SPR_BOOKE_MCSR]) },
> -    { "tsr", offsetof(CPUState, spr[SPR_BOOKE_TSR]) },
> -    { "tcr", offsetof(CPUState, spr[SPR_BOOKE_TCR]) },
> -    { "vrsave", offsetof(CPUState, spr[SPR_VRSAVE]) },
> -    { "pir", offsetof(CPUState, spr[SPR_BOOKE_PIR]) },
> -    { "mcsrr0", offsetof(CPUState, spr[SPR_BOOKE_MCSRR0]) },
> -    { "mcsrr1", offsetof(CPUState, spr[SPR_BOOKE_MCSRR1]) },
> -    { "decar", offsetof(CPUState, spr[SPR_BOOKE_DECAR]) },
> -    { "ivpr", offsetof(CPUState, spr[SPR_BOOKE_IVPR]) },
> -    { "epcr", offsetof(CPUState, spr[SPR_BOOKE_EPCR]) },
> -    { "sprg8", offsetof(CPUState, spr[SPR_BOOKE_SPRG8]) },
> -    { "ivor0", offsetof(CPUState, spr[SPR_BOOKE_IVOR0]) },
> -    { "ivor1", offsetof(CPUState, spr[SPR_BOOKE_IVOR1]) },
> -    { "ivor2", offsetof(CPUState, spr[SPR_BOOKE_IVOR2]) },
> -    { "ivor3", offsetof(CPUState, spr[SPR_BOOKE_IVOR3]) },
> -    { "ivor4", offsetof(CPUState, spr[SPR_BOOKE_IVOR4]) },
> -    { "ivor5", offsetof(CPUState, spr[SPR_BOOKE_IVOR5]) },
> -    { "ivor6", offsetof(CPUState, spr[SPR_BOOKE_IVOR6]) },
> -    { "ivor7", offsetof(CPUState, spr[SPR_BOOKE_IVOR7]) },
> -    { "ivor8", offsetof(CPUState, spr[SPR_BOOKE_IVOR8]) },
> -    { "ivor9", offsetof(CPUState, spr[SPR_BOOKE_IVOR9]) },
> -    { "ivor10", offsetof(CPUState, spr[SPR_BOOKE_IVOR10]) },
> -    { "ivor11", offsetof(CPUState, spr[SPR_BOOKE_IVOR11]) },
> -    { "ivor12", offsetof(CPUState, spr[SPR_BOOKE_IVOR12]) },
> -    { "ivor13", offsetof(CPUState, spr[SPR_BOOKE_IVOR13]) },
> -    { "ivor14", offsetof(CPUState, spr[SPR_BOOKE_IVOR14]) },
> -    { "ivor15", offsetof(CPUState, spr[SPR_BOOKE_IVOR15]) },
> -    { "ivor32", offsetof(CPUState, spr[SPR_BOOKE_IVOR32]) },
> -    { "ivor33", offsetof(CPUState, spr[SPR_BOOKE_IVOR33]) },
> -    { "ivor34", offsetof(CPUState, spr[SPR_BOOKE_IVOR34]) },
> -    { "ivor35", offsetof(CPUState, spr[SPR_BOOKE_IVOR35]) },
> -    { "ivor36", offsetof(CPUState, spr[SPR_BOOKE_IVOR36]) },
> -    { "ivor37", offsetof(CPUState, spr[SPR_BOOKE_IVOR37]) },
> -    { "mas0", offsetof(CPUState, spr[SPR_BOOKE_MAS0]) },
> -    { "mas1", offsetof(CPUState, spr[SPR_BOOKE_MAS1]) },
> -    { "mas2", offsetof(CPUState, spr[SPR_BOOKE_MAS2]) },
> -    { "mas3", offsetof(CPUState, spr[SPR_BOOKE_MAS3]) },
> -    { "mas4", offsetof(CPUState, spr[SPR_BOOKE_MAS4]) },
> -    { "mas6", offsetof(CPUState, spr[SPR_BOOKE_MAS6]) },
> -    { "mas7", offsetof(CPUState, spr[SPR_BOOKE_MAS7]) },
> -    { "mmucfg", offsetof(CPUState, spr[SPR_MMUCFG]) },
> -    { "tlb0cfg", offsetof(CPUState, spr[SPR_BOOKE_TLB0CFG]) },
> -    { "tlb1cfg", offsetof(CPUState, spr[SPR_BOOKE_TLB1CFG]) },
> -    { "epr", offsetof(CPUState, spr[SPR_BOOKE_EPR]) },
> -    { "eplc", offsetof(CPUState, spr[SPR_BOOKE_EPLC]) },
> -    { "epsc", offsetof(CPUState, spr[SPR_BOOKE_EPSC]) },
> -    { "svr", offsetof(CPUState, spr[SPR_E500_SVR]) },
> -    { "mcar", offsetof(CPUState, spr[SPR_Exxx_MCAR]) },
> -    { "pid1", offsetof(CPUState, spr[SPR_BOOKE_PID1]) },
> -    { "pid2", offsetof(CPUState, spr[SPR_BOOKE_PID2]) },
> -    { "hid0", offsetof(CPUState, spr[SPR_HID0]) },
> +    { "srr0", offsetof(CPUPPCState, spr[SPR_SRR0]) },
> +    { "srr1", offsetof(CPUPPCState, spr[SPR_SRR1]) },
> +    { "sprg0", offsetof(CPUPPCState, spr[SPR_SPRG0]) },
> +    { "sprg1", offsetof(CPUPPCState, spr[SPR_SPRG1]) },
> +    { "sprg2", offsetof(CPUPPCState, spr[SPR_SPRG2]) },
> +    { "sprg3", offsetof(CPUPPCState, spr[SPR_SPRG3]) },
> +    { "sprg4", offsetof(CPUPPCState, spr[SPR_SPRG4]) },
> +    { "sprg5", offsetof(CPUPPCState, spr[SPR_SPRG5]) },
> +    { "sprg6", offsetof(CPUPPCState, spr[SPR_SPRG6]) },
> +    { "sprg7", offsetof(CPUPPCState, spr[SPR_SPRG7]) },
> +    { "pid", offsetof(CPUPPCState, spr[SPR_BOOKE_PID]) },
> +    { "csrr0", offsetof(CPUPPCState, spr[SPR_BOOKE_CSRR0]) },
> +    { "csrr1", offsetof(CPUPPCState, spr[SPR_BOOKE_CSRR1]) },
> +    { "esr", offsetof(CPUPPCState, spr[SPR_BOOKE_ESR]) },
> +    { "dear", offsetof(CPUPPCState, spr[SPR_BOOKE_DEAR]) },
> +    { "mcsr", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSR]) },
> +    { "tsr", offsetof(CPUPPCState, spr[SPR_BOOKE_TSR]) },
> +    { "tcr", offsetof(CPUPPCState, spr[SPR_BOOKE_TCR]) },
> +    { "vrsave", offsetof(CPUPPCState, spr[SPR_VRSAVE]) },
> +    { "pir", offsetof(CPUPPCState, spr[SPR_BOOKE_PIR]) },
> +    { "mcsrr0", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSRR0]) },
> +    { "mcsrr1", offsetof(CPUPPCState, spr[SPR_BOOKE_MCSRR1]) },
> +    { "decar", offsetof(CPUPPCState, spr[SPR_BOOKE_DECAR]) },
> +    { "ivpr", offsetof(CPUPPCState, spr[SPR_BOOKE_IVPR]) },
> +    { "epcr", offsetof(CPUPPCState, spr[SPR_BOOKE_EPCR]) },
> +    { "sprg8", offsetof(CPUPPCState, spr[SPR_BOOKE_SPRG8]) },
> +    { "ivor0", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR0]) },
> +    { "ivor1", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR1]) },
> +    { "ivor2", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR2]) },
> +    { "ivor3", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR3]) },
> +    { "ivor4", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR4]) },
> +    { "ivor5", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR5]) },
> +    { "ivor6", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR6]) },
> +    { "ivor7", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR7]) },
> +    { "ivor8", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR8]) },
> +    { "ivor9", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR9]) },
> +    { "ivor10", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR10]) },
> +    { "ivor11", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR11]) },
> +    { "ivor12", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR12]) },
> +    { "ivor13", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR13]) },
> +    { "ivor14", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR14]) },
> +    { "ivor15", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR15]) },
> +    { "ivor32", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR32]) },
> +    { "ivor33", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR33]) },
> +    { "ivor34", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR34]) },
> +    { "ivor35", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR35]) },
> +    { "ivor36", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR36]) },
> +    { "ivor37", offsetof(CPUPPCState, spr[SPR_BOOKE_IVOR37]) },
> +    { "mas0", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS0]) },
> +    { "mas1", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS1]) },
> +    { "mas2", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS2]) },
> +    { "mas3", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS3]) },
> +    { "mas4", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS4]) },
> +    { "mas6", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS6]) },
> +    { "mas7", offsetof(CPUPPCState, spr[SPR_BOOKE_MAS7]) },
> +    { "mmucfg", offsetof(CPUPPCState, spr[SPR_MMUCFG]) },
> +    { "tlb0cfg", offsetof(CPUPPCState, spr[SPR_BOOKE_TLB0CFG]) },
> +    { "tlb1cfg", offsetof(CPUPPCState, spr[SPR_BOOKE_TLB1CFG]) },
> +    { "epr", offsetof(CPUPPCState, spr[SPR_BOOKE_EPR]) },
> +    { "eplc", offsetof(CPUPPCState, spr[SPR_BOOKE_EPLC]) },
> +    { "epsc", offsetof(CPUPPCState, spr[SPR_BOOKE_EPSC]) },
> +    { "svr", offsetof(CPUPPCState, spr[SPR_E500_SVR]) },
> +    { "mcar", offsetof(CPUPPCState, spr[SPR_Exxx_MCAR]) },
> +    { "pid1", offsetof(CPUPPCState, spr[SPR_BOOKE_PID1]) },
> +    { "pid2", offsetof(CPUPPCState, spr[SPR_BOOKE_PID2]) },
> +    { "hid0", offsetof(CPUPPCState, spr[SPR_HID0]) },
 
>  #elif defined(TARGET_SPARC)
> -    { "g0", offsetof(CPUState, gregs[0]) },
> -    { "g1", offsetof(CPUState, gregs[1]) },
> -    { "g2", offsetof(CPUState, gregs[2]) },
> -    { "g3", offsetof(CPUState, gregs[3]) },
> -    { "g4", offsetof(CPUState, gregs[4]) },
> -    { "g5", offsetof(CPUState, gregs[5]) },
> -    { "g6", offsetof(CPUState, gregs[6]) },
> -    { "g7", offsetof(CPUState, gregs[7]) },
> +    { "g0", offsetof(CPUSPARCState, gregs[0]) },
> +    { "g1", offsetof(CPUSPARCState, gregs[1]) },
> +    { "g2", offsetof(CPUSPARCState, gregs[2]) },
> +    { "g3", offsetof(CPUSPARCState, gregs[3]) },
> +    { "g4", offsetof(CPUSPARCState, gregs[4]) },
> +    { "g5", offsetof(CPUSPARCState, gregs[5]) },
> +    { "g6", offsetof(CPUSPARCState, gregs[6]) },
> +    { "g7", offsetof(CPUSPARCState, gregs[7]) },
>      { "o0", 0, monitor_get_reg },
>      { "o1", 1, monitor_get_reg },
>      { "o2", 2, monitor_get_reg },
> @@ -2931,72 +2931,72 @@ static const MonitorDef monitor_defs[] = {
>      { "i5", 21, monitor_get_reg },
>      { "i6", 22, monitor_get_reg },
>      { "i7", 23, monitor_get_reg },
> -    { "pc", offsetof(CPUState, pc) },
> -    { "npc", offsetof(CPUState, npc) },
> -    { "y", offsetof(CPUState, y) },
> +    { "pc", offsetof(CPUSPARCState, pc) },
> +    { "npc", offsetof(CPUSPARCState, npc) },
> +    { "y", offsetof(CPUSPARCState, y) },
>  #ifndef TARGET_SPARC64
>      { "psr", 0, &monitor_get_psr, },
> -    { "wim", offsetof(CPUState, wim) },
> +    { "wim", offsetof(CPUSPARCState, wim) },
>  #endif
> -    { "tbr", offsetof(CPUState, tbr) },
> -    { "fsr", offsetof(CPUState, fsr) },
> -    { "f0", offsetof(CPUState, fpr[0].l.upper) },
> -    { "f1", offsetof(CPUState, fpr[0].l.lower) },
> -    { "f2", offsetof(CPUState, fpr[1].l.upper) },
> -    { "f3", offsetof(CPUState, fpr[1].l.lower) },
> -    { "f4", offsetof(CPUState, fpr[2].l.upper) },
> -    { "f5", offsetof(CPUState, fpr[2].l.lower) },
> -    { "f6", offsetof(CPUState, fpr[3].l.upper) },
> -    { "f7", offsetof(CPUState, fpr[3].l.lower) },
> -    { "f8", offsetof(CPUState, fpr[4].l.upper) },
> -    { "f9", offsetof(CPUState, fpr[4].l.lower) },
> -    { "f10", offsetof(CPUState, fpr[5].l.upper) },
> -    { "f11", offsetof(CPUState, fpr[5].l.lower) },
> -    { "f12", offsetof(CPUState, fpr[6].l.upper) },
> -    { "f13", offsetof(CPUState, fpr[6].l.lower) },
> -    { "f14", offsetof(CPUState, fpr[7].l.upper) },
> -    { "f15", offsetof(CPUState, fpr[7].l.lower) },
> -    { "f16", offsetof(CPUState, fpr[8].l.upper) },
> -    { "f17", offsetof(CPUState, fpr[8].l.lower) },
> -    { "f18", offsetof(CPUState, fpr[9].l.upper) },
> -    { "f19", offsetof(CPUState, fpr[9].l.lower) },
> -    { "f20", offsetof(CPUState, fpr[10].l.upper) },
> -    { "f21", offsetof(CPUState, fpr[10].l.lower) },
> -    { "f22", offsetof(CPUState, fpr[11].l.upper) },
> -    { "f23", offsetof(CPUState, fpr[11].l.lower) },
> -    { "f24", offsetof(CPUState, fpr[12].l.upper) },
> -    { "f25", offsetof(CPUState, fpr[12].l.lower) },
> -    { "f26", offsetof(CPUState, fpr[13].l.upper) },
> -    { "f27", offsetof(CPUState, fpr[13].l.lower) },
> -    { "f28", offsetof(CPUState, fpr[14].l.upper) },
> -    { "f29", offsetof(CPUState, fpr[14].l.lower) },
> -    { "f30", offsetof(CPUState, fpr[15].l.upper) },
> -    { "f31", offsetof(CPUState, fpr[15].l.lower) },
> +    { "tbr", offsetof(CPUSPARCState, tbr) },
> +    { "fsr", offsetof(CPUSPARCState, fsr) },
> +    { "f0", offsetof(CPUSPARCState, fpr[0].l.upper) },
> +    { "f1", offsetof(CPUSPARCState, fpr[0].l.lower) },
> +    { "f2", offsetof(CPUSPARCState, fpr[1].l.upper) },
> +    { "f3", offsetof(CPUSPARCState, fpr[1].l.lower) },
> +    { "f4", offsetof(CPUSPARCState, fpr[2].l.upper) },
> +    { "f5", offsetof(CPUSPARCState, fpr[2].l.lower) },
> +    { "f6", offsetof(CPUSPARCState, fpr[3].l.upper) },
> +    { "f7", offsetof(CPUSPARCState, fpr[3].l.lower) },
> +    { "f8", offsetof(CPUSPARCState, fpr[4].l.upper) },
> +    { "f9", offsetof(CPUSPARCState, fpr[4].l.lower) },
> +    { "f10", offsetof(CPUSPARCState, fpr[5].l.upper) },
> +    { "f11", offsetof(CPUSPARCState, fpr[5].l.lower) },
> +    { "f12", offsetof(CPUSPARCState, fpr[6].l.upper) },
> +    { "f13", offsetof(CPUSPARCState, fpr[6].l.lower) },
> +    { "f14", offsetof(CPUSPARCState, fpr[7].l.upper) },
> +    { "f15", offsetof(CPUSPARCState, fpr[7].l.lower) },
> +    { "f16", offsetof(CPUSPARCState, fpr[8].l.upper) },
> +    { "f17", offsetof(CPUSPARCState, fpr[8].l.lower) },
> +    { "f18", offsetof(CPUSPARCState, fpr[9].l.upper) },
> +    { "f19", offsetof(CPUSPARCState, fpr[9].l.lower) },
> +    { "f20", offsetof(CPUSPARCState, fpr[10].l.upper) },
> +    { "f21", offsetof(CPUSPARCState, fpr[10].l.lower) },
> +    { "f22", offsetof(CPUSPARCState, fpr[11].l.upper) },
> +    { "f23", offsetof(CPUSPARCState, fpr[11].l.lower) },
> +    { "f24", offsetof(CPUSPARCState, fpr[12].l.upper) },
> +    { "f25", offsetof(CPUSPARCState, fpr[12].l.lower) },
> +    { "f26", offsetof(CPUSPARCState, fpr[13].l.upper) },
> +    { "f27", offsetof(CPUSPARCState, fpr[13].l.lower) },
> +    { "f28", offsetof(CPUSPARCState, fpr[14].l.upper) },
> +    { "f29", offsetof(CPUSPARCState, fpr[14].l.lower) },
> +    { "f30", offsetof(CPUSPARCState, fpr[15].l.upper) },
> +    { "f31", offsetof(CPUSPARCState, fpr[15].l.lower) },
>  #ifdef TARGET_SPARC64
> -    { "f32", offsetof(CPUState, fpr[16]) },
> -    { "f34", offsetof(CPUState, fpr[17]) },
> -    { "f36", offsetof(CPUState, fpr[18]) },
> -    { "f38", offsetof(CPUState, fpr[19]) },
> -    { "f40", offsetof(CPUState, fpr[20]) },
> -    { "f42", offsetof(CPUState, fpr[21]) },
> -    { "f44", offsetof(CPUState, fpr[22]) },
> -    { "f46", offsetof(CPUState, fpr[23]) },
> -    { "f48", offsetof(CPUState, fpr[24]) },
> -    { "f50", offsetof(CPUState, fpr[25]) },
> -    { "f52", offsetof(CPUState, fpr[26]) },
> -    { "f54", offsetof(CPUState, fpr[27]) },
> -    { "f56", offsetof(CPUState, fpr[28]) },
> -    { "f58", offsetof(CPUState, fpr[29]) },
> -    { "f60", offsetof(CPUState, fpr[30]) },
> -    { "f62", offsetof(CPUState, fpr[31]) },
> -    { "asi", offsetof(CPUState, asi) },
> -    { "pstate", offsetof(CPUState, pstate) },
> -    { "cansave", offsetof(CPUState, cansave) },
> -    { "canrestore", offsetof(CPUState, canrestore) },
> -    { "otherwin", offsetof(CPUState, otherwin) },
> -    { "wstate", offsetof(CPUState, wstate) },
> -    { "cleanwin", offsetof(CPUState, cleanwin) },
> -    { "fprs", offsetof(CPUState, fprs) },
> +    { "f32", offsetof(CPUSPARCState, fpr[16]) },
> +    { "f34", offsetof(CPUSPARCState, fpr[17]) },
> +    { "f36", offsetof(CPUSPARCState, fpr[18]) },
> +    { "f38", offsetof(CPUSPARCState, fpr[19]) },
> +    { "f40", offsetof(CPUSPARCState, fpr[20]) },
> +    { "f42", offsetof(CPUSPARCState, fpr[21]) },
> +    { "f44", offsetof(CPUSPARCState, fpr[22]) },
> +    { "f46", offsetof(CPUSPARCState, fpr[23]) },
> +    { "f48", offsetof(CPUSPARCState, fpr[24]) },
> +    { "f50", offsetof(CPUSPARCState, fpr[25]) },
> +    { "f52", offsetof(CPUSPARCState, fpr[26]) },
> +    { "f54", offsetof(CPUSPARCState, fpr[27]) },
> +    { "f56", offsetof(CPUSPARCState, fpr[28]) },
> +    { "f58", offsetof(CPUSPARCState, fpr[29]) },
> +    { "f60", offsetof(CPUSPARCState, fpr[30]) },
> +    { "f62", offsetof(CPUSPARCState, fpr[31]) },
> +    { "asi", offsetof(CPUSPARCState, asi) },
> +    { "pstate", offsetof(CPUSPARCState, pstate) },
> +    { "cansave", offsetof(CPUSPARCState, cansave) },
> +    { "canrestore", offsetof(CPUSPARCState, canrestore) },
> +    { "otherwin", offsetof(CPUSPARCState, otherwin) },
> +    { "wstate", offsetof(CPUSPARCState, wstate) },
> +    { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
> +    { "fprs", offsetof(CPUSPARCState, fprs) },
>  #endif
>  #endif
>      { NULL },
> -- 
> 1.7.7

Am 15.03.2012 17:15, schrieb Lluís Vilanova:
> Andreas Färber writes:
> 
>> Use CPUX86State etc. instead (hand-converted).
> 
> Is there any reason not to move this into CPU${arch}State and instead provide
> virtual methods in CPUState?

Yes, a very simple one: This is one of the prerequisite patches to
having a QOM CPUState struct we can place such virtual methods in.

Originally, not even reset was in there, but there were comments that my
series was getting too long, so I moved it out of the ARM series and
squashed it into what is now patch 43.

> For example (a CPUState argument is implicit on all methods):
> 
>     int reg_get_number(const char *regname);
>     const char* reg_get_name(int regnum);
> 
>     /* register validity for a specific instantiation must be chekced somehow */
>     bool reg_valid(int regnum);
> 
>     size_t reg_get_size(int regnum);
> 
>     /* multiple sizes must be handled somehow */
>     /* precondition: get_reg_validity(regnum) == true */
>     void reg_get_value(int regnum, void *buf);
>     void reg_set_value(int regnum, void *buf);
> 
> 
> This way, target-specific code would be moved where it belongs, partially
> merging (for example) code from both monitor.c, gdbstub.c and the target
> itself.

That is definitely the direction I want to go with QOM CPUs. I haven't
dug into gdbstub or monitor too deeply yet, beyond looking for ways to
make the CPUState -> CPUArchState conversion as small as possible.

In this case, I am not sure why the original authors chose not to place
the code into target-*. Maybe those reasons no longer apply.

Anyway, once all CPUs are converted - or if you're adventurous based on
my qom-cpu-wip branch - I'd be happy to review patches moving more stuff
into CPUState. :) Moving properties from CPUArchState into CPUState
seemed fairly mechanical, so I attacked the more challenging problem of
statically calculated offsets for TCG's icount first.

I'd also envision some of the #defines we have flying around in cpu.h to
turn into read-only CPU properties, like page size or number of MMU
modes, so that the CPU becomes more self-describing and doesn't pollute
the global namespace.

> The downside is that using pointers to set/get the value is pretty
> uncomfortable.

We can add cpu_...(CPUState, ...) wrapper functions, like cpu_reset().

Cheers,
Andreas

Andreas Färber writes:

> Am 15.03.2012 17:15, schrieb Lluís Vilanova:
>> Andreas Färber writes:
>> 
>>> Use CPUX86State etc. instead (hand-converted).
>> 
>> Is there any reason not to move this into CPU${arch}State and instead provide
>> virtual methods in CPUState?

> Yes, a very simple one: This is one of the prerequisite patches to
> having a QOM CPUState struct we can place such virtual methods in.

Sure, I meant besides that :)


> Originally, not even reset was in there, but there were comments that my
> series was getting too long, so I moved it out of the ARM series and
> squashed it into what is now patch 43.

>> For example (a CPUState argument is implicit on all methods):
>> 
>> int reg_get_number(const char *regname);
>> const char* reg_get_name(int regnum);
>> 
>> /* register validity for a specific instantiation must be chekced somehow */
>> bool reg_valid(int regnum);
>> 
>> size_t reg_get_size(int regnum);
>> 
>> /* multiple sizes must be handled somehow */
>> /* precondition: get_reg_validity(regnum) == true */
>> void reg_get_value(int regnum, void *buf);
>> void reg_set_value(int regnum, void *buf);
>> 
>> 
>> This way, target-specific code would be moved where it belongs, partially
>> merging (for example) code from both monitor.c, gdbstub.c and the target
>> itself.

> That is definitely the direction I want to go with QOM CPUs. I haven't
> dug into gdbstub or monitor too deeply yet, beyond looking for ways to
> make the CPUState -> CPUArchState conversion as small as possible.

Well, I just saw that when looking at your series.


> In this case, I am not sure why the original authors chose not to place
> the code into target-*. Maybe those reasons no longer apply.

> Anyway, once all CPUs are converted - or if you're adventurous based on
> my qom-cpu-wip branch - I'd be happy to review patches moving more stuff
> into CPUState. :) Moving properties from CPUArchState into CPUState
> seemed fairly mechanical, so I attacked the more challenging problem of
> statically calculated offsets for TCG's icount first.

> I'd also envision some of the #defines we have flying around in cpu.h to
> turn into read-only CPU properties, like page size or number of MMU
> modes, so that the CPU becomes more self-describing and doesn't pollute
> the global namespace.

>> The downside is that using pointers to set/get the value is pretty
>> uncomfortable.

> We can add cpu_...(CPUState, ...) wrapper functions, like cpu_reset().

What I meant is that it's much more straightforward to have

    type_t v = cpu_reg_get_value(REG_ARCH_REGID);

rather than

    type_t v;
    cpu_reg_get_value(REG_ARCH_REGID, &v);

The problem is that different registers can have different width. Even if you
decide for uint64_t (which seems reasonable), you can still have wider registers
(e.g., 128bit FP regs). But maybe those "uncommon" wider registers can be
handled separately.


Lluis