[v1,3/7] target-arm: Add helper macros and defines for CCNT register

Include a helper function to determine if the CCNT counter
is enabled as well as the constants used to mask the pmccfiltr_el0
register.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 target-arm/cpu.h |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

Hi Alistair,

On 06/23/2014 09:12 PM, Alistair Francis wrote:
> Include a helper function to determine if the CCNT counter
> is enabled as well as the constants used to mask the pmccfiltr_el0
> register.
> 
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> 
>  target-arm/cpu.h |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 6a2efd8..31aa09c 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -111,6 +111,25 @@ typedef struct ARMGenericTimer {
>  #define GTIMER_VIRT 1
>  #define NUM_GTIMERS 2
>  
> +#ifndef CONFIG_USER_ONLY
> +    /* Definitions for the PMCCFILTR_EL0 and PMXEVTYPER registers */
> +    #define PMCP 0x80000000
> +    #define PMCU 0x40000000

These names are very similar to what one might use for the PMCR, which has its
P bit somewhere completely different. A prefix derived from PMXEVTYPER might
be clearer.

> +    /* This implements the PMCCFILTR_EL0:P and U bits; the PMXEVTYPER:P and U
> +     * bits and the c9_pmcr:E bit.
> +     *
> +     * It does not suppor the secure/non-secure componenets of the

Nit: support, components

> +     * PMCCFILTR_EL0 register
> +     */
> +    #define CCNT_ENABLED(env) \
> +        ((env->cp15.c9_pmcr & PMCRE) && \
> +        !(env->cp15.pmccfiltr_el0 & PMCP && arm_current_pl(env) == 1) && \
> +        !(env->cp15.pmccfiltr_el0 & PMCU && arm_current_pl(env) == 0) && \
> +        !(env->cp15.c9_pmxevtyper & PMCP && arm_current_pl(env) == 1) && \
> +        !(env->cp15.c9_pmxevtyper & PMCU && arm_current_pl(env) == 0))
> +#endif
> +
>  typedef struct CPUARMState {
>      /* Regs for current mode.  */
>      uint32_t regs[16];
> 

Christopher

On 24 June 2014 02:12, Alistair Francis <alistair.francis@xilinx.com> wrote:
> +    /* This implements the PMCCFILTR_EL0:P and U bits; the PMXEVTYPER:P and U
> +     * bits and the c9_pmcr:E bit.
> +     *
> +     * It does not suppor the secure/non-secure componenets of the
> +     * PMCCFILTR_EL0 register
> +     */
> +    #define CCNT_ENABLED(env) \
> +        ((env->cp15.c9_pmcr & PMCRE) && \
> +        !(env->cp15.pmccfiltr_el0 & PMCP && arm_current_pl(env) == 1) && \
> +        !(env->cp15.pmccfiltr_el0 & PMCU && arm_current_pl(env) == 0) && \
> +        !(env->cp15.c9_pmxevtyper & PMCP && arm_current_pl(env) == 1) && \
> +        !(env->cp15.c9_pmxevtyper & PMCU && arm_current_pl(env) == 0))
> +#endif

This really should be a function, not a macro. (And it probably
ought to live in internals.h.)

Can you write the check so that it won't blow up and need
fixing when arm_current_pl() returns 2 or 3 in the future?

thanks
-- PMM

On Wed, Jun 25, 2014 at 1:31 AM, Christopher Covington
<cov@codeaurora.org> wrote:
> Hi Alistair,
>
> On 06/23/2014 09:12 PM, Alistair Francis wrote:
>> Include a helper function to determine if the CCNT counter
>> is enabled as well as the constants used to mask the pmccfiltr_el0
>> register.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>>  target-arm/cpu.h |   19 +++++++++++++++++++
>>  1 files changed, 19 insertions(+), 0 deletions(-)
>>
>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> index 6a2efd8..31aa09c 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -111,6 +111,25 @@ typedef struct ARMGenericTimer {
>>  #define GTIMER_VIRT 1
>>  #define NUM_GTIMERS 2
>>
>> +#ifndef CONFIG_USER_ONLY
>> +    /* Definitions for the PMCCFILTR_EL0 and PMXEVTYPER registers */
>> +    #define PMCP 0x80000000
>> +    #define PMCU 0x40000000
>
> These names are very similar to what one might use for the PMCR, which has its
> P bit somewhere completely different. A prefix derived from PMXEVTYPER might
> be clearer.

Yep, can do

>
>> +    /* This implements the PMCCFILTR_EL0:P and U bits; the PMXEVTYPER:P and U
>> +     * bits and the c9_pmcr:E bit.
>> +     *
>> +     * It does not suppor the secure/non-secure componenets of the
>
> Nit: support, components

Thanks, will fix in next version

>
>> +     * PMCCFILTR_EL0 register
>> +     */
>> +    #define CCNT_ENABLED(env) \
>> +        ((env->cp15.c9_pmcr & PMCRE) && \
>> +        !(env->cp15.pmccfiltr_el0 & PMCP && arm_current_pl(env) == 1) && \
>> +        !(env->cp15.pmccfiltr_el0 & PMCU && arm_current_pl(env) == 0) && \
>> +        !(env->cp15.c9_pmxevtyper & PMCP && arm_current_pl(env) == 1) && \
>> +        !(env->cp15.c9_pmxevtyper & PMCU && arm_current_pl(env) == 0))
>> +#endif
>> +
>>  typedef struct CPUARMState {
>>      /* Regs for current mode.  */
>>      uint32_t regs[16];
>>
>
> Christopher
>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by the Linux Foundation.
>

On Wed, Jun 25, 2014 at 1:56 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 24 June 2014 02:12, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> +    /* This implements the PMCCFILTR_EL0:P and U bits; the PMXEVTYPER:P and U
>> +     * bits and the c9_pmcr:E bit.
>> +     *
>> +     * It does not suppor the secure/non-secure componenets of the
>> +     * PMCCFILTR_EL0 register
>> +     */
>> +    #define CCNT_ENABLED(env) \
>> +        ((env->cp15.c9_pmcr & PMCRE) && \
>> +        !(env->cp15.pmccfiltr_el0 & PMCP && arm_current_pl(env) == 1) && \
>> +        !(env->cp15.pmccfiltr_el0 & PMCU && arm_current_pl(env) == 0) && \
>> +        !(env->cp15.c9_pmxevtyper & PMCP && arm_current_pl(env) == 1) && \
>> +        !(env->cp15.c9_pmxevtyper & PMCU && arm_current_pl(env) == 0))
>> +#endif
>
> This really should be a function, not a macro. (And it probably
> ought to live in internals.h.)
>
> Can you write the check so that it won't blow up and need
> fixing when arm_current_pl() returns 2 or 3 in the future?

Ok, I can move it to a function and it shouldn't be too hard to allow
it to work with EL 2 or 3 (from memory there isn't a specific check for EL2,
just the secure/non-secure stuff)

>
> thanks
> -- PMM
>