From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54077) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKZ3N-00069f-76 for qemu-devel@nongnu.org; Fri, 13 Sep 2013 15:29:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VKZ3F-00009W-6i for qemu-devel@nongnu.org; Fri, 13 Sep 2013 15:29:17 -0400 Received: from mail-pb0-f53.google.com ([209.85.160.53]:47007) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKZ3F-00009F-0e for qemu-devel@nongnu.org; Fri, 13 Sep 2013 15:29:09 -0400 Received: by mail-pb0-f53.google.com with SMTP id up15so1610144pbc.40 for ; Fri, 13 Sep 2013 12:29:07 -0700 (PDT) Date: Fri, 13 Sep 2013 12:29:20 -0700 From: Christoffer Dall Message-ID: <20130913192920.GD30894@cbox> References: <1377288624-7418-1-git-send-email-christoffer.dall@linaro.org> <1377288624-7418-4-git-send-email-christoffer.dall@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH 3/5] hw: arm_gic: Keep track of SGI sources List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: "linaro-kernel@lists.linaro.org" , QEMU Developers , Patch Tracking , "kvmarm@lists.cs.columbia.edu" On Fri, Sep 06, 2013 at 03:08:16PM +0100, Peter Maydell wrote: > On 23 August 2013 21:10, Christoffer Dall wrote: > > Right now the arm gic emulation doesn't keep track of the source of an > > SGI (which apparently Linux guests don't use, or they're fine with > > assuming CPU 0 always). > > > > Add the necessary matrix on the GICState structure and maintain the data > > when setting and clearing the pending state of an IRQ. > > > > Note that we always choose to present the source as the lowest-numbered > > CPU in case multiple cores have signalled the same SGI number to a core > > on the system. > > > @@ -525,6 +538,11 @@ static void gic_dist_writel(void *opaque, hwaddr offset, > > break; > > } > > GIC_SET_PENDING(irq, mask); > > + target_cpu = (unsigned)ffs(mask) - 1; > > + while (target_cpu < NCPU) { > > + s->sgi_source[irq][target_cpu] |= (1 << cpu); > > + target_cpu = (unsigned)ffs(mask) - 1; > > + } > > This is an infinite loop, because you don't do anything > with mask inside the loop, so target_cpu is always > the same each time round. gcc with optimization is > smart enough to notice this: > > => 0x00005555556c1625 <+229>: jmp 0x5555556c1625 > > :-) > > Unsurprisingly, my test vexpress-a9 image hangs on startup. > that's unfortunate, I think I meant to use find_next_bit and pass target_cpu, but ended up deciding to just clear the bit in the mask, but forgot to actually clear the bit. Whoops. -Christoffer > > --- a/hw/intc/arm_gic_common.c > > +++ b/hw/intc/arm_gic_common.c > > @@ -71,6 +71,7 @@ static const VMStateDescription vmstate_gic = { > > VMSTATE_UINT8_2DARRAY(priority1, GICState, GIC_INTERNAL, NCPU), > > VMSTATE_UINT8_ARRAY(priority2, GICState, GIC_MAXIRQ - GIC_INTERNAL), > > VMSTATE_UINT16_2DARRAY(last_active, GICState, GIC_MAXIRQ, NCPU), > > + VMSTATE_UINT8_2DARRAY(sgi_source, GICState, GIC_NR_SGIS, NCPU), > > VMSTATE_UINT16_ARRAY(priority_mask, GICState, NCPU), > > VMSTATE_UINT16_ARRAY(running_irq, GICState, NCPU), > > VMSTATE_UINT16_ARRAY(running_priority, GICState, NCPU), > > You need to bump the version_id and minimum_version_id > if you add a new field here. >