On Wed, Jan 29, 2014 at 10:28 PM, Peter Maydell
<peter.mayd...@linaro.org> wrote:
> On 28 January 2014 06:25, Alistair Francis <alistair.fran...@xilinx.com>
> wrote:
>> This patch implements the ARM PMCCNTR register including
>> the disable and reset components of the PMCR register.
>>
>> Signed-off-by: Alistair Francis <alistair.fran...@xilinx.com>
>> ---
>> This patch assumes that non-invasive debugging is not permitted
>> when determining if the counter is disabled
>> V4: Some bug fixes pointed out by Peter Crosthwaite. Including
>> increasing the accuracy of the timer.
>> V3: Fixed up incorrect reset, disable and enable handling that
>> was submitted in V2. The patch should now also handle changing
>> of the clock scaling.
>> V2: Incorporated the comments that Peter Maydell and Peter
>> Crosthwaite had. Now the implementation only requires one
>> CPU state
>>
>> target-arm/cpu.h | 3 ++
>> target-arm/helper.c | 77
>> +++++++++++++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 78 insertions(+), 2 deletions(-)
>>
>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> index 198b6b8..2fdab58 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -215,6 +215,9 @@ typedef struct CPUARMState {
>> uint32_t c15_diagnostic; /* diagnostic register */
>> uint32_t c15_power_diagnostic;
>> uint32_t c15_power_control; /* power control */
>> + /* If the counter is enabled, this stores the last time the counter
>> + * was reset. Otherwise it stores the counter value */
>
> Newline before the "*/" please.
>
>> + uint32_t c15_ccnt;
>> } cp15;
>>
>> /* System registers (AArch64) */
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index c708f15..f6c57c8 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -13,6 +13,12 @@ static inline int get_phys_addr(CPUARMState *env,
>> uint32_t address,
>> target_ulong *page_size);
>> #endif
>>
>> +/* Definitions for the PMCCNTR and PMCR registers */
>> +#define PMCRDP 0x20
>> +#define PMCRD 0x8
>> +#define PMCRC 0x4
>> +#define PMCRE 0x1
>> +
>> static int vfp_gdb_get_reg(CPUARMState *env, uint8_t *buf, int reg)
>> {
>> int nregs;
>> @@ -502,12 +508,46 @@ static int pmreg_read(CPUARMState *env, const
>> ARMCPRegInfo *ri,
>> static int pmcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>> uint64_t value)
>> {
>> + uint32_t temp_ticks;
>> +
>> if (arm_current_pl(env) == 0 && !env->cp15.c9_pmuserenr) {
>> return EXCP_UDEF;
>> }
>> +
>> + temp_ticks = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) *
>> + get_ticks_per_sec() / 1000000;
>> +
>> + /* This assumes that non-invasive debugging is not permitted */
>> + if (!(env->cp15.c9_pmcr & PMCRDP) ||
>> + env->cp15.c9_pmcr & PMCRE) {
>> + /* If the counter is enabled */
>> + if (env->cp15.c9_pmcr & PMCRDP) {
>> + /* Increment once every 64 processor clock cycles */
>> + env->cp15.c15_ccnt = (temp_ticks/64) - env->cp15.c15_ccnt;
>> + } else {
>> + env->cp15.c15_ccnt = temp_ticks - env->cp15.c15_ccnt;
>> + }
>> + }
>> +
>> + if (value & PMCRC) {
>> + /* The counter has been reset */
>> + env->cp15.c15_ccnt = 0;
>> + }
>> +
>> /* only the DP, X, D and E bits are writable */
>> env->cp15.c9_pmcr &= ~0x39;
>> env->cp15.c9_pmcr |= (value & 0x39);
>> +
>> + /* This assumes that non-invasive debugging is not permitted */
>> + if (!(env->cp15.c9_pmcr & PMCRDP) ||
>> + env->cp15.c9_pmcr & PMCRE) {
>> + if (env->cp15.c9_pmcr & PMCRDP) {
>> + /* Increment once every 64 processor clock cycles */
>> + temp_ticks /= 64;
>> + }
>> + env->cp15.c15_ccnt = temp_ticks;
>> + }
>> +
>> return 0;
>> }
>>
>> @@ -584,6 +624,39 @@ static int vbar_write(CPUARMState *env, const
>> ARMCPRegInfo *ri,
>> return 0;
>> }
>>
>> +static int pmccntr_read(CPUARMState *env, const ARMCPRegInfo *ri,
>> + uint64_t *value)
>> +{
>> + uint32_t total_ticks;
>> +
>> + /* This assumes that non-invasive debugging is not permitted */
>> + if (env->cp15.c9_pmcr & PMCRDP ||
>> + !(env->cp15.c9_pmcr & PMCRE)) {
>> + /* Counter is disabled, do not change value */
>> + *value = env->cp15.c15_ccnt;
>> + return 0;
>> + }
>> +
>> + total_ticks = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL) *
>> + get_ticks_per_sec() / 1000000;
>> +
>> + if (env->cp15.c9_pmcr & PMCRDP) {
>> + /* Increment once every 64 processor clock cycles */
>> + total_ticks /= 64;
>> + }
>> + *value = total_ticks - env->cp15.c15_ccnt;
>> +
>> + return 0;
>> +}
>> +
>> +static int pmccntr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>> + uint64_t value)
>> +{
>> + /* A NOP write */
>> + qemu_log_mask(LOG_UNIMP, "CCNT: Write not implemented\n");
>> + return 0;
>> +}
>
> This will break migration. You must provide a mechanism for the
> migration to do a "read register on source end; write value to register
> at destination". (You also in this case need to make sure migration
> works whether the migration process writes the control register
> first and the counter second or the other way around.)
>
Is this as simple as hooking up the default raw_write/raw_read
handlers? From a migration perspective is should be a case of simply
saving and loading the state value with no fancyness. The vm timer
should migrate so there should be no need for any of the syncing
effects on either end of a migration.
Regards,
Peter
>> +
>> static int ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri,
>> uint64_t *value)
>> {
>> @@ -644,9 +717,9 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
>> */
>> { .name = "PMSELR", .cp = 15, .crn = 9, .crm = 12, .opc1 = 0, .opc2 = 5,
>> .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
>> - /* Unimplemented, RAZ/WI. XXX PMUSERENR */
>> { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 =
>> 0,
>> - .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
>> + .access = PL1_RW, .readfn = pmccntr_read, .writefn = pmccntr_write,
>> + .resetvalue = 0, .type = ARM_CP_IO },
>> { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2
>> = 1,
>> .access = PL0_RW,
>> .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper),
>
> thanks
> -- PMM
>
