From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Jones <drjones@redhat.com>
Subject: Re: [PATCH 9/9] arm: add vectors support
Date: Sun, 20 Oct 2013 18:35:34 +0200
Message-ID: <20131020163533.GA2072@hawk.usersys.redhat.com>
References: <1381767815-12510-1-git-send-email-drjones@redhat.com>
 <1381767815-12510-10-git-send-email-drjones@redhat.com>
 <20131017010642.GL24837@cbox>
 <20131017103859.GE2172@hawk.usersys.redhat.com>
 <20131017185843.GQ24837@cbox>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, gleb@redhat.com
To: Christoffer Dall <christoffer.dall@linaro.org>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:28394 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1751231Ab3JTQf5 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sun, 20 Oct 2013 12:35:57 -0400
Content-Disposition: inline
In-Reply-To: <20131017185843.GQ24837@cbox>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Thu, Oct 17, 2013 at 11:58:43AM -0700, Christoffer Dall wrote:
> On Thu, Oct 17, 2013 at 12:38:59PM +0200, Andrew Jones wrote:
> > On Wed, Oct 16, 2013 at 06:06:42PM -0700, Christoffer Dall wrote:
> > > > +++ b/arm/cstart.S
> > > > @@ -1,5 +1,6 @@
> > > >  
> > > >  #define CR_B	(1 << 7)
> > > > +#define CR_V	(1 << 13)
> > > >  
> > > >  .arm
> > > >  
> > > > @@ -12,6 +13,13 @@ start:
> > > >  	push	{ r0-r3 }		@push r3 too for 8-byte alignment
> > > >  
> > > >  	mrc	p15, 0, r8, c1, c0, 0	@r8 = sctrl
> > > > +
> > > > +	/* set up vector table */
> > > > +	bic	r8, #CR_V		@sctrl.V = 0
> > > > +	mcr	p15, 0, r8, c1, c0, 0
> > > > +	ldr	r0, =vector_table	@vbar = vector_table
> > > > +	mcr     p15, 0, r0, c12, c0, 0
> > > > +
> > > >  	bl	get_endianness
> > > >  	bl	io_init
> > > >  
> > > > @@ -41,6 +49,44 @@ halt:
> > > >  1:	wfi
> > > >  	b	1b
> > > >  
> > > > +vector_common:
> > > > +	add	r2, sp, #(14 * 4)
> > > 
> > > this looks weird, what are you pointing to here?
> > 
> > What sp was at the time of exception. So if you look at ex_regs->sp,
> > then you'll see what sp was when the exception occurred, not that plus
> > what we're pushing on now for the handler.
> > 
> 
> Hmmm, so you're assuming that all exceptions will be taken from SVC
> mode?  I assume we will run tests in more than SVC mode, no?
> 
> Also note that the lr you're pushing here is not the lr at the time the
> exception occurs, but the return address from the exception.  If the SVC
> instruction is executed from SVC mode, the original lr is lost iirc, and
> the caller needs to save it.  If you're from user mode, something like
> 
>     stm sp, {r0-lr}^
> 
> will take care of this for you, and if you're from svc
> mode, you may want consider doing something like
> 
>     push {sp, lr}
>     push {r0-r12}
> 
> instead (assuming this is only ever compiled in ARM mode, not Thumb2, in
> which case the whole thing gets more complicated.

I think the lr pushing should be ok. That part is done in the macro that
all vectors start with.  It got snipped from this mail, so here it is

.macro m_vector, v
	push    { r0-r12,lr }
	mov     r1, \v
	b       vector_common
.endm

I realize it may still not be correct, just as the calculated sp
may not be correct, depending on which mode,vector combo is used,
but I was expecting to have different paths in the C code for
fixing it up. Although that said, I haven't thought about it a
bunch yet, so maybe it won't work that way...

drew