| Message ID | 20210902161543.417092-22-f4bug@amsat.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | accel: Move has_work() from SysemuCPUOps to AccelOpsClass | expand |
On Thu, Sep 02, 2021 at 06:15:34PM +0200, Philippe Mathieu-Daudé wrote: > Each POWER cpu has its own has_work() implementation. Instead of > overloading CPUClass on each PowerPCCPUClass init, register the > generic ppc_cpu_has_work() handler, and have it call the POWER > specific has_work(). I don't quite see the rationale for introducing a second layer of indirection here. What's wrong with switching the base has_work for each cpu variant? > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > target/ppc/cpu-qom.h | 3 +++ > target/ppc/cpu_init.c | 26 ++++++++++++++++++-------- > 2 files changed, 21 insertions(+), 8 deletions(-) > > diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h > index 5800fa324e8..ff2bafcde6f 100644 > --- a/target/ppc/cpu-qom.h > +++ b/target/ppc/cpu-qom.h > @@ -189,6 +189,9 @@ struct PowerPCCPUClass { > int bfd_mach; > uint32_t l1_dcache_size, l1_icache_size; > #ifndef CONFIG_USER_ONLY > +#ifdef CONFIG_TCG > + bool (*has_work)(CPUState *cpu); > +#endif /* CONFIG_TCG */ > unsigned int gdb_num_sprs; > const char *gdb_spr_xml; > #endif > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c > index e2e721c2b81..bbad16cc1ec 100644 > --- a/target/ppc/cpu_init.c > +++ b/target/ppc/cpu_init.c > @@ -7583,6 +7583,7 @@ static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr) > return false; > } > > +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) > static bool cpu_has_work_POWER7(CPUState *cs) > { > PowerPCCPU *cpu = POWERPC_CPU(cs); > @@ -7616,12 +7617,12 @@ static bool cpu_has_work_POWER7(CPUState *cs) > return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); > } > } > +#endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ > > POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); > - CPUClass *cc = CPU_CLASS(oc); > > dc->fw_name = "PowerPC,POWER7"; > dc->desc = "POWER7"; > @@ -7630,7 +7631,6 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) > pcc->pcr_supported = PCR_COMPAT_2_06 | PCR_COMPAT_2_05; > pcc->init_proc = init_proc_POWER7; > pcc->check_pow = check_pow_nocheck; > - cc->has_work = cpu_has_work_POWER7; > pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | > PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | > PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | > @@ -7673,6 +7673,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) > pcc->lpcr_pm = LPCR_P7_PECE0 | LPCR_P7_PECE1 | LPCR_P7_PECE2; > pcc->mmu_model = POWERPC_MMU_2_06; > #if defined(CONFIG_SOFTMMU) > + pcc->has_work = cpu_has_work_POWER7; > pcc->hash64_opts = &ppc_hash64_opts_POWER7; > pcc->lrg_decr_bits = 32; > #endif > @@ -7743,6 +7744,7 @@ static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr) > return false; > } > > +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) > static bool cpu_has_work_POWER8(CPUState *cs) > { > PowerPCCPU *cpu = POWERPC_CPU(cs); > @@ -7784,12 +7786,12 @@ static bool cpu_has_work_POWER8(CPUState *cs) > return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); > } > } > +#endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ > > POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); > - CPUClass *cc = CPU_CLASS(oc); > > dc->fw_name = "PowerPC,POWER8"; > dc->desc = "POWER8"; > @@ -7798,7 +7800,6 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) > pcc->pcr_supported = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05; > pcc->init_proc = init_proc_POWER8; > pcc->check_pow = check_pow_nocheck; > - cc->has_work = cpu_has_work_POWER8; > pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | > PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | > PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | > @@ -7848,6 +7849,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) > LPCR_P8_PECE3 | LPCR_P8_PECE4; > pcc->mmu_model = POWERPC_MMU_2_07; > #if defined(CONFIG_SOFTMMU) > + pcc->has_work = cpu_has_work_POWER8; > pcc->hash64_opts = &ppc_hash64_opts_POWER7; > pcc->lrg_decr_bits = 32; > pcc->n_host_threads = 8; > @@ -7941,6 +7943,7 @@ static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr) > return false; > } > > +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) > static bool cpu_has_work_POWER9(CPUState *cs) > { > PowerPCCPU *cpu = POWERPC_CPU(cs); > @@ -7998,12 +8001,12 @@ static bool cpu_has_work_POWER9(CPUState *cs) > return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); > } > } > +#endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ > > POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); > - CPUClass *cc = CPU_CLASS(oc); > > dc->fw_name = "PowerPC,POWER9"; > dc->desc = "POWER9"; > @@ -8013,7 +8016,6 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data) > PCR_COMPAT_2_05; > pcc->init_proc = init_proc_POWER9; > pcc->check_pow = check_pow_nocheck; > - cc->has_work = cpu_has_work_POWER9; > pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | > PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | > PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | > @@ -8062,6 +8064,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data) > pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE; > pcc->mmu_model = POWERPC_MMU_3_00; > #if defined(CONFIG_SOFTMMU) > + pcc->has_work = cpu_has_work_POWER9; > /* segment page size remain the same */ > pcc->hash64_opts = &ppc_hash64_opts_POWER7; > pcc->radix_page_info = &POWER9_radix_page_info; > @@ -8150,6 +8153,7 @@ static bool ppc_pvr_match_power10(PowerPCCPUClass *pcc, uint32_t pvr) > return false; > } > > +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) > static bool cpu_has_work_POWER10(CPUState *cs) > { > PowerPCCPU *cpu = POWERPC_CPU(cs); > @@ -8207,12 +8211,12 @@ static bool cpu_has_work_POWER10(CPUState *cs) > return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); > } > } > +#endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ > > POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); > - CPUClass *cc = CPU_CLASS(oc); > > dc->fw_name = "PowerPC,POWER10"; > dc->desc = "POWER10"; > @@ -8223,7 +8227,6 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data) > PCR_COMPAT_2_06 | PCR_COMPAT_2_05; > pcc->init_proc = init_proc_POWER10; > pcc->check_pow = check_pow_nocheck; > - cc->has_work = cpu_has_work_POWER10; > pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | > PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | > PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | > @@ -8275,6 +8278,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data) > pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE; > pcc->mmu_model = POWERPC_MMU_3_00; > #if defined(CONFIG_SOFTMMU) > + pcc->has_work = cpu_has_work_POWER10; > /* segment page size remain the same */ > pcc->hash64_opts = &ppc_hash64_opts_POWER7; > pcc->radix_page_info = &POWER10_radix_page_info; > @@ -8796,6 +8800,12 @@ static bool ppc_cpu_has_work(CPUState *cs) > PowerPCCPU *cpu = POWERPC_CPU(cs); > CPUPPCState *env = &cpu->env; > > + if (cs->halted) { > + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); > + > + return pcc->has_work(cs); > + } > + > return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); > } > #endif /* CONFIG_TCG && !CONFIG_USER_ONLY */
On 9/3/21 2:50 AM, David Gibson wrote: > On Thu, Sep 02, 2021 at 06:15:34PM +0200, Philippe Mathieu-Daudé wrote: >> Each POWER cpu has its own has_work() implementation. Instead of >> overloading CPUClass on each PowerPCCPUClass init, register the >> generic ppc_cpu_has_work() handler, and have it call the POWER >> specific has_work(). > > I don't quite see the rationale for introducing a second layer of > indirection here. What's wrong with switching the base has_work for > each cpu variant? > >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> target/ppc/cpu-qom.h | 3 +++ >> target/ppc/cpu_init.c | 26 ++++++++++++++++++-------- >> 2 files changed, 21 insertions(+), 8 deletions(-) >> @@ -8796,6 +8800,12 @@ static bool ppc_cpu_has_work(CPUState *cs) >> PowerPCCPU *cpu = POWERPC_CPU(cs); >> CPUPPCState *env = &cpu->env; >> >> + if (cs->halted) { >> + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); Maybe I should have kept the 'if (cs->halted)' for the next patch, simply dispatch here, then in the next patch the code simplification is more apparent. I thought this approach would involve less #ifdef'ry but haven't checked the other way around. Will do now. >> + return pcc->has_work(cs); >> + } >> + >> return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); >> } >> #endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ >
On 9/3/21 2:50 AM, David Gibson wrote: > On Thu, Sep 02, 2021 at 06:15:34PM +0200, Philippe Mathieu-Daudé wrote: >> Each POWER cpu has its own has_work() implementation. Instead of >> overloading CPUClass on each PowerPCCPUClass init, register the >> generic ppc_cpu_has_work() handler, and have it call the POWER >> specific has_work(). > > I don't quite see the rationale for introducing a second layer of > indirection here. What's wrong with switching the base has_work for > each cpu variant? We're moving the hook from CPUState to TCGCPUOps. Phil was trying to avoid creating N versions of static const struct TCGCPUOps ppc_tcg_ops = { ... }; A plausible alternative is to remove the const from this struct and modify it, just as we do for CPUState, on the assumption that we cannot mix and match ppc cpu types in any one machine. r~
On 9/3/21 10:42 PM, Richard Henderson wrote: > On 9/3/21 2:50 AM, David Gibson wrote: >> On Thu, Sep 02, 2021 at 06:15:34PM +0200, Philippe Mathieu-Daudé wrote: >>> Each POWER cpu has its own has_work() implementation. Instead of >>> overloading CPUClass on each PowerPCCPUClass init, register the >>> generic ppc_cpu_has_work() handler, and have it call the POWER >>> specific has_work(). >> >> I don't quite see the rationale for introducing a second layer of >> indirection here. What's wrong with switching the base has_work for >> each cpu variant? > > We're moving the hook from CPUState to TCGCPUOps. > Phil was trying to avoid creating N versions of > > static const struct TCGCPUOps ppc_tcg_ops = { > ... > }; Ah yes this is the reason! Too many context switching so I forgot about it. > A plausible alternative is to remove the const from this struct and > modify it, just as we do for CPUState, on the assumption that we cannot > mix and match ppc cpu types in any one machine. I thought about this case and remembered how it works on the ARM arch, i.e. ZynqMP machine uses both Cortex-R5F and Cortex-A53. Even if no similar PPC machine exists, IMHO we should try to generally allow to possibility to experiment machine with different CPUs. Restricting it on PPC goes the other way around. Thoughts?
On 9/3/21 11:11 PM, Philippe Mathieu-Daudé wrote: > On 9/3/21 10:42 PM, Richard Henderson wrote: >> On 9/3/21 2:50 AM, David Gibson wrote: >>> On Thu, Sep 02, 2021 at 06:15:34PM +0200, Philippe Mathieu-Daudé wrote: >>>> Each POWER cpu has its own has_work() implementation. Instead of >>>> overloading CPUClass on each PowerPCCPUClass init, register the >>>> generic ppc_cpu_has_work() handler, and have it call the POWER >>>> specific has_work(). >>> >>> I don't quite see the rationale for introducing a second layer of >>> indirection here. What's wrong with switching the base has_work for >>> each cpu variant? >> >> We're moving the hook from CPUState to TCGCPUOps. >> Phil was trying to avoid creating N versions of >> >> static const struct TCGCPUOps ppc_tcg_ops = { >> ... >> }; > > Ah yes this is the reason! Too many context switching so > I forgot about it. > >> A plausible alternative is to remove the const from this struct and >> modify it, just as we do for CPUState, on the assumption that we cannot >> mix and match ppc cpu types in any one machine. > > I thought about this case and remembered how it works on the ARM arch, > i.e. ZynqMP machine uses both Cortex-R5F and Cortex-A53. Even if no > similar PPC machine exists, IMHO we should try to generally allow to > possibility to experiment machine with different CPUs. Restricting it > on PPC goes the other way around. Thoughts? I'm running out of ideas to do avoid the indirection and multiple copies of TCGCPUOps. I'm not giving up, I suppose I'm simply not seeing it... David, any suggestions?
On 9/11/21 3:31 PM, Philippe Mathieu-Daudé wrote: > On 9/3/21 11:11 PM, Philippe Mathieu-Daudé wrote: >> On 9/3/21 10:42 PM, Richard Henderson wrote: >>> On 9/3/21 2:50 AM, David Gibson wrote: >>>> On Thu, Sep 02, 2021 at 06:15:34PM +0200, Philippe Mathieu-Daudé wrote: >>>>> Each POWER cpu has its own has_work() implementation. Instead of >>>>> overloading CPUClass on each PowerPCCPUClass init, register the >>>>> generic ppc_cpu_has_work() handler, and have it call the POWER >>>>> specific has_work(). >>>> >>>> I don't quite see the rationale for introducing a second layer of >>>> indirection here. What's wrong with switching the base has_work for >>>> each cpu variant? >>> >>> We're moving the hook from CPUState to TCGCPUOps. >>> Phil was trying to avoid creating N versions of >>> >>> static const struct TCGCPUOps ppc_tcg_ops = { >>> ... >>> }; >> >> Ah yes this is the reason! Too many context switching so >> I forgot about it. >> >>> A plausible alternative is to remove the const from this struct and >>> modify it, just as we do for CPUState, on the assumption that we cannot >>> mix and match ppc cpu types in any one machine. >> >> I thought about this case and remembered how it works on the ARM arch, >> i.e. ZynqMP machine uses both Cortex-R5F and Cortex-A53. Even if no >> similar PPC machine exists, IMHO we should try to generally allow to >> possibility to experiment machine with different CPUs. Restricting it >> on PPC goes the other way around. Thoughts? > > I'm running out of ideas to do avoid the indirection and multiple > copies of TCGCPUOps. I'm not giving up, I suppose I'm simply not > seeing it... David, any suggestions? I think multiple copies of TCGCPUOps is the solution. Macro-ized, perhaps, so that the amount of typing is minimal across the versions. r~
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h index 5800fa324e8..ff2bafcde6f 100644 --- a/target/ppc/cpu-qom.h +++ b/target/ppc/cpu-qom.h @@ -189,6 +189,9 @@ struct PowerPCCPUClass { int bfd_mach; uint32_t l1_dcache_size, l1_icache_size; #ifndef CONFIG_USER_ONLY +#ifdef CONFIG_TCG + bool (*has_work)(CPUState *cpu); +#endif /* CONFIG_TCG */ unsigned int gdb_num_sprs; const char *gdb_spr_xml; #endif diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index e2e721c2b81..bbad16cc1ec 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -7583,6 +7583,7 @@ static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr) return false; } +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) static bool cpu_has_work_POWER7(CPUState *cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -7616,12 +7617,12 @@ static bool cpu_has_work_POWER7(CPUState *cs) return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); } } +#endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); - CPUClass *cc = CPU_CLASS(oc); dc->fw_name = "PowerPC,POWER7"; dc->desc = "POWER7"; @@ -7630,7 +7631,6 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) pcc->pcr_supported = PCR_COMPAT_2_06 | PCR_COMPAT_2_05; pcc->init_proc = init_proc_POWER7; pcc->check_pow = check_pow_nocheck; - cc->has_work = cpu_has_work_POWER7; pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | @@ -7673,6 +7673,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) pcc->lpcr_pm = LPCR_P7_PECE0 | LPCR_P7_PECE1 | LPCR_P7_PECE2; pcc->mmu_model = POWERPC_MMU_2_06; #if defined(CONFIG_SOFTMMU) + pcc->has_work = cpu_has_work_POWER7; pcc->hash64_opts = &ppc_hash64_opts_POWER7; pcc->lrg_decr_bits = 32; #endif @@ -7743,6 +7744,7 @@ static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr) return false; } +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) static bool cpu_has_work_POWER8(CPUState *cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -7784,12 +7786,12 @@ static bool cpu_has_work_POWER8(CPUState *cs) return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); } } +#endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); - CPUClass *cc = CPU_CLASS(oc); dc->fw_name = "PowerPC,POWER8"; dc->desc = "POWER8"; @@ -7798,7 +7800,6 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) pcc->pcr_supported = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05; pcc->init_proc = init_proc_POWER8; pcc->check_pow = check_pow_nocheck; - cc->has_work = cpu_has_work_POWER8; pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | @@ -7848,6 +7849,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data) LPCR_P8_PECE3 | LPCR_P8_PECE4; pcc->mmu_model = POWERPC_MMU_2_07; #if defined(CONFIG_SOFTMMU) + pcc->has_work = cpu_has_work_POWER8; pcc->hash64_opts = &ppc_hash64_opts_POWER7; pcc->lrg_decr_bits = 32; pcc->n_host_threads = 8; @@ -7941,6 +7943,7 @@ static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr) return false; } +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) static bool cpu_has_work_POWER9(CPUState *cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -7998,12 +8001,12 @@ static bool cpu_has_work_POWER9(CPUState *cs) return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); } } +#endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); - CPUClass *cc = CPU_CLASS(oc); dc->fw_name = "PowerPC,POWER9"; dc->desc = "POWER9"; @@ -8013,7 +8016,6 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data) PCR_COMPAT_2_05; pcc->init_proc = init_proc_POWER9; pcc->check_pow = check_pow_nocheck; - cc->has_work = cpu_has_work_POWER9; pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | @@ -8062,6 +8064,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data) pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE; pcc->mmu_model = POWERPC_MMU_3_00; #if defined(CONFIG_SOFTMMU) + pcc->has_work = cpu_has_work_POWER9; /* segment page size remain the same */ pcc->hash64_opts = &ppc_hash64_opts_POWER7; pcc->radix_page_info = &POWER9_radix_page_info; @@ -8150,6 +8153,7 @@ static bool ppc_pvr_match_power10(PowerPCCPUClass *pcc, uint32_t pvr) return false; } +#if defined(CONFIG_TCG) && !defined(CONFIG_USER_ONLY) static bool cpu_has_work_POWER10(CPUState *cs) { PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -8207,12 +8211,12 @@ static bool cpu_has_work_POWER10(CPUState *cs) return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); } } +#endif /* CONFIG_TCG && !CONFIG_USER_ONLY */ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); - CPUClass *cc = CPU_CLASS(oc); dc->fw_name = "PowerPC,POWER10"; dc->desc = "POWER10"; @@ -8223,7 +8227,6 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data) PCR_COMPAT_2_06 | PCR_COMPAT_2_05; pcc->init_proc = init_proc_POWER10; pcc->check_pow = check_pow_nocheck; - cc->has_work = cpu_has_work_POWER10; pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | @@ -8275,6 +8278,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data) pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE; pcc->mmu_model = POWERPC_MMU_3_00; #if defined(CONFIG_SOFTMMU) + pcc->has_work = cpu_has_work_POWER10; /* segment page size remain the same */ pcc->hash64_opts = &ppc_hash64_opts_POWER7; pcc->radix_page_info = &POWER10_radix_page_info; @@ -8796,6 +8800,12 @@ static bool ppc_cpu_has_work(CPUState *cs) PowerPCCPU *cpu = POWERPC_CPU(cs); CPUPPCState *env = &cpu->env; + if (cs->halted) { + PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); + + return pcc->has_work(cs); + } + return msr_ee && (cs->interrupt_request & CPU_INTERRUPT_HARD); } #endif /* CONFIG_TCG && !CONFIG_USER_ONLY */
Each POWER cpu has its own has_work() implementation. Instead of overloading CPUClass on each PowerPCCPUClass init, register the generic ppc_cpu_has_work() handler, and have it call the POWER specific has_work(). Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- target/ppc/cpu-qom.h | 3 +++ target/ppc/cpu_init.c | 26 ++++++++++++++++++-------- 2 files changed, 21 insertions(+), 8 deletions(-)