On Nov 16 14:50, Peter Maydell wrote: > On 5 November 2018 at 18:51, Aaron Lindsay <aa...@os.amperecomputing.com> > wrote: > > pmccntr_read and pmccntr_write contained duplicate code that was already > > being handled by pmccntr_sync. Consolidate the duplicated code into two > > functions: pmccntr_op_start and pmccntr_op_finish. Add a companion to > > c15_ccnt in CPUARMState so that we can simultaneously save both the > > architectural register value and the last underlying cycle count - this > > ensures time isn't lost and will also allow us to access the 'old' > > architectural register value in order to detect overflows in later > > patches. > > > > Signed-off-by: Aaron Lindsay <alind...@codeaurora.org> > > Signed-off-by: Aaron Lindsay <aclin...@gmail.com> > > > > /** > > - * pmccntr_sync > > + * pmccntr_op_start/finish > > + * @env: CPUARMState > > + * > > + * Convert the counter in the PMCCNTR between its delta form (the typical > > mode > > + * when it's enabled) and the guest-visible value. These two calls must > > always > > + * surround any action which might affect the counter. > > + */ > > +void pmccntr_op_start(CPUARMState *env); > > +void pmccntr_op_finish(CPUARMState *env); > > + > > +/** > > + * pmu_op_start/finish > > * @env: CPUARMState > > * > > - * Synchronises the counter in the PMCCNTR. This must always be called > > twice, > > - * once before any action that might affect the timer and again afterwards. > > - * The function is used to swap the state of the register if required. > > - * This only happens when not in user mode (!CONFIG_USER_ONLY) > > + * Convert all PMU counters between their delta form (the typical mode when > > + * they are enabled) and the guest-visible values. These two calls must > > + * surround any action which might affect the counters, and the return > > value > > + * from pmu_op_start must be supplied as the second argument to > > pmu_op_finish. > > */ > > -void pmccntr_sync(CPUARMState *env); > > +void pmu_op_start(CPUARMState *env); > > +void pmu_op_finish(CPUARMState *env); > > The comment says that pmu_op_start returns a value and pmu_op_finish > has a second argument, but the prototypes disagree...
Doh, I updated the comment for pmccntr_op_* but missed pmu_op_*. The last sentence should end after the comma: > > + * Convert all PMU counters between their delta form (the typical mode when > > + * they are enabled) and the guest-visible values. These two calls must > > + * surround any action which might affect the counters. > > */ > Otherwise > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > so if this turns out to be the only problem I can fix it up when > I apply it to my tree, if you provide suitable new comment text. I've also updated the comment text in my tree, so if/when there is a v8 of this patchset, it won't be lost. -Aaron
