Re: [Xen-devel] [PATCH V2 1/2] xen: arm: introduce Cortex-A7 support

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2013-07-03 at 14:09 +0100, Tim Deegan wrote:
> At 06:43 -0600 on 03 Jul (1372833795), Bamvor Jian Zhang wrote:
> > Hi, Ian
> > 
> >  >>>Wrote "Ian Campbell <Ian.Campbell@xxxxxxxxxx>"> On Tue,
> >  Tue, 2013-07-02 at 21:11 +0800, Bamvor Jian Zhang wrote:
> > > >  
> > > > +        /* Set return address(PIC) */
> > > > +        /* XXX: it only work while thumb2 is not enable in xen */
> > > 
> > > That's true of lots/all of our asm. Lets ignore that for now (no need to
> > > comment).
> > ok
> > > Since we already have physoffset in r10 would it be clearer to make use
> > > of that?
> > sorry if i am wrong. do u mean change "adr lr, 1f" to something like 
> > "add lr, pc, r10"?
> 
> No, I think this code is correct as it stands. 

Tim pointed out IRL that bl can't take a register...

> But it is a bit strange that the CPUinfo struct contains _code_ to jump
> to a routine,

I hadn't noticed that!

>  rather than just containing the address of the routine
> itself.  If it just contained the address, the structures could be
> defined in C (so we'd be certain that they matched the offset and size
> constants we use here).  And then this asm would indeed use physoffset,
> something like:
> 
>       ldr   r1, [r1, #PROCINFO_cpu_init]   /* r1 := vaddr(init func) */
>       adr   lr, 1f                         /* Save return address */
>       add   pc, r1, r10                    /* Call paddr(init func) */
> 1:
> 
> The way you've done it is certainly very clever, but we don't need to
> optimize single instructions out of the boot time and I think this is
> clearer.
> 
> Cheers,
> 
> Tim.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



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