Re: [Xen-devel] [PATCH 02/17] xen/arm64: head: Don't clobber x30/lr in the macro PRINT

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 26 Jun 2019, Julien Grall wrote:
> Hi Stefano,
> 
> On 26/06/2019 00:59, Stefano Stabellini wrote:
> > On Tue, 25 Jun 2019, Stefano Stabellini wrote:
> > > On Mon, 10 Jun 2019, Julien Grall wrote:
> > > > >   The current implementation of the macro PRINT will clobber x30/lr.
> > > > > This
> > > > means the user should save lr if it cares about it.
> > > 
> > > By x30/lr, do you mean x0-x3 and lr? I would prefer a clearer
> > > expression.
> > 
> > No of course not! You meant x30 which is a synonym of lr! It is just
> > that in this case it is also supposed to clobber x0-x3 -- I got
> > confused! The commit message is also fine as is then. More below.
> > 
> > 
> > > Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > 
> > > 
> > > > Follow-up patches will introduce more use of PRINT in place where lr
> > > > should be preserved. Rather than requiring all the users to preserve lr,
> > > > the macro PRINT is modified to save and restore it.
> > > > 
> > > > While the comment state x3 will be clobbered, this is not the case. So
> > > > PRINT will use x3 to preserve lr.
> > > > 
> > > > Lastly, take the opportunity to move the comment on top of PRINT and use
> > > > PRINT in init_uart. Both changes will be helpful in a follow-up patch.
> > > > 
> > > > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > > > ---
> > > >   xen/arch/arm/arm64/head.S | 14 +++++++++-----
> > > >   1 file changed, 9 insertions(+), 5 deletions(-)
> > > > 
> > > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > > > index c8bbdf05a6..a5147c8d80 100644
> > > > --- a/xen/arch/arm/arm64/head.S
> > > > +++ b/xen/arch/arm/arm64/head.S
> > > > @@ -78,12 +78,17 @@
> > > >    *  x30 - lr
> > > >    */
> > > >   -/* Macro to print a string to the UART, if there is one.
> > > > - * Clobbers x0-x3. */
> > > >   #ifdef CONFIG_EARLY_PRINTK
> > > > +/*
> > > > + * Macro to print a string to the UART, if there is one.
> > > > + *
> > > > + * Clobbers x0 - x3
> > > > + */
> > > >   #define PRINT(_s)           \
> > > > +        mov   x3, lr  ;     \
> > > >           adr   x0, 98f ;     \
> > > >           bl    puts    ;     \
> > > > +        mov   lr, x3  ;     \
> > > >           RODATA_STR(98, _s)
> > 
> > Strangely enough I get a build error with gcc 7.3.1, but if I use x30
> > instead of lr, it builds fine. Have you seen this before?
> 
> Hmmm, I can't to reproduce it even on older compiler (4.9). My guess is not
> all the assembler is able to understand "lr".
> 
> In the file entry.S we have the following line:
> 
> lr      .req    x30             // link register
> 
> 
> Could you give a try to add the line in head.S?

That works

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel