From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34896) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wkwu7-0004G2-0S for qemu-devel@nongnu.org; Thu, 15 May 2014 10:45:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wkwu2-0004rM-JF for qemu-devel@nongnu.org; Thu, 15 May 2014 10:45:02 -0400 Received: from edge10.ethz.ch ([82.130.75.186]:5102) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wkwu2-0004rH-AL for qemu-devel@nongnu.org; Thu, 15 May 2014 10:44:58 -0400 Message-ID: <5374D2E4.6000104@ethz.ch> Date: Thu, 15 May 2014 16:44:52 +0200 From: Fabian Aggeler MIME-Version: 1.0 References: <1399997768-32014-1-git-send-email-aggelerf@ethz.ch> <1399997768-32014-11-git-send-email-aggelerf@ethz.ch> <537307E8.3080405@gmail.com> <5373B84C.5070707@gmail.com> In-Reply-To: <5373B84C.5070707@gmail.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 10/23] target-arm: implement CPACR register logic List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fedorov Sergey , qemu-devel@nongnu.org Cc: edgar.iglesias@gmail.com, Sergey Fedorov , peter.maydell@linaro.org On 14/05/14 20:39, Fedorov Sergey wrote: > > 14.05.2014 10:06, Sergey Fedorov =D0=BF=D0=B8=D1=88=D0=B5=D1=82: >> On 13.05.2014 20:15, Fabian Aggeler wrote: >>> From: Sergey Fedorov >>> >>> CPACR register allows to control access rights to coprocessor 0-13 >>> interfaces. Bits corresponding to unimplemented coprocessors should be >>> RAZ/WI. QEMU implements only VFP coprocessor on ARMv6+ targets. So only >>> cp10 & cp11 bits are writable. >>> >>> Signed-off-by: Sergey Fedorov >>> Signed-off-by: Fabian Aggeler >>> --- >>> target-arm/helper.c | 6 ++++++ >>> target-arm/translate.c | 26 +++++++++++++++++++++++--- >>> 2 files changed, 29 insertions(+), 3 deletions(-) >>> >>> diff --git a/target-arm/helper.c b/target-arm/helper.c >>> index cf1f88c..4e82259 100644 >>> --- a/target-arm/helper.c >>> +++ b/target-arm/helper.c >>> @@ -477,6 +477,12 @@ static const ARMCPRegInfo not_v7_cp_reginfo[] =3D = { >>> static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri, >>> uint64_t value) >>> { >>> + uint32_t mask =3D 0; >>> + >>> + if (arm_feature(env, ARM_FEATURE_VFP)) { >>> + mask |=3D 0x00f00000; /* VFP coprocessor: cp10 & cp11 */ >>> + } >>> + value &=3D mask; >>> if (env->cp15.c1_coproc !=3D value) { >>> env->cp15.c1_coproc =3D value; >>> /* ??? Is this safe when called from within a TB? */ >>> diff --git a/target-arm/translate.c b/target-arm/translate.c >>> index 87d0918..c815fb3 100644 >>> --- a/target-arm/translate.c >>> +++ b/target-arm/translate.c >>> @@ -6866,9 +6866,29 @@ static int disas_coproc_insn(CPUARMState * env, = DisasContext *s, uint32_t insn) >>> const ARMCPRegInfo *ri; >>> >>> cpnum =3D (insn >> 8) & 0xf; >>> - if (arm_feature(env, ARM_FEATURE_XSCALE) >>> - && ((env->cp15.c15_cpar ^ 0x3fff) & (1 << cpnum))) >>> - return 1; >>> + if (cpnum < 14) { >>> + if (arm_feature(env, ARM_FEATURE_XSCALE)) { >>> + if (~env->cp15.c15_cpar & (1 << cpnum)) { >>> + return 1; >>> + } >>> + } else { >>> + /* Bits [20:21] of CPACR control access to cp10 >>> + * Bits [23:22] of CPACR control access to cp11 */ >>> + switch ((env->cp15.c1_coproc >> (cpnum * 2)) & 3) { >>> + case 0: /* access denied */ >>> + return 1; >>> + case 1: /* privileged mode access only */ >>> + if (IS_USER(s)) { >>> + return 1; >>> + } >>> + break; >>> + case 2: /* reserved */ >>> + return 1; >>> + case 3: /* privileged and user mode access */ >>> + break; >>> + } >>> + } >>> + } >>> >>> /* First check for coprocessor space used for actual instructions= */ >>> switch (cpnum) { >> Please, look at disas_vfp_insn() and disas_neon_*_insn() functions. >> Looks like them should be updated. In that case do not forget to adjust >> arm_cpu_reset() so user emulation would be able to execute VFP/NEON >> instructions. > > See ARM ARM v7-AR B1.11.1 > I don't quite get what you mean. Bits 20-24 of c1_coproc already get set=20 to 1 for user emulation in arm_cpu_reset(). And disas_cfp_insn and=20 disas_neon_*_insn() all check s->cpacr_fpen in the beginning (which gets=20 set in cpu_get_tb_cpu_state() if bits 20-22 of c1_coproc are set to 3 or=20 (1 && cpu is in user mode)). So I guess we should add some checks for NSACR, to only set that flag if=20 the corresponding NSACR bit is set. >> >> Thanks, >> Sergey. >