Cédric Le Goater <c...@kaod.org> writes: > Hello, > > On 12/1/21 16:17, Daniel Henrique Barboza wrote: >> Hi, >> >> In this new version the most significant change is in patch 6, >> where a new hflag allows us to not call the instruction helper >> inside translate.c unless we're absolutely certain that there >> is an instruction count event being sampled and active in the >> PMU. This change turned out to be a big boost in performance >> in the PMU emulation overall, most notably when dealing with >> cycle events that were calling the helper needlessly. >> >> This and all other changes were suggested by David in his review >> of the previous version. > > > patch 1-8 look good. I still have some questions on the exception > handling and how EBB are gated. > > I am asking to get the model right for the next step which should > be to modify the XIVE interrupt controller to generate External > EBB exceptions. > > One more comment, not for now, since the EBB patchset is nearly > ready. > > May be, it is time to think about introducing a per-CPU model > excp_handlers[] array indexed by POWERPC_EXCP_* exception > numbers and to duplicate some code for the sake of clarity. > > Fabiano, isn't it what you had in mind ?
I had basically changed env->excp_vectors to be an array of objects of the kind: struct PPCInterrupt { Object parent; int id; const char *name; target_ulong addr; ppc_intr_fn_t setup_regs; }; we would access it from powerpc_excp() with: intr = &env->excp_vectors[excp]; if (intr->setup_regs) { intr->setup_regs(cpu, intr, excp_model, ®s, &ignore); } I also had another series to move the exception models into QOM like this: struct PPCIntrModel { Object parent; int id; const char *name; target_ulong hreset_vector; target_ulong ivor_mask; target_ulong ivpr_mask; target_ulong excp_prefix; PPCInterrupt excp_vectors[POWERPC_EXCP_NB]; }; struct PPCIntrModelClass { ObjectClass parent_class; bool (*intr_little_endian)(CPUPPCState *env, bool hv); bool (*lpar_env_selection)(CPUPPCState *env); target_ulong (*filter_msr)(CPUPPCState *env); bool (*set_sixty_four_bit_mode)(CPUPPCState *env, target_ulong *msr); bool (*set_ail)(CPUPPCState *env, bool mmu_all_on, bool hv_escalation, bool hv, int *_ail); void (*prepare_tlb_miss)(PowerPCCPU *cpu, int excp, target_ulong *new_msr, target_ulong *msr); void (*debug_software_tlb)(CPUPPCState *env, int excp); void (*init_excp)(PPCIntrModel *im); }; So the powerpc_excp() code would become: PPCIntrModel *intr_model = &env->im; PPCInterrupt *intr; ... intr = &intr_model->entry_points[excp]; if (!intr->setup_regs) { cpu_abort(cs, "Raised an exception without defined vector %d\n", excp); } regs.new_nip = intr->addr | intr_model->excp_prefix; intr->setup_regs(cpu, intr, intr_model, ®s, &ignore); I'll rebase it all and work on reducing some of the complexity around QOM, which was pointed out by David in the previous version: https://lists.nongnu.org/archive/html/qemu-ppc/2021-06/msg00140.html Any other suggestions are welcome. > > Thanks, > > C.