From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44799) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zqdx4-0002ld-1M for qemu-devel@nongnu.org; Mon, 26 Oct 2015 05:20:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zqdx0-0003kP-SH for qemu-devel@nongnu.org; Mon, 26 Oct 2015 05:20:26 -0400 Received: from mail-bn1on0090.outbound.protection.outlook.com ([157.56.110.90]:50721 helo=na01-bn1-obe.outbound.protection.outlook.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zqdx0-0003kJ-ND for qemu-devel@nongnu.org; Mon, 26 Oct 2015 05:20:22 -0400 Date: Mon, 26 Oct 2015 10:20:18 +0100 From: "Edgar E. Iglesias" Message-ID: <20151026092018.GA3751@toto> References: <1444863346-9711-1-git-send-email-edgar.iglesias@gmail.com> <1444863346-9711-4-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 03/13] target-arm: Add support for AArch32 S2 negative t0sz 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 04:29:35PM +0100, Peter Maydell wrote: > On 14 October 2015 at 23:55, Edgar E. Iglesias wrote: > > From: "Edgar E. Iglesias" > > > > Add support for AArch32 S2 negative t0sz. In preparation for > > using 40bit IPAs on AArch32. > > > > Signed-off-by: Edgar E. Iglesias > > --- > > target-arm/helper.c | 11 +++++++++++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/target-arm/helper.c b/target-arm/helper.c > > index 4e19838..a8a46db 100644 > > --- a/target-arm/helper.c > > +++ b/target-arm/helper.c > > @@ -6475,6 +6475,17 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, > > if (va_size == 64) { > > t0sz = MIN(t0sz, 39); > > t0sz = MAX(t0sz, 16); > > + } else { > > + bool sext = extract32(t0sz, 4, 1); > > + bool sign = extract32(t0sz, 3, 1); > > + t0sz = sextract32(t0sz, 0, 4); > > + > > + /* If the sign-extend bit is not the same as t0sz[3], the result > > + * is unpredictable. Flag this as a guest error. */ > > + if (sign != sext) { > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "AArch32: VTCR.S / VTCR.T0SZ[3] missmatch\n"); > > + } > > Shouldn't this be guarded by a check on whether this is an s2 > translation, since the 4-bit signed T0SZ and the S bit are only for > the VTCR, not for the normal TTBCRs ? Yes, sounds good. I've changed the patch to the following: @@ -6521,8 +6521,24 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address, */ int32_t t0sz = extract32(tcr->raw_tcr, 0, 6); if (va_size == 64) { + /* AArch64 translation. */ t0sz = MIN(t0sz, 39); t0sz = MAX(t0sz, 16); + } else if (mmu_idx != ARMMMUIdx_S2NS) { + /* AArch32 stage 1 translation. */ + t0sz = extract32(t0sz, 0, 3); + } else { + /* AArch32 stage 2 translation. */ + bool sext = extract32(t0sz, 4, 1); + bool sign = extract32(t0sz, 3, 1); + t0sz = sextract32(t0sz, 0, 4); + + /* If the sign-extend bit is not the same as t0sz[3], the result + * is unpredictable. Flag this as a guest error. */ + if (sign != sext) { + qemu_log_mask(LOG_GUEST_ERROR, + "AArch32: VTCR.S / VTCR.T0SZ[3] missmatch\n"); + } } We can also remove the error log and add more complete checks in future patches if you prefer... Cheers, Edgar > > That is, we have 3 cases here for determining t0sz: > * AArch64 6-bit unsigned field > * AArch32 stage 1 3-bit unsigned field > * AArch32 stage 2 4-bit signed field > so we need more than just a single if/else. > > It's true that bits 3 and 4 are RES0 for TTBCR, but if we're > going to actually start logging guest errors here maybe we > should actually report the real problem (RES0 bits being set) > for that case. > > thanks > -- PMM