On Apr 12 17:49, Peter Maydell wrote: > On 16 March 2018 at 20:31, Aaron Lindsay <alind...@codeaurora.org> wrote: > > Because the design of the PMU requires that the counter values be > > converted between their delta and guest-visible forms for mode > > filtering, an additional hook which occurs before the EL is changed is > > necessary. > > > > Signed-off-by: Aaron Lindsay <alind...@codeaurora.org> > > --- > > target/arm/cpu.c | 13 +++++++++++++ > > target/arm/cpu.h | 12 ++++++++---- > > target/arm/helper.c | 14 ++++++++------ > > target/arm/internals.h | 7 +++++++ > > target/arm/op_helper.c | 8 ++++++++ > > 5 files changed, 44 insertions(+), 10 deletions(-) > > > > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > > index 5f782bf..a2cb21e 100644 > > --- a/target/arm/cpu.c > > +++ b/target/arm/cpu.c > > @@ -55,6 +55,18 @@ static bool arm_cpu_has_work(CPUState *cs) > > | CPU_INTERRUPT_EXITTB); > > } > > > > +void arm_register_pre_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook, > > + void *opaque) > > +{ > > + ARMELChangeHook *entry; > > + entry = g_malloc0(sizeof (*entry)); > > g_new0(). > > > + > > + entry->hook = hook; > > + entry->opaque = opaque; > > + > > + QLIST_INSERT_HEAD(&cpu->pre_el_change_hooks, entry, node); > > +} > > + > > void arm_register_el_change_hook(ARMCPU *cpu, ARMELChangeHookFn *hook, > > void *opaque) > > { > > @@ -747,6 +759,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error > > **errp) > > return; > > } > > > > + QLIST_INIT(&cpu->pre_el_change_hooks); > > QLIST_INIT(&cpu->el_change_hooks); > > > > /* Some features automatically imply others: */ > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > > index 3b45d3d..b0ef727 100644 > > --- a/target/arm/cpu.h > > +++ b/target/arm/cpu.h > > @@ -832,6 +832,7 @@ struct ARMCPU { > > */ > > bool cfgend; > > > > + QLIST_HEAD(, ARMELChangeHook) pre_el_change_hooks; > > QLIST_HEAD(, ARMELChangeHook) el_change_hooks; > > > > int32_t node_id; /* NUMA node this CPU belongs to */ > > @@ -2895,12 +2896,15 @@ static inline AddressSpace > > *arm_addressspace(CPUState *cs, MemTxAttrs attrs) > > #endif > > > > /** > > + * arm_register_pre_el_change_hook: > > * arm_register_el_change_hook: > > - * Register a hook function which will be called back whenever this > > - * CPU changes exception level or mode. The hook function will be > > - * passed a pointer to the ARMCPU and the opaque data pointer passed > > - * to this function when the hook was registered. > > + * Register a hook function which will be called back before or after this > > CPU > > + * changes exception level or mode. The hook function will be passed a > > pointer > > + * to the ARMCPU and the opaque data pointer passed to this function when > > the > > + * hook was registered. > > */ > > I would just have one doc comment for each function, rather than > trying to share. (Some day we may actually autogenerate HTML docs > from these comments...) > > Do we make the guarantee that if we call the pre-change hook > then we will definitely subsequently call the post-change hook? > (ie does the PMU hook rely on that?)
Yes, the PMU relies on the pre- and post- hooks being called in pairs since they drive the state machine of sorts that exists in the variables holding the counter values/deltas. And unless I've really screwed up the implementation, I believe the change hooks in my patchset make that guarantee. -Aaron > > Otherwise looks OK. > > thanks > -- PMM -- Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.