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.


Changes from v8:
- patch 5:
  * overflow timer of PMC5 is now marked as NULL instead of absent
- patch 6:
  * new hflags HFLAGS_INSN_CNT added to track instruction count state
  * previous HFLAGS_MMCR0FC flag removed
  * pmu_count_insns() now works partially with user mode
- patch 9:
  * fixed interrupt comment
- v8 link: https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg05160.html


Daniel Henrique Barboza (9):
  target/ppc: introduce PMUEventType and PMU overflow timers
  target/ppc: PMU basic cycle count for pseries TCG
  target/ppc: PMU: update counters on PMCs r/w
  target/ppc: PMU: update counters on MMCR1 write
  target/ppc: enable PMU counter overflow with cycle events
  target/ppc: enable PMU instruction count
  target/ppc/power8-pmu.c: add PM_RUN_INST_CMPL (0xFA) event
  PPC64/TCG: Implement 'rfebb' instruction
  target/ppc/excp_helper.c: EBB handling adjustments

Gustavo Romero (1):
  target/ppc: PMU Event-Based exception support

 hw/ppc/spapr_cpu_core.c                |   1 +
 target/ppc/cpu.h                       |  60 +++-
 target/ppc/cpu_init.c                  |  46 ++-
 target/ppc/excp_helper.c               |  93 ++++++
 target/ppc/helper.h                    |   6 +
 target/ppc/helper_regs.c               |   7 +
 target/ppc/insn32.decode               |   5 +
 target/ppc/meson.build                 |   1 +
 target/ppc/power8-pmu-regs.c.inc       |  69 ++++-
 target/ppc/power8-pmu.c                | 386 +++++++++++++++++++++++++
 target/ppc/power8-pmu.h                |  26 ++
 target/ppc/spr_tcg.h                   |   5 +
 target/ppc/translate.c                 |  78 +++++
 target/ppc/translate/branch-impl.c.inc |  33 +++
 14 files changed, 801 insertions(+), 15 deletions(-)
 create mode 100644 target/ppc/power8-pmu.c
 create mode 100644 target/ppc/power8-pmu.h
 create mode 100644 target/ppc/translate/branch-impl.c.inc

-- 
2.31.1

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.
> 
> 
> Changes from v8:
> - patch 5:
>    * overflow timer of PMC5 is now marked as NULL instead of absent
> - patch 6:
>    * new hflags HFLAGS_INSN_CNT added to track instruction count state
>    * previous HFLAGS_MMCR0FC flag removed
>    * pmu_count_insns() now works partially with user mode
> - patch 9:
>    * fixed interrupt comment
> - v8 link: https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg05160.html
Applied patches 1-8 to ppc-next.


Thanks,

C.

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 ?

Thanks,

C.

Cédric Le Goater <clg@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, &regs, &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, &regs, &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.