* [Xen-devel] [XEN PATCH v3 0/2] xen/arm: Configure early printk via Kconfig
@ 2020-03-09 17:45 Anthony PERARD
2020-03-09 17:45 ` [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro Anthony PERARD
2020-03-09 17:45 ` [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
0 siblings, 2 replies; 18+ messages in thread
From: Anthony PERARD @ 2020-03-09 17:45 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
Ian Jackson, George Dunlap, Jan Beulich, Anthony PERARD,
Volodymyr Babchuk, Roger Pau Monné
Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.arm-early-printk-v3
v3:
- many changes listed in patch notes.
Hi,
That two patchs is to unblock my work on "xen: Build system improvements".
There is an easier fix to build with early printk, but it is just better to use
Kconfig.
It is base on earlier work by Julien, "xen/arm: Add Skeleton for using
configuring early printk using Kconfig".
The changes which rename all macros is in a separated patch, as Stefano wanted.
Hope you like the changes.
(I might add those patches to my other series if I need to resent it before the
early printk changes are applied.)
Cheers,
Anthony PERARD (2):
xen/arm: Rename all early printk macro
xen/arm: Configure early printk via Kconfig
docs/misc/arm/early-printk.txt | 71 +++----
xen/Kconfig.debug | 2 +
xen/arch/arm/Kconfig.debug | 287 +++++++++++++++++++++++++++++
xen/arch/arm/Makefile | 2 +-
xen/arch/arm/Rules.mk | 74 +-------
xen/arch/arm/arm32/Makefile | 2 +-
xen/arch/arm/arm32/debug-8250.inc | 2 +-
xen/arch/arm/arm32/debug-pl011.inc | 4 +-
xen/arch/arm/arm32/debug-scif.inc | 4 +-
xen/arch/arm/arm32/debug.S | 4 +-
xen/arch/arm/arm32/head.S | 10 +-
xen/arch/arm/arm64/Makefile | 2 +-
xen/arch/arm/arm64/debug-8250.inc | 4 +-
xen/arch/arm/arm64/debug-pl011.inc | 4 +-
xen/arch/arm/arm64/debug.S | 4 +-
xen/arch/arm/arm64/head.S | 10 +-
xen/arch/x86/Kconfig.debug | 0
xen/include/asm-arm/early_printk.h | 2 +-
18 files changed, 341 insertions(+), 147 deletions(-)
create mode 100644 xen/arch/arm/Kconfig.debug
create mode 100644 xen/arch/x86/Kconfig.debug
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro
2020-03-09 17:45 [Xen-devel] [XEN PATCH v3 0/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
@ 2020-03-09 17:45 ` Anthony PERARD
2020-03-11 13:57 ` Julien Grall
2020-03-09 17:45 ` [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
1 sibling, 1 reply; 18+ messages in thread
From: Anthony PERARD @ 2020-03-09 17:45 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Volodymyr Babchuk, Stefano Stabellini, Julien Grall
We are going to move the generation of the early printk macro into
Kconfig. This means all macro will be prefix with CONFIG_. We do that
ahead of the change.
We also take the opportunity to better name some variables, which are
used by only one driver and wouldn't make sens for other UART driver.
Thus,
- EARLY_UART_REG_SHIFT became CONFIG_EARLY_UART_8250_REG_SHIFT
- EARLY_PRINTK_VERSION_* became CONFIG_EARLY_UART_SCIF_VERSION_*
The other variables are change to have the prefix CONFIG_EARLY_UART_
when they change a parameter of the driver. So we have now:
- CONFIG_EARLY_UART_BAUD_RATE
- CONFIG_EARLY_UART_BASE_ADDRESS
- CONFIG_EARLY_UART_INIT
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
That's based on early work by Julien
[PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk using Kconfig
<20190913103953.8182-1-julien.grall@arm.com>
---
Notes:
v3:
- Revert the renaming of EARLY_PRINTK to CONFIG_EARLY_PRINTK in the
makefiles, as this doesn't work well with user provided
CONFIG_EARLY_PRINTK.
This is done in the following patch instead.
- rename CONFIG_EARLY_UART_BAUD_RATE to CONFIG_EARLY_UART_PL011_BAUD_RATE
xen/arch/arm/Rules.mk | 14 +++++++-------
xen/arch/arm/arm32/debug-8250.inc | 2 +-
xen/arch/arm/arm32/debug-pl011.inc | 4 ++--
xen/arch/arm/arm32/debug-scif.inc | 4 ++--
xen/arch/arm/arm32/debug.S | 4 ++--
xen/arch/arm/arm32/head.S | 10 +++++-----
xen/arch/arm/arm64/debug-8250.inc | 4 ++--
xen/arch/arm/arm64/debug-pl011.inc | 4 ++--
xen/arch/arm/arm64/debug.S | 4 ++--
xen/arch/arm/arm64/head.S | 10 +++++-----
xen/include/asm-arm/early_printk.h | 2 +-
11 files changed, 31 insertions(+), 31 deletions(-)
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index 022a3a6f82ba..faa09ea111ec 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -66,9 +66,9 @@ endif
endif
ifeq ($(EARLY_PRINTK_INC),scif)
ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
-CFLAGS-y += -DEARLY_PRINTK_VERSION_$(word 3,$(EARLY_PRINTK_CFG))
+CFLAGS-y += -DCONFIG_EARLY_UART_SCIF_VERSION_$(word 3,$(EARLY_PRINTK_CFG))
else
-CFLAGS-y += -DEARLY_PRINTK_VERSION_NONE
+CFLAGS-y += -DCONFIG_EARLY_UART_SCIF_VERSION_NONE
endif
endif
@@ -77,11 +77,11 @@ EARLY_PRINTK := y
endif
CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
-CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DEARLY_PRINTK_INIT_UART
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK_BAUD=$(EARLY_PRINTK_BAUD)
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
-CFLAGS-$(EARLY_PRINTK) += -DEARLY_UART_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
+CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DCONFIG_EARLY_UART_INIT
+CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
+CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_UART_PL011_BAUD_RATE=$(EARLY_PRINTK_BAUD)
+CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
+CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_UART_8250_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
else # !CONFIG_DEBUG
diff --git a/xen/arch/arm/arm32/debug-8250.inc b/xen/arch/arm/arm32/debug-8250.inc
index 0759a27ee157..c47e8be4aaf3 100644
--- a/xen/arch/arm/arm32/debug-8250.inc
+++ b/xen/arch/arm/arm32/debug-8250.inc
@@ -23,7 +23,7 @@
*/
.macro early_uart_ready rb rc
1:
- ldr \rc, [\rb, #(UART_LSR << EARLY_UART_REG_SHIFT)] /* Read LSR */
+ ldr \rc, [\rb, #(UART_LSR << CONFIG_EARLY_UART_8250_REG_SHIFT)] /* Read LSR */
tst \rc, #UART_LSR_THRE /* Check Xmit holding register flag */
beq 1b /* Wait for the UART to be ready */
.endm
diff --git a/xen/arch/arm/arm32/debug-pl011.inc b/xen/arch/arm/arm32/debug-pl011.inc
index ec462eabab5c..214f68dc95bd 100644
--- a/xen/arch/arm/arm32/debug-pl011.inc
+++ b/xen/arch/arm/arm32/debug-pl011.inc
@@ -25,9 +25,9 @@
* rd: scratch register 2 (unused here)
*/
.macro early_uart_init rb, rc, rd
- mov \rc, #(7372800 / EARLY_PRINTK_BAUD % 16)
+ mov \rc, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE % 16)
str \rc, [\rb, #FBRD] /* -> UARTFBRD (Baud divisor fraction) */
- mov \rc, #(7372800 / EARLY_PRINTK_BAUD / 16)
+ mov \rc, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE / 16)
str \rc, [\rb, #IBRD] /* -> UARTIBRD (Baud divisor integer) */
mov \rc, #0x60 /* 8n1 */
str \rc, [\rb, #LCR_H] /* -> UARTLCR_H (Line control) */
diff --git a/xen/arch/arm/arm32/debug-scif.inc b/xen/arch/arm/arm32/debug-scif.inc
index 3f01c909c238..b2b82501e792 100644
--- a/xen/arch/arm/arm32/debug-scif.inc
+++ b/xen/arch/arm/arm32/debug-scif.inc
@@ -19,10 +19,10 @@
#include <asm/scif-uart.h>
-#ifdef EARLY_PRINTK_VERSION_NONE
+#ifdef CONFIG_EARLY_UART_SCIF_VERSION_NONE
#define STATUS_REG SCIF_SCFSR
#define TX_FIFO_REG SCIF_SCFTDR
-#elif EARLY_PRINTK_VERSION_A
+#elif CONFIG_EARLY_UART_SCIF_VERSION_A
#define STATUS_REG SCIFA_SCASSR
#define TX_FIFO_REG SCIFA_SCAFTDR
#endif
diff --git a/xen/arch/arm/arm32/debug.S b/xen/arch/arm/arm32/debug.S
index 1829b29915e0..e77c76d0debc 100644
--- a/xen/arch/arm/arm32/debug.S
+++ b/xen/arch/arm/arm32/debug.S
@@ -19,8 +19,8 @@
#include <asm/early_printk.h>
-#ifdef EARLY_PRINTK_INC
-#include EARLY_PRINTK_INC
+#if defined (CONFIG_EARLY_PRINTK_INC)
+#include CONFIG_EARLY_PRINTK_INC
#endif
/*
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index e9d356f05c2b..2b593c5ef99a 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -36,8 +36,8 @@
#define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START)
#define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
-#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
-#include EARLY_PRINTK_INC
+#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
+#include CONFIG_EARLY_PRINTK_INC
#endif
/*
@@ -223,7 +223,7 @@ GLOBAL(init_secondary)
1:
#ifdef CONFIG_EARLY_PRINTK
- mov_w r11, EARLY_UART_BASE_ADDRESS /* r11 := UART base address */
+ mov_w r11, CONFIG_EARLY_UART_BASE_ADDRESS /* r11 := UART base address */
PRINT("- CPU ")
print_reg r7
PRINT(" booting -\r\n")
@@ -706,8 +706,8 @@ ENTRY(switch_ttbr)
* Clobbers r0 - r3
*/
init_uart:
- mov_w r11, EARLY_UART_BASE_ADDRESS
-#ifdef EARLY_PRINTK_INIT_UART
+ mov_w r11, CONFIG_EARLY_UART_BASE_ADDRESS
+#ifdef CONFIG_EARLY_UART_INIT
early_uart_init r11, r1, r2
#endif
PRINT("- UART enabled -\r\n")
diff --git a/xen/arch/arm/arm64/debug-8250.inc b/xen/arch/arm/arm64/debug-8250.inc
index 53d6828bfafe..30ea13077e22 100644
--- a/xen/arch/arm/arm64/debug-8250.inc
+++ b/xen/arch/arm/arm64/debug-8250.inc
@@ -25,7 +25,7 @@
*/
.macro early_uart_ready xb c
1:
- ldrb w\c, [\xb, #UART_LSR << EARLY_UART_REG_SHIFT]
+ ldrb w\c, [\xb, #UART_LSR << CONFIG_EARLY_UART_8250_REG_SHIFT]
and w\c, w\c, #UART_LSR_THRE
cmp w\c, #UART_LSR_THRE
b.ne 1b
@@ -38,7 +38,7 @@
*/
.macro early_uart_transmit xb wt
/* UART_THR transmit holding */
- strb \wt, [\xb, #UART_THR << EARLY_UART_REG_SHIFT]
+ strb \wt, [\xb, #UART_THR << CONFIG_EARLY_UART_8250_REG_SHIFT]
.endm
/*
diff --git a/xen/arch/arm/arm64/debug-pl011.inc b/xen/arch/arm/arm64/debug-pl011.inc
index 569c3dfbcf47..385deff49b1b 100644
--- a/xen/arch/arm/arm64/debug-pl011.inc
+++ b/xen/arch/arm/arm64/debug-pl011.inc
@@ -24,9 +24,9 @@
* c: scratch register number
*/
.macro early_uart_init xb, c
- mov x\c, #(7372800 / EARLY_PRINTK_BAUD % 16)
+ mov x\c, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE % 16)
strh w\c, [\xb, #0x28] /* -> UARTFBRD (Baud divisor fraction) */
- mov x\c, #(7372800 / EARLY_PRINTK_BAUD / 16)
+ mov x\c, #(7372800 / CONFIG_EARLY_UART_PL011_BAUD_RATE / 16)
strh w\c, [\xb, #0x24] /* -> UARTIBRD (Baud divisor integer) */
mov x\c, #0x60 /* 8n1 */
str w\c, [\xb, #0x2C] /* -> UARTLCR_H (Line control) */
diff --git a/xen/arch/arm/arm64/debug.S b/xen/arch/arm/arm64/debug.S
index b7f53ac0519b..71cad9d762b2 100644
--- a/xen/arch/arm/arm64/debug.S
+++ b/xen/arch/arm/arm64/debug.S
@@ -19,8 +19,8 @@
#include <asm/early_printk.h>
-#ifdef EARLY_PRINTK_INC
-#include EARLY_PRINTK_INC
+#ifdef CONFIG_EARLY_PRINTK_INC
+#include CONFIG_EARLY_PRINTK_INC
#endif
/*
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index e5015f93a2d8..4d45ea3dac3c 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -45,8 +45,8 @@
#define __HEAD_FLAGS ((__HEAD_FLAG_PAGE_SIZE << 1) | \
(__HEAD_FLAG_PHYS_BASE << 3))
-#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
-#include EARLY_PRINTK_INC
+#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
+#include CONFIG_EARLY_PRINTK_INC
#endif
/*
@@ -363,7 +363,7 @@ GLOBAL(init_secondary)
1:
#ifdef CONFIG_EARLY_PRINTK
- ldr x23, =EARLY_UART_BASE_ADDRESS /* x23 := UART base address */
+ ldr x23, =CONFIG_EARLY_UART_BASE_ADDRESS /* x23 := UART base address */
PRINT("- CPU ")
print_reg x24
PRINT(" booting -\r\n")
@@ -843,8 +843,8 @@ ENTRY(switch_ttbr)
* Clobbers x0 - x1
*/
init_uart:
- ldr x23, =EARLY_UART_BASE_ADDRESS
-#ifdef EARLY_PRINTK_INIT_UART
+ ldr x23, =CONFIG_EARLY_UART_BASE_ADDRESS
+#ifdef CONFIG_EARLY_UART_INIT
early_uart_init x23, 0
#endif
PRINT("- UART enabled -\r\n")
diff --git a/xen/include/asm-arm/early_printk.h b/xen/include/asm-arm/early_printk.h
index 078cf701dcb0..d5485decfa9f 100644
--- a/xen/include/asm-arm/early_printk.h
+++ b/xen/include/asm-arm/early_printk.h
@@ -15,7 +15,7 @@
/* need to add the uart address offset in page to the fixmap address */
#define EARLY_UART_VIRTUAL_ADDRESS \
- (FIXMAP_ADDR(FIXMAP_CONSOLE) +(EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
+ (FIXMAP_ADDR(FIXMAP_CONSOLE) + (CONFIG_EARLY_UART_BASE_ADDRESS & ~PAGE_MASK))
#endif /* !CONFIG_EARLY_PRINTK */
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-09 17:45 [Xen-devel] [XEN PATCH v3 0/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
2020-03-09 17:45 ` [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro Anthony PERARD
@ 2020-03-09 17:45 ` Anthony PERARD
2020-03-11 14:18 ` Julien Grall
2020-03-13 23:12 ` Stefano Stabellini
1 sibling, 2 replies; 18+ messages in thread
From: Anthony PERARD @ 2020-03-09 17:45 UTC (permalink / raw)
To: xen-devel
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
Ian Jackson, George Dunlap, Julien Grall, Jan Beulich,
Anthony PERARD, Volodymyr Babchuk, Roger Pau Monné
At the moment, early printk can only be configured on the make command
line. It is not very handy because a user has to remove the option
everytime it is using another command other than compiling the
hypervisor.
Furthermore, early printk is one of the few odds one that are not
using Kconfig.
So this is about time to move it to Kconfig.
The new kconfigs options allow a user to eather select a UART driver
to use at boot time, and set the parameters, or it is still possible
to select a platform which will set the parameters.
If CONFIG_EARLY_PRINTK is present in the environment or on the make
command line, make will return an error.
Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
Original patch:
[PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk using Kconfig
<20190913103953.8182-1-julien.grall@arm.com>
---
Notes:
v3:
- rename EARLY_PRINK to CONFIG_EARLY_PRINTK in makefile here (which
select which object to build).
- rename EARLY_UART_BAUD_RATE to EARLY_UART_PL011_BAUD_RATE
- typos
- drop the list of aliases in early-printk.txt. Kconfig choice menu
should be enough.
- reword early-printk.txt.
- rework how EARLY_PRINTK is set to Y
and use that instead of a list of all EARLY_UART_*
- Add a check to ask user to use Kconfig to set early printk.
- rework the possible choice to have all uart driver and platform
specific option together.
- have added or reword prompt and help messages of the different
options. The platform specific option don't have extended help, the
prompt is probably enough.
(The non-platform specific options have the help message that Julien
have written in the first version.)
- have made EARLY_UART_INIT dependent on the value of
EARLY_UART_PL011_BAUD_RATE so that there is no need to expose _INIT to
users.
docs/misc/arm/early-printk.txt | 71 +++-----
xen/Kconfig.debug | 2 +
xen/arch/arm/Kconfig.debug | 287 +++++++++++++++++++++++++++++++++
xen/arch/arm/Makefile | 2 +-
xen/arch/arm/Rules.mk | 74 +--------
xen/arch/arm/arm32/Makefile | 2 +-
xen/arch/arm/arm64/Makefile | 2 +-
xen/arch/x86/Kconfig.debug | 0
8 files changed, 317 insertions(+), 123 deletions(-)
create mode 100644 xen/arch/arm/Kconfig.debug
create mode 100644 xen/arch/x86/Kconfig.debug
diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
index 89e081e51eaf..c61973013097 100644
--- a/docs/misc/arm/early-printk.txt
+++ b/docs/misc/arm/early-printk.txt
@@ -1,64 +1,39 @@
How to enable early printk
-Early printk can only be enabled if debug=y. You may want to enable it if
-you are debbuging code that executes before the console is initialized.
+Early printk can only be enabled if CONFIG_DEBUG=y. You may want to enable
+it if you are debugging code that executes before the console is
+initialized.
Note that selecting this option will limit Xen to a single UART definition.
Attempting to boot Xen image on a different platform *will not work*, so this
option should not be enable for Xens that are intended to be portable.
-CONFIG_EARLY_PRINTK=<INC>,<BASE_ADDRESS>,<OTHER_OPTIONS>
+Select one of the "Early printk via * UART" in the choice possible for
+"Early printk" in the "Debugging options" of Kconfig. You will then need to
+set other options, which depends on the driver selected.
-<INC> and <BASE_ADDRESS> are mandatory arguments:
+CONFIG_EARLY_UART_BASE_ADDRESS is a mandatory argument, it is the base
+physical address of the UART to use.
- - <INC> is the name of the driver, see xen/arch/arm/arm{32,64}/debug-*.inc
- (where <INC> corresponds to the wildcarded *).
- - <BASE_ADDRESS> is the base physical address of the UART to use
+Other options depends on the driver selected:
+ - 8250
+ - CONFIG_EARLY_UART_8250_REG_SHIFT is, optionally, the left-shift to
+ apply to the register offsets within the uart.
+ - pl011
+ - CONFIG_EARLY_UART_PL011_BAUD_RATE is, optionally, a baud rate which
+ should be used to configure the UART at start of day.
-<OTHER_OPTIONS> varies depending on <INC>:
+ If CONFIG_EARLY_UART_PL011_BAUD_RATE is set to 0 then the code will
+ not try to initialize the UART, so that bootloader or firmware
+ settings can be used for maximum compatibility.
+ - scif
+ - CONFIG_EARLY_UART_SCIF_VERSION_* is, optionally, the interface version
+ of the UART. Default to version NONE.
- - 8250,<BASE_ADDRESS>,<REG_SHIFT>
- - <REG_SHIFT> is, optionally, the left-shift to apply to the
- register offsets within the uart.
- - pl011,<BASE_ADDRESS>,<BAUD_RATE>
- - <BAUD_RATE> is, optionally a baud rate which should be used to
- configure the UART at start of day.
-
- If <BAUD_RATE> is not given then the code will not try to
- initialize the UART, so that bootloader or firmware settings can
- be used for maximum compatibility.
- - scif,<BASE_ADDRESS>,<VERSION>
- - SCIF<VERSION> is, optionally, the interface version of the UART.
-
- If <VERSION> is not given then the default interface version (SCIF)
- will be used.
- For all other uarts there are no additional options.
As a convenience it is also possible to select from a list of
-predefined configurations using CONFIG_EARLY_PRINTK=mach where mach is
-the name of the machine:
-
- - brcm: printk with 8250 on Broadcom 7445D0 boards with A15 processors.
- - dra7: printk with 8250 on DRA7 platform
- - exynos5250: printk with the second UART
- - fastmodel: printk on ARM Fastmodel software emulators
- - hikey960: printk with pl011 with Hikey 960
- - juno: printk with pl011 on Juno platform
- - lager: printk with SCIF0 on Renesas Lager board (R-Car H2 processor)
- - midway: printk with the pl011 on Calxeda Midway processors
- - mvebu: printk with the MVEBU for Marvell Armada 3700 SoCs
- - omap5432: printk with UART3 on TI OMAP5432 processors
- - rcar3: printk with SCIF2 on Renesas R-Car Gen3 processors
- - seattle: printk with pl011 for AMD Seattle processor
- - sun6i: printk with 8250 on Allwinner A31 processors
- - sun7i: printk with 8250 on Allwinner A20 processors
- - thunderx: printk with pl011 for Cavium ThunderX processor
- - vexpress: printk with pl011 for versatile express
- - xgene-mcdivitt: printk with 820 on Xgene mcdivitt platform
- - xgene-storm: printk with 820 on Xgene storm platform
- - zynqmp: printk with Cadence UART for Xilinx ZynqMP SoCs
-
-These settings are is hardcoded in xen/arch/arm/Rules.mk,
-see there when adding support for new machines.
+predefined configurations available in the list of choice for "Early
+printk" for specific platform.
By default early printk is disabled.
diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
index b3511e81a275..ee6ee33b69be 100644
--- a/xen/Kconfig.debug
+++ b/xen/Kconfig.debug
@@ -128,6 +128,8 @@ config XMEM_POOL_POISON
Poison free blocks with 0xAA bytes and verify them when a block is
allocated in order to spot use-after-free issues.
+source "arch/$(SRCARCH)/Kconfig.debug"
+
endif # DEBUG || EXPERT
endmenu
diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
new file mode 100644
index 000000000000..ffb21e8ac20a
--- /dev/null
+++ b/xen/arch/arm/Kconfig.debug
@@ -0,0 +1,287 @@
+choice
+ bool "Early printk"
+ optional
+ help
+ You may want to enable early printk if you are debugging code
+ that executes before the console is initialized.
+
+ Note that selecting this option will limit Xen to a single UART
+ definition. Attempting to boot Xen image on a different
+ platform *will not work*, so this option should not be enable
+ for Xens that are intended to be portable.
+
+ Choose one of the UART drivers for early printk, then you'll
+ have to specify the parameters, like the base address.
+
+ Alternatively, there are platform specific options which will
+ have default values for the various parameters.
+
+ config EARLY_UART_CHOICE_8250
+ select EARLY_UART_8250
+ bool "Early printk via 8250 UART"
+ help
+ Say Y here if you wish the early printk to direct their
+ output to a 8250 UART. You can use this option to
+ provide the parameters for the 8250 UART rather than
+ selecting one of the platform specific options below if
+ you know the parameters for the port.
+
+ This option is preferred over the platform specific
+ options; the platform specific options are deprecated
+ and will soon be removed.
+ config EARLY_UART_CHOICE_CADENCE
+ select EARLY_UART_CADENCE
+ depends on ARM_64
+ bool "Early printk via Cadence UART"
+ help
+ Say Y here if you wish the early printk to direct their
+ output to a Cadence UART. You can use this option to
+ provide the parameters for the Cadence UART rather than
+ selecting one of the platform specific options below if
+ you know the parameters for the port.
+
+ This option is preferred over the platform specific
+ options; the platform specific options are deprecated
+ and will soon be removed.
+
+ config EARLY_UART_CHOICE_EXYNOS4210
+ select EARLY_UART_EXYNOS4210
+ depends on ARM_32
+ bool "Early printk via Exynos4210 UART"
+ help
+ Say Y here if you wish the early printk to direct their
+ output to a Exynos 4210 UART. You can use this option to
+ provide the parameters for the Exynos 4210 UART rather than
+ selecting one of the platform specific options below if
+ you know the parameters for the port.
+
+ This option is preferred over the platform specific
+ options; the platform specific options are deprecated
+ and will soon be removed.
+ config EARLY_UART_CHOICE_MESON
+ select EARLY_UART_MESON
+ depends on ARM_64
+ bool "Early printk via MESON UART"
+ help
+ Say Y here if you wish the early printk to direct their
+ output to a MESON UART. You can use this option to
+ provide the parameters for the MESON UART rather than
+ selecting one of the platform specific options below if
+ you know the parameters for the port.
+
+ This option is preferred over the platform specific
+ options; the platform specific options are deprecated
+ and will soon be removed.
+ config EARLY_UART_CHOICE_MVEBU
+ select EARLY_UART_MVEBU
+ depends on ARM_64
+ bool "Early printk via MVEBU UART"
+ help
+ Say Y here if you wish the early printk to direct their
+ output to a MVEBU UART. You can use this option to
+ provide the parameters for the MVEBU UART rather than
+ selecting one of the platform specific options below if
+ you know the parameters for the port.
+
+ This option is preferred over the platform specific
+ options; the platform specific options are deprecated
+ and will soon be removed.
+ config EARLY_UART_CHOICE_PL011
+ select EARLY_UART_PL011
+ bool "Early printk via PL011 UART"
+ help
+ Say Y here if you wish the early printk to direct their
+ output to a PL011 UART. You can use this option to
+ provide the parameters for the PL011 UART rather than
+ selecting one of the platform specific options below if
+ you know the parameters for the port.
+
+ This option is preferred over the platform specific
+ options; the platform specific options are deprecated
+ and will soon be removed.
+ config EARLY_UART_CHOICE_SCIF
+ select EARLY_UART_SCIF
+ bool "Early printk via SCIF UART"
+ help
+ Say Y here if you wish the early printk to direct their
+ output to a SCIF UART. You can use this option to
+ provide the parameters for the SCIF UART rather than
+ selecting one of the platform specific options below if
+ you know the parameters for the port.
+
+ This option is preferred over the platform specific
+ options; the platform specific options are deprecated
+ and will soon be removed.
+
+ config EARLY_PRINTK_BRCM
+ bool "Early printk with 8250 on Broadcom 7445D0 boards with A15 processors"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_DRA7
+ bool "Early printk with 8250 on DRA7 platform"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_EXYNOS5250
+ bool "Early printk with the second UART on Exynos5250"
+ select EARLY_UART_EXYNOS4210
+ depends on ARM_32
+ config EARLY_PRINTK_FASTMODEL
+ bool "Early printk with pl011 on ARM Fastmodel software emulators"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_HIKEY960
+ bool "Early printk with pl011 with Hikey 960"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_JUNO
+ bool "Early printk with pl011 on Juno platform"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_LAGER
+ bool "Early printk with SCIF0 on Renesas Lager board (R-Car H2 processor)"
+ select EARLY_UART_SCIF
+ config EARLY_PRINTK_MIDWAY
+ bool "Early printk with pl011 on Calxeda Midway processors"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_MVEBU
+ bool "Early printk with MVEBU for Marvell Armada 3700 SoCs"
+ select EARLY_UART_MVEBU
+ depends on ARM_64
+ config EARLY_PRINTK_OMAP5432
+ bool "Early printk with UART3 on TI OMAP5432 processors"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_RCAR3
+ bool "Early printk with SCIF2 on Renesas R-Car Gen3 processors"
+ select EARLY_UART_SCIF
+ config EARLY_PRINTK_SEATTLE
+ bool "Early printk with pl011 for AMD Seattle processor"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_SUN6I
+ bool "Early printk with 8250 on Allwinner A31 processors"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_SUN7I
+ bool "Early printk with 8250 on Allwinner A20 processors"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_THUNDERX
+ bool "Early printk with pl011 for Cavium ThunderX processor"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_VEXPRESS
+ bool "Early printk with pl011 for versatile express"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_XGENE_MCDIVITT
+ bool "Early printk with 820 on Xgene mcdivitt platform"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_XGENE_STORM
+ bool "Early printk with 820 on Xgene storm platform"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_ZYNQMP
+ bool "Early printk with Cadence UART for Xilinx ZynqMP SoCs"
+ select EARLY_UART_CADENCE
+ depends on ARM_64
+endchoice
+
+
+config EARLY_UART_8250
+ select EARLY_PRINTK
+ bool
+config EARLY_UART_CADENCE
+ select EARLY_PRINTK
+ bool
+config EARLY_UART_EXYNOS4210
+ select EARLY_PRINTK
+ bool
+config EARLY_UART_MESON
+ select EARLY_PRINTK
+ bool
+config EARLY_UART_MVEBU
+ select EARLY_PRINTK
+ bool
+config EARLY_UART_PL011
+ select EARLY_PRINTK
+ bool
+config EARLY_UART_SCIF
+ select EARLY_PRINTK
+ bool
+
+config EARLY_PRINTK
+ bool
+
+config EARLY_UART_BASE_ADDRESS
+ depends on EARLY_PRINTK
+ hex "Early printk, physical base address of debug UART"
+ default 0xF040AB00 if EARLY_PRINTK_BRCM
+ default 0x4806A000 if EARLY_PRINTK_DRA7
+ default 0x1c090000 if EARLY_PRINTK_FASTMODEL
+ default 0x12c20000 if EARLY_PRINTK_EXYNOS5250
+ default 0xfff32000 if EARLY_PRINTK_HIKEY960
+ default 0x7ff80000 if EARLY_PRINTK_JUNO
+ default 0xe6e60000 if EARLY_PRINTK_LAGER
+ default 0xfff36000 if EARLY_PRINTK_MIDWAY
+ default 0xd0012000 if EARLY_PRINTK_MVEBU
+ default 0x48020000 if EARLY_PRINTK_OMAP5432
+ default 0xe6e88000 if EARLY_PRINTK_RCAR3
+ default 0xe1010000 if EARLY_PRINTK_SEATTLE
+ default 0x01c28000 if EARLY_PRINTK_SUN6I
+ default 0x01c28000 if EARLY_PRINTK_SUN7I
+ default 0x87e024000000 if EARLY_PRINTK_THUNDERX
+ default 0x1c090000 if EARLY_PRINTK_VEXPRESS
+ default 0x1c021000 if EARLY_PRINTK_XGENE_MCDIVITT
+ default 0x1c020000 if EARLY_PRINTK_XGENE_STORM
+ default 0xff000000 if EARLY_PRINTK_ZYNQMP
+
+config EARLY_UART_PL011_BAUD_RATE
+ depends on EARLY_UART_PL011
+ int "Early printk UART baud rate for pl011"
+ help
+ Optionally sets the baud rate which should be used to configure
+ the UART at start of day.
+
+ If EARLY_UART_PL011_BAUD_RATE is set to 0 then the code will
+ not try to initialize the UART, so that bootloader or firmware
+ settings can be used for maximum compatibility.
+
+ default 115200 if EARLY_PRINTK_FASTMODEL
+ default 0
+
+config EARLY_UART_INIT
+ depends on EARLY_UART_PL011 && EARLY_UART_PL011_BAUD_RATE != 0
+ def_bool y
+
+config EARLY_UART_8250_REG_SHIFT
+ depends on EARLY_UART_8250
+ int "Early printk, left-shift to apply to the register offsets within the 8250 UART"
+ help
+ EARLY_UART_8250_REG_SHIFT is, optionally, the left-shift to
+ apply to the register offsets within the UART with early
+ printk.
+
+ Default to 0.
+
+ default 2 if EARLY_PRINTK_BRCM
+ default 2 if EARLY_PRINTK_DRA7
+ default 2 if EARLY_PRINTK_OMAP5432
+ default 2 if EARLY_PRINTK_SUN6I
+ default 2 if EARLY_PRINTK_SUN7I
+ default 2 if EARLY_PRINTK_XGENE_MCDIVITT
+ default 2 if EARLY_PRINTK_XGENE_STORM
+ default 0
+
+choice EARLY_UART_SCIF_VERSION
+ prompt "Early printk UART SCIF interface version"
+ depends on EARLY_UART_SCIF
+ default EARLY_UART_SCIF_VERSION_NONE
+ help
+ Select the interface version of the SCIF UART.
+
+ Select EARLY_UART_SCIF_VERSION_NONE to use the default
+ interface version (SCIF).
+ config EARLY_UART_SCIF_VERSION_NONE
+ bool "default SCIF UART interface"
+ config EARLY_UART_SCIF_VERSION_A
+ bool "SCIF UART interface version A"
+endchoice
+
+config EARLY_PRINTK_INC
+ string
+ default "debug-8250.inc" if EARLY_UART_8250
+ default "debug-cadence.inc" if EARLY_UART_CADENCE
+ default "debug-exynos4210.inc" if EARLY_UART_EXYNOS4210
+ default "debug-meson.inc" if EARLY_UART_MESON
+ default "debug-mvebu.inc" if EARLY_UART_MVEBU
+ default "debug-pl011.inc" if EARLY_UART_PL011
+ default "debug-scif.inc" if EARLY_UART_SCIF
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 1044c2298a05..12f92a4bd3f9 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -16,7 +16,7 @@ obj-y += device.o
obj-y += domain.o
obj-y += domain_build.init.o
obj-y += domctl.o
-obj-$(EARLY_PRINTK) += early_printk.o
+obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
obj-y += gic.o
obj-y += gic-v2.o
obj-$(CONFIG_GICV3) += gic-v3.o
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index faa09ea111ec..3ad284aa71a4 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -18,76 +18,6 @@ CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-a15
CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
-EARLY_PRINTK := n
-
-ifeq ($(CONFIG_DEBUG),y)
-
-# See docs/misc/arm/early-printk.txt for syntax
-
-EARLY_PRINTK_brcm := 8250,0xF040AB00,2
-EARLY_PRINTK_dra7 := 8250,0x4806A000,2
-EARLY_PRINTK_fastmodel := pl011,0x1c090000,115200
-EARLY_PRINTK_exynos5250 := exynos4210,0x12c20000
-EARLY_PRINTK_hikey960 := pl011,0xfff32000
-EARLY_PRINTK_juno := pl011,0x7ff80000
-EARLY_PRINTK_lager := scif,0xe6e60000
-EARLY_PRINTK_midway := pl011,0xfff36000
-EARLY_PRINTK_mvebu := mvebu,0xd0012000
-EARLY_PRINTK_omap5432 := 8250,0x48020000,2
-EARLY_PRINTK_rcar3 := scif,0xe6e88000
-EARLY_PRINTK_seattle := pl011,0xe1010000
-EARLY_PRINTK_sun6i := 8250,0x01c28000,2
-EARLY_PRINTK_sun7i := 8250,0x01c28000,2
-EARLY_PRINTK_thunderx := pl011,0x87e024000000
-EARLY_PRINTK_vexpress := pl011,0x1c090000
-EARLY_PRINTK_xgene-mcdivitt := 8250,0x1c021000,2
-EARLY_PRINTK_xgene-storm := 8250,0x1c020000,2
-EARLY_PRINTK_zynqmp := cadence,0xff000000
-
-ifneq ($(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)),)
-EARLY_PRINTK_CFG := $(subst $(comma), ,$(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)))
-else
-EARLY_PRINTK_CFG := $(subst $(comma), ,$(CONFIG_EARLY_PRINTK))
-endif
-
-# Extract configuration from string
-EARLY_PRINTK_INC := $(word 1,$(EARLY_PRINTK_CFG))
-EARLY_UART_BASE_ADDRESS := $(word 2,$(EARLY_PRINTK_CFG))
-
-# UART specific options
-ifeq ($(EARLY_PRINTK_INC),8250)
-EARLY_UART_REG_SHIFT := $(word 3,$(EARLY_PRINTK_CFG))
-endif
-ifeq ($(EARLY_PRINTK_INC),pl011)
-ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
-EARLY_PRINTK_INIT_UART := y
-EARLY_PRINTK_BAUD := $(word 3,$(EARLY_PRINTK_CFG))
-endif
-endif
-ifeq ($(EARLY_PRINTK_INC),scif)
-ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
-CFLAGS-y += -DCONFIG_EARLY_UART_SCIF_VERSION_$(word 3,$(EARLY_PRINTK_CFG))
-else
-CFLAGS-y += -DCONFIG_EARLY_UART_SCIF_VERSION_NONE
-endif
-endif
-
-ifneq ($(EARLY_PRINTK_INC),)
-EARLY_PRINTK := y
-endif
-
-CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
-CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DCONFIG_EARLY_UART_INIT
-CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
-CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_UART_PL011_BAUD_RATE=$(EARLY_PRINTK_BAUD)
-CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
-CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_UART_8250_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
-
-else # !CONFIG_DEBUG
-
-ifneq ($(CONFIG_EARLY_PRINTK),)
-# Early printk is dependant on a debug build.
-$(error CONFIG_EARLY_PRINTK enabled for non-debug build)
-endif
-
+ifneq ($(filter command line environment,$(origin CONFIG_EARLY_PRINTK)),)
+ $(error You must use 'make menuconfig' to enable/disable early printk now)
endif
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 539bbef298a7..96105d238307 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -1,6 +1,6 @@
obj-y += lib/
-obj-$(EARLY_PRINTK) += debug.o
+obj-$(CONFIG_EARLY_PRINTK) += debug.o
obj-y += domctl.o
obj-y += domain.o
obj-y += entry.o
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index db8565b71a33..40642ff57494 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -2,7 +2,7 @@ obj-y += lib/
obj-y += cache.o
obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
-obj-$(EARLY_PRINTK) += debug.o
+obj-$(CONFIG_EARLY_PRINTK) += debug.o
obj-y += domctl.o
obj-y += domain.o
obj-y += entry.o
diff --git a/xen/arch/x86/Kconfig.debug b/xen/arch/x86/Kconfig.debug
new file mode 100644
index 000000000000..e69de29bb2d1
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro
2020-03-09 17:45 ` [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro Anthony PERARD
@ 2020-03-11 13:57 ` Julien Grall
2020-03-11 14:46 ` Anthony PERARD
0 siblings, 1 reply; 18+ messages in thread
From: Julien Grall @ 2020-03-11 13:57 UTC (permalink / raw)
To: Anthony PERARD, xen-devel; +Cc: Stefano Stabellini, Volodymyr Babchuk
Hi Anthony,
On 09/03/2020 17:45, Anthony PERARD wrote:
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index e9d356f05c2b..2b593c5ef99a 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -36,8 +36,8 @@
> #define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START)
> #define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
>
> -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> -#include EARLY_PRINTK_INC
> +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
NIT: I would also take the opportunity to clean-up the line by remove
the extra () and the space before (. Something like:
#if define(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)
[...]
> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> index e5015f93a2d8..4d45ea3dac3c 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -45,8 +45,8 @@
> #define __HEAD_FLAGS ((__HEAD_FLAG_PAGE_SIZE << 1) | \
> (__HEAD_FLAG_PHYS_BASE << 3))
>
> -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> -#include EARLY_PRINTK_INC
> +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
Same here.
I am happy to fix both cases on commit:
Acked-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-09 17:45 ` [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
@ 2020-03-11 14:18 ` Julien Grall
[not found] ` <20200311152613.GJ2152@perard.uk.xensource.com>
2020-03-13 23:12 ` Stefano Stabellini
1 sibling, 1 reply; 18+ messages in thread
From: Julien Grall @ 2020-03-11 14:18 UTC (permalink / raw)
To: Anthony PERARD, xen-devel
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
George Dunlap, Julien Grall, Jan Beulich, Volodymyr Babchuk,
Roger Pau Monné
Hi Anthony,
On 09/03/2020 17:45, Anthony PERARD wrote:
> At the moment, early printk can only be configured on the make command
> line. It is not very handy because a user has to remove the option
> everytime it is using another command other than compiling the
> hypervisor.
>
> Furthermore, early printk is one of the few odds one that are not
> using Kconfig.
>
> So this is about time to move it to Kconfig.
>
> The new kconfigs options allow a user to eather select a UART driver
> to use at boot time, and set the parameters, or it is still possible
> to select a platform which will set the parameters.
>
> If CONFIG_EARLY_PRINTK is present in the environment or on the make
> command line, make will return an error.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
The patch looks good to me. I have only a couple of comments (see below).
>
> ---
>
> Original patch:
> [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk using Kconfig
> <20190913103953.8182-1-julien.grall@arm.com>
> ---
>
> Notes:
> v3:
> - rename EARLY_PRINK to CONFIG_EARLY_PRINTK in makefile here (which
> select which object to build).
> - rename EARLY_UART_BAUD_RATE to EARLY_UART_PL011_BAUD_RATE
> - typos
> - drop the list of aliases in early-printk.txt. Kconfig choice menu
> should be enough.
> - reword early-printk.txt.
> - rework how EARLY_PRINTK is set to Y
> and use that instead of a list of all EARLY_UART_*
> - Add a check to ask user to use Kconfig to set early printk.
> - rework the possible choice to have all uart driver and platform
> specific option together.
> - have added or reword prompt and help messages of the different
> options. The platform specific option don't have extended help, the
> prompt is probably enough.
> (The non-platform specific options have the help message that Julien
> have written in the first version.)
> - have made EARLY_UART_INIT dependent on the value of
> EARLY_UART_PL011_BAUD_RATE so that there is no need to expose _INIT to
> users.
>
> docs/misc/arm/early-printk.txt | 71 +++-----
> xen/Kconfig.debug | 2 +
> xen/arch/arm/Kconfig.debug | 287 +++++++++++++++++++++++++++++++++
> xen/arch/arm/Makefile | 2 +-
> xen/arch/arm/Rules.mk | 74 +--------
> xen/arch/arm/arm32/Makefile | 2 +-
> xen/arch/arm/arm64/Makefile | 2 +-
> xen/arch/x86/Kconfig.debug | 0
> 8 files changed, 317 insertions(+), 123 deletions(-)
> create mode 100644 xen/arch/arm/Kconfig.debug
> create mode 100644 xen/arch/x86/Kconfig.debug
>
> diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
> index 89e081e51eaf..c61973013097 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -1,64 +1,39 @@
> How to enable early printk
>
> -Early printk can only be enabled if debug=y. You may want to enable it if
> -you are debbuging code that executes before the console is initialized.
> +Early printk can only be enabled if CONFIG_DEBUG=y. You may want to enable
NIT: AFAICT, the file is using one space after full stop. I would like
to keep it like that for consistency :).
> +it if you are debugging code that executes before the console is
> +initialized.
>
> Note that selecting this option will limit Xen to a single UART definition.
> Attempting to boot Xen image on a different platform *will not work*, so this
> option should not be enable for Xens that are intended to be portable.
>
> -CONFIG_EARLY_PRINTK=<INC>,<BASE_ADDRESS>,<OTHER_OPTIONS>
> +Select one of the "Early printk via * UART" in the choice possible for
> +"Early printk" in the "Debugging options" of Kconfig. You will then need to
> +set other options, which depends on the driver selected.
>
> -<INC> and <BASE_ADDRESS> are mandatory arguments:
> +CONFIG_EARLY_UART_BASE_ADDRESS is a mandatory argument, it is the base
> +physical address of the UART to use.
>
> - - <INC> is the name of the driver, see xen/arch/arm/arm{32,64}/debug-*.inc
> - (where <INC> corresponds to the wildcarded *).
> - - <BASE_ADDRESS> is the base physical address of the UART to use
> +Other options depends on the driver selected:
> + - 8250
> + - CONFIG_EARLY_UART_8250_REG_SHIFT is, optionally, the left-shift to
> + apply to the register offsets within the uart.
> + - pl011
> + - CONFIG_EARLY_UART_PL011_BAUD_RATE is, optionally, a baud rate which
> + should be used to configure the UART at start of day.
>
> -<OTHER_OPTIONS> varies depending on <INC>:
> + If CONFIG_EARLY_UART_PL011_BAUD_RATE is set to 0 then the code will
> + not try to initialize the UART, so that bootloader or firmware
> + settings can be used for maximum compatibility.
> + - scif
> + - CONFIG_EARLY_UART_SCIF_VERSION_* is, optionally, the interface version
> + of the UART. Default to version NONE.
>
> - - 8250,<BASE_ADDRESS>,<REG_SHIFT>
> - - <REG_SHIFT> is, optionally, the left-shift to apply to the
> - register offsets within the uart.
> - - pl011,<BASE_ADDRESS>,<BAUD_RATE>
> - - <BAUD_RATE> is, optionally a baud rate which should be used to
> - configure the UART at start of day.
> -
> - If <BAUD_RATE> is not given then the code will not try to
> - initialize the UART, so that bootloader or firmware settings can
> - be used for maximum compatibility.
> - - scif,<BASE_ADDRESS>,<VERSION>
> - - SCIF<VERSION> is, optionally, the interface version of the UART.
> -
> - If <VERSION> is not given then the default interface version (SCIF)
> - will be used.
> - For all other uarts there are no additional options.
>
> As a convenience it is also possible to select from a list of
> -predefined configurations using CONFIG_EARLY_PRINTK=mach where mach is
> -the name of the machine:
> -
> - - brcm: printk with 8250 on Broadcom 7445D0 boards with A15 processors.
> - - dra7: printk with 8250 on DRA7 platform
> - - exynos5250: printk with the second UART
> - - fastmodel: printk on ARM Fastmodel software emulators
> - - hikey960: printk with pl011 with Hikey 960
> - - juno: printk with pl011 on Juno platform
> - - lager: printk with SCIF0 on Renesas Lager board (R-Car H2 processor)
> - - midway: printk with the pl011 on Calxeda Midway processors
> - - mvebu: printk with the MVEBU for Marvell Armada 3700 SoCs
> - - omap5432: printk with UART3 on TI OMAP5432 processors
> - - rcar3: printk with SCIF2 on Renesas R-Car Gen3 processors
> - - seattle: printk with pl011 for AMD Seattle processor
> - - sun6i: printk with 8250 on Allwinner A31 processors
> - - sun7i: printk with 8250 on Allwinner A20 processors
> - - thunderx: printk with pl011 for Cavium ThunderX processor
> - - vexpress: printk with pl011 for versatile express
> - - xgene-mcdivitt: printk with 820 on Xgene mcdivitt platform
> - - xgene-storm: printk with 820 on Xgene storm platform
> - - zynqmp: printk with Cadence UART for Xilinx ZynqMP SoCs
> -
> -These settings are is hardcoded in xen/arch/arm/Rules.mk,
> -see there when adding support for new machines.
> +predefined configurations available in the list of choice for "Early
> +printk" for specific platform.
>
> By default early printk is disabled.
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index b3511e81a275..ee6ee33b69be 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -128,6 +128,8 @@ config XMEM_POOL_POISON
> Poison free blocks with 0xAA bytes and verify them when a block is
> allocated in order to spot use-after-free issues.
>
> +source "arch/$(SRCARCH)/Kconfig.debug"
To double check, this means that earlyprintk can be selected in EXPERT
mode now. However, in the document early-printk.txt, the feature is said
to only be enabled with CONFIG_DEBUG=y.
I like the idea of allowing a user to enable earlyprintk in EXPERT mode
(some early boot bug may only occur in non-debug build). So I am happy
to keep the code like. Can you update the doc accordingly?
> +
> endif # DEBUG || EXPERT
>
> endmenu
> diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
> new file mode 100644
> index 000000000000..ffb21e8ac20a
> --- /dev/null
> +++ b/xen/arch/arm/Kconfig.debug
> @@ -0,0 +1,287 @@
> +choice
> + bool "Early printk"
> + optional
> + help
> + You may want to enable early printk if you are debugging code
> + that executes before the console is initialized.
> +
> + Note that selecting this option will limit Xen to a single UART
> + definition. Attempting to boot Xen image on a different
> + platform *will not work*, so this option should not be enable
> + for Xens that are intended to be portable.
> +
> + Choose one of the UART drivers for early printk, then you'll
> + have to specify the parameters, like the base address.
> +
> + Alternatively, there are platform specific options which will
> + have default values for the various parameters.
Would it be worth to mention such options are deprecated?
> +
> + config EARLY_UART_CHOICE_8250
> + select EARLY_UART_8250
> + bool "Early printk via 8250 UART"
> + help
> + Say Y here if you wish the early printk to direct their
> + output to a 8250 UART. You can use this option to
> + provide the parameters for the 8250 UART rather than
> + selecting one of the platform specific options below if
> + you know the parameters for the port.
> +
> + This option is preferred over the platform specific
> + options; the platform specific options are deprecated
> + and will soon be removed.
> + config EARLY_UART_CHOICE_CADENCE
> + select EARLY_UART_CADENCE
> + depends on ARM_64
> + bool "Early printk via Cadence UART"
> + help
> + Say Y here if you wish the early printk to direct their
> + output to a Cadence UART. You can use this option to
> + provide the parameters for the Cadence UART rather than
> + selecting one of the platform specific options below if
> + you know the parameters for the port.
> +
> + This option is preferred over the platform specific
> + options; the platform specific options are deprecated
> + and will soon be removed.
> +
> + config EARLY_UART_CHOICE_EXYNOS4210
> + select EARLY_UART_EXYNOS4210
> + depends on ARM_32
> + bool "Early printk via Exynos4210 UART"
> + help
> + Say Y here if you wish the early printk to direct their
> + output to a Exynos 4210 UART. You can use this option to
> + provide the parameters for the Exynos 4210 UART rather than
> + selecting one of the platform specific options below if
> + you know the parameters for the port.
> +
> + This option is preferred over the platform specific
> + options; the platform specific options are deprecated
> + and will soon be removed.
> + config EARLY_UART_CHOICE_MESON
> + select EARLY_UART_MESON
> + depends on ARM_64
> + bool "Early printk via MESON UART"
> + help
> + Say Y here if you wish the early printk to direct their
> + output to a MESON UART. You can use this option to
> + provide the parameters for the MESON UART rather than
> + selecting one of the platform specific options below if
> + you know the parameters for the port.
> +
> + This option is preferred over the platform specific
> + options; the platform specific options are deprecated
> + and will soon be removed.
> + config EARLY_UART_CHOICE_MVEBU
> + select EARLY_UART_MVEBU
> + depends on ARM_64
> + bool "Early printk via MVEBU UART"
> + help
> + Say Y here if you wish the early printk to direct their
> + output to a MVEBU UART. You can use this option to
> + provide the parameters for the MVEBU UART rather than
> + selecting one of the platform specific options below if
> + you know the parameters for the port.
> +
> + This option is preferred over the platform specific
> + options; the platform specific options are deprecated
> + and will soon be removed.
> + config EARLY_UART_CHOICE_PL011
> + select EARLY_UART_PL011
> + bool "Early printk via PL011 UART"
> + help
> + Say Y here if you wish the early printk to direct their
> + output to a PL011 UART. You can use this option to
> + provide the parameters for the PL011 UART rather than
> + selecting one of the platform specific options below if
> + you know the parameters for the port.
> +
> + This option is preferred over the platform specific
> + options; the platform specific options are deprecated
> + and will soon be removed.
> + config EARLY_UART_CHOICE_SCIF
> + select EARLY_UART_SCIF
> + bool "Early printk via SCIF UART"
> + help
> + Say Y here if you wish the early printk to direct their
> + output to a SCIF UART. You can use this option to
> + provide the parameters for the SCIF UART rather than
> + selecting one of the platform specific options below if
> + you know the parameters for the port.
> +
> + This option is preferred over the platform specific
> + options; the platform specific options are deprecated
> + and will soon be removed.
> +
> + config EARLY_PRINTK_BRCM
> + bool "Early printk with 8250 on Broadcom 7445D0 boards with A15 processors"
> + select EARLY_UART_8250
I noticed below you added "depends on ARM_64" on the Xilinx SoC. In
general, platform specific options are tied to either arm32 or arm64,
even if the UART "driver" is arch agnostic.
You could technically boot Xen on Arm 32-bit on Armv8 HW provided they
support 32-bit at the hypervisor level, but we never supported this
case. So I am wondering whether we should add depends on each
earlyprintk. Stefano, any opinions?
> + config EARLY_PRINTK_DRA7
> + bool "Early printk with 8250 on DRA7 platform"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_EXYNOS5250
> + bool "Early printk with the second UART on Exynos5250"
> + select EARLY_UART_EXYNOS4210
> + depends on ARM_32
> + config EARLY_PRINTK_FASTMODEL
> + bool "Early printk with pl011 on ARM Fastmodel software emulators"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_HIKEY960
> + bool "Early printk with pl011 with Hikey 960"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_JUNO
> + bool "Early printk with pl011 on Juno platform"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_LAGER
> + bool "Early printk with SCIF0 on Renesas Lager board (R-Car H2 processor)"
> + select EARLY_UART_SCIF
> + config EARLY_PRINTK_MIDWAY
> + bool "Early printk with pl011 on Calxeda Midway processors"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_MVEBU
> + bool "Early printk with MVEBU for Marvell Armada 3700 SoCs"
> + select EARLY_UART_MVEBU
> + depends on ARM_64
> + config EARLY_PRINTK_OMAP5432
> + bool "Early printk with UART3 on TI OMAP5432 processors"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_RCAR3
> + bool "Early printk with SCIF2 on Renesas R-Car Gen3 processors"
> + select EARLY_UART_SCIF
> + config EARLY_PRINTK_SEATTLE
> + bool "Early printk with pl011 for AMD Seattle processor"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_SUN6I
> + bool "Early printk with 8250 on Allwinner A31 processors"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_SUN7I
> + bool "Early printk with 8250 on Allwinner A20 processors"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_THUNDERX
> + bool "Early printk with pl011 for Cavium ThunderX processor"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_VEXPRESS
> + bool "Early printk with pl011 for versatile express"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_XGENE_MCDIVITT
> + bool "Early printk with 820 on Xgene mcdivitt platform"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_XGENE_STORM
> + bool "Early printk with 820 on Xgene storm platform"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_ZYNQMP
> + bool "Early printk with Cadence UART for Xilinx ZynqMP SoCs"
> + select EARLY_UART_CADENCE
> + depends on ARM_64
> +endchoice
> +
> +
> +config EARLY_UART_8250
> + select EARLY_PRINTK
> + bool
> +config EARLY_UART_CADENCE
> + select EARLY_PRINTK
> + bool
> +config EARLY_UART_EXYNOS4210
> + select EARLY_PRINTK
> + bool
> +config EARLY_UART_MESON
> + select EARLY_PRINTK
> + bool
> +config EARLY_UART_MVEBU
> + select EARLY_PRINTK
> + bool
> +config EARLY_UART_PL011
> + select EARLY_PRINTK
> + bool
> +config EARLY_UART_SCIF
> + select EARLY_PRINTK
> + bool
> +
> +config EARLY_PRINTK
> + bool
> +
> +config EARLY_UART_BASE_ADDRESS
> + depends on EARLY_PRINTK
> + hex "Early printk, physical base address of debug UART"
> + default 0xF040AB00 if EARLY_PRINTK_BRCM
> + default 0x4806A000 if EARLY_PRINTK_DRA7
> + default 0x1c090000 if EARLY_PRINTK_FASTMODEL
> + default 0x12c20000 if EARLY_PRINTK_EXYNOS5250
> + default 0xfff32000 if EARLY_PRINTK_HIKEY960
> + default 0x7ff80000 if EARLY_PRINTK_JUNO
> + default 0xe6e60000 if EARLY_PRINTK_LAGER
> + default 0xfff36000 if EARLY_PRINTK_MIDWAY
> + default 0xd0012000 if EARLY_PRINTK_MVEBU
> + default 0x48020000 if EARLY_PRINTK_OMAP5432
> + default 0xe6e88000 if EARLY_PRINTK_RCAR3
> + default 0xe1010000 if EARLY_PRINTK_SEATTLE
> + default 0x01c28000 if EARLY_PRINTK_SUN6I
> + default 0x01c28000 if EARLY_PRINTK_SUN7I
> + default 0x87e024000000 if EARLY_PRINTK_THUNDERX
You are allowing EARLY_PRINTK_THUNDERX to be selected on Arm32 platform
but the address is above 4G. I suspect this would break randconfig build.
> + default 0x1c090000 if EARLY_PRINTK_VEXPRESS
> + default 0x1c021000 if EARLY_PRINTK_XGENE_MCDIVITT
> + default 0x1c020000 if EARLY_PRINTK_XGENE_STORM
> + default 0xff000000 if EARLY_PRINTK_ZYNQMP
> +
> +config EARLY_UART_PL011_BAUD_RATE
> + depends on EARLY_UART_PL011
> + int "Early printk UART baud rate for pl011"
> + help
> + Optionally sets the baud rate which should be used to configure
> + the UART at start of day.
> +
> + If EARLY_UART_PL011_BAUD_RATE is set to 0 then the code will
> + not try to initialize the UART, so that bootloader or firmware
> + settings can be used for maximum compatibility.
> +
> + default 115200 if EARLY_PRINTK_FASTMODEL
> + default 0
> +
> +config EARLY_UART_INIT
> + depends on EARLY_UART_PL011 && EARLY_UART_PL011_BAUD_RATE != 0
> + def_bool y
> +
> +config EARLY_UART_8250_REG_SHIFT
> + depends on EARLY_UART_8250
> + int "Early printk, left-shift to apply to the register offsets within the 8250 UART"
> + help
> + EARLY_UART_8250_REG_SHIFT is, optionally, the left-shift to
> + apply to the register offsets within the UART with early
> + printk.
> +
> + Default to 0.
> +
> + default 2 if EARLY_PRINTK_BRCM
> + default 2 if EARLY_PRINTK_DRA7
> + default 2 if EARLY_PRINTK_OMAP5432
> + default 2 if EARLY_PRINTK_SUN6I
> + default 2 if EARLY_PRINTK_SUN7I
> + default 2 if EARLY_PRINTK_XGENE_MCDIVITT
> + default 2 if EARLY_PRINTK_XGENE_STORM
> + default 0
> +
> +choice EARLY_UART_SCIF_VERSION
> + prompt "Early printk UART SCIF interface version"
> + depends on EARLY_UART_SCIF
> + default EARLY_UART_SCIF_VERSION_NONE
> + help
> + Select the interface version of the SCIF UART.
> +
> + Select EARLY_UART_SCIF_VERSION_NONE to use the default
> + interface version (SCIF).
> + config EARLY_UART_SCIF_VERSION_NONE
> + bool "default SCIF UART interface"
> + config EARLY_UART_SCIF_VERSION_A
> + bool "SCIF UART interface version A"
> +endchoice
> +
> +config EARLY_PRINTK_INC
> + string
> + default "debug-8250.inc" if EARLY_UART_8250
> + default "debug-cadence.inc" if EARLY_UART_CADENCE
> + default "debug-exynos4210.inc" if EARLY_UART_EXYNOS4210
> + default "debug-meson.inc" if EARLY_UART_MESON
> + default "debug-mvebu.inc" if EARLY_UART_MVEBU
> + default "debug-pl011.inc" if EARLY_UART_PL011
> + default "debug-scif.inc" if EARLY_UART_SCIF
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 1044c2298a05..12f92a4bd3f9 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -16,7 +16,7 @@ obj-y += device.o
> obj-y += domain.o
> obj-y += domain_build.init.o
> obj-y += domctl.o
> -obj-$(EARLY_PRINTK) += early_printk.o
> +obj-$(CONFIG_EARLY_PRINTK) += early_printk.o
> obj-y += gic.o
> obj-y += gic-v2.o
> obj-$(CONFIG_GICV3) += gic-v3.o
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index faa09ea111ec..3ad284aa71a4 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -18,76 +18,6 @@ CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-a15
> CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
> CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
>
> -EARLY_PRINTK := n
> -
> -ifeq ($(CONFIG_DEBUG),y)
> -
> -# See docs/misc/arm/early-printk.txt for syntax
> -
> -EARLY_PRINTK_brcm := 8250,0xF040AB00,2
> -EARLY_PRINTK_dra7 := 8250,0x4806A000,2
> -EARLY_PRINTK_fastmodel := pl011,0x1c090000,115200
> -EARLY_PRINTK_exynos5250 := exynos4210,0x12c20000
> -EARLY_PRINTK_hikey960 := pl011,0xfff32000
> -EARLY_PRINTK_juno := pl011,0x7ff80000
> -EARLY_PRINTK_lager := scif,0xe6e60000
> -EARLY_PRINTK_midway := pl011,0xfff36000
> -EARLY_PRINTK_mvebu := mvebu,0xd0012000
> -EARLY_PRINTK_omap5432 := 8250,0x48020000,2
> -EARLY_PRINTK_rcar3 := scif,0xe6e88000
> -EARLY_PRINTK_seattle := pl011,0xe1010000
> -EARLY_PRINTK_sun6i := 8250,0x01c28000,2
> -EARLY_PRINTK_sun7i := 8250,0x01c28000,2
> -EARLY_PRINTK_thunderx := pl011,0x87e024000000
> -EARLY_PRINTK_vexpress := pl011,0x1c090000
> -EARLY_PRINTK_xgene-mcdivitt := 8250,0x1c021000,2
> -EARLY_PRINTK_xgene-storm := 8250,0x1c020000,2
> -EARLY_PRINTK_zynqmp := cadence,0xff000000
> -
> -ifneq ($(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)),)
> -EARLY_PRINTK_CFG := $(subst $(comma), ,$(EARLY_PRINTK_$(CONFIG_EARLY_PRINTK)))
> -else
> -EARLY_PRINTK_CFG := $(subst $(comma), ,$(CONFIG_EARLY_PRINTK))
> -endif
> -
> -# Extract configuration from string
> -EARLY_PRINTK_INC := $(word 1,$(EARLY_PRINTK_CFG))
> -EARLY_UART_BASE_ADDRESS := $(word 2,$(EARLY_PRINTK_CFG))
> -
> -# UART specific options
> -ifeq ($(EARLY_PRINTK_INC),8250)
> -EARLY_UART_REG_SHIFT := $(word 3,$(EARLY_PRINTK_CFG))
> -endif
> -ifeq ($(EARLY_PRINTK_INC),pl011)
> -ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
> -EARLY_PRINTK_INIT_UART := y
> -EARLY_PRINTK_BAUD := $(word 3,$(EARLY_PRINTK_CFG))
> -endif
> -endif
> -ifeq ($(EARLY_PRINTK_INC),scif)
> -ifneq ($(word 3,$(EARLY_PRINTK_CFG)),)
> -CFLAGS-y += -DCONFIG_EARLY_UART_SCIF_VERSION_$(word 3,$(EARLY_PRINTK_CFG))
> -else
> -CFLAGS-y += -DCONFIG_EARLY_UART_SCIF_VERSION_NONE
> -endif
> -endif
> -
> -ifneq ($(EARLY_PRINTK_INC),)
> -EARLY_PRINTK := y
> -endif
> -
> -CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
> -CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DCONFIG_EARLY_UART_INIT
> -CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
> -CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_UART_PL011_BAUD_RATE=$(EARLY_PRINTK_BAUD)
> -CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
> -CFLAGS-$(EARLY_PRINTK) += -DCONFIG_EARLY_UART_8250_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
> -
> -else # !CONFIG_DEBUG
> -
> -ifneq ($(CONFIG_EARLY_PRINTK),)
> -# Early printk is dependant on a debug build.
> -$(error CONFIG_EARLY_PRINTK enabled for non-debug build)
> -endif
> -
> +ifneq ($(filter command line environment,$(origin CONFIG_EARLY_PRINTK)),)
> + $(error You must use 'make menuconfig' to enable/disable early printk now)
> endif
> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
> index 539bbef298a7..96105d238307 100644
> --- a/xen/arch/arm/arm32/Makefile
> +++ b/xen/arch/arm/arm32/Makefile
> @@ -1,6 +1,6 @@
> obj-y += lib/
>
> -obj-$(EARLY_PRINTK) += debug.o
> +obj-$(CONFIG_EARLY_PRINTK) += debug.o
> obj-y += domctl.o
> obj-y += domain.o
> obj-y += entry.o
> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index db8565b71a33..40642ff57494 100644
> --- a/xen/arch/arm/arm64/Makefile
> +++ b/xen/arch/arm/arm64/Makefile
> @@ -2,7 +2,7 @@ obj-y += lib/
>
> obj-y += cache.o
> obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
> -obj-$(EARLY_PRINTK) += debug.o
> +obj-$(CONFIG_EARLY_PRINTK) += debug.o
> obj-y += domctl.o
> obj-y += domain.o
> obj-y += entry.o
> diff --git a/xen/arch/x86/Kconfig.debug b/xen/arch/x86/Kconfig.debug
> new file mode 100644
> index 000000000000..e69de29bb2d1
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro
2020-03-11 13:57 ` Julien Grall
@ 2020-03-11 14:46 ` Anthony PERARD
2020-03-12 20:18 ` Julien Grall
0 siblings, 1 reply; 18+ messages in thread
From: Anthony PERARD @ 2020-03-11 14:46 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, Stefano Stabellini, Volodymyr Babchuk
On Wed, Mar 11, 2020 at 01:57:37PM +0000, Julien Grall wrote:
> Hi Anthony,
>
> On 09/03/2020 17:45, Anthony PERARD wrote:
> > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > index e9d356f05c2b..2b593c5ef99a 100644
> > --- a/xen/arch/arm/arm32/head.S
> > +++ b/xen/arch/arm/arm32/head.S
> > @@ -36,8 +36,8 @@
> > #define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START)
> > #define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
> > -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> > -#include EARLY_PRINTK_INC
> > +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
>
> NIT: I would also take the opportunity to clean-up the line by remove the
> extra () and the space before (. Something like:
>
> #if define(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)
>
> [...]
>
> > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > index e5015f93a2d8..4d45ea3dac3c 100644
> > --- a/xen/arch/arm/arm64/head.S
> > +++ b/xen/arch/arm/arm64/head.S
> > @@ -45,8 +45,8 @@
> > #define __HEAD_FLAGS ((__HEAD_FLAG_PAGE_SIZE << 1) | \
> > (__HEAD_FLAG_PHYS_BASE << 3))
> > -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> > -#include EARLY_PRINTK_INC
> > +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
>
> Same here.
Those clean-up sounds good.
> I am happy to fix both cases on commit:
>
> Acked-by: Julien Grall <jgrall@amazon.com>
Thanks,
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
[not found] ` <20200311152613.GJ2152@perard.uk.xensource.com>
@ 2020-03-11 17:21 ` Julien Grall
2020-03-11 17:38 ` Anthony PERARD
0 siblings, 1 reply; 18+ messages in thread
From: Julien Grall @ 2020-03-11 17:21 UTC (permalink / raw)
To: Anthony PERARD
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
George Dunlap, Julien Grall, Jan Beulich, xen-devel,
Volodymyr Babchuk, Roger Pau Monné
Hi Anthony,
On 11/03/2020 15:26, Anthony PERARD wrote:
> On Wed, Mar 11, 2020 at 02:18:20PM +0000, Julien Grall wrote:
>>> diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
>>> index 89e081e51eaf..c61973013097 100644
>>> --- a/docs/misc/arm/early-printk.txt
>>> +++ b/docs/misc/arm/early-printk.txt
>>> @@ -1,64 +1,39 @@
>>> How to enable early printk
>>> -Early printk can only be enabled if debug=y. You may want to enable it if
>>> -you are debbuging code that executes before the console is initialized.
>>> +Early printk can only be enabled if CONFIG_DEBUG=y. You may want to enable
>>
>> NIT: AFAICT, the file is using one space after full stop. I would like to
>> keep it like that for consistency :).
>
> Sound good, I should look at how to fix my vim configuration so it stop
> adding extra spaces :-(
>
> :set nojoinspaces
>
> Won't happen again :-).
>
>>> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
>>> index b3511e81a275..ee6ee33b69be 100644
>>> --- a/xen/Kconfig.debug
>>> +++ b/xen/Kconfig.debug
>>> @@ -128,6 +128,8 @@ config XMEM_POOL_POISON
>>> Poison free blocks with 0xAA bytes and verify them when a block is
>>> allocated in order to spot use-after-free issues.
>>> +source "arch/$(SRCARCH)/Kconfig.debug"
>>
>> To double check, this means that earlyprintk can be selected in EXPERT mode
>> now. However, in the document early-printk.txt, the feature is said to only
>> be enabled with CONFIG_DEBUG=y.
>>
>> I like the idea of allowing a user to enable earlyprintk in EXPERT mode
>> (some early boot bug may only occur in non-debug build). So I am happy to
>> keep the code like. Can you update the doc accordingly?
>
> Will do.
>
>>> +
>>> endif # DEBUG || EXPERT
>>> endmenu
>>> diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
>>> new file mode 100644
>>> index 000000000000..ffb21e8ac20a
>>> --- /dev/null
>>> +++ b/xen/arch/arm/Kconfig.debug
>>> @@ -0,0 +1,287 @@
>>> +choice
>>> + bool "Early printk"
>>> + optional
>>> + help
>>> + You may want to enable early printk if you are debugging code
>>> + that executes before the console is initialized.
>>> +
>>> + Note that selecting this option will limit Xen to a single UART
>>> + definition. Attempting to boot Xen image on a different
>>> + platform *will not work*, so this option should not be enable
>>> + for Xens that are intended to be portable.
>>> +
>>> + Choose one of the UART drivers for early printk, then you'll
>>> + have to specify the parameters, like the base address.
>>> +
>>> + Alternatively, there are platform specific options which will
>>> + have default values for the various parameters.
>>
>> Would it be worth to mention such options are deprecated?
>
> Yes, I should mention that here. (And probably in the early-printk.txt
> doc as well.)
>
> How about:
> Alternatively, there are platform specific options which will
> have default values for the various parameters. But such option are
> deprecated and will soon be removed.
>
> Or it would be better to highlight the fact that they are deprecated, so
> maybe the following would be better:
> Deprecated: Alternatively, there are platform specific options which
> will have default values for the various parameters. But such option
> will soon be removed.
The second version looks better to me.
>>> +
>>> + config EARLY_PRINTK_BRCM
>>> + bool "Early printk with 8250 on Broadcom 7445D0 boards with A15 processors"
>>> + select EARLY_UART_8250
>>
>> I noticed below you added "depends on ARM_64" on the Xilinx SoC. In general,
>> platform specific options are tied to either arm32 or arm64, even if the
>> UART "driver" is arch agnostic.
>
> Those "depends" are only there because the early uart driver is only
> available for one arch. "debug-cadence.inc" can only be found in
> "arch/arm/arm64/", not in arm32, for example.
>
>> You could technically boot Xen on Arm 32-bit on Armv8 HW provided they
>> support 32-bit at the hypervisor level, but we never supported this case. So
>> I am wondering whether we should add depends on each earlyprintk. Stefano,
>> any opinions?
>>
>>> +config EARLY_UART_BASE_ADDRESS
>>> + depends on EARLY_PRINTK
>>> + hex "Early printk, physical base address of debug UART"
>>> + default 0x87e024000000 if EARLY_PRINTK_THUNDERX
>>
>> You are allowing EARLY_PRINTK_THUNDERX to be selected on Arm32 platform but
>> the address is above 4G. I suspect this would break randconfig build.
>
> gcc doesn't seems to complain :-).
I was expecting GAS to throw an error because the 64-bit value does not
fit in a 32-bit register. But... it looks like GAS will silently
truncate the value to 0x24000000 :(.
> (I mean "arm-none-eabi-gcc (Arch Repository) 9.2.0")
>
> But I can have thunderx depends on arm_64.
Is there a way to constrainst the address in Kconfig?
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-11 17:21 ` Julien Grall
@ 2020-03-11 17:38 ` Anthony PERARD
2020-03-12 20:16 ` Julien Grall
0 siblings, 1 reply; 18+ messages in thread
From: Anthony PERARD @ 2020-03-11 17:38 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
George Dunlap, Julien Grall, Jan Beulich, xen-devel,
Volodymyr Babchuk, Roger Pau Monné
On Wed, Mar 11, 2020 at 05:21:24PM +0000, Julien Grall wrote:
> On 11/03/2020 15:26, Anthony PERARD wrote:
> > On Wed, Mar 11, 2020 at 02:18:20PM +0000, Julien Grall wrote:
> > > > +config EARLY_UART_BASE_ADDRESS
> > > > + depends on EARLY_PRINTK
> > > > + hex "Early printk, physical base address of debug UART"
> > > > + default 0x87e024000000 if EARLY_PRINTK_THUNDERX
> > >
> > > You are allowing EARLY_PRINTK_THUNDERX to be selected on Arm32 platform but
> > > the address is above 4G. I suspect this would break randconfig build.
> >
> > gcc doesn't seems to complain :-).
>
> I was expecting GAS to throw an error because the 64-bit value does not fit
> in a 32-bit register. But... it looks like GAS will silently truncate the
> value to 0x24000000 :(.
>
> > (I mean "arm-none-eabi-gcc (Arch Repository) 9.2.0")
> >
> > But I can have thunderx depends on arm_64.
> Is there a way to constrainst the address in Kconfig?
There is! I can add "range 0x0 0xffffffff if ARM_32".
But Kconfig doesn't say anything if a default value is too high, and
silently set the value to the maximum.
Still, it's better. I just need to add depends on ARM_64 for thunderx,
and that should be fine.
And that prevent users from setting a too hight value, as kconfig will
not accept a value outside the range.
Thanks,
--
Anthony PERARD
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-11 17:38 ` Anthony PERARD
@ 2020-03-12 20:16 ` Julien Grall
0 siblings, 0 replies; 18+ messages in thread
From: Julien Grall @ 2020-03-12 20:16 UTC (permalink / raw)
To: Anthony PERARD
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
George Dunlap, Julien Grall, Jan Beulich, xen-devel,
Volodymyr Babchuk, Roger Pau Monné
Hi,
On 11/03/2020 17:38, Anthony PERARD wrote:
> On Wed, Mar 11, 2020 at 05:21:24PM +0000, Julien Grall wrote:
>> On 11/03/2020 15:26, Anthony PERARD wrote:
>>> On Wed, Mar 11, 2020 at 02:18:20PM +0000, Julien Grall wrote:
>>>>> +config EARLY_UART_BASE_ADDRESS
>>>>> + depends on EARLY_PRINTK
>>>>> + hex "Early printk, physical base address of debug UART"
>>>>> + default 0x87e024000000 if EARLY_PRINTK_THUNDERX
>>>>
>>>> You are allowing EARLY_PRINTK_THUNDERX to be selected on Arm32 platform but
>>>> the address is above 4G. I suspect this would break randconfig build.
>>>
>>> gcc doesn't seems to complain :-).
>>
>> I was expecting GAS to throw an error because the 64-bit value does not fit
>> in a 32-bit register. But... it looks like GAS will silently truncate the
>> value to 0x24000000 :(.
>>
>>> (I mean "arm-none-eabi-gcc (Arch Repository) 9.2.0")
>>>
>>> But I can have thunderx depends on arm_64.
>> Is there a way to constrainst the address in Kconfig?
>
> There is! I can add "range 0x0 0xffffffff if ARM_32".
> But Kconfig doesn't say anything if a default value is too high, and
> silently set the value to the maximum.
Lovely. It would have been nice from Kconfig to warn you if the default
is not in the range. Maybe a future improvement?
> Still, it's better. I just need to add depends on ARM_64 for thunderx,
> and that should be fine.
I agree that this would make things better.
> And that prevent users from setting a too hight value, as kconfig will
> not accept a value outside the range.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro
2020-03-11 14:46 ` Anthony PERARD
@ 2020-03-12 20:18 ` Julien Grall
2020-03-13 23:01 ` Stefano Stabellini
0 siblings, 1 reply; 18+ messages in thread
From: Julien Grall @ 2020-03-12 20:18 UTC (permalink / raw)
To: Anthony PERARD; +Cc: xen-devel, Stefano Stabellini, Volodymyr Babchuk
Hi,
On 11/03/2020 14:46, Anthony PERARD wrote:
> On Wed, Mar 11, 2020 at 01:57:37PM +0000, Julien Grall wrote:
>> Hi Anthony,
>>
>> On 09/03/2020 17:45, Anthony PERARD wrote:
>>> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
>>> index e9d356f05c2b..2b593c5ef99a 100644
>>> --- a/xen/arch/arm/arm32/head.S
>>> +++ b/xen/arch/arm/arm32/head.S
>>> @@ -36,8 +36,8 @@
>>> #define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START)
>>> #define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
>>> -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
>>> -#include EARLY_PRINTK_INC
>>> +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
>>
>> NIT: I would also take the opportunity to clean-up the line by remove the
>> extra () and the space before (. Something like:
>>
>> #if define(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)
>>
>> [...]
>>
>>> diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
>>> index e5015f93a2d8..4d45ea3dac3c 100644
>>> --- a/xen/arch/arm/arm64/head.S
>>> +++ b/xen/arch/arm/arm64/head.S
>>> @@ -45,8 +45,8 @@
>>> #define __HEAD_FLAGS ((__HEAD_FLAG_PAGE_SIZE << 1) | \
>>> (__HEAD_FLAG_PHYS_BASE << 3))
>>> -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
>>> -#include EARLY_PRINTK_INC
>>> +#if (defined (CONFIG_EARLY_PRINTK)) && (defined (CONFIG_EARLY_PRINTK_INC))
>>
>> Same here.
>
> Those clean-up sounds good.
I will give a couple of days so Stefano can have an opportunity to
comment. If I don't hear anything by Monday, I will commit it.
>
>> I am happy to fix both cases on commit:
>>
>> Acked-by: Julien Grall <jgrall@amazon.com>
>
> Thanks,
>
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro
2020-03-12 20:18 ` Julien Grall
@ 2020-03-13 23:01 ` Stefano Stabellini
0 siblings, 0 replies; 18+ messages in thread
From: Stefano Stabellini @ 2020-03-13 23:01 UTC (permalink / raw)
To: Julien Grall
Cc: Anthony PERARD, xen-devel, Stefano Stabellini, Volodymyr Babchuk
On Thu, 12 Mar 2020, Julien Grall wrote:
> On 11/03/2020 14:46, Anthony PERARD wrote:
> > On Wed, Mar 11, 2020 at 01:57:37PM +0000, Julien Grall wrote:
> > > Hi Anthony,
> > >
> > > On 09/03/2020 17:45, Anthony PERARD wrote:
> > > > diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> > > > index e9d356f05c2b..2b593c5ef99a 100644
> > > > --- a/xen/arch/arm/arm32/head.S
> > > > +++ b/xen/arch/arm/arm32/head.S
> > > > @@ -36,8 +36,8 @@
> > > > #define XEN_FIRST_SLOT first_table_offset(XEN_VIRT_START)
> > > > #define XEN_SECOND_SLOT second_table_offset(XEN_VIRT_START)
> > > > -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> > > > -#include EARLY_PRINTK_INC
> > > > +#if (defined (CONFIG_EARLY_PRINTK)) && (defined
> > > > (CONFIG_EARLY_PRINTK_INC))
> > >
> > > NIT: I would also take the opportunity to clean-up the line by remove the
> > > extra () and the space before (. Something like:
> > >
> > > #if define(CONFIG_EARLY_PRINTK) && defined(CONFIG_EARLY_PRINTK_INC)
> > >
> > > [...]
> > >
> > > > diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
> > > > index e5015f93a2d8..4d45ea3dac3c 100644
> > > > --- a/xen/arch/arm/arm64/head.S
> > > > +++ b/xen/arch/arm/arm64/head.S
> > > > @@ -45,8 +45,8 @@
> > > > #define __HEAD_FLAGS ((__HEAD_FLAG_PAGE_SIZE << 1) | \
> > > > (__HEAD_FLAG_PHYS_BASE << 3))
> > > > -#if (defined (CONFIG_EARLY_PRINTK)) && (defined (EARLY_PRINTK_INC))
> > > > -#include EARLY_PRINTK_INC
> > > > +#if (defined (CONFIG_EARLY_PRINTK)) && (defined
> > > > (CONFIG_EARLY_PRINTK_INC))
> > >
> > > Same here.
> >
> > Those clean-up sounds good.
>
> I will give a couple of days so Stefano can have an opportunity to comment. If
> I don't hear anything by Monday, I will commit it.
It's fine by me. Also:
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-09 17:45 ` [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
2020-03-11 14:18 ` Julien Grall
@ 2020-03-13 23:12 ` Stefano Stabellini
2020-03-13 23:14 ` Stefano Stabellini
2020-03-13 23:19 ` Julien Grall
1 sibling, 2 replies; 18+ messages in thread
From: Stefano Stabellini @ 2020-03-13 23:12 UTC (permalink / raw)
To: Anthony PERARD
Cc: Stefano Stabellini, Julien Grall, Wei Liu, Andrew Cooper,
Ian Jackson, George Dunlap, Julien Grall, Jan Beulich, xen-devel,
Volodymyr Babchuk, Roger Pau Monné
On Mon, 9 Mar 2020, Anthony PERARD wrote:
> At the moment, early printk can only be configured on the make command
> line. It is not very handy because a user has to remove the option
> everytime it is using another command other than compiling the
> hypervisor.
>
> Furthermore, early printk is one of the few odds one that are not
> using Kconfig.
>
> So this is about time to move it to Kconfig.
>
> The new kconfigs options allow a user to eather select a UART driver
> to use at boot time, and set the parameters, or it is still possible
> to select a platform which will set the parameters.
>
> If CONFIG_EARLY_PRINTK is present in the environment or on the make
> command line, make will return an error.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>
> ---
>
> Original patch:
> [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk using Kconfig
> <20190913103953.8182-1-julien.grall@arm.com>
> ---
>
> Notes:
> v3:
> - rename EARLY_PRINK to CONFIG_EARLY_PRINTK in makefile here (which
> select which object to build).
> - rename EARLY_UART_BAUD_RATE to EARLY_UART_PL011_BAUD_RATE
> - typos
> - drop the list of aliases in early-printk.txt. Kconfig choice menu
> should be enough.
> - reword early-printk.txt.
> - rework how EARLY_PRINTK is set to Y
> and use that instead of a list of all EARLY_UART_*
> - Add a check to ask user to use Kconfig to set early printk.
> - rework the possible choice to have all uart driver and platform
> specific option together.
> - have added or reword prompt and help messages of the different
> options. The platform specific option don't have extended help, the
> prompt is probably enough.
> (The non-platform specific options have the help message that Julien
> have written in the first version.)
> - have made EARLY_UART_INIT dependent on the value of
> EARLY_UART_PL011_BAUD_RATE so that there is no need to expose _INIT to
> users.
>
The patch is fine by me. I only have one very minor comment below.
> + config EARLY_UART_CHOICE_CADENCE
> + select EARLY_UART_CADENCE
> + depends on ARM_64
> + bool "Early printk via Cadence UART"
> + help
> + Say Y here if you wish the early printk to direct their
> + output to a Cadence UART. You can use this option to
> + provide the parameters for the Cadence UART rather than
> + selecting one of the platform specific options below if
> + you know the parameters for the port.
> +
> + This option is preferred over the platform specific
> + options; the platform specific options are deprecated
> + and will soon be removed.
[...]
> + config EARLY_PRINTK_ZYNQMP
> + bool "Early printk with Cadence UART for Xilinx ZynqMP SoCs"
> + select EARLY_UART_CADENCE
> + depends on ARM_64
> +endchoice
[...]
> +config EARLY_UART_BASE_ADDRESS
> + depends on EARLY_PRINTK
> + hex "Early printk, physical base address of debug UART"
> + default 0xF040AB00 if EARLY_PRINTK_BRCM
> + default 0x4806A000 if EARLY_PRINTK_DRA7
> + default 0x1c090000 if EARLY_PRINTK_FASTMODEL
> + default 0x12c20000 if EARLY_PRINTK_EXYNOS5250
> + default 0xfff32000 if EARLY_PRINTK_HIKEY960
> + default 0x7ff80000 if EARLY_PRINTK_JUNO
> + default 0xe6e60000 if EARLY_PRINTK_LAGER
> + default 0xfff36000 if EARLY_PRINTK_MIDWAY
> + default 0xd0012000 if EARLY_PRINTK_MVEBU
> + default 0x48020000 if EARLY_PRINTK_OMAP5432
> + default 0xe6e88000 if EARLY_PRINTK_RCAR3
> + default 0xe1010000 if EARLY_PRINTK_SEATTLE
> + default 0x01c28000 if EARLY_PRINTK_SUN6I
> + default 0x01c28000 if EARLY_PRINTK_SUN7I
> + default 0x87e024000000 if EARLY_PRINTK_THUNDERX
> + default 0x1c090000 if EARLY_PRINTK_VEXPRESS
> + default 0x1c021000 if EARLY_PRINTK_XGENE_MCDIVITT
> + default 0x1c020000 if EARLY_PRINTK_XGENE_STORM
> + default 0xff000000 if EARLY_PRINTK_ZYNQMP
Today we only have one board with CADENCE UART which is ZynqMP. However,
only if EARLY_PRINTK_ZYNQMP is selected the BASE_ADDRESS is default to
0xff000000.
Ideally, BASE_ADDRESS would default to 0xff000000 if EARLY_PRINTK_ZYNQMP
or if EARLY_UART_CADENCE. (There is one more similar example which is
EARLY_UART_EXYNOS4210.)
I don't know if it is worth optimizing, I'll let you and Julien decide.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-13 23:12 ` Stefano Stabellini
@ 2020-03-13 23:14 ` Stefano Stabellini
2020-03-17 9:36 ` Julien Grall
2020-03-13 23:19 ` Julien Grall
1 sibling, 1 reply; 18+ messages in thread
From: Stefano Stabellini @ 2020-03-13 23:14 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Julien Grall, Wei Liu, Andrew Cooper, Ian Jackson, George Dunlap,
Julien Grall, Jan Beulich, Anthony PERARD, xen-devel,
Volodymyr Babchuk, Roger Pau Monné
On Fri, 13 Mar 2020, Stefano Stabellini wrote:
> On Mon, 9 Mar 2020, Anthony PERARD wrote:
> > At the moment, early printk can only be configured on the make command
> > line. It is not very handy because a user has to remove the option
> > everytime it is using another command other than compiling the
> > hypervisor.
> >
> > Furthermore, early printk is one of the few odds one that are not
> > using Kconfig.
> >
> > So this is about time to move it to Kconfig.
> >
> > The new kconfigs options allow a user to eather select a UART driver
> > to use at boot time, and set the parameters, or it is still possible
> > to select a platform which will set the parameters.
> >
> > If CONFIG_EARLY_PRINTK is present in the environment or on the make
> > command line, make will return an error.
> >
> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >
> > ---
> >
> > Original patch:
> > [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk using Kconfig
> > <20190913103953.8182-1-julien.grall@arm.com>
> > ---
> >
> > Notes:
> > v3:
> > - rename EARLY_PRINK to CONFIG_EARLY_PRINTK in makefile here (which
> > select which object to build).
> > - rename EARLY_UART_BAUD_RATE to EARLY_UART_PL011_BAUD_RATE
> > - typos
> > - drop the list of aliases in early-printk.txt. Kconfig choice menu
> > should be enough.
> > - reword early-printk.txt.
> > - rework how EARLY_PRINTK is set to Y
> > and use that instead of a list of all EARLY_UART_*
> > - Add a check to ask user to use Kconfig to set early printk.
> > - rework the possible choice to have all uart driver and platform
> > specific option together.
> > - have added or reword prompt and help messages of the different
> > options. The platform specific option don't have extended help, the
> > prompt is probably enough.
> > (The non-platform specific options have the help message that Julien
> > have written in the first version.)
> > - have made EARLY_UART_INIT dependent on the value of
> > EARLY_UART_PL011_BAUD_RATE so that there is no need to expose _INIT to
> > users.
> >
>
> The patch is fine by me. I only have one very minor comment below.
I forgot to add
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-13 23:12 ` Stefano Stabellini
2020-03-13 23:14 ` Stefano Stabellini
@ 2020-03-13 23:19 ` Julien Grall
2020-03-13 23:37 ` Julien Grall
1 sibling, 1 reply; 18+ messages in thread
From: Julien Grall @ 2020-03-13 23:19 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Wei Liu, Andrew Cooper, Ian Jackson, George Dunlap, Julien Grall,
Jan Beulich, Anthony PERARD, xen-devel, Volodymyr Babchuk,
Roger Pau Monné
On Fri, 13 Mar 2020 at 23:12, Stefano Stabellini <sstabellini@kernel.org> wrote:
>
> On Mon, 9 Mar 2020, Anthony PERARD wrote:
> > At the moment, early printk can only be configured on the make command
> > line. It is not very handy because a user has to remove the option
> > everytime it is using another command other than compiling the
> > hypervisor.
> >
> > Furthermore, early printk is one of the few odds one that are not
> > using Kconfig.
> >
> > So this is about time to move it to Kconfig.
> >
> > The new kconfigs options allow a user to eather select a UART driver
> > to use at boot time, and set the parameters, or it is still possible
> > to select a platform which will set the parameters.
> >
> > If CONFIG_EARLY_PRINTK is present in the environment or on the make
> > command line, make will return an error.
> >
> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >
> > ---
> >
> > Original patch:
> > [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk using Kconfig
> > <20190913103953.8182-1-julien.grall@arm.com>
> > ---
> >
> > Notes:
> > v3:
> > - rename EARLY_PRINK to CONFIG_EARLY_PRINTK in makefile here (which
> > select which object to build).
> > - rename EARLY_UART_BAUD_RATE to EARLY_UART_PL011_BAUD_RATE
> > - typos
> > - drop the list of aliases in early-printk.txt. Kconfig choice menu
> > should be enough.
> > - reword early-printk.txt.
> > - rework how EARLY_PRINTK is set to Y
> > and use that instead of a list of all EARLY_UART_*
> > - Add a check to ask user to use Kconfig to set early printk.
> > - rework the possible choice to have all uart driver and platform
> > specific option together.
> > - have added or reword prompt and help messages of the different
> > options. The platform specific option don't have extended help, the
> > prompt is probably enough.
> > (The non-platform specific options have the help message that Julien
> > have written in the first version.)
> > - have made EARLY_UART_INIT dependent on the value of
> > EARLY_UART_PL011_BAUD_RATE so that there is no need to expose _INIT to
> > users.
> >
>
> The patch is fine by me. I only have one very minor comment below.
>
>
> > + config EARLY_UART_CHOICE_CADENCE
> > + select EARLY_UART_CADENCE
> > + depends on ARM_64
> > + bool "Early printk via Cadence UART"
> > + help
> > + Say Y here if you wish the early printk to direct their
> > + output to a Cadence UART. You can use this option to
> > + provide the parameters for the Cadence UART rather than
> > + selecting one of the platform specific options below if
> > + you know the parameters for the port.
> > +
> > + This option is preferred over the platform specific
> > + options; the platform specific options are deprecated
> > + and will soon be removed.
>
> [...]
>
> > + config EARLY_PRINTK_ZYNQMP
> > + bool "Early printk with Cadence UART for Xilinx ZynqMP SoCs"
> > + select EARLY_UART_CADENCE
> > + depends on ARM_64
> > +endchoice
>
> [...]
>
> > +config EARLY_UART_BASE_ADDRESS
> > + depends on EARLY_PRINTK
> > + hex "Early printk, physical base address of debug UART"
> > + default 0xF040AB00 if EARLY_PRINTK_BRCM
> > + default 0x4806A000 if EARLY_PRINTK_DRA7
> > + default 0x1c090000 if EARLY_PRINTK_FASTMODEL
> > + default 0x12c20000 if EARLY_PRINTK_EXYNOS5250
> > + default 0xfff32000 if EARLY_PRINTK_HIKEY960
> > + default 0x7ff80000 if EARLY_PRINTK_JUNO
> > + default 0xe6e60000 if EARLY_PRINTK_LAGER
> > + default 0xfff36000 if EARLY_PRINTK_MIDWAY
> > + default 0xd0012000 if EARLY_PRINTK_MVEBU
> > + default 0x48020000 if EARLY_PRINTK_OMAP5432
> > + default 0xe6e88000 if EARLY_PRINTK_RCAR3
> > + default 0xe1010000 if EARLY_PRINTK_SEATTLE
> > + default 0x01c28000 if EARLY_PRINTK_SUN6I
> > + default 0x01c28000 if EARLY_PRINTK_SUN7I
> > + default 0x87e024000000 if EARLY_PRINTK_THUNDERX
> > + default 0x1c090000 if EARLY_PRINTK_VEXPRESS
> > + default 0x1c021000 if EARLY_PRINTK_XGENE_MCDIVITT
> > + default 0x1c020000 if EARLY_PRINTK_XGENE_STORM
> > + default 0xff000000 if EARLY_PRINTK_ZYNQMP
>
> Today we only have one board with CADENCE UART which is ZynqMP. However,
> only if EARLY_PRINTK_ZYNQMP is selected the BASE_ADDRESS is default to
> 0xff000000.
>
> Ideally, BASE_ADDRESS would default to 0xff000000 if EARLY_PRINTK_ZYNQMP
> or if EARLY_UART_CADENCE. (There is one more similar example which is
> EARLY_UART_EXYNOS4210.)
As you say *today*. How about in the future? There are no promise any
platform using cadence UART will be wired at the same address.
If you specify a default address, then the risk is the user will
forget to update it and see no log at all (Xen may crash if the
address is invalid).
By not specifying a default address, the build system should shout at
you and therefore you will know that you need to configure the
address.
So the slight inconvenience to the user to specify an address is not
worth the risk.
>
> I don't know if it is worth optimizing, I'll let you and Julien decide.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-13 23:19 ` Julien Grall
@ 2020-03-13 23:37 ` Julien Grall
0 siblings, 0 replies; 18+ messages in thread
From: Julien Grall @ 2020-03-13 23:37 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Wei Liu, Andrew Cooper, Ian Jackson, George Dunlap, Julien Grall,
Jan Beulich, Anthony PERARD, xen-devel, Volodymyr Babchuk,
Roger Pau Monné
On Fri, 13 Mar 2020 at 23:19, Julien Grall <julien.grall.oss@gmail.com> wrote:
>
> On Fri, 13 Mar 2020 at 23:12, Stefano Stabellini <sstabellini@kernel.org> wrote:
> >
> > On Mon, 9 Mar 2020, Anthony PERARD wrote:
> > > At the moment, early printk can only be configured on the make command
> > > line. It is not very handy because a user has to remove the option
> > > everytime it is using another command other than compiling the
> > > hypervisor.
> > >
> > > Furthermore, early printk is one of the few odds one that are not
> > > using Kconfig.
> > >
> > > So this is about time to move it to Kconfig.
> > >
> > > The new kconfigs options allow a user to eather select a UART driver
> > > to use at boot time, and set the parameters, or it is still possible
> > > to select a platform which will set the parameters.
> > >
> > > If CONFIG_EARLY_PRINTK is present in the environment or on the make
> > > command line, make will return an error.
> > >
> > > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > >
> > > ---
> > >
> > > Original patch:
> > > [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk using Kconfig
> > > <20190913103953.8182-1-julien.grall@arm.com>
> > > ---
> > >
> > > Notes:
> > > v3:
> > > - rename EARLY_PRINK to CONFIG_EARLY_PRINTK in makefile here (which
> > > select which object to build).
> > > - rename EARLY_UART_BAUD_RATE to EARLY_UART_PL011_BAUD_RATE
> > > - typos
> > > - drop the list of aliases in early-printk.txt. Kconfig choice menu
> > > should be enough.
> > > - reword early-printk.txt.
> > > - rework how EARLY_PRINTK is set to Y
> > > and use that instead of a list of all EARLY_UART_*
> > > - Add a check to ask user to use Kconfig to set early printk.
> > > - rework the possible choice to have all uart driver and platform
> > > specific option together.
> > > - have added or reword prompt and help messages of the different
> > > options. The platform specific option don't have extended help, the
> > > prompt is probably enough.
> > > (The non-platform specific options have the help message that Julien
> > > have written in the first version.)
> > > - have made EARLY_UART_INIT dependent on the value of
> > > EARLY_UART_PL011_BAUD_RATE so that there is no need to expose _INIT to
> > > users.
> > >
> >
> > The patch is fine by me. I only have one very minor comment below.
> >
> >
> > > + config EARLY_UART_CHOICE_CADENCE
> > > + select EARLY_UART_CADENCE
> > > + depends on ARM_64
> > > + bool "Early printk via Cadence UART"
> > > + help
> > > + Say Y here if you wish the early printk to direct their
> > > + output to a Cadence UART. You can use this option to
> > > + provide the parameters for the Cadence UART rather than
> > > + selecting one of the platform specific options below if
> > > + you know the parameters for the port.
> > > +
> > > + This option is preferred over the platform specific
> > > + options; the platform specific options are deprecated
> > > + and will soon be removed.
> >
> > [...]
> >
> > > + config EARLY_PRINTK_ZYNQMP
> > > + bool "Early printk with Cadence UART for Xilinx ZynqMP SoCs"
> > > + select EARLY_UART_CADENCE
> > > + depends on ARM_64
> > > +endchoice
> >
> > [...]
> >
> > > +config EARLY_UART_BASE_ADDRESS
> > > + depends on EARLY_PRINTK
> > > + hex "Early printk, physical base address of debug UART"
> > > + default 0xF040AB00 if EARLY_PRINTK_BRCM
> > > + default 0x4806A000 if EARLY_PRINTK_DRA7
> > > + default 0x1c090000 if EARLY_PRINTK_FASTMODEL
> > > + default 0x12c20000 if EARLY_PRINTK_EXYNOS5250
> > > + default 0xfff32000 if EARLY_PRINTK_HIKEY960
> > > + default 0x7ff80000 if EARLY_PRINTK_JUNO
> > > + default 0xe6e60000 if EARLY_PRINTK_LAGER
> > > + default 0xfff36000 if EARLY_PRINTK_MIDWAY
> > > + default 0xd0012000 if EARLY_PRINTK_MVEBU
> > > + default 0x48020000 if EARLY_PRINTK_OMAP5432
> > > + default 0xe6e88000 if EARLY_PRINTK_RCAR3
> > > + default 0xe1010000 if EARLY_PRINTK_SEATTLE
> > > + default 0x01c28000 if EARLY_PRINTK_SUN6I
> > > + default 0x01c28000 if EARLY_PRINTK_SUN7I
> > > + default 0x87e024000000 if EARLY_PRINTK_THUNDERX
> > > + default 0x1c090000 if EARLY_PRINTK_VEXPRESS
> > > + default 0x1c021000 if EARLY_PRINTK_XGENE_MCDIVITT
> > > + default 0x1c020000 if EARLY_PRINTK_XGENE_STORM
> > > + default 0xff000000 if EARLY_PRINTK_ZYNQMP
> >
> > Today we only have one board with CADENCE UART which is ZynqMP. However,
> > only if EARLY_PRINTK_ZYNQMP is selected the BASE_ADDRESS is default to
> > 0xff000000.
> >
> > Ideally, BASE_ADDRESS would default to 0xff000000 if EARLY_PRINTK_ZYNQMP
> > or if EARLY_UART_CADENCE. (There is one more similar example which is
> > EARLY_UART_EXYNOS4210.)
>
> As you say *today*. How about in the future? There are no promise any
> platform using cadence UART will be wired at the same address.
> If you specify a default address, then the risk is the user will
> forget to update it and see no log at all (Xen may crash if the
> address is invalid).
>
> By not specifying a default address, the build system should shout at
> you and therefore you will know that you need to configure the
> address.
>
> So the slight inconvenience to the user to specify an address is not
I probably should have used "advantage" rather than "inconvenience" here.
> worth the risk.
>
> >
> > I don't know if it is worth optimizing, I'll let you and Julien decide.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-13 23:14 ` Stefano Stabellini
@ 2020-03-17 9:36 ` Julien Grall
2020-03-17 17:51 ` Stefano Stabellini
0 siblings, 1 reply; 18+ messages in thread
From: Julien Grall @ 2020-03-17 9:36 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Wei Liu, Andrew Cooper, Ian Jackson, George Dunlap, Julien Grall,
Jan Beulich, Anthony PERARD, xen-devel, Volodymyr Babchuk,
Roger Pau Monné
Hi,
On 13/03/2020 23:14, Stefano Stabellini wrote:
> On Fri, 13 Mar 2020, Stefano Stabellini wrote:
>> On Mon, 9 Mar 2020, Anthony PERARD wrote:
>>> At the moment, early printk can only be configured on the make command
>>> line. It is not very handy because a user has to remove the option
>>> everytime it is using another command other than compiling the
>>> hypervisor.
>>>
>>> Furthermore, early printk is one of the few odds one that are not
>>> using Kconfig.
>>>
>>> So this is about time to move it to Kconfig.
>>>
>>> The new kconfigs options allow a user to eather select a UART driver
>>> to use at boot time, and set the parameters, or it is still possible
>>> to select a platform which will set the parameters.
>>>
>>> If CONFIG_EARLY_PRINTK is present in the environment or on the make
>>> command line, make will return an error.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>>>
>>> ---
>>>
>>> Original patch:
>>> [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk using Kconfig
>>> <20190913103953.8182-1-julien.grall@arm.com>
>>> ---
>>>
>>> Notes:
>>> v3:
>>> - rename EARLY_PRINK to CONFIG_EARLY_PRINTK in makefile here (which
>>> select which object to build).
>>> - rename EARLY_UART_BAUD_RATE to EARLY_UART_PL011_BAUD_RATE
>>> - typos
>>> - drop the list of aliases in early-printk.txt. Kconfig choice menu
>>> should be enough.
>>> - reword early-printk.txt.
>>> - rework how EARLY_PRINTK is set to Y
>>> and use that instead of a list of all EARLY_UART_*
>>> - Add a check to ask user to use Kconfig to set early printk.
>>> - rework the possible choice to have all uart driver and platform
>>> specific option together.
>>> - have added or reword prompt and help messages of the different
>>> options. The platform specific option don't have extended help, the
>>> prompt is probably enough.
>>> (The non-platform specific options have the help message that Julien
>>> have written in the first version.)
>>> - have made EARLY_UART_INIT dependent on the value of
>>> EARLY_UART_PL011_BAUD_RATE so that there is no need to expose _INIT to
>>> users.
>>>
>>
>> The patch is fine by me. I only have one very minor comment below.
>
> I forgot to add
>
> Tested-by: Stefano Stabellini <sstabellini@kernel.org>
Thank you for the testing. This patch contains a question for you (see
[1]). Can you please take a look?
Cheers,
[1] <7720f54e-7ef7-9dae-dfdb-38b89a0892a6@xen.org>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-17 9:36 ` Julien Grall
@ 2020-03-17 17:51 ` Stefano Stabellini
2020-03-24 14:48 ` Anthony PERARD
0 siblings, 1 reply; 18+ messages in thread
From: Stefano Stabellini @ 2020-03-17 17:51 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Wei Liu, Andrew Cooper, Ian Jackson,
George Dunlap, Julien Grall, Jan Beulich, Anthony PERARD,
xen-devel, Volodymyr Babchuk, Roger Pau Monné
On Tue, 17 Mar 2020, Julien Grall wrote:
> Hi,
>
> On 13/03/2020 23:14, Stefano Stabellini wrote:
> > On Fri, 13 Mar 2020, Stefano Stabellini wrote:
> > > On Mon, 9 Mar 2020, Anthony PERARD wrote:
> > > > At the moment, early printk can only be configured on the make command
> > > > line. It is not very handy because a user has to remove the option
> > > > everytime it is using another command other than compiling the
> > > > hypervisor.
> > > >
> > > > Furthermore, early printk is one of the few odds one that are not
> > > > using Kconfig.
> > > >
> > > > So this is about time to move it to Kconfig.
> > > >
> > > > The new kconfigs options allow a user to eather select a UART driver
> > > > to use at boot time, and set the parameters, or it is still possible
> > > > to select a platform which will set the parameters.
> > > >
> > > > If CONFIG_EARLY_PRINTK is present in the environment or on the make
> > > > command line, make will return an error.
> > > >
> > > > Signed-off-by: Julien Grall <julien.grall@arm.com>
> > > > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> > > >
> > > > ---
> > > >
> > > > Original patch:
> > > > [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early
> > > > printk using Kconfig
> > > > <20190913103953.8182-1-julien.grall@arm.com>
> > > > ---
> > > >
> > > > Notes:
> > > > v3:
> > > > - rename EARLY_PRINK to CONFIG_EARLY_PRINTK in makefile here (which
> > > > select which object to build).
> > > > - rename EARLY_UART_BAUD_RATE to EARLY_UART_PL011_BAUD_RATE
> > > > - typos
> > > > - drop the list of aliases in early-printk.txt. Kconfig choice menu
> > > > should be enough.
> > > > - reword early-printk.txt.
> > > > - rework how EARLY_PRINTK is set to Y
> > > > and use that instead of a list of all EARLY_UART_*
> > > > - Add a check to ask user to use Kconfig to set early printk.
> > > > - rework the possible choice to have all uart driver and platform
> > > > specific option together.
> > > > - have added or reword prompt and help messages of the different
> > > > options. The platform specific option don't have extended help,
> > > > the
> > > > prompt is probably enough.
> > > > (The non-platform specific options have the help message that
> > > > Julien
> > > > have written in the first version.)
> > > > - have made EARLY_UART_INIT dependent on the value of
> > > > EARLY_UART_PL011_BAUD_RATE so that there is no need to expose
> > > > _INIT to
> > > > users.
> > > >
> > >
> > > The patch is fine by me. I only have one very minor comment below.
> >
> > I forgot to add
> >
> > Tested-by: Stefano Stabellini <sstabellini@kernel.org>
>
> Thank you for the testing. This patch contains a question for you (see [1]).
> Can you please take a look?
>
> Cheers,
>
> [1] <7720f54e-7ef7-9dae-dfdb-38b89a0892a6@xen.org>
So the question is:
> I noticed below you added "depends on ARM_64" on the Xilinx SoC. In
> general, platform specific options are tied to either arm32 or arm64,
> even if the UART "driver" is arch agnostic.
>
> You could technically boot Xen on Arm 32-bit on Armv8 HW provided they
> support 32-bit at the hypervisor level, but we never supported this
> case. So I am wondering whether we should add depends on each
> earlyprintk. Stefano, any opinions?
Well spotted.
Xilinx doesn't support 32-bit Xen on their boards, "support" as in test,
run or validate. So it would not be a problem from Xilinx point of view
to add a "depends on ARM_64".
I take that you are suggesting adding "depends on ARM_64/32" under the
legacy platform earlyprintk options, from EARLY_PRINTK_BRCM to
EARLY_PRINTK_ZYNQMP right? If so, I am fine with it, and it seems like a
good idea.
The other new generic earlyprintk options, the ones that only depend on
the uart driver, from EARLY_UART_CHOICE_8250 to EARLY_UART_CHOICE_SCIF,
it feels more natural to leave them without a specific arch dependency.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig
2020-03-17 17:51 ` Stefano Stabellini
@ 2020-03-24 14:48 ` Anthony PERARD
0 siblings, 0 replies; 18+ messages in thread
From: Anthony PERARD @ 2020-03-24 14:48 UTC (permalink / raw)
To: Stefano Stabellini
Cc: Julien Grall, Wei Liu, Andrew Cooper, Ian Jackson, George Dunlap,
Julien Grall, Jan Beulich, xen-devel, Volodymyr Babchuk,
Roger Pau Monné
On Tue, Mar 17, 2020 at 10:51:34AM -0700, Stefano Stabellini wrote:
> On Tue, 17 Mar 2020, Julien Grall wrote:
> > I noticed below you added "depends on ARM_64" on the Xilinx SoC. In
> > general, platform specific options are tied to either arm32 or arm64,
> > even if the UART "driver" is arch agnostic.
> >
> > You could technically boot Xen on Arm 32-bit on Armv8 HW provided they
> > support 32-bit at the hypervisor level, but we never supported this
> > case. So I am wondering whether we should add depends on each
> > earlyprintk. Stefano, any opinions?
>
> Well spotted.
>
> Xilinx doesn't support 32-bit Xen on their boards, "support" as in test,
> run or validate. So it would not be a problem from Xilinx point of view
> to add a "depends on ARM_64".
>
> I take that you are suggesting adding "depends on ARM_64/32" under the
> legacy platform earlyprintk options, from EARLY_PRINTK_BRCM to
> EARLY_PRINTK_ZYNQMP right? If so, I am fine with it, and it seems like a
> good idea.
I don't have useful information on which Xen bitness each platform can
boot or support, so I can't really add those "depends on". But that
could be done in a follow-up.
> The other new generic earlyprintk options, the ones that only depend on
> the uart driver, from EARLY_UART_CHOICE_8250 to EARLY_UART_CHOICE_SCIF,
> it feels more natural to leave them without a specific arch dependency.
That would mean adding drivers for both arm32 and arm64. For example,
debug-cadence.inc is only available in arm64/. So if someone selects
arm32 and the cadence early uart driver, there's going to be a compile
error. That's the only reason on why I've added "depends on" on all
EARLY_UART_CHOICE_*.
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-03-24 14:49 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09 17:45 [Xen-devel] [XEN PATCH v3 0/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
2020-03-09 17:45 ` [Xen-devel] [XEN PATCH v3 1/2] xen/arm: Rename all early printk macro Anthony PERARD
2020-03-11 13:57 ` Julien Grall
2020-03-11 14:46 ` Anthony PERARD
2020-03-12 20:18 ` Julien Grall
2020-03-13 23:01 ` Stefano Stabellini
2020-03-09 17:45 ` [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
2020-03-11 14:18 ` Julien Grall
[not found] ` <20200311152613.GJ2152@perard.uk.xensource.com>
2020-03-11 17:21 ` Julien Grall
2020-03-11 17:38 ` Anthony PERARD
2020-03-12 20:16 ` Julien Grall
2020-03-13 23:12 ` Stefano Stabellini
2020-03-13 23:14 ` Stefano Stabellini
2020-03-17 9:36 ` Julien Grall
2020-03-17 17:51 ` Stefano Stabellini
2020-03-24 14:48 ` Anthony PERARD
2020-03-13 23:19 ` Julien Grall
2020-03-13 23:37 ` Julien Grall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.