Re: [Xen-devel] [PATCH] arm: restore ELR_hyp and SPSR_hyp on return from hypervisor to hypervisor.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2012-02-21 at 10:53 +0000, Stefano Stabellini wrote:
> On Mon, 20 Feb 2012, Ian Campbell wrote:
> > On Mon, 2012-02-20 at 15:22 +0000, Ian Campbell wrote:
> > > This is necessary to handle nested traps to the hypervisor more than one 
> > > deep.
> > 
> > A sort of corollary to this patch is the following.
> > 
> > The situation with the LR register when in hyp mode is a bit odd, since
> > this is normally a banked register but for hyp mode it actually accesses
> > LR_usr instead.
> > 
> > However, although I think the following is correct I'm not sure if it is
> > useful to combine LR_usr and LR like this -- in particular I fear it
> > might be more confusing than helpful. Making the return-to-user case a
> > proper superset of return-to-hypervisor (as is the case on the save
> > path) is nice though.
> > 
> > What do people think?
> 
> I am OK with this.

Is that an Acked-by?

> 
> > 8<---------------------------------------------------------------
> > 
> > >From 3132c2976ae4c83d6d9b94ad80ee04c4880f13da Mon Sep 17 00:00:00 2001
> > From: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Date: Mon, 20 Feb 2012 15:07:09 +0000
> > Subject: [PATCH] arm: lr register in hyp mode is really LR_usr.
> > 
> > Save and restore it in the same way for both hypervisor and user stack 
> > frames
> > rather than saving both individually in the user stack frame.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> >  xen/arch/arm/entry.S          |   14 ++------------
> >  xen/include/public/arch-arm.h |   15 ++++++++++++---
> >  2 files changed, 14 insertions(+), 15 deletions(-)
> > 
> > diff --git a/xen/arch/arm/entry.S b/xen/arch/arm/entry.S
> > index 36f1119..0e85c3c 100644
> > --- a/xen/arch/arm/entry.S
> > +++ b/xen/arch/arm/entry.S
> > @@ -29,8 +29,6 @@
> >     blne save_guest_regs
> >  
> >  save_guest_regs:
> > -   ldr r11, [sp, #UREGS_lr]
> > -   str r11, [sp, #UREGS_LR_usr]
> >     ldr r11, =0xffffffff  /* Clobber SP which is only valid for hypervisor 
> > frames. */
> >     str r11, [sp, #UREGS_sp]
> 
> It would be nice to have a comment here saying "no need to save LR again
> because LR_usr and LR are the same physical register"

I'll add something next to both the save and restore of SP_usr saying
why LR_usr isn't also saved/restored there.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel