From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47304) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aSnvR-0007ia-Qe for qemu-devel@nongnu.org; Mon, 08 Feb 2016 10:40:31 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aSnvQ-0001QO-LI for qemu-devel@nongnu.org; Mon, 08 Feb 2016 10:40:29 -0500 References: <1454690704-16233-1-git-send-email-peter.maydell@linaro.org> <1454690704-16233-3-git-send-email-peter.maydell@linaro.org> From: Sergey Fedorov Message-ID: <56B8B6E5.1060809@gmail.com> Date: Mon, 8 Feb 2016 18:40:21 +0300 MIME-Version: 1.0 In-Reply-To: <1454690704-16233-3-git-send-email-peter.maydell@linaro.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH 2/6] target-arm: Fix handling of SCR.SMD List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell , qemu-devel@nongnu.org Cc: qemu-arm@nongnu.org, patches@linaro.org On 05.02.2016 19:45, Peter Maydell wrote: > We weren't quite implementing the handling of SCR.SMD correctly. > The condition governing whether the SMD bit should apply only > for NS state is "is EL3 is AArch32", not "is the current EL AArch32". > Fix the condition, and clarify the comment both to reflect this and > to expand slightly on what's going on for the v7-no-Virtualization case. > > Signed-off-by: Peter Maydell Reviewed-by: Sergey Fedorov > --- > The bit about forcing SMD to zero confused me, anyway, since I > expected it to mean "in this function", not elsewhere... > --- > target-arm/op_helper.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c > index 313c0f8..4fedae5 100644 > --- a/target-arm/op_helper.c > +++ b/target-arm/op_helper.c > @@ -614,12 +614,14 @@ void HELPER(pre_smc)(CPUARMState *env, uint32_t syndrome) > int cur_el = arm_current_el(env); > bool secure = arm_is_secure(env); > bool smd = env->cp15.scr_el3 & SCR_SMD; > - /* On ARMv8 AArch32, SMD only applies to NS state. > - * On ARMv7 SMD only applies to NS state and only if EL2 is available. > - * For ARMv7 non EL2, we force SMD to zero so we don't need to re-check > - * the EL2 condition here. > + /* On ARMv8 with EL3 AArch64, SMD applies to both S and NS state. > + * On ARMv8 with EL3 AArch32, or ARMv7 with the Virtualization > + * extensions, SMD only applies to NS state. > + * On ARMv7 without the Virtualization extensions, the SMD bit > + * doesn't exist, but we forbid the guest to set it to 1 in scr_write(), > + * so we need not special case this here. > */ > - bool undef = is_a64(env) ? smd : (!secure && smd); > + bool undef = arm_feature(env, ARM_FEATURE_AARCH64) ? smd : smd && !secure; > > if (arm_is_psci_call(cpu, EXCP_SMC)) { > /* If PSCI is enabled and this looks like a valid PSCI call then