On 7 February 2014 07:35, Hu Tao <hutao@xxxxxxxxxxxxxx> wrote: > On Fri, Jan 31, 2014 at 03:45:27PM +0000, Peter Maydell wrote: >> Make the cache ID system registers (CLIDR, CCSELR, CCSIDR, CTR) > > s/CCSELR/CSSELR/ > >> visible to AArch64. These are mostly simple 64-bit extensions of the >> existing 32 bit system registers and so can share reginfo definitions. > > According to the document(ARM DDI 0487A.a), some AArch64 system > registers are 32-bit, for example CCSIDR_EL1 is 32-bit. But System_Put() > and System_Get() writes/reads 64-bit values, which makes me confused. We've been round this issue before. The documentation uses "32-bit" as a shorthand for "64 bit but the top 32 bits are RES0". There is no way in AArch64 to do a 32 bit read or write of a system register -- the only instructions are MSR/MRS, which are always 64 bits. Similarly, the KVM kernel API exposes all registers as 64 bits wide. We could in theory have a mechanism that allowed a 64 bit system register access to be backed by a 32 bit field value, but it's simpler not to. >> CTR needs to have a split definition, but we can clean up the >> temporary user-mode implementation in favour of using the CPU-specified >> reset value, and implement the system-mode-required semantics of >> restricting its EL0 accessibility if SCTLR.UCT is not set. >> >> Signed-off-by: Peter Maydell <peter.maydell@xxxxxxxxxx> >> --- >> target-arm/cpu.c | 2 ++ >> target-arm/cpu.h | 2 +- >> target-arm/cpu64.c | 1 + >> target-arm/helper.c | 31 +++++++++++++++++++++---------- >> 4 files changed, 25 insertions(+), 11 deletions(-) >> >> diff --git a/target-arm/cpu.c b/target-arm/cpu.c >> index fe18b65..8fed098 100644 >> --- a/target-arm/cpu.c >> +++ b/target-arm/cpu.c >> @@ -91,6 +91,8 @@ static void arm_cpu_reset(CPUState *s) >> env->aarch64 = 1; >> #if defined(CONFIG_USER_ONLY) >> env->pstate = PSTATE_MODE_EL0t; >> + /* Userspace expects access to CTL_EL0 */ >> + env->cp15.c1_sys |= SCTLR_UCT; >> #else >> env->pstate = PSTATE_D | PSTATE_A | PSTATE_I | PSTATE_F >> | PSTATE_MODE_EL1h; >> diff --git a/target-arm/cpu.h b/target-arm/cpu.h >> index d1ed423..f5b706e 100644 >> --- a/target-arm/cpu.h >> +++ b/target-arm/cpu.h >> @@ -166,7 +166,7 @@ typedef struct CPUARMState { >> /* System control coprocessor (cp15) */ >> struct { >> uint32_t c0_cpuid; >> - uint32_t c0_cssel; /* Cache size selection. */ >> + uint64_t c0_cssel; /* Cache size selection. */ > > I see all backing fields for AArch64 system registers are converted to > uint64_t, why not convert ARMCPU.ccsidr which backs CCSIDR? It's not directly accessed via a .fieldoffset() entry, so there's no requirement for it to be 64 bits wide. I've also generally left the ARMCPU values which are just constants to be used in ".resetvalue =" specifications alone. It's only the cases where a struct field is the backing storage for the register that need widening. thanks -- PMM _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm