Re: [Qemu-devel] [PATCH target-arm v2 07/13] target-arm/helper.c: define MPUIR register

[Peter Maydell]

On 12 June 2015 at 20:10, Peter Crosthwaite
<address@hidden> wrote:
> Define the MPUIR register for MPU supporting systems V6 onwards.

"(ARMv6 and onwards)".

> Currently only support unified MPU.

"we only"

> The size of the unified MPU is supported via the number of "dregions".
> So just a single config as added to specify this size. (When split MPU
> is implemented iregions will accompany).

Try:

    The size of the unified MPU is defined via the number of "dregions".
    So just a single config is added to specify this size. (When split MPU
    is implemented we will add an extra iregions config).

> Signed-off-by: Peter Crosthwaite <address@hidden>
> ---
> changed since v1:
> Add #regions configuration
> conditionalise MPUIR existence
>
>  target-arm/cpu-qom.h |  2 ++
>  target-arm/cpu.c     |  8 ++++++++
>  target-arm/helper.c  | 10 ++++++++++
>  3 files changed, 20 insertions(+)
>
> diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
> index 30832d9..05c33ac 100644
> --- a/target-arm/cpu-qom.h
> +++ b/target-arm/cpu-qom.h
> @@ -105,6 +105,8 @@ typedef struct ARMCPU {
>
>      /* CPU has memory protection unit */
>      bool has_mpu;
> +    /* PMSAv7 MPU number of supported regions */
> +    uint32_t pmsav7_dregion;
>
>      /* PSCI conduit used to invoke PSCI methods
>       * 0 - disabled, 1 - smc, 2 - hvc
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 82ac52d..c967763 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -445,6 +445,9 @@ static Property arm_cpu_has_el3_property =
>  static Property arm_cpu_has_mpu_property =
>              DEFINE_PROP_BOOL("has-mpu", ARMCPU, has_mpu, true);
>
> +static Property arm_cpu_pmsav7_dregion_property =
> +            DEFINE_PROP_UINT32("pmsav7-dregion", ARMCPU, pmsav7_dregion, 16);
> +
>  static void arm_cpu_post_init(Object *obj)
>  {
>      ARMCPU *cpu = ARM_CPU(obj);
> @@ -476,6 +479,11 @@ static void arm_cpu_post_init(Object *obj)
>      if (arm_feature(&cpu->env, ARM_FEATURE_MPU)) {
>          qdev_property_add_static(DEVICE(obj), &arm_cpu_has_mpu_property,
>                                   &error_abort);
> +        if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
> +            qdev_property_add_static(DEVICE(obj),
> +                                     &arm_cpu_pmsav7_dregion_property,
> +                                     &error_abort);
> +        }
>      }


Worth making bogus values (<0, >255, at least) a realize error,
I think, especially since we start allocating memory based on
the number of regions later on.

Otherwise OK.

-- PMM