On 31 May 2014, at 02:09, Peter Crosthwaite <peter.crosthwa...@xilinx.com> wrote:
> On Fri, May 16, 2014 at 10:43 PM, Fabian Aggeler <aggel...@ethz.ch> wrote: >> This patch changes some readfns/writefns to use raw_write >> and raw_read functions, wich use the fieldoffset specified > > "which" > >> in ARMCPRegInfo instead of directly accessing the field. >> This will simplify patches for EL3 & Security Extensions. >> > > Yes I like this idea is generally and universally. It makes the code > more self documenting as these raw_write/raw_read sites clearly > indicate that "this is the actual register state value", which > everything else in the fn is then side effects. It does also mean any > renaming of variables in the env now only have to be changed twice (in > the env and in the .fieldoffset of CPRegInfo) rather than three/four > times (in read/write handlers as well). > >> Signed-off-by: Fabian Aggeler <aggel...@ethz.ch> >> --- >> This patch was previously part of the Security Extension patchset >> but is not really Sec-Ext specific. >> >> target-arm/helper.c | 30 +++++++++++++++--------------- >> 1 file changed, 15 insertions(+), 15 deletions(-) >> >> diff --git a/target-arm/helper.c b/target-arm/helper.c >> index 417161e..6302d67 100644 >> --- a/target-arm/helper.c >> +++ b/target-arm/helper.c >> @@ -319,7 +319,7 @@ static void dacr_write(CPUARMState *env, const >> ARMCPRegInfo *ri, uint64_t value) >> { >> ARMCPU *cpu = arm_env_get_cpu(env); >> >> - env->cp15.c3 = value; >> + raw_write(env, ri, value); >> tlb_flush(CPU(cpu), 1); /* Flush TLB as domain not tracked in TLB */ >> } >> >> @@ -327,12 +327,12 @@ static void fcse_write(CPUARMState *env, const >> ARMCPRegInfo *ri, uint64_t value) >> { >> ARMCPU *cpu = arm_env_get_cpu(env); >> >> - if (env->cp15.c13_fcse != value) { >> + if (raw_read(env, ri) != value) { >> /* Unlike real hardware the qemu TLB uses virtual addresses, >> * not modified virtual addresses, so this causes a TLB flush. >> */ >> tlb_flush(CPU(cpu), 1); >> - env->cp15.c13_fcse = value; >> + raw_write(env, ri, value); >> } >> } >> >> @@ -341,7 +341,7 @@ static void contextidr_write(CPUARMState *env, const >> ARMCPRegInfo *ri, >> { >> ARMCPU *cpu = arm_env_get_cpu(env); >> >> - if (env->cp15.contextidr_el1 != value && !arm_feature(env, >> ARM_FEATURE_MPU) >> + if (raw_read(env, ri) != value && !arm_feature(env, ARM_FEATURE_MPU) >> && !extended_addresses_enabled(env)) { >> /* For VMSA (when not using the LPAE long descriptor page table >> * format) this register includes the ASID, so do a TLB flush. >> @@ -349,7 +349,7 @@ static void contextidr_write(CPUARMState *env, const >> ARMCPRegInfo *ri, >> */ >> tlb_flush(CPU(cpu), 1); >> } >> - env->cp15.contextidr_el1 = value; >> + raw_write(env, ri, value); >> } >> >> static void tlbiall_write(CPUARMState *env, const ARMCPRegInfo *ri, >> @@ -657,7 +657,7 @@ static void vbar_write(CPUARMState *env, const >> ARMCPRegInfo *ri, >> * contexts. (ARMv8 would permit us to do no masking at all, but ARMv7 >> * requires the bottom five bits to be RAZ/WI because they're UNK/SBZP.) >> */ >> - env->cp15.c12_vbar = value & ~0x1FULL; >> + raw_write(env, ri, value & ~0x1Ful); > > This one was already done in Edgar's series (now merged) so best to > rebase to catch any other conflicts. > > But otherwise, > > Reviewed-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com> Thanks, I will rebase and send v2 to avoid conflicts. Fabian > >> } >> >> static uint64_t ccsidr_read(CPUARMState *env, const ARMCPRegInfo *ri) >> @@ -669,7 +669,7 @@ static uint64_t ccsidr_read(CPUARMState *env, const >> ARMCPRegInfo *ri) >> static void csselr_write(CPUARMState *env, const ARMCPRegInfo *ri, >> uint64_t value) >> { >> - env->cp15.c0_cssel = value & 0xf; >> + raw_write(env, ri, value & 0xf); >> } >> >> static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri) >> @@ -1192,11 +1192,11 @@ static const ARMCPRegInfo generic_timer_cp_reginfo[] >> = { >> static void par_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t >> value) >> { >> if (arm_feature(env, ARM_FEATURE_LPAE)) { >> - env->cp15.par_el1 = value; >> + raw_write(env, ri, value); >> } else if (arm_feature(env, ARM_FEATURE_V7)) { >> - env->cp15.par_el1 = value & 0xfffff6ff; >> + raw_write(env, ri, value & 0xfffff6ff); >> } else { >> - env->cp15.par_el1 = value & 0xfffff1ff; >> + raw_write(env, ri, value & 0xfffff1ff); >> } >> } >> >> @@ -1399,7 +1399,7 @@ static void vmsa_ttbcr_raw_write(CPUARMState *env, >> const ARMCPRegInfo *ri, >> * for long-descriptor tables the TTBCR fields are used differently >> * and the c2_mask and c2_base_mask values are meaningless. >> */ >> - env->cp15.c2_control = value; >> + raw_write(env, ri, value); >> env->cp15.c2_mask = ~(((uint32_t)0xffffffffu) >> maskshift); >> env->cp15.c2_base_mask = ~((uint32_t)0x3fffu >> maskshift); >> } >> @@ -1421,7 +1421,7 @@ static void vmsa_ttbcr_write(CPUARMState *env, const >> ARMCPRegInfo *ri, >> static void vmsa_ttbcr_reset(CPUARMState *env, const ARMCPRegInfo *ri) >> { >> env->cp15.c2_base_mask = 0xffffc000u; >> - env->cp15.c2_control = 0; >> + raw_write(env, ri, 0); >> env->cp15.c2_mask = 0; >> } >> >> @@ -1432,7 +1432,7 @@ static void vmsa_tcr_el1_write(CPUARMState *env, const >> ARMCPRegInfo *ri, >> >> /* For AArch64 the A1 bit could result in a change of ASID, so TLB >> flush. */ >> tlb_flush(CPU(cpu), 1); >> - env->cp15.c2_control = value; >> + raw_write(env, ri, value); >> } >> >> static void vmsa_ttbr_write(CPUARMState *env, const ARMCPRegInfo *ri, >> @@ -2081,14 +2081,14 @@ static void sctlr_write(CPUARMState *env, const >> ARMCPRegInfo *ri, >> { >> ARMCPU *cpu = arm_env_get_cpu(env); >> >> - if (env->cp15.c1_sys == value) { >> + if (raw_read(env, ri) == value) { >> /* Skip the TLB flush if nothing actually changed; Linux likes >> * to do a lot of pointless SCTLR writes. >> */ >> return; >> } >> >> - env->cp15.c1_sys = value; >> + raw_write(env, ri, value); >> /* ??? Lots of these bits are not implemented. */ >> /* This may enable/disable the MMU, so do a TLB flush. */ >> tlb_flush(CPU(cpu), 1); >> -- >> 1.8.3.2