[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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?

The error is:

arm64/head.S: Assembler messages:
arm64/head.S:272: Error: operand 1 must be an integer register -- `mov lr,x3'
[...]
arm64/head.S:272: Error: undefined symbol lr used as an immediate value
[...]


Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.