Message ID | 0838927eb19397cae7bc9b2bd805483e068ff56c.1391143296.git.alistair.francis@xilinx.com |
---|---|
State | New |
Headers | show |
Ping On Fri, Jan 31, 2014 at 2:44 PM, Alistair Francis <alistair.francis@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.francis@xilinx.com> > --- > This patch assumes that non-invasive debugging is not permitted > when determining if the counter is disabled > V5: Implement the actual write function to make sure that > migration works correctly. Also includes the raw_read/write as > the normal read/write functions depend on the pmcr register. So > they don't allow for the pmccntr register to be written first. > 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 | 4 ++ > target-arm/helper.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 97 insertions(+), 2 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 198b6b8..5f96a4d 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -215,6 +215,10 @@ 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 > + */ > + uint32_t c15_ccnt; > } cp15; > > /* System registers (AArch64) */ > diff --git a/target-arm/helper.c b/target-arm/helper.c > index c708f15..090953a 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 - env->cp15.c15_ccnt; > + } > + > return 0; > } > > @@ -584,6 +624,56 @@ 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) > +{ > + 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, set the absolute value */ > + env->cp15.c15_ccnt = value; > + 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; > + } > + env->cp15.c15_ccnt = total_ticks - value; > + > + return 0; > +} > + > static int ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri, > uint64_t *value) > { > @@ -644,9 +734,10 @@ 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, .resetvalue = 0, .type = ARM_CP_IO, > + .readfn = pmccntr_read, .writefn = pmccntr_write, > + .raw_readfn = raw_read, .raw_writefn = raw_write }, > { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1, > .access = PL0_RW, > .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper), > -- > 1.7.1 >
On 31 January 2014 04:44, Alistair Francis <alistair.francis@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.francis@xilinx.com> I'm afraid you'll need to respin this against my target-arm.next branch, because I've landed the patches which update the API for read/write functions. git://git.linaro.org/people/peter.maydell/qemu-arm.git target-arm.next > + > static int ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri, > uint64_t *value) > { > @@ -644,9 +734,10 @@ 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 */ Your patch has dropped this XXX comment without fixing the issue. Luckily you'll find that in target-arm.next it's been fixed, by providing a suitable accessfn for the register. > { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0, > - .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, > + .access = PL1_RW, .resetvalue = 0, .type = ARM_CP_IO, This changes the access rights, incorrectly. > + .readfn = pmccntr_read, .writefn = pmccntr_write, > + .raw_readfn = raw_read, .raw_writefn = raw_write }, This is wrong, because raw_read/write need to return the same value as the actual register, just without odd side effects. > { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1, > .access = PL0_RW, > .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper), Thanks, and sorry for the inconvenience resulting from the clashing patchset. -- PMM
On Thu, Feb 13, 2014 at 5:32 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 31 January 2014 04:44, Alistair Francis <alistair.francis@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.francis@xilinx.com> > > I'm afraid you'll need to respin this against my target-arm.next > branch, because I've landed the patches which update the > API for read/write functions. > git://git.linaro.org/people/peter.maydell/qemu-arm.git target-arm.next > >> + >> static int ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri, >> uint64_t *value) >> { >> @@ -644,9 +734,10 @@ 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 */ > > Your patch has dropped this XXX comment without fixing the > issue. Luckily you'll find that in target-arm.next it's been fixed, > by providing a suitable accessfn for the register. > >> { .name = "PMCCNTR", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 0, >> - .access = PL0_RW, .type = ARM_CP_CONST, .resetvalue = 0 }, >> + .access = PL1_RW, .resetvalue = 0, .type = ARM_CP_IO, > > This changes the access rights, incorrectly. > >> + .readfn = pmccntr_read, .writefn = pmccntr_write, >> + .raw_readfn = raw_read, .raw_writefn = raw_write }, > > This is wrong, because raw_read/write need to return the same > value as the actual register, just without odd side effects. > My understanding is that the _read and _write functions as you have implemented are this exact semantic. So you should be able to just drop the raw_'s. Regards, Peter >> { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1, >> .access = PL0_RW, >> .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper), > > Thanks, and sorry for the inconvenience resulting from > the clashing patchset. > > -- PMM >
diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 198b6b8..5f96a4d 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -215,6 +215,10 @@ 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 + */ + uint32_t c15_ccnt; } cp15; /* System registers (AArch64) */ diff --git a/target-arm/helper.c b/target-arm/helper.c index c708f15..090953a 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 - env->cp15.c15_ccnt; + } + return 0; } @@ -584,6 +624,56 @@ 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) +{ + 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, set the absolute value */ + env->cp15.c15_ccnt = value; + 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; + } + env->cp15.c15_ccnt = total_ticks - value; + + return 0; +} + static int ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t *value) { @@ -644,9 +734,10 @@ 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, .resetvalue = 0, .type = ARM_CP_IO, + .readfn = pmccntr_read, .writefn = pmccntr_write, + .raw_readfn = raw_read, .raw_writefn = raw_write }, { .name = "PMXEVTYPER", .cp = 15, .crn = 9, .crm = 13, .opc1 = 0, .opc2 = 1, .access = PL0_RW, .fieldoffset = offsetof(CPUARMState, cp15.c9_pmxevtyper),
This patch implements the ARM PMCCNTR register including the disable and reset components of the PMCR register. Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> --- This patch assumes that non-invasive debugging is not permitted when determining if the counter is disabled V5: Implement the actual write function to make sure that migration works correctly. Also includes the raw_read/write as the normal read/write functions depend on the pmcr register. So they don't allow for the pmccntr register to be written first. 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 | 4 ++ target-arm/helper.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 97 insertions(+), 2 deletions(-)