From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33108) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9YZS-0004dp-JE for qemu-devel@nongnu.org; Thu, 17 Dec 2015 08:26:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a9YZR-0005rX-Mc for qemu-devel@nongnu.org; Thu, 17 Dec 2015 08:26:14 -0500 Received: from mail-ob0-x22f.google.com ([2607:f8b0:4003:c01::22f]:35094) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a9YZR-0005rI-Gz for qemu-devel@nongnu.org; Thu, 17 Dec 2015 08:26:13 -0500 Received: by mail-ob0-x22f.google.com with SMTP id 18so56754847obc.2 for ; Thu, 17 Dec 2015 05:26:13 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <1449101933-24928-4-git-send-email-mdavidsaver@gmail.com> References: <1449101933-24928-1-git-send-email-mdavidsaver@gmail.com> <1449101933-24928-4-git-send-email-mdavidsaver@gmail.com> From: Peter Maydell Date: Thu, 17 Dec 2015 13:25:53 +0000 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v2 03/26] armv7m: Explicit error for bad vector table List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Davidsaver Cc: Peter Crosthwaite , qemu-arm , QEMU Developers On 3 December 2015 at 00:18, Michael Davidsaver wrote: > Give an explicit error and abort when a load > from VECBASE fails. Otherwise would likely > jump to 0, which for v7-m holds the reset stack > pointer address. > --- > target-arm/helper.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 2c631e3..7b76f32 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -5414,6 +5414,25 @@ static void do_v7m_exception_exit(CPUARMState *env) > pointer. */ > } > > +static > +uint32_t arm_v7m_load_vector(ARMCPU *cpu) > + > +{ > + CPUState *cs = &cpu->parent_obj; This isn't the right way to cast to the base class of a QOM object. You want: CPUState *cs = CPU(cpu); > + CPUARMState *env = &cpu->env; > + MemTxResult result; > + hwaddr vec = env->v7m.vecbase + env->v7m.exception * 4; > + uint32_t addr; > + > + addr = address_space_ldl(cs->as, vec, > + MEMTXATTRS_UNSPECIFIED, &result); > + if (result != MEMTX_OK) { We could use a comment here: /* Architecturally this should cause a HardFault setting HSFR.VECTTBL, * which would then be immediately followed by our failing to load * the entry vector for that HardFault, which is a Lockup case. * Since we don't model Lockup, we just report this guest error * via cpu_abort(). */ > + cpu_abort(cs, "Failed to read from exception vector table " > + "entry %08x\n", (unsigned)vec); > + } > + return addr; > +} > + > void arm_v7m_cpu_do_interrupt(CPUState *cs) > { > ARMCPU *cpu = ARM_CPU(cs); > @@ -5495,7 +5514,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) > /* Clear IT bits */ > env->condexec_bits = 0; > env->regs[14] = lr; > - addr = ldl_phys(cs->as, env->v7m.vecbase + env->v7m.exception * 4); > + addr = arm_v7m_load_vector(cpu); > env->regs[15] = addr & 0xfffffffe; > env->thumb = addr & 1; > } The rest of this patch looks OK though. thanks -- PMM