From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38780) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zqgxm-0000il-6W for qemu-devel@nongnu.org; Mon, 26 Oct 2015 08:33:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zqgxi-0003e6-TB for qemu-devel@nongnu.org; Mon, 26 Oct 2015 08:33:22 -0400 Received: from mail-bn1bon0085.outbound.protection.outlook.com ([157.56.111.85]:43472 helo=na01-bn1-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zqgxi-0003e2-Lj for qemu-devel@nongnu.org; Mon, 26 Oct 2015 08:33:18 -0400 Date: Mon, 26 Oct 2015 13:33:14 +0100 From: "Edgar E. Iglesias" Message-ID: <20151026123314.GF3751@toto> References: <1444863346-9711-1-git-send-email-edgar.iglesias@gmail.com> <1444863346-9711-14-git-send-email-edgar.iglesias@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v4 13/13] target-arm: Add support for S1 + S2 MMU translations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: QEMU Developers , Alexander Graf , Sergey Fedorov , Laurent Desnogues , "Edgar E. Iglesias" , Alex =?iso-8859-1?Q?Benn=E9e?= On Fri, Oct 23, 2015 at 06:09:24PM +0100, Peter Maydell wrote: > On 14 October 2015 at 23:55, Edgar E. Iglesias wrote: > > From: "Edgar E. Iglesias" > > > > Signed-off-by: Edgar E. Iglesias > > --- > > target-arm/helper.c | 44 +++++++++++++++++++++++++++++++++++++------- > > 1 file changed, 37 insertions(+), 7 deletions(-) > > > > diff --git a/target-arm/helper.c b/target-arm/helper.c > > index 69e24e1..9d70ef2 100644 > > --- a/target-arm/helper.c > > +++ b/target-arm/helper.c > > @@ -7129,14 +7129,44 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address, > > ARMMMUFaultInfo *fi) > > { > > if (mmu_idx == ARMMMUIdx_S12NSE0 || mmu_idx == ARMMMUIdx_S12NSE1) { > > - /* TODO: when we support EL2 we should here call ourselves recursively > > - * to do the stage 1 and then stage 2 translations. The arm_ld*_ptw > > - * functions will also need changing to perform ARMMMUIdx_S2NS loads > > - * rather than direct physical memory loads when appropriate. > > - * For non-EL2 CPUs a stage1+stage2 translation is just stage 1. > > + /* Call ourselves recursively to do the stage 1 and then stage 2 > > + * translations. > > */ > > - assert(!arm_feature(env, ARM_FEATURE_EL2)); > > - mmu_idx += ARMMMUIdx_S1NSE0; > > + if (arm_feature(env, ARM_FEATURE_EL2)) { > > + hwaddr ipa; > > + int s2_prot; > > + int ret; > > + > > + ret = get_phys_addr(env, address, access_type, > > + mmu_idx + ARMMMUIdx_S1NSE0, &ipa, attrs, > > + prot, page_size, fsr, fi); > > + > > + /* If S1 fails or S2 is disabled, return early. */ > > + if (ret || regime_translation_disabled(env, ARMMMUIdx_S2NS)) { > > + if (ret && fi->stage2) { > > + /* This is a S2 error while doing S1 PTW. */ > > + env->cp15.hpfar_el2 = extract64(fi->s2addr, 12, 47) << 4; > > Might be worth a note that it's OK to set the HPFAR here because > this always results in a fault (even if from an AT instruction), > whereas we can't set the FAR registers here because that doesn't > happen for stage 1 faults from AT instructions. > > ...I think we still need to add the code to cause the exception > if a stage 1 AT instruction results in a stage 2 fault, right? Yes, those faults are still missing... I can try to add them in the next round/series together with the detailed error reporting. > If the caller has to look into the FaultInfo struct anyway, maybe > we should just let the caller set the HPFAR_EL2 from the s2addr > if it's going to send the exception to EL2. Agreed, I've moved the setting of HPFAR_EL2 to the callers. Thanks! Edgar > > > + } > > + *phys_ptr = ipa; > > + return ret; > > + } > > + > > + /* S1 is done. Now do S2 translation. */ > > + ret = get_phys_addr_lpae(env, ipa, access_type, ARMMMUIdx_S2NS, > > + phys_ptr, attrs, &s2_prot, > > + page_size, fsr, fi); > > + if (ret) { > > + env->cp15.hpfar_el2 = extract64(ipa, 12, 47) << 4; > > + } > > + /* Combine the S1 and S2 perms. */ > > + *prot &= s2_prot; > > + return ret; > > + } else { > > + /* > > + * For non-EL2 CPUs a stage1+stage2 translation is just stage 1. > > + */ > > + mmu_idx += ARMMMUIdx_S1NSE0; > > + } > > } > > > > /* The page table entries may downgrade secure to non-secure, but > > -- > > 1.9.1 > > > > thanks > -- PMM