On Mon, Jun 1, 2015 at 11:56 AM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 1 June 2015 at 19:04, Peter Crosthwaite <peter.crosthwa...@xilinx.com>
> wrote:
>> define the arm CP registers for PMSAv7 and their accessor functions.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
>> ---
>> target-arm/cpu.h | 6 ++++++
>> target-arm/helper.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 54 insertions(+)
>>
>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
>> index 09cc16d..9cb2e49 100644
>> --- a/target-arm/cpu.h
>> +++ b/target-arm/cpu.h
>> @@ -286,6 +286,12 @@ typedef struct CPUARMState {
>> };
>> uint64_t par_el[4];
>> };
>> +
>> + uint32_t c6_rgnr;
>> + uint32_t c6_drbar[PMSAV7_MPU_NUM_REGIONS];
>> + uint32_t c6_drsr[PMSAV7_MPU_NUM_REGIONS];
>> + uint32_t c6_dracr[PMSAV7_MPU_NUM_REGIONS];
>> +
>> uint32_t c9_insn; /* Cache lockdown registers. */
>> uint32_t c9_data;
>> uint64_t c9_pmcr; /* performance monitor control register */
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index cb21bbf..f11efea 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -1709,6 +1709,54 @@ static uint64_t pmsav5_insn_ap_read(CPUARMState *env,
>> const ARMCPRegInfo *ri)
>> return simple_mpu_ap_bits(env->cp15.pmsav5_insn_ap);
>> }
>>
>> +static uint64_t pmsav7_read(CPUARMState *env, const ARMCPRegInfo *ri)
>> +{
>> + uint32_t *u32p = (void *)env + ri->fieldoffset;
>
> This is raw_ptr(env, ri);
>
Fixed.
>> +
>> + u32p += env->cp15.c6_rgnr;
>> + return *u32p;
>> +}
>> +
>> +static void pmsav7_write(CPUARMState *env, const ARMCPRegInfo *ri,
>> + uint64_t value)
>> +{
>> + ARMCPU *cpu = arm_env_get_cpu(env);
>> + uint32_t *u32p = (void *)env + ri->fieldoffset;
>> +
>> + u32p += env->cp15.c6_rgnr;
>> + tlb_flush(CPU(cpu), 1); /* Mappings may have changed - purge! */
>> + *u32p = value;
>
> Since you're not boundary-checking c6_rgnr either when the guest
> tries to set it (or on inbound migration) or at point of use.
> this is a handy way for the guest to do controlled writes to
> anywhere in QEMU's address space...
>
Boundary check added at time of set.
>> +}
>> +
>> +static void pmsav7_reset(CPUARMState *env, const ARMCPRegInfo *ri)
>> +{
>> + int i;
>> + uint32_t *u32p = (void *)env + ri->fieldoffset;
>> +
>> + for (i = 0; i < 16; ++i) {
>
> You have a #define for number of regions...
> In any case you could just memset() the whole thing rather
> than looping.
>
Memset added. Hard constant removed.
>> + u32p[i] = 0;
>> + }
>> +}
>> +
>> +static const ARMCPRegInfo pmsav7_cp_reginfo[] = {
>> + { .name = "DRBAR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 0,
>> + .access = PL1_RW,
>> + .fieldoffset = offsetof(CPUARMState, cp15.c6_drbar[0]),
>> + .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn =
>> pmsav7_reset },
>> + { .name = "DRSR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 2,
>> + .access = PL1_RW,
>> + .fieldoffset = offsetof(CPUARMState, cp15.c6_drsr[0]),
>> + .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn =
>> pmsav7_reset },
>> + { .name = "DRACR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 1, .opc2 = 4,
>> + .access = PL1_RW,
>> + .fieldoffset = offsetof(CPUARMState, cp15.c6_dracr[0]),
>> + .readfn = pmsav7_read, .writefn = pmsav7_write, .resetfn =
>> pmsav7_reset },
>> + { .name = "RGNR", .cp = 15, .crn = 6, .opc1 = 0, .crm = 2, .opc2 = 0,
>> + .access = PL1_RW,
>> + .fieldoffset = offsetof(CPUARMState, cp15.c6_rgnr), .resetvalue = 0,
>> },
>> + REGINFO_SENTINEL
>> +};
>
> This won't handle migration/state save/load properly.
>
So this was non trivial in the end. I have marked the arrays as
ARM_CP_NO_RAW as they simply don't have state in their own right. I
have then added a subsection in machine.c that VMSTATEs the arrays. I
could have used VMSTATE_UINT32_ARRAY but decided to refactor the
series to have the MPU number or regions available as variable in
ARMCPU. This means we can get the VMSTATE right straight away with a
VMSTATE_VARRAY for these arrays.
The tricky part is the RGNR incoming boundary validation. I have added
this to the new subsection but it is disjunct from the actual VMSD for
RGNR itself which just uses the automatic CP reginfo VMSD.
Regards,
Peter
> -- PMM
>
