Re: [Qemu-devel] [PATCH target-arm v5 1/1] target-arm: Implements the ARM PMCCNTR register

[Peter Maydell]

On 31 January 2014 04:44, Alistair Francis <address@hidden> wrote:
> This patch implements the ARM PMCCNTR register including
> the disable and reset components of the PMCR register.
>
> Signed-off-by: Alistair Francis <address@hidden>

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