[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] xen/arm32: head: Improve logging in head.S
Hi Michal,
On 12/01/2024 11:25, Michal Orzel wrote:
On 12/01/2024 11:58, Julien Grall wrote:
On 12/01/2024 08:49, Michal Orzel wrote:
Hi Julien,
Hi Michal,
On 11/01/2024 19:34, Julien Grall wrote:
From: Julien Grall <jgrall@xxxxxxxxxx>
The sequence to enable the MMU on arm32 is quite complex as we may need
to jump to a temporary mapping to map Xen.
Recently, we had one bug in the logic (see f5a49eb7f8b3 ("xen/arm32:
head: Add mising isb in switch_to_runtime_mapping()") and it was
a pain to debug because there are no logging.
In order to improve the logging in the MMU switch we need to add
support for early printk while running on the identity mapping
and also on the temporary mapping.
For the identity mapping, we have only the first page of Xen mapped.
So all the strings should reside in the first page. For that purpose
a new macro PRINT_ID is introduced.
For the temporary mapping, the fixmap is already linked in the temporary
area (and so does the UART). So we just need to update the register
storing the UART address (i.e. r11) to point to the UART temporary
mapping.
Take the opportunity to introduce mov_w_on_cond in order to
conditionally execute mov_w and avoid branches.
Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx>
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
Thanks!
/*
@@ -29,16 +33,26 @@
#ifdef CONFIG_EARLY_PRINTK
/*
- * Macro to print a string to the UART, if there is one.
+ * Macros to print a string to the UART, if there is one.
+ *
+ * There are multiple flavors:
+ * - PRINT_SECT(section, string): The @string will be located in @section
+ * - PRINT(): The string will be located in .rodata.str.
+ * - PRINT_ID(): When Xen is running on the Identity Mapping, it is
+ * only possible to have a limited amount of Xen. This will create
+ * the string in .rodata.idmap which will always be mapped.
*
* Clobbers r0 - r3
*/
-#define PRINT(_s) \
- mov r3, lr ;\
- adr_l r0, 98f ;\
- bl asm_puts ;\
- mov lr, r3 ;\
- RODATA_STR(98, _s)
+#define PRINT_SECT(section, string) \
+ mov r3, lr ;\
+ adr_l r0, 98f ;\
+ bl asm_puts ;\
+ mov lr, r3 ;\
+ RODATA_SECT(section, 98, string)
+
+#define PRINT(string) PRINT_SECT(.rodata.str, string)
+#define PRINT_ID(string) PRINT_SECT(.rodata.idmap, string)
I know this is just a macro but does it make sense to have something MMU
specific in common header?
I don't expect MPU to use it.
For cache coloring, I would like secondary boot CPUs to start directly
on the colored Xen. This means that any message used before enabling the
MMU will need to be part of the .rodata.idmap.
I know that 32-bit is not in scope for the cache coloring series. But I
would like to keep 32-bit and 64-bit boot logic fairly similar.
With that in mind, would you be happy if I keep PRINT_ID() in macros.h?
Note that I would be ok to move in mmu/head.S and move back in macros.h
later on. I just wanted to avoid code movement :).
With the above explanation it does not make sense to move it back and forth, so
let's keep it as is.
Thanks! If that's change, we will move PRINT_ID() to mmu/head.S
I have committed the patch.
Cheers,
--
Julien Grall
|
