[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH v7 02/12] target/arm: Reorganize PMCCNTR accesses
From: |
Aaron Lindsay |
Subject: |
Re: [Qemu-arm] [PATCH v7 02/12] target/arm: Reorganize PMCCNTR accesses |
Date: |
Fri, 16 Nov 2018 15:41:57 +0000 |
On Nov 16 14:50, Peter Maydell wrote:
> On 5 November 2018 at 18:51, Aaron Lindsay <address@hidden> 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 <address@hidden>
> > Signed-off-by: Aaron Lindsay <address@hidden>
>
>
> > /**
> > - * 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 <address@hidden>
> 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
- [Qemu-arm] [PATCH v7 01/12] migration: Add post_save function to VMStateDescription, (continued)
[Qemu-arm] [PATCH v7 02/12] target/arm: Reorganize PMCCNTR accesses, Aaron Lindsay, 2018/11/05
[Qemu-arm] [PATCH v7 05/12] target/arm: Allow AArch32 access for PMCCFILTR, Aaron Lindsay, 2018/11/05
[Qemu-arm] [PATCH v7 04/12] target/arm: Filter cycle counter based on PMCCFILTR_EL0, Aaron Lindsay, 2018/11/05
[Qemu-arm] [PATCH v7 06/12] target/arm: Implement PMOVSSET, Aaron Lindsay, 2018/11/05
[Qemu-arm] [PATCH v7 08/12] target/arm: Finish implementation of PM[X]EVCNTR and PM[X]EVTYPER, Aaron Lindsay, 2018/11/05
[Qemu-arm] [PATCH v7 07/12] target/arm: Add array for supported PMU events, generate PMCEID[01], Aaron Lindsay, 2018/11/05
[Qemu-arm] [PATCH v7 09/12] target/arm: PMU: Add instruction and cycle events, Aaron Lindsay, 2018/11/05
[Qemu-arm] [PATCH v7 10/12] target/arm: PMU: Set PMCR.N to 4, Aaron Lindsay, 2018/11/05