From patchwork Tue Dec 15 17:31:39 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 557034 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41ABD1402BF for ; Wed, 16 Dec 2015 04:32:33 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b=TjU89H5j; dkim-atps=neutral Received: from localhost ([::1]:38412 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8tSh-0001g1-3Y for incoming@patchwork.ozlabs.org; Tue, 15 Dec 2015 12:32:31 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37828) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8tSC-0000yM-BH for qemu-devel@nongnu.org; Tue, 15 Dec 2015 12:32:06 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a8tSB-0007gI-CO for qemu-devel@nongnu.org; Tue, 15 Dec 2015 12:32:00 -0500 Received: from mail-vk0-x230.google.com ([2607:f8b0:400c:c05::230]:35875) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8tSB-0007g6-6N for qemu-devel@nongnu.org; Tue, 15 Dec 2015 12:31:59 -0500 Received: by mail-vk0-x230.google.com with SMTP id y187so10221555vka.3 for ; Tue, 15 Dec 2015 09:31:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc:content-type; bh=YPo3sFMjLZJ3xALcC04tgXaZKC58B58uUPXGQF7NpLA=; b=TjU89H5jFojR5eJYuAkpBaQv3PQgB7VILuhvn3WdFFvcIgCwlCjdlq1hvpIDPD7ceF f68xNsqq+BM9FVB2/0WZnTJHzyFUWQ3PSSg5UiDUTeZf0DYZdfMkRxSjsTeLiSiaUaPy RIZoYocc40h10iQ7qXo+nxaqEDJ23hjRj0TK4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=YPo3sFMjLZJ3xALcC04tgXaZKC58B58uUPXGQF7NpLA=; b=V0FA82biWQXzm3edR/CCyenxVxFP6Lp3ai9+1n7rErcDvZpW8cnBIMCjySFi4TrhRq qgmMLDTQhwMljSvkKF8e7UlH8qQL3mOgdLr9xIcj1fzWVW7CdZ1gLfcSEpoWThPEIGZl EqPz3wK+oCy+dZoDG8jZP93gGW3YMHootKVU0mh/PNLjuDFuzZBSBMCmELuzX08fki2e Pd+M0/r8GnG0X3YwZa0JaxElZgoZfD9rzp83ajbfSA/RyetSnwxmUQsixifGy8NXlCjy 76dkTFc7uCntaUvw4QOY142UgAVsg0yW1rcowFe2+tB2HpRfh0jVIXkeFdby4sOpC3LL tvJg== X-Gm-Message-State: ALoCoQlqY6ODJnLjjYr/dWI2yXLw6nYsW2sR+OKZ4ODGXNt0RixMc0QYteUJxb72K/Z+gDAneSd23q8Sk+gFkSqk9vIfd6md1+PxeWanP7MNAWqV/1fvmcw= X-Received: by 10.31.47.197 with SMTP id v188mr29743842vkv.6.1450200718661; Tue, 15 Dec 2015 09:31:58 -0800 (PST) MIME-Version: 1.0 Received: by 10.31.153.15 with HTTP; Tue, 15 Dec 2015 09:31:39 -0800 (PST) In-Reply-To: <1449167808-5656-1-git-send-email-Andrew.Baumann@microsoft.com> References: <1449167808-5656-1-git-send-email-Andrew.Baumann@microsoft.com> From: Peter Maydell Date: Tue, 15 Dec 2015 17:31:39 +0000 Message-ID: To: Andrew Baumann X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2607:f8b0:400c:c05::230 Cc: Laurent Desnogues , qemu-arm , QEMU Developers Subject: Re: [Qemu-devel] [PATCH v2] target-arm: raise exception on misaligned LDREX operands X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 3 December 2015 at 18:36, Andrew Baumann wrote: > Qemu does not generally perform alignment checks. However, the ARM ARM > requires implementation of alignment exceptions for a number of cases > including LDREX, and Windows-on-ARM relies on this. > > This change adds plumbing to enable alignment checks on loads using > MO_ALIGN, a do_unaligned_access hook to raise the exception (data > abort), and uses the new aligned loads in LDREX (for all but > single-byte loads). > > Signed-off-by: Andrew Baumann > --- > Thanks for the feedback on v1! I wish I had known about (or gone > looking for) MO_ALIGN sooner... > > arm_regime_using_lpae_format() is a no-op wrapper I added to export > regime_using_lpae_format (which is a static inline). Would it be > preferable to simply export the existing function, and rename it? If > so, is this still the correct name to use for the function? The way you have it seems OK to me. > +/* Raise a data fault alignment exception for the specified virtual address */ > +void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, int is_write, > + int is_user, uintptr_t retaddr) > +{ > + ARMCPU *cpu = ARM_CPU(cs); > + CPUARMState *env = &cpu->env; > + int target_el; > + bool same_el; > + > + if (retaddr) { > + /* now we have a real cpu fault */ > + cpu_restore_state(cs, retaddr); > + } > + > + target_el = exception_target_el(env); > + same_el = (arm_current_el(env) == target_el); > + > + env->exception.vaddress = vaddr; > + > + /* the DFSR for an alignment fault depends on whether we're using > + * the LPAE long descriptor format, or the short descriptor format */ > + if (arm_regime_using_lpae_format(env, cpu_mmu_index(env, false))) { > + env->exception.fsr = 0x21; > + } else { > + env->exception.fsr = 0x1; > + } > + > + raise_exception(env, EXCP_DATA_ABORT, > + syn_data_abort(same_el, 0, 0, 0, 0, 0x21), > + target_el); > +} This isn't propagating the 'read or write' information from is_write into the syndrome and DFSR. You need this minor tweak: (compare the similar code in tlb_fill()). I'm just going to squash that in when I apply this to target-arm.next, to save you having to respin. thanks -- PMM diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c index c6995ca..3e5e0d3 100644 --- a/target-arm/op_helper.c +++ b/target-arm/op_helper.c @@ -154,8 +154,12 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr, int is_write, env->exception.fsr = 0x1; } + if (is_write == 1 && arm_feature(env, ARM_FEATURE_V6)) { + env->exception.fsr |= (1 << 11); + } + raise_exception(env, EXCP_DATA_ABORT, - syn_data_abort(same_el, 0, 0, 0, 0, 0x21), + syn_data_abort(same_el, 0, 0, 0, is_write == 1, 0x21), target_el); }