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@xxxxxxxxx>
> ---
> 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 */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
