* [PATCH v10 0/3] PMU-EBB support for PPC64 TCG @ 2022-02-08 19:48 Daniel Henrique Barboza 2022-02-08 19:48 ` [PATCH v10 1/3] target/ppc: fix indent of function parameters Daniel Henrique Barboza ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Daniel Henrique Barboza @ 2022-02-08 19:48 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, clg, david Hi, This small series finalizes the pending PMU-EBB support for PPC64 TCG. In theory this would be a re-send of patches 09 and 10 of the v9, but those patches were so off the mark with the recent exception changes that I ended up discarding them and doing from the start. Patch 1 is a trivial indent fix. I've read a lot of code in excp_helper.c and this mis-indent was being annoying. Patch 2 is the last bits of the PMU specific code before triggering EBBs. I chose to do it separately to not mix PMU specific code and EBB logic. Patch 3 is the re-implementation of what patches 09+10 of the v9 were doing back then. The most significant change is that we're now gating the exception before reaching powerpc_excp_books(). v9 link: https://lists.gnu.org/archive/html/qemu-devel/2021-12/msg00073.html Daniel Henrique Barboza (3): target/ppc: fix indent of function parameters target/ppc: finalize pre-EBB PMU logic target/ppc: EBB exception implementation target/ppc/excp_helper.c | 59 +++++++++++++++++++++++++++++++++++----- target/ppc/helper.h | 1 + target/ppc/power8-pmu.c | 48 +++++++++++++++++++++++++++++--- 3 files changed, 97 insertions(+), 11 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v10 1/3] target/ppc: fix indent of function parameters 2022-02-08 19:48 [PATCH v10 0/3] PMU-EBB support for PPC64 TCG Daniel Henrique Barboza @ 2022-02-08 19:48 ` Daniel Henrique Barboza 2022-02-10 2:20 ` David Gibson 2022-02-08 19:48 ` [PATCH v10 2/3] target/ppc: finalize pre-EBB PMU logic Daniel Henrique Barboza 2022-02-08 19:48 ` [PATCH v10 3/3] target/ppc: EBB exception implementation Daniel Henrique Barboza 2 siblings, 1 reply; 7+ messages in thread From: Daniel Henrique Barboza @ 2022-02-08 19:48 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, clg, david Fix indentation of powerpc_set_excp_state() and ppc_excp_apply_ail() parameters. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/excp_helper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index c107953dec..8a49a4ab90 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -265,9 +265,9 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp, * +--------------------------------------------------------------------+ */ static void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp, - target_ulong msr, - target_ulong *new_msr, - target_ulong *vector) + target_ulong msr, + target_ulong *new_msr, + target_ulong *vector) { #if defined(TARGET_PPC64) CPUPPCState *env = &cpu->env; @@ -362,7 +362,7 @@ static void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp, } static void powerpc_set_excp_state(PowerPCCPU *cpu, - target_ulong vector, target_ulong msr) + target_ulong vector, target_ulong msr) { CPUState *cs = CPU(cpu); CPUPPCState *env = &cpu->env; -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v10 1/3] target/ppc: fix indent of function parameters 2022-02-08 19:48 ` [PATCH v10 1/3] target/ppc: fix indent of function parameters Daniel Henrique Barboza @ 2022-02-10 2:20 ` David Gibson 0 siblings, 0 replies; 7+ messages in thread From: David Gibson @ 2022-02-10 2:20 UTC (permalink / raw) To: Daniel Henrique Barboza; +Cc: qemu-ppc, qemu-devel, clg [-- Attachment #1: Type: text/plain, Size: 1864 bytes --] On Tue, Feb 08, 2022 at 04:48:36PM -0300, Daniel Henrique Barboza wrote: > Fix indentation of powerpc_set_excp_state() and ppc_excp_apply_ail() > parameters. > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > target/ppc/excp_helper.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index c107953dec..8a49a4ab90 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -265,9 +265,9 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp, > * +--------------------------------------------------------------------+ > */ > static void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp, > - target_ulong msr, > - target_ulong *new_msr, > - target_ulong *vector) > + target_ulong msr, > + target_ulong *new_msr, > + target_ulong *vector) > { > #if defined(TARGET_PPC64) > CPUPPCState *env = &cpu->env; > @@ -362,7 +362,7 @@ static void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp, > } > > static void powerpc_set_excp_state(PowerPCCPU *cpu, > - target_ulong vector, target_ulong msr) > + target_ulong vector, target_ulong msr) > { > CPUState *cs = CPU(cpu); > CPUPPCState *env = &cpu->env; -- 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v10 2/3] target/ppc: finalize pre-EBB PMU logic 2022-02-08 19:48 [PATCH v10 0/3] PMU-EBB support for PPC64 TCG Daniel Henrique Barboza 2022-02-08 19:48 ` [PATCH v10 1/3] target/ppc: fix indent of function parameters Daniel Henrique Barboza @ 2022-02-08 19:48 ` Daniel Henrique Barboza 2022-02-08 19:48 ` [PATCH v10 3/3] target/ppc: EBB exception implementation Daniel Henrique Barboza 2 siblings, 0 replies; 7+ messages in thread From: Daniel Henrique Barboza @ 2022-02-08 19:48 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, clg, david There are still PMU exclusive bits to handle in fire_PMC_interrupt() before implementing the EBB support. Let's finalize it now to avoid dealing with PMU and EBB logic at the same time in the next patches. fire_PMC_interrupt() will fire an Performance Monitor alert depending on MMCR0_PMAE. If we are required to freeze the timers (MMCR0_FCECE) we'll also need to update summaries and delete the existing overflow timers. In all cases we're going to update the cycle counters. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/power8-pmu.c | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c index 236e8e66e9..d245663158 100644 --- a/target/ppc/power8-pmu.c +++ b/target/ppc/power8-pmu.c @@ -222,6 +222,20 @@ static void pmu_update_overflow_timers(CPUPPCState *env) } } +static void pmu_delete_timers(CPUPPCState *env) +{ + QEMUTimer *pmc_overflow_timer; + int sprn; + + for (sprn = SPR_POWER_PMC1; sprn <= SPR_POWER_PMC6; sprn++) { + pmc_overflow_timer = get_cyc_overflow_timer(env, sprn); + + if (pmc_overflow_timer) { + timer_del(pmc_overflow_timer); + } + } +} + void helper_store_mmcr0(CPUPPCState *env, target_ulong value) { bool hflags_pmcc0 = (value & MMCR0_PMCC0) != 0; @@ -271,8 +285,26 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu) { CPUPPCState *env = &cpu->env; - if (!(env->spr[SPR_POWER_MMCR0] & MMCR0_EBE)) { - return; + pmu_update_cycles(env); + + if (env->spr[SPR_POWER_MMCR0] & MMCR0_FCECE) { + env->spr[SPR_POWER_MMCR0] &= ~MMCR0_FCECE; + env->spr[SPR_POWER_MMCR0] |= MMCR0_FC; + + /* Changing MMCR0_FC requires a new HFLAGS_INSN_CNT calc */ + pmu_update_summaries(env); + + /* + * Delete all pending timers if we need to freeze + * the PMC. We'll restart them when the PMC starts + * running again. + */ + pmu_delete_timers(env); + } + + if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMAE) { + env->spr[SPR_POWER_MMCR0] &= ~MMCR0_PMAE; + env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO; } /* PMC interrupt not implemented yet */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v10 3/3] target/ppc: EBB exception implementation 2022-02-08 19:48 [PATCH v10 0/3] PMU-EBB support for PPC64 TCG Daniel Henrique Barboza 2022-02-08 19:48 ` [PATCH v10 1/3] target/ppc: fix indent of function parameters Daniel Henrique Barboza 2022-02-08 19:48 ` [PATCH v10 2/3] target/ppc: finalize pre-EBB PMU logic Daniel Henrique Barboza @ 2022-02-08 19:48 ` Daniel Henrique Barboza 2022-02-08 23:20 ` Fabiano Rosas 2 siblings, 1 reply; 7+ messages in thread From: Daniel Henrique Barboza @ 2022-02-08 19:48 UTC (permalink / raw) To: qemu-devel; +Cc: Daniel Henrique Barboza, qemu-ppc, clg, david This patch adds the EBB exception support that are triggered by Performance Monitor alerts. This happens when a Performance Monitor alert occurs and MMCR0_EBE, BESCR_PME and BESCR_GE are set. A 'ebb_excp_enabled' helper is called at the end of fire_PMC_interrupt() to fire the EBB exception, checking for FSCR and HFSCR support beforehand. In ppc_hw_interrupt() the generated EBB exception will be taken only if running in problem state and with BESCR_GE set. The check for BESCR_GE bit in this step is needed to avoid race conditions where we take an EBB, while the previous EBB is still inflight (BESCR_GE cleared), and SPR_EBBHR is not set yet. In this case we'll branch to env->nip = 0 and the guest will crash. The Linux kernel selftest 'lost_exception_test' is an example where this racing will occur. The code in powerpc_excp_books() is the default EBB handling described in the PowerISA v3.1: clear BESCR_GE, set BESCR_PMEO, save env->nip in SPR_EBBRR and redirect the execution to the address pointed by SPR_EBBHR. The already implemented 'rbebb' instruction is then able to return from the EBB by retrieving the NIP in SPR_EBBRR. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- target/ppc/excp_helper.c | 51 +++++++++++++++++++++++++++++++++++++--- target/ppc/helper.h | 1 + target/ppc/power8-pmu.c | 12 ++++++++-- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 8a49a4ab90..2a95cec39e 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -19,6 +19,7 @@ #include "qemu/osdep.h" #include "qemu/main-loop.h" #include "cpu.h" +#include "hw/ppc/ppc.h" #include "exec/exec-all.h" #include "internal.h" #include "helper_regs.h" @@ -990,8 +991,22 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) new_msr |= (target_ulong)MSR_HVB; new_msr |= env->msr & ((target_ulong)1 << MSR_RI); break; - case POWERPC_EXCP_THERM: /* Thermal interrupt */ case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */ + env->spr[SPR_BESCR] &= ~BESCR_GE; + env->spr[SPR_BESCR] |= BESCR_PMEO; + + /* + * Save NIP for rfebb insn in SPR_EBBRR. Next nip is + * stored in the EBB Handler SPR_EBBHR. + */ + env->spr[SPR_EBBRR] = env->nip; + powerpc_set_excp_state(cpu, env->spr[SPR_EBBHR], env->msr); + + /* + * This exception is handled in userspace. No need to proceed. + */ + return; + case POWERPC_EXCP_THERM: /* Thermal interrupt */ case POWERPC_EXCP_VPUA: /* Vector assist exception */ case POWERPC_EXCP_MAINT: /* Maintenance exception */ case POWERPC_EXCP_SDOOR: /* Doorbell interrupt */ @@ -1671,8 +1686,14 @@ static void ppc_hw_interrupt(CPUPPCState *env) return; } if (env->pending_interrupts & (1 << PPC_INTERRUPT_PERFM)) { - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM); - powerpc_excp(cpu, POWERPC_EXCP_PERFM); + /* + * PERFM EBB must be taken in problem state and + * with BESCR_GE set. + */ + if (msr_pr == 1 && env->spr[SPR_BESCR] & BESCR_GE) { + env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM); + powerpc_excp(cpu, POWERPC_EXCP_PERFM); + } return; } /* Thermal interrupt */ @@ -1915,6 +1936,30 @@ void helper_rfebb(CPUPPCState *env, target_ulong s) env->spr[SPR_BESCR] &= ~BESCR_GE; } } + +void helper_ebb_perfm_int(CPUPPCState *env) +{ + PowerPCCPU *cpu = env_archcpu(env); + + /* + * FSCR_EBB and FSCR_IC_EBB are the same bits used with + * HFSCR. + */ + helper_fscr_facility_check(env, FSCR_EBB, 0, FSCR_IC_EBB); + helper_hfscr_facility_check(env, FSCR_EBB, "EBB", FSCR_IC_EBB); + + /* + * Setting "env->pending_interrupts |= 1 << PPC_INTERRUPT_PERFM" + * instead of calling "ppc_set_irq()"" works in most cases, but under + * certain race conditions (e.g. lost_exception_test EBB kernel + * selftest) this hits an assert when dealing with the BQL: + * + * tcg_handle_interrupt: assertion failed: (qemu_mutex_iothread_locked()) + * + * We ended up using ppc_set_irq() because it handles the BQL. + */ + ppc_set_irq(cpu, PPC_INTERRUPT_PERFM, 1); +} #endif /*****************************************************************************/ diff --git a/target/ppc/helper.h b/target/ppc/helper.h index f2e5060910..bb26da6176 100644 --- a/target/ppc/helper.h +++ b/target/ppc/helper.h @@ -19,6 +19,7 @@ DEF_HELPER_1(rfid, void, env) DEF_HELPER_1(rfscv, void, env) DEF_HELPER_1(hrfid, void, env) DEF_HELPER_2(rfebb, void, env, tl) +DEF_HELPER_1(ebb_perfm_int, void, env) DEF_HELPER_2(store_lpcr, void, env, tl) DEF_HELPER_2(store_pcr, void, env, tl) DEF_HELPER_2(store_mmcr0, void, env, tl) diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c index d245663158..41409e609f 100644 --- a/target/ppc/power8-pmu.c +++ b/target/ppc/power8-pmu.c @@ -281,6 +281,13 @@ void helper_store_pmc(CPUPPCState *env, uint32_t sprn, uint64_t value) pmc_update_overflow_timer(env, sprn); } +static bool ebb_excp_enabled(CPUPPCState *env) +{ + return env->spr[SPR_POWER_MMCR0] & MMCR0_EBE && + env->spr[SPR_BESCR] & BESCR_PME && + env->spr[SPR_BESCR] & BESCR_GE; +} + static void fire_PMC_interrupt(PowerPCCPU *cpu) { CPUPPCState *env = &cpu->env; @@ -307,8 +314,9 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu) env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO; } - /* PMC interrupt not implemented yet */ - return; + if (ebb_excp_enabled(env)) { + helper_ebb_perfm_int(env); + } } /* This helper assumes that the PMC is running. */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v10 3/3] target/ppc: EBB exception implementation 2022-02-08 19:48 ` [PATCH v10 3/3] target/ppc: EBB exception implementation Daniel Henrique Barboza @ 2022-02-08 23:20 ` Fabiano Rosas 2022-02-09 12:40 ` Daniel Henrique Barboza 0 siblings, 1 reply; 7+ messages in thread From: Fabiano Rosas @ 2022-02-08 23:20 UTC (permalink / raw) To: Daniel Henrique Barboza, qemu-devel Cc: Daniel Henrique Barboza, qemu-ppc, clg, david Daniel Henrique Barboza <danielhb413@gmail.com> writes: > This patch adds the EBB exception support that are triggered by > Performance Monitor alerts. This happens when a Performance Monitor > alert occurs and MMCR0_EBE, BESCR_PME and BESCR_GE are set. > > A 'ebb_excp_enabled' helper is called at the end of fire_PMC_interrupt() > to fire the EBB exception, checking for FSCR and HFSCR support > beforehand. > > In ppc_hw_interrupt() the generated EBB exception will be taken only if > running in problem state and with BESCR_GE set. The check for BESCR_GE > bit in this step is needed to avoid race conditions where we take an > EBB, while the previous EBB is still inflight (BESCR_GE cleared), and > SPR_EBBHR is not set yet. In this case we'll branch to env->nip = 0 and the > guest will crash. The Linux kernel selftest 'lost_exception_test' is an > example where this racing will occur. > > The code in powerpc_excp_books() is the default EBB handling described > in the PowerISA v3.1: clear BESCR_GE, set BESCR_PMEO, save env->nip in > SPR_EBBRR and redirect the execution to the address pointed by > SPR_EBBHR. The already implemented 'rbebb' instruction is then able to > return from the EBB by retrieving the NIP in SPR_EBBRR. > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> Now that BookS code is separate from the other CPUs, let me leave this here: Why do we have "interrupts" before "exceptions"? As in ppc_hw_interrupt calling powerpc_excp. If anyone has a consistent mental model on how this that they could share I'd appreciate it. Now onto the patch: > --- > target/ppc/excp_helper.c | 51 +++++++++++++++++++++++++++++++++++++--- > target/ppc/helper.h | 1 + > target/ppc/power8-pmu.c | 12 ++++++++-- > 3 files changed, 59 insertions(+), 5 deletions(-) > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index 8a49a4ab90..2a95cec39e 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -19,6 +19,7 @@ > #include "qemu/osdep.h" > #include "qemu/main-loop.h" > #include "cpu.h" > +#include "hw/ppc/ppc.h" > #include "exec/exec-all.h" > #include "internal.h" > #include "helper_regs.h" > @@ -990,8 +991,22 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) > new_msr |= (target_ulong)MSR_HVB; > new_msr |= env->msr & ((target_ulong)1 << MSR_RI); > break; > - case POWERPC_EXCP_THERM: /* Thermal interrupt */ > case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */ We would need some way to tell apart EBB from other Performance Monitor interrupts here. Unless you want to leave that to the next person that looks at Performance Monitor interrupts. > + env->spr[SPR_BESCR] &= ~BESCR_GE; > + env->spr[SPR_BESCR] |= BESCR_PMEO; > + > + /* > + * Save NIP for rfebb insn in SPR_EBBRR. Next nip is > + * stored in the EBB Handler SPR_EBBHR. > + */ > + env->spr[SPR_EBBRR] = env->nip; > + powerpc_set_excp_state(cpu, env->spr[SPR_EBBHR], env->msr); > + > + /* > + * This exception is handled in userspace. No need to proceed. > + */ > + return; > + case POWERPC_EXCP_THERM: /* Thermal interrupt */ > case POWERPC_EXCP_VPUA: /* Vector assist exception */ > case POWERPC_EXCP_MAINT: /* Maintenance exception */ > case POWERPC_EXCP_SDOOR: /* Doorbell interrupt */ > @@ -1671,8 +1686,14 @@ static void ppc_hw_interrupt(CPUPPCState *env) > return; > } > if (env->pending_interrupts & (1 << PPC_INTERRUPT_PERFM)) { > - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM); > - powerpc_excp(cpu, POWERPC_EXCP_PERFM); > + /* > + * PERFM EBB must be taken in problem state and > + * with BESCR_GE set. > + */ > + if (msr_pr == 1 && env->spr[SPR_BESCR] & BESCR_GE) { > + env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM); > + powerpc_excp(cpu, POWERPC_EXCP_PERFM); > + } This is masking other Performance Interrupts (for all CPUs). Can we move these checks into the helper? > return; > } > /* Thermal interrupt */ > @@ -1915,6 +1936,30 @@ void helper_rfebb(CPUPPCState *env, target_ulong s) > env->spr[SPR_BESCR] &= ~BESCR_GE; > } > } > + > +void helper_ebb_perfm_int(CPUPPCState *env) > +{ > + PowerPCCPU *cpu = env_archcpu(env); > + > + /* > + * FSCR_EBB and FSCR_IC_EBB are the same bits used with > + * HFSCR. > + */ > + helper_fscr_facility_check(env, FSCR_EBB, 0, FSCR_IC_EBB); > + helper_hfscr_facility_check(env, FSCR_EBB, "EBB", FSCR_IC_EBB); > + > + /* > + * Setting "env->pending_interrupts |= 1 << PPC_INTERRUPT_PERFM" > + * instead of calling "ppc_set_irq()"" works in most cases, but under > + * certain race conditions (e.g. lost_exception_test EBB kernel > + * selftest) this hits an assert when dealing with the BQL: > + * > + * tcg_handle_interrupt: assertion failed: (qemu_mutex_iothread_locked()) > + * > + * We ended up using ppc_set_irq() because it handles the BQL. > + */ > + ppc_set_irq(cpu, PPC_INTERRUPT_PERFM, 1); > +} > #endif > > /*****************************************************************************/ > diff --git a/target/ppc/helper.h b/target/ppc/helper.h > index f2e5060910..bb26da6176 100644 > --- a/target/ppc/helper.h > +++ b/target/ppc/helper.h > @@ -19,6 +19,7 @@ DEF_HELPER_1(rfid, void, env) > DEF_HELPER_1(rfscv, void, env) > DEF_HELPER_1(hrfid, void, env) > DEF_HELPER_2(rfebb, void, env, tl) > +DEF_HELPER_1(ebb_perfm_int, void, env) > DEF_HELPER_2(store_lpcr, void, env, tl) > DEF_HELPER_2(store_pcr, void, env, tl) > DEF_HELPER_2(store_mmcr0, void, env, tl) > diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c > index d245663158..41409e609f 100644 > --- a/target/ppc/power8-pmu.c > +++ b/target/ppc/power8-pmu.c > @@ -281,6 +281,13 @@ void helper_store_pmc(CPUPPCState *env, uint32_t sprn, uint64_t value) > pmc_update_overflow_timer(env, sprn); > } > > +static bool ebb_excp_enabled(CPUPPCState *env) > +{ > + return env->spr[SPR_POWER_MMCR0] & MMCR0_EBE && > + env->spr[SPR_BESCR] & BESCR_PME && > + env->spr[SPR_BESCR] & BESCR_GE; > +} > + > static void fire_PMC_interrupt(PowerPCCPU *cpu) > { > CPUPPCState *env = &cpu->env; > @@ -307,8 +314,9 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu) > env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO; > } > > - /* PMC interrupt not implemented yet */ > - return; > + if (ebb_excp_enabled(env)) { > + helper_ebb_perfm_int(env); > + } > } > > /* This helper assumes that the PMC is running. */ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v10 3/3] target/ppc: EBB exception implementation 2022-02-08 23:20 ` Fabiano Rosas @ 2022-02-09 12:40 ` Daniel Henrique Barboza 0 siblings, 0 replies; 7+ messages in thread From: Daniel Henrique Barboza @ 2022-02-09 12:40 UTC (permalink / raw) To: Fabiano Rosas, qemu-devel; +Cc: qemu-ppc, clg, david On 2/8/22 20:20, Fabiano Rosas wrote: > Daniel Henrique Barboza <danielhb413@gmail.com> writes: > >> This patch adds the EBB exception support that are triggered by >> Performance Monitor alerts. This happens when a Performance Monitor >> alert occurs and MMCR0_EBE, BESCR_PME and BESCR_GE are set. >> >> A 'ebb_excp_enabled' helper is called at the end of fire_PMC_interrupt() >> to fire the EBB exception, checking for FSCR and HFSCR support >> beforehand. >> >> In ppc_hw_interrupt() the generated EBB exception will be taken only if >> running in problem state and with BESCR_GE set. The check for BESCR_GE >> bit in this step is needed to avoid race conditions where we take an >> EBB, while the previous EBB is still inflight (BESCR_GE cleared), and >> SPR_EBBHR is not set yet. In this case we'll branch to env->nip = 0 and the >> guest will crash. The Linux kernel selftest 'lost_exception_test' is an >> example where this racing will occur. >> >> The code in powerpc_excp_books() is the default EBB handling described >> in the PowerISA v3.1: clear BESCR_GE, set BESCR_PMEO, save env->nip in >> SPR_EBBRR and redirect the execution to the address pointed by >> SPR_EBBHR. The already implemented 'rbebb' instruction is then able to >> return from the EBB by retrieving the NIP in SPR_EBBRR. >> >> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> > > Now that BookS code is separate from the other CPUs, let me leave this > here: > > Why do we have "interrupts" before "exceptions"? As in ppc_hw_interrupt > calling powerpc_excp. > > If anyone has a consistent mental model on how this that they could > share I'd appreciate it. > > Now onto the patch: >> --- >> target/ppc/excp_helper.c | 51 +++++++++++++++++++++++++++++++++++++--- >> target/ppc/helper.h | 1 + >> target/ppc/power8-pmu.c | 12 ++++++++-- >> 3 files changed, 59 insertions(+), 5 deletions(-) >> >> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c >> index 8a49a4ab90..2a95cec39e 100644 >> --- a/target/ppc/excp_helper.c >> +++ b/target/ppc/excp_helper.c >> @@ -19,6 +19,7 @@ >> #include "qemu/osdep.h" >> #include "qemu/main-loop.h" >> #include "cpu.h" >> +#include "hw/ppc/ppc.h" >> #include "exec/exec-all.h" >> #include "internal.h" >> #include "helper_regs.h" >> @@ -990,8 +991,22 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp) >> new_msr |= (target_ulong)MSR_HVB; >> new_msr |= env->msr & ((target_ulong)1 << MSR_RI); >> break; >> - case POWERPC_EXCP_THERM: /* Thermal interrupt */ >> case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */ > > We would need some way to tell apart EBB from other Performance Monitor > interrupts here. Unless you want to leave that to the next person that > looks at Performance Monitor interrupts. I don't mind doing that, but bear in mind that EBB isn't an official interrupt/exception vector in the ISA. > >> + env->spr[SPR_BESCR] &= ~BESCR_GE; >> + env->spr[SPR_BESCR] |= BESCR_PMEO; >> + >> + /* >> + * Save NIP for rfebb insn in SPR_EBBRR. Next nip is >> + * stored in the EBB Handler SPR_EBBHR. >> + */ >> + env->spr[SPR_EBBRR] = env->nip; >> + powerpc_set_excp_state(cpu, env->spr[SPR_EBBHR], env->msr); >> + >> + /* >> + * This exception is handled in userspace. No need to proceed. >> + */ >> + return; >> + case POWERPC_EXCP_THERM: /* Thermal interrupt */ >> case POWERPC_EXCP_VPUA: /* Vector assist exception */ >> case POWERPC_EXCP_MAINT: /* Maintenance exception */ >> case POWERPC_EXCP_SDOOR: /* Doorbell interrupt */ >> @@ -1671,8 +1686,14 @@ static void ppc_hw_interrupt(CPUPPCState *env) >> return; >> } >> if (env->pending_interrupts & (1 << PPC_INTERRUPT_PERFM)) { >> - env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM); >> - powerpc_excp(cpu, POWERPC_EXCP_PERFM); >> + /* >> + * PERFM EBB must be taken in problem state and >> + * with BESCR_GE set. >> + */ >> + if (msr_pr == 1 && env->spr[SPR_BESCR] & BESCR_GE) { >> + env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM); >> + powerpc_excp(cpu, POWERPC_EXCP_PERFM); >> + } > > This is masking other Performance Interrupts (for all CPUs). Can we move > these checks into the helper? Probably. I'll try it out. Daniel > >> return; >> } >> /* Thermal interrupt */ >> @@ -1915,6 +1936,30 @@ void helper_rfebb(CPUPPCState *env, target_ulong s) >> env->spr[SPR_BESCR] &= ~BESCR_GE; >> } >> } >> + >> +void helper_ebb_perfm_int(CPUPPCState *env) >> +{ >> + PowerPCCPU *cpu = env_archcpu(env); >> + >> + /* >> + * FSCR_EBB and FSCR_IC_EBB are the same bits used with >> + * HFSCR. >> + */ >> + helper_fscr_facility_check(env, FSCR_EBB, 0, FSCR_IC_EBB); >> + helper_hfscr_facility_check(env, FSCR_EBB, "EBB", FSCR_IC_EBB); >> + >> + /* >> + * Setting "env->pending_interrupts |= 1 << PPC_INTERRUPT_PERFM" >> + * instead of calling "ppc_set_irq()"" works in most cases, but under >> + * certain race conditions (e.g. lost_exception_test EBB kernel >> + * selftest) this hits an assert when dealing with the BQL: >> + * >> + * tcg_handle_interrupt: assertion failed: (qemu_mutex_iothread_locked()) >> + * >> + * We ended up using ppc_set_irq() because it handles the BQL. >> + */ >> + ppc_set_irq(cpu, PPC_INTERRUPT_PERFM, 1); >> +} >> #endif >> >> /*****************************************************************************/ >> diff --git a/target/ppc/helper.h b/target/ppc/helper.h >> index f2e5060910..bb26da6176 100644 >> --- a/target/ppc/helper.h >> +++ b/target/ppc/helper.h >> @@ -19,6 +19,7 @@ DEF_HELPER_1(rfid, void, env) >> DEF_HELPER_1(rfscv, void, env) >> DEF_HELPER_1(hrfid, void, env) >> DEF_HELPER_2(rfebb, void, env, tl) >> +DEF_HELPER_1(ebb_perfm_int, void, env) >> DEF_HELPER_2(store_lpcr, void, env, tl) >> DEF_HELPER_2(store_pcr, void, env, tl) >> DEF_HELPER_2(store_mmcr0, void, env, tl) >> diff --git a/target/ppc/power8-pmu.c b/target/ppc/power8-pmu.c >> index d245663158..41409e609f 100644 >> --- a/target/ppc/power8-pmu.c >> +++ b/target/ppc/power8-pmu.c >> @@ -281,6 +281,13 @@ void helper_store_pmc(CPUPPCState *env, uint32_t sprn, uint64_t value) >> pmc_update_overflow_timer(env, sprn); >> } >> >> +static bool ebb_excp_enabled(CPUPPCState *env) >> +{ >> + return env->spr[SPR_POWER_MMCR0] & MMCR0_EBE && >> + env->spr[SPR_BESCR] & BESCR_PME && >> + env->spr[SPR_BESCR] & BESCR_GE; >> +} >> + >> static void fire_PMC_interrupt(PowerPCCPU *cpu) >> { >> CPUPPCState *env = &cpu->env; >> @@ -307,8 +314,9 @@ static void fire_PMC_interrupt(PowerPCCPU *cpu) >> env->spr[SPR_POWER_MMCR0] |= MMCR0_PMAO; >> } >> >> - /* PMC interrupt not implemented yet */ >> - return; >> + if (ebb_excp_enabled(env)) { >> + helper_ebb_perfm_int(env); >> + } >> } >> >> /* This helper assumes that the PMC is running. */ ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-02-10 4:27 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-08 19:48 [PATCH v10 0/3] PMU-EBB support for PPC64 TCG Daniel Henrique Barboza 2022-02-08 19:48 ` [PATCH v10 1/3] target/ppc: fix indent of function parameters Daniel Henrique Barboza 2022-02-10 2:20 ` David Gibson 2022-02-08 19:48 ` [PATCH v10 2/3] target/ppc: finalize pre-EBB PMU logic Daniel Henrique Barboza 2022-02-08 19:48 ` [PATCH v10 3/3] target/ppc: EBB exception implementation Daniel Henrique Barboza 2022-02-08 23:20 ` Fabiano Rosas 2022-02-09 12:40 ` Daniel Henrique Barboza
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.