* [Xen-devel] [XEN PATCH v2 0/2] xen/arm: Configure early printk via Kconfig
@ 2020-03-06 17:42 Anthony PERARD
2020-03-06 17:42 ` [Xen-devel] [XEN PATCH v2 1/2] xen/arm: Rename all early printk macro Anthony PERARD
2020-03-06 17:42 ` [Xen-devel] [XEN PATCH v2 2/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
0 siblings, 2 replies; 10+ messages in thread
From: Anthony PERARD @ 2020-03-06 17:42 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-v2
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" but instead of having a single choice
menu where platform and driver are mixed, I have two separated menu. Selecting
a UART driver for early printk will hide the platform specific options. (As I
understand, you want to remove the platform specific options)
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 (1):
xen/arm: Rename all early printk macro
Julien Grall (1):
xen/arm: Configure early printk via Kconfig
docs/misc/arm/early-printk.txt | 50 ++++---
xen/Kconfig.debug | 2 +
xen/arch/arm/Kconfig.debug | 208 +++++++++++++++++++++++++++++
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, 261 insertions(+), 127 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] 10+ messages in thread
* [Xen-devel] [XEN PATCH v2 1/2] xen/arm: Rename all early printk macro
2020-03-06 17:42 [Xen-devel] [XEN PATCH v2 0/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
@ 2020-03-06 17:42 ` Anthony PERARD
2020-03-06 23:57 ` Stefano Stabellini
2020-03-08 17:57 ` Julien Grall
2020-03-06 17:42 ` [Xen-devel] [XEN PATCH v2 2/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
1 sibling, 2 replies; 10+ messages in thread
From: Anthony PERARD @ 2020-03-06 17:42 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
We also reuse CONFIG_EARLY_PRINTK provided by users to enable or not
early printk, thus we need to override the value in makefile.
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>
---
xen/arch/arm/Makefile | 2 +-
xen/arch/arm/Rules.mk | 20 +++++++++-----------
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/include/asm-arm/early_printk.h | 2 +-
14 files changed, 36 insertions(+), 38 deletions(-)
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 022a3a6f82ba..85f8a4c6f914 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -18,8 +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
@@ -66,22 +64,22 @@ 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
ifneq ($(EARLY_PRINTK_INC),)
-EARLY_PRINTK := y
+override CONFIG_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-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
+CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DCONFIG_EARLY_UART_INIT
+CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
+CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_BAUD_RATE=$(EARLY_PRINTK_BAUD)
+CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
+CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_8250_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
else # !CONFIG_DEBUG
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/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..82a22984d9b5 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_BAUD_RATE % 16)
str \rc, [\rb, #FBRD] /* -> UARTFBRD (Baud divisor fraction) */
- mov \rc, #(7372800 / EARLY_PRINTK_BAUD / 16)
+ mov \rc, #(7372800 / CONFIG_EARLY_UART_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/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/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..117b5b256405 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_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_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] 10+ messages in thread
* [Xen-devel] [XEN PATCH v2 2/2] xen/arm: Configure early printk via Kconfig
2020-03-06 17:42 [Xen-devel] [XEN PATCH v2 0/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
2020-03-06 17:42 ` [Xen-devel] [XEN PATCH v2 1/2] xen/arm: Rename all early printk macro Anthony PERARD
@ 2020-03-06 17:42 ` Anthony PERARD
2020-03-06 23:57 ` Stefano Stabellini
2020-03-08 18:29 ` Julien Grall
1 sibling, 2 replies; 10+ messages in thread
From: Anthony PERARD @ 2020-03-06 17:42 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é
From: Julien Grall <julien.grall@arm.com>
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 a UART driver has been selected, the choice to select a platform
won't be possible.
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>
---
docs/misc/arm/early-printk.txt | 50 ++++----
xen/Kconfig.debug | 2 +
xen/arch/arm/Kconfig.debug | 208 +++++++++++++++++++++++++++++++++
xen/arch/arm/Rules.mk | 72 ------------
xen/arch/x86/Kconfig.debug | 0
5 files changed, 234 insertions(+), 98 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..7dff6c314549 100644
--- a/docs/misc/arm/early-printk.txt
+++ b/docs/misc/arm/early-printk.txt
@@ -1,42 +1,40 @@
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 debbuging 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 "UART drivers for early printk" in the "Debugging options" of
+Kconfig. You will then need to set other options, which depends on the drivers
+selected.
-<INC> and <BASE_ADDRESS> are mandatory arguments:
+CONFIG_EARLY_UART_BASE_ADDRESS is a mandatory arguments, 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_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>:
+ Select CONFIG_EARLY_UART_INIT to have the option, if that's set to N
+ 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:
+predefined configurations via "Enable early printk for a specific platform
+(deprecated)".
- brcm: printk with 8250 on Broadcom 7445D0 boards with A15 processors.
- dra7: printk with 8250 on DRA7 platform
@@ -58,7 +56,7 @@ the name of the machine:
- 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,
+These settings are is hardcoded in xen/arch/arm/Kconfig.debug,
see there when adding support for new machines.
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..5111f89043ca
--- /dev/null
+++ b/xen/arch/arm/Kconfig.debug
@@ -0,0 +1,208 @@
+choice
+ bool "UART drivers for early printk"
+ optional
+ help
+ Choose one of the UART driver, then you'll have to specifie the
+ parameters, like the base address.
+
+ Alternatively, there are platform specific options
+ config EARLY_UART_CHOICE_8250
+ select EARLY_UART_8250
+ bool "8250 driver"
+ config EARLY_UART_CHOICE_CADENCE
+ select EARLY_UART_CADENCE
+ depends on ARM_64
+ bool "Enable early printk via Cadence UART"
+ config EARLY_UART_CHOICE_EXYNOS4210
+ select EARLY_UART_EXYNOS4210
+ depends on ARM_32
+ bool "exynos 4210 driver"
+ config EARLY_UART_CHOICE_MESON
+ select EARLY_UART_MESON
+ depends on ARM_64
+ bool "meson driver"
+ config EARLY_UART_CHOICE_MVEBU
+ select EARLY_UART_MVEBU
+ depends on ARM_64
+ bool "mvebu driver"
+ config EARLY_UART_CHOICE_PL011
+ select EARLY_UART_PL011
+ bool "pl011 driver"
+ config EARLY_UART_CHOICE_SCIF
+ select EARLY_UART_SCIF
+ bool "scif driver"
+endchoice
+
+
+choice
+ bool "Enable early printk for a specific platform (deprecated)"
+ depends on !(EARLY_UART_CHOICE_8250 || EARLY_UART_CHOICE_CADENCE || EARLY_UART_CHOICE_EXYNOS4210 || EARLY_UART_CHOICE_MESON || EARLY_UART_CHOICE_MVEBU || EARLY_UART_CHOICE_PL011 || EARLY_UART_CHOICE_SCIF)
+ optional
+ help
+ Those are platform specific options for early printk. This are
+ deprecated and will soon be removed.
+
+ Select a UART driver instead.
+
+ config EARLY_PRINTK_BRCM
+ bool "printk with 8250 on Broadcom 7445D0 boards with A15 processors"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_DRA7
+ bool "printk with 8250 on DRA7 platform"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_EXYNOS5250
+ bool "printk with the second UART on Exynos5250"
+ select EARLY_UART_EXYNOS4210
+ depends on ARM_32
+ config EARLY_PRINTK_FASTMODEL
+ bool "printk on ARM Fastmodel software emulators"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_HIKEY960
+ bool "printk with pl011 with Hikey 960"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_JUNO
+ bool "printk with pl011 on Juno platform"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_LAGER
+ bool "printk with SCIF0 on Renesas Lager board (R-Car H2 processor)"
+ select EARLY_UART_SCIF
+ config EARLY_PRINTK_MIDWAY
+ bool "printk with the pl011 on Calxeda Midway processors"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_MVEBU
+ bool "printk with the MVEBU for Marvell Armada 3700 SoCs"
+ select EARLY_UART_MVEBU
+ depends on ARM_64
+ config EARLY_PRINTK_OMAP5432
+ bool "printk with UART3 on TI OMAP5432 processors"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_RCAR3
+ bool "printk with SCIF2 on Renesas R-Car Gen3 processors"
+ select EARLY_UART_SCIF
+ config EARLY_PRINTK_SEATTLE
+ bool "printk with pl011 for AMD Seattle processor"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_SUN6I
+ bool "printk with 8250 on Allwinner A31 processors"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_SUN7I
+ bool "printk with 8250 on Allwinner A20 processors"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_THUNDERX
+ bool "printk with pl011 for Cavium ThunderX processor"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_VEXPRESS
+ bool "printk with pl011 for versatile express"
+ select EARLY_UART_PL011
+ config EARLY_PRINTK_XGENE_MCDIVITT
+ bool "printk with 820 on Xgene mcdivitt platform"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_XGENE_STORM
+ bool "printk with 820 on Xgene storm platform"
+ select EARLY_UART_8250
+ config EARLY_PRINTK_ZYNQMP
+ bool "printk with Cadence UART for Xilinx ZynqMP SoCs"
+ select EARLY_UART_CADENCE
+ depends on ARM_64
+ help
+ Say Y here if you want the early printk support on Xilinx
+ ZynQMP platform.
+
+endchoice
+
+
+config EARLY_UART_8250
+ bool
+config EARLY_UART_CADENCE
+ bool
+config EARLY_UART_EXYNOS4210
+ bool
+config EARLY_UART_MESON
+ bool
+config EARLY_UART_MVEBU
+ bool
+config EARLY_UART_PL011
+ bool
+config EARLY_UART_SCIF
+ bool
+
+config EARLY_PRINTK
+ depends on EARLY_UART_8250 || EARLY_UART_CADENCE || EARLY_UART_EXYNOS4210 || EARLY_UART_MESON || EARLY_UART_MVEBU || EARLY_UART_PL011 || EARLY_UART_SCIF
+ def_bool y
+
+config EARLY_UART_BASE_ADDRESS
+ depends on EARLY_UART_8250 || EARLY_UART_CADENCE || EARLY_UART_EXYNOS4210 || EARLY_UART_MESON || EARLY_UART_MVEBU || EARLY_UART_PL011 || EARLY_UART_SCIF
+ hex "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_INIT
+ depends on EARLY_UART_PL011
+ bool "Initialize UART early"
+ default y if EARLY_PRINTK_FASTMODEL
+ help
+ Select N to keep the settings that the bootloader or firmware
+ have selected, for maximum compatibility.
+
+ Select Y to initialize the UART with a new baud rate.
+
+config EARLY_UART_BAUD_RATE
+ depends on EARLY_UART_PL011 && EARLY_UART_INIT
+ int "Early printk UART baud rate"
+ help
+ Sets the baud rate which should be used to configure the UART
+ at start of day.
+
+ To avoid initialize the UART and reuse the bootloader or
+ firmware settings, set CONFIG_EARLY_UART_INIT to N (for maximum
+ compatibility).
+ default 115200 if EARLY_PRINTK_FASTMODEL
+
+config EARLY_UART_8250_REG_SHIFT
+ depends on EARLY_UART_8250
+ int "left-shift to apply to the register offsets within the uart"
+ 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 "UART SCIF interface version"
+ depends on EARLY_UART_SCIF
+ default EARLY_UART_SCIF_VERSION_NONE
+ config EARLY_UART_SCIF_VERSION_NONE
+ bool "default scif interface"
+ config EARLY_UART_SCIF_VERSION_A
+ bool "SCIF 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/Rules.mk b/xen/arch/arm/Rules.mk
index 85f8a4c6f914..43f6f3d38b9b 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -17,75 +17,3 @@ CFLAGS-$(CONFIG_ARM_32) += -mcpu=cortex-a15
CFLAGS-$(CONFIG_ARM_64) += -mcpu=generic
CFLAGS-$(CONFIG_ARM_64) += -mgeneral-regs-only # No fp registers etc
-
-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),)
-override CONFIG_EARLY_PRINTK := y
-endif
-
-CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
-CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DCONFIG_EARLY_UART_INIT
-CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
-CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_BAUD_RATE=$(EARLY_PRINTK_BAUD)
-CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
-CFLAGS-$(CONFIG_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
-
-endif
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] 10+ messages in thread
* Re: [Xen-devel] [XEN PATCH v2 1/2] xen/arm: Rename all early printk macro
2020-03-06 17:42 ` [Xen-devel] [XEN PATCH v2 1/2] xen/arm: Rename all early printk macro Anthony PERARD
@ 2020-03-06 23:57 ` Stefano Stabellini
2020-03-09 11:34 ` Anthony PERARD
2020-03-08 17:57 ` Julien Grall
1 sibling, 1 reply; 10+ messages in thread
From: Stefano Stabellini @ 2020-03-06 23:57 UTC (permalink / raw)
To: Anthony PERARD
Cc: xen-devel, Stefano Stabellini, Julien Grall, Volodymyr Babchuk
On Fri, 6 Mar 2020, Anthony PERARD wrote:
> 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
>
> We also reuse CONFIG_EARLY_PRINTK provided by users to enable or not
> early printk, thus we need to override the value in makefile.
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
I tried this patch and it breaks the build with EARLY_PRINTK. With:
export CONFIG_EARLY_PRINTK=zynqmp
I get:
/local/repos/gcc-linaro-5.3.1-2016.05-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc -D__ASSEMBLY__ -DBUILD_ID -fno-strict-aliasing -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include /local/repos/xen-upstream/xen/include/xen/config.h '-D__OBJECT_FILE__="arm64/head.o"' -Wa,--strip-local-absolute -g -MMD -MF arm64/.head.o.d -mcpu=generic -mgeneral-regs-only -DCONFIG_EARLY_PRINTK -DCONFIG_EARLY_PRINTK_INC=\"debug-y.inc\" -DCONFIG_EARLY_UART_BAUD_RATE= -DCONFIG_EARLY_UART_BASE_ADDRESS= -DCONFIG_EARLY_UART_8250_REG_SHIFT= -I/local/repos/xen-upstream/xen/include -fno-stack-protector -fno-exceptions -Wnested-externs -c arm64/head.S -o arm64/head.o
arm64/head.S:49:33: fatal error: debug-y.inc: No such file or directory
I take the patch was not expected to do that?
> ---
> 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>
> ---
> xen/arch/arm/Makefile | 2 +-
> xen/arch/arm/Rules.mk | 20 +++++++++-----------
> 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/include/asm-arm/early_printk.h | 2 +-
> 14 files changed, 36 insertions(+), 38 deletions(-)
>
> 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 022a3a6f82ba..85f8a4c6f914 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -18,8 +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
> @@ -66,22 +64,22 @@ 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
>
> ifneq ($(EARLY_PRINTK_INC),)
> -EARLY_PRINTK := y
> +override CONFIG_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-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
> +CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DCONFIG_EARLY_UART_INIT
> +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
> +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_BAUD_RATE=$(EARLY_PRINTK_BAUD)
> +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
> +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_8250_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
>
> else # !CONFIG_DEBUG
>
> 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/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..82a22984d9b5 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_BAUD_RATE % 16)
> str \rc, [\rb, #FBRD] /* -> UARTFBRD (Baud divisor fraction) */
> - mov \rc, #(7372800 / EARLY_PRINTK_BAUD / 16)
> + mov \rc, #(7372800 / CONFIG_EARLY_UART_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/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/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..117b5b256405 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_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_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 [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [XEN PATCH v2 2/2] xen/arm: Configure early printk via Kconfig
2020-03-06 17:42 ` [Xen-devel] [XEN PATCH v2 2/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
@ 2020-03-06 23:57 ` Stefano Stabellini
2020-03-08 18:29 ` Julien Grall
1 sibling, 0 replies; 10+ messages in thread
From: Stefano Stabellini @ 2020-03-06 23:57 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 Fri, 6 Mar 2020, Anthony PERARD wrote:
> From: Julien Grall <julien.grall@arm.com>
>
> 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 a UART driver has been selected, the choice to select a platform
> won't be possible.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks for the cleanup, much needed
You can add
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>
> Original patch:
> [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk using Kconfig
> <20190913103953.8182-1-julien.grall@arm.com>
> ---
> docs/misc/arm/early-printk.txt | 50 ++++----
> xen/Kconfig.debug | 2 +
> xen/arch/arm/Kconfig.debug | 208 +++++++++++++++++++++++++++++++++
> xen/arch/arm/Rules.mk | 72 ------------
> xen/arch/x86/Kconfig.debug | 0
> 5 files changed, 234 insertions(+), 98 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..7dff6c314549 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -1,42 +1,40 @@
> 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 debbuging 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 "UART drivers for early printk" in the "Debugging options" of
> +Kconfig. You will then need to set other options, which depends on the drivers
> +selected.
>
> -<INC> and <BASE_ADDRESS> are mandatory arguments:
> +CONFIG_EARLY_UART_BASE_ADDRESS is a mandatory arguments, 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_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>:
> + Select CONFIG_EARLY_UART_INIT to have the option, if that's set to N
> + 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:
> +predefined configurations via "Enable early printk for a specific platform
> +(deprecated)".
>
> - brcm: printk with 8250 on Broadcom 7445D0 boards with A15 processors.
> - dra7: printk with 8250 on DRA7 platform
> @@ -58,7 +56,7 @@ the name of the machine:
> - 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,
> +These settings are is hardcoded in xen/arch/arm/Kconfig.debug,
> see there when adding support for new machines.
>
> 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..5111f89043ca
> --- /dev/null
> +++ b/xen/arch/arm/Kconfig.debug
> @@ -0,0 +1,208 @@
> +choice
> + bool "UART drivers for early printk"
> + optional
> + help
> + Choose one of the UART driver, then you'll have to specifie the
^ drivers ^ specify
> + parameters, like the base address.
> +
> + Alternatively, there are platform specific options
> + config EARLY_UART_CHOICE_8250
> + select EARLY_UART_8250
> + bool "8250 driver"
> + config EARLY_UART_CHOICE_CADENCE
> + select EARLY_UART_CADENCE
> + depends on ARM_64
> + bool "Enable early printk via Cadence UART"
> + config EARLY_UART_CHOICE_EXYNOS4210
> + select EARLY_UART_EXYNOS4210
> + depends on ARM_32
> + bool "exynos 4210 driver"
> + config EARLY_UART_CHOICE_MESON
> + select EARLY_UART_MESON
> + depends on ARM_64
> + bool "meson driver"
> + config EARLY_UART_CHOICE_MVEBU
> + select EARLY_UART_MVEBU
> + depends on ARM_64
> + bool "mvebu driver"
> + config EARLY_UART_CHOICE_PL011
> + select EARLY_UART_PL011
> + bool "pl011 driver"
> + config EARLY_UART_CHOICE_SCIF
> + select EARLY_UART_SCIF
> + bool "scif driver"
> +endchoice
> +
> +
> +choice
> + bool "Enable early printk for a specific platform (deprecated)"
> + depends on !(EARLY_UART_CHOICE_8250 || EARLY_UART_CHOICE_CADENCE || EARLY_UART_CHOICE_EXYNOS4210 || EARLY_UART_CHOICE_MESON || EARLY_UART_CHOICE_MVEBU || EARLY_UART_CHOICE_PL011 || EARLY_UART_CHOICE_SCIF)
> + optional
> + help
> + Those are platform specific options for early printk. This are
^ They
> + deprecated and will soon be removed.
> +
> + Select a UART driver instead.
Could you please say "Select a UART driver for early printk instead" or
"Select an early printk UART driver instead". I know it is supposed to
be obvious but we also have UART device drivers under "Device Drivers"
and it actually got me confused for a while.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Xen-devel] [XEN PATCH v2 1/2] xen/arm: Rename all early printk macro
2020-03-06 17:42 ` [Xen-devel] [XEN PATCH v2 1/2] xen/arm: Rename all early printk macro Anthony PERARD
2020-03-06 23:57 ` Stefano Stabellini
@ 2020-03-08 17:57 ` Julien Grall
2020-03-09 12:06 ` Anthony PERARD
1 sibling, 1 reply; 10+ messages in thread
From: Julien Grall @ 2020-03-08 17:57 UTC (permalink / raw)
To: Anthony PERARD, xen-devel; +Cc: Stefano Stabellini, Volodymyr Babchuk
Hi,
On 06/03/2020 17:42, Anthony PERARD wrote:
> 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
>
> We also reuse CONFIG_EARLY_PRINTK provided by users to enable or not
> early printk, thus we need to override the value in makefile.
>
> 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>
> ---
> xen/arch/arm/Makefile | 2 +-
> xen/arch/arm/Rules.mk | 20 +++++++++-----------
> 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/include/asm-arm/early_printk.h | 2 +-
> 14 files changed, 36 insertions(+), 38 deletions(-)
>
> 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 022a3a6f82ba..85f8a4c6f914 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -18,8 +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
> @@ -66,22 +64,22 @@ 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
>
> ifneq ($(EARLY_PRINTK_INC),)
> -EARLY_PRINTK := y
> +override CONFIG_EARLY_PRINTK := y
I am not sure to understand why you are using the directive override
here. Why can't you just prefix the variable with CONFIG_?
> 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-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
> +CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DCONFIG_EARLY_UART_INIT
> +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
> +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_BAUD_RATE=$(EARLY_PRINTK_BAUD)
The baud rate is only used by the PL011 in rare cases. So I would add
PL011 in the name.
> +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
> +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_8250_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
The name clearly suggests that this should be protected with an "if
8250". Maybe in the similar way as for the scif specific variables.
But... I am not going to push for it as the next patch is going to
remove it.
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] 10+ messages in thread
* Re: [Xen-devel] [XEN PATCH v2 2/2] xen/arm: Configure early printk via Kconfig
2020-03-06 17:42 ` [Xen-devel] [XEN PATCH v2 2/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
2020-03-06 23:57 ` Stefano Stabellini
@ 2020-03-08 18:29 ` Julien Grall
2020-03-09 15:04 ` Anthony PERARD
1 sibling, 1 reply; 10+ messages in thread
From: Julien Grall @ 2020-03-08 18:29 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,
On 06/03/2020 17:42, Anthony PERARD wrote:
> From: Julien Grall <julien.grall@arm.com>
>
> 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 a UART driver has been selected, the choice to select a platform
> won't be possible.
>
> 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>
> ---
> docs/misc/arm/early-printk.txt | 50 ++++----
> xen/Kconfig.debug | 2 +
> xen/arch/arm/Kconfig.debug | 208 +++++++++++++++++++++++++++++++++
> xen/arch/arm/Rules.mk | 72 ------------
> xen/arch/x86/Kconfig.debug | 0
> 5 files changed, 234 insertions(+), 98 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..7dff6c314549 100644
> --- a/docs/misc/arm/early-printk.txt
> +++ b/docs/misc/arm/early-printk.txt
> @@ -1,42 +1,40 @@
> 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 debbuging 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 "UART drivers for early printk" in the "Debugging options" of
> +Kconfig. You will then need to set other options, which depends on the drivers
> +selected.
>
> -<INC> and <BASE_ADDRESS> are mandatory arguments:
> +CONFIG_EARLY_UART_BASE_ADDRESS is a mandatory arguments, it is the base
s/arguments/argument/
> +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_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>:
> + Select CONFIG_EARLY_UART_INIT to have the option, if that's set to N
> + 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.
Why did this paragraph and...
> - - 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.
... this one were removed? They actually provide information to the user
of what will happen if they parameters are left to their default value.
> - 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:
> +predefined configurations via "Enable early printk for a specific platform
> +(deprecated)".
>
> - brcm: printk with 8250 on Broadcom 7445D0 boards with A15 processors.
> - dra7: printk with 8250 on DRA7 platform
> @@ -58,7 +56,7 @@ the name of the machine:
> - xgene-storm: printk with 820 on Xgene storm platform
> - zynqmp: printk with Cadence UART for Xilinx ZynqMP SoCs
I think you want to drop the list of early printk alias as they will be
invalid after this patch.
>
> -These settings are is hardcoded in xen/arch/arm/Rules.mk,
> +These settings are is hardcoded in xen/arch/arm/Kconfig.debug,
> see there when adding support for new machines.
>
> 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..5111f89043ca
> --- /dev/null
> +++ b/xen/arch/arm/Kconfig.debug
> @@ -0,0 +1,208 @@
> +choice
> + bool "UART drivers for early printk"
> + optional
> + help
> + Choose one of the UART driver, then you'll have to specifie the
s/specifie/specify/
> + parameters, like the base address.
> +
> + Alternatively, there are platform specific options
> + config EARLY_UART_CHOICE_8250
> + select EARLY_UART_8250
> + bool "8250 driver"
> + config EARLY_UART_CHOICE_CADENCE
> + select EARLY_UART_CADENCE
> + depends on ARM_64
> + bool "Enable early printk via Cadence UART"
> + config EARLY_UART_CHOICE_EXYNOS4210
> + select EARLY_UART_EXYNOS4210
> + depends on ARM_32
> + bool "exynos 4210 driver"
> + config EARLY_UART_CHOICE_MESON
> + select EARLY_UART_MESON
> + depends on ARM_64
> + bool "meson driver"
> + config EARLY_UART_CHOICE_MVEBU
> + select EARLY_UART_MVEBU
> + depends on ARM_64
> + bool "mvebu driver"
> + config EARLY_UART_CHOICE_PL011
> + select EARLY_UART_PL011
> + bool "pl011 driver"
> + config EARLY_UART_CHOICE_SCIF
> + select EARLY_UART_SCIF
> + bool "scif driver"
> +endchoice
> +
> +
> +choice
> + bool "Enable early printk for a specific platform (deprecated)"
> + depends on !(EARLY_UART_CHOICE_8250 || EARLY_UART_CHOICE_CADENCE || EARLY_UART_CHOICE_EXYNOS4210 || EARLY_UART_CHOICE_MESON || EARLY_UART_CHOICE_MVEBU || EARLY_UART_CHOICE_PL011 || EARLY_UART_CHOICE_SCIF)
The split is going to cause confusion to the users because he/she may
select the UART type first and then lose access to this list.
Furthermore, the depends on !(...) is pretty horrible to have. This is
also going to make more difficult to add new UART type (there are a few
more existing...).
So I would prefer if we have one list.
> + optional
> + help
> + Those are platform specific options for early printk. This are
> + deprecated and will soon be removed.
> +
> + Select a UART driver instead.
> +
> + config EARLY_PRINTK_BRCM
> + bool "printk with 8250 on Broadcom 7445D0 boards with A15 processors"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_DRA7
> + bool "printk with 8250 on DRA7 platform"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_EXYNOS5250
> + bool "printk with the second UART on Exynos5250"
> + select EARLY_UART_EXYNOS4210
> + depends on ARM_32
> + config EARLY_PRINTK_FASTMODEL
> + bool "printk on ARM Fastmodel software emulators"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_HIKEY960
> + bool "printk with pl011 with Hikey 960"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_JUNO
> + bool "printk with pl011 on Juno platform"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_LAGER
> + bool "printk with SCIF0 on Renesas Lager board (R-Car H2 processor)"
> + select EARLY_UART_SCIF
> + config EARLY_PRINTK_MIDWAY
> + bool "printk with the pl011 on Calxeda Midway processors"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_MVEBU
> + bool "printk with the MVEBU for Marvell Armada 3700 SoCs"
> + select EARLY_UART_MVEBU
> + depends on ARM_64
> + config EARLY_PRINTK_OMAP5432
> + bool "printk with UART3 on TI OMAP5432 processors"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_RCAR3
> + bool "printk with SCIF2 on Renesas R-Car Gen3 processors"
> + select EARLY_UART_SCIF
> + config EARLY_PRINTK_SEATTLE
> + bool "printk with pl011 for AMD Seattle processor"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_SUN6I
> + bool "printk with 8250 on Allwinner A31 processors"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_SUN7I
> + bool "printk with 8250 on Allwinner A20 processors"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_THUNDERX
> + bool "printk with pl011 for Cavium ThunderX processor"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_VEXPRESS
> + bool "printk with pl011 for versatile express"
> + select EARLY_UART_PL011
> + config EARLY_PRINTK_XGENE_MCDIVITT
> + bool "printk with 820 on Xgene mcdivitt platform"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_XGENE_STORM
> + bool "printk with 820 on Xgene storm platform"
> + select EARLY_UART_8250
> + config EARLY_PRINTK_ZYNQMP
> + bool "printk with Cadence UART for Xilinx ZynqMP SoCs"
> + select EARLY_UART_CADENCE
> + depends on ARM_64
> + help
> + Say Y here if you want the early printk support on Xilinx
> + ZynQMP platform.
This is a bit odd to add a description for one Kconfig and not all the
other. My preference would be to describe all of them, but I understand
this will require extra work.
> +
> +endchoice
> +
> +
> +config EARLY_UART_8250
> + bool
> +config EARLY_UART_CADENCE
> + bool
> +config EARLY_UART_EXYNOS4210
> + bool
> +config EARLY_UART_MESON
> + bool
> +config EARLY_UART_MVEBU
> + bool
> +config EARLY_UART_PL011
> + bool
> +config EARLY_UART_SCIF
> + bool
> +
> +config EARLY_PRINTK
> + depends on EARLY_UART_8250 || EARLY_UART_CADENCE || EARLY_UART_EXYNOS4210 || EARLY_UART_MESON || EARLY_UART_MVEBU || EARLY_UART_PL011 || EARLY_UART_SCIF
Please rework this and let each EARLY_UART_* to select EARLY_PRINTK.
> + def_bool y
> +
> +config EARLY_UART_BASE_ADDRESS
> + depends on EARLY_UART_8250 || EARLY_UART_CADENCE || EARLY_UART_EXYNOS4210 || EARLY_UART_MESON || EARLY_UART_MVEBU || EARLY_UART_PL011 || EARLY_UART_SCIF
This can depends on EARLY_PRINTK.
> + hex "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_INIT
> + depends on EARLY_UART_PL011
> + bool "Initialize UART early"
> + default y if EARLY_PRINTK_FASTMODEL
> + help
> + Select N to keep the settings that the bootloader or firmware
> + have selected, for maximum compatibility.
> +
> + Select Y to initialize the UART with a new baud rate.
At the moment, we rely on the firmware to initialize the UART correctly
(and not only the baud rate...). But it may be possible that it was done
incorrectly. So the earlyprintk code may require to reset the UART. In
the case, the user should have no choice as this as a pretty low impact.
Can we instead select EARLY_UART_INIT based on whether the BAUD_RATE has
been selected?
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] 10+ messages in thread
* Re: [Xen-devel] [XEN PATCH v2 1/2] xen/arm: Rename all early printk macro
2020-03-06 23:57 ` Stefano Stabellini
@ 2020-03-09 11:34 ` Anthony PERARD
0 siblings, 0 replies; 10+ messages in thread
From: Anthony PERARD @ 2020-03-09 11:34 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel, Julien Grall, Volodymyr Babchuk
On Fri, Mar 06, 2020 at 03:57:23PM -0800, Stefano Stabellini wrote:
> On Fri, 6 Mar 2020, Anthony PERARD wrote:
> > We also reuse CONFIG_EARLY_PRINTK provided by users to enable or not
> > early printk, thus we need to override the value in makefile.
> >
> > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>
> I tried this patch and it breaks the build with EARLY_PRINTK. With:
>
> export CONFIG_EARLY_PRINTK=zynqmp
>
> I get:
>
> /local/repos/gcc-linaro-5.3.1-2016.05-x86_64_aarch64-linux-gnu/bin/aarch64-linux-gnu-gcc -D__ASSEMBLY__ -DBUILD_ID -fno-strict-aliasing -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include /local/repos/xen-upstream/xen/include/xen/config.h '-D__OBJECT_FILE__="arm64/head.o"' -Wa,--strip-local-absolute -g -MMD -MF arm64/.head.o.d -mcpu=generic -mgeneral-regs-only -DCONFIG_EARLY_PRINTK -DCONFIG_EARLY_PRINTK_INC=\"debug-y.inc\" -DCONFIG_EARLY_UART_BAUD_RATE= -DCONFIG_EARLY_UART_BASE_ADDRESS= -DCONFIG_EARLY_UART_8250_REG_SHIFT= -I/local/repos/xen-upstream/xen/include -fno-stack-protector -fno-exceptions -Wnested-externs -c arm64/head.S -o arm64/head.o
> arm64/head.S:49:33: fatal error: debug-y.inc: No such file or directory
>
> I take the patch was not expected to do that?
I didn't about users providing CONFIG_EARLY_PRINTK via the environment,
and I'm changing the value, so that fails.
There's two way to provide CONFIG_EARLY_PRINTK:
- via make
make CONFIG_EARLY_PRINTK=zynqmp
- via the environment
CONFIG_EARLY_PRINTK=zynqmp make
I've only tested the first scenario, that why I have an override. But
that doesn't work with the second scenario.
So renaming the make variable EARLY_PRINTK to CONFIG_EARLY_PRINTK will
have to wait until the second patch of the series.
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] 10+ messages in thread
* Re: [Xen-devel] [XEN PATCH v2 1/2] xen/arm: Rename all early printk macro
2020-03-08 17:57 ` Julien Grall
@ 2020-03-09 12:06 ` Anthony PERARD
0 siblings, 0 replies; 10+ messages in thread
From: Anthony PERARD @ 2020-03-09 12:06 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, Stefano Stabellini, Volodymyr Babchuk
On Sun, Mar 08, 2020 at 05:57:32PM +0000, Julien Grall wrote:
> On 06/03/2020 17:42, Anthony PERARD wrote:
> > We also reuse CONFIG_EARLY_PRINTK provided by users to enable or not
> > early printk, thus we need to override the value in makefile.
[...]
> > ifneq ($(EARLY_PRINTK_INC),)
> > -EARLY_PRINTK := y
> > +override CONFIG_EARLY_PRINTK := y
>
> I am not sure to understand why you are using the directive override here.
> Why can't you just prefix the variable with CONFIG_?
override is needed if someone run make like this:
make CONFIG_EARLY_PRINTK=sun7i
otherwise the value can't be changed.
But that dosn't work when run like this:
export CONFIG_EARLY_PRINTK=sun7i
make
So I'm going to have to rename the variable in the second patch.
> > 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-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK
> > +CFLAGS-$(EARLY_PRINTK_INIT_UART) += -DCONFIG_EARLY_UART_INIT
> > +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_PRINTK_INC=\"debug-$(EARLY_PRINTK_INC).inc\"
> > +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_BAUD_RATE=$(EARLY_PRINTK_BAUD)
>
> The baud rate is only used by the PL011 in rare cases. So I would add PL011
> in the name.
Sound fine, I'll rename it.
> > +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
> > +CFLAGS-$(CONFIG_EARLY_PRINTK) += -DCONFIG_EARLY_UART_8250_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
>
> The name clearly suggests that this should be protected with an "if 8250".
> Maybe in the similar way as for the scif specific variables. But... I am not
> going to push for it as the next patch is going to remove it.
Yep, some macro gets defined without a value. :-)
But that gets fix in the next patch.
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] 10+ messages in thread
* Re: [Xen-devel] [XEN PATCH v2 2/2] xen/arm: Configure early printk via Kconfig
2020-03-08 18:29 ` Julien Grall
@ 2020-03-09 15:04 ` Anthony PERARD
0 siblings, 0 replies; 10+ messages in thread
From: Anthony PERARD @ 2020-03-09 15:04 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 Sun, Mar 08, 2020 at 06:29:02PM +0000, Julien Grall wrote:
> On 06/03/2020 17:42, Anthony PERARD wrote:
> > - - 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.
>
> Why did this paragraph and...
>
> > - - 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.
>
> ... this one were removed? They actually provide information to the user of
> what will happen if they parameters are left to their default value.
It was replaced by:
- pl011
- CONFIG_EARLY_UART_BAUD_RATE is, optionally a baud rate which should
be used to configure the UART at start of day.
Select CONFIG_EARLY_UART_INIT to have the option, if that's set to N
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.
So they aren't really removed, just reworked I think. But I probably
need to rework the pl011 one as they may not need to expose
EARLY_UART_INIT to users.
> > - 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:
> > +predefined configurations via "Enable early printk for a specific platform
> > +(deprecated)".
> > - brcm: printk with 8250 on Broadcom 7445D0 boards with A15 processors.
> > - dra7: printk with 8250 on DRA7 platform
> > @@ -58,7 +56,7 @@ the name of the machine:
> > - xgene-storm: printk with 820 on Xgene storm platform
> > - zynqmp: printk with Cadence UART for Xilinx ZynqMP SoCs
>
>
> I think you want to drop the list of early printk alias as they will be
> invalid after this patch.
Will do.
> > diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
> > new file mode 100644
> > index 000000000000..5111f89043ca
> > --- /dev/null
> > +++ b/xen/arch/arm/Kconfig.debug
> > @@ -0,0 +1,208 @@
> > +choice
> > + bool "UART drivers for early printk"
> > + optional
> > + help
> > + Choose one of the UART driver, then you'll have to specifie the
>
> s/specifie/specify/
>
> > + parameters, like the base address.
> > +
> > + Alternatively, there are platform specific options
> > + config EARLY_UART_CHOICE_8250
> > + select EARLY_UART_8250
> > + bool "8250 driver"
[...]
> > +endchoice
> > +
> > +
> > +choice
> > + bool "Enable early printk for a specific platform (deprecated)"
> > + depends on !(EARLY_UART_CHOICE_8250 || EARLY_UART_CHOICE_CADENCE || EARLY_UART_CHOICE_EXYNOS4210 || EARLY_UART_CHOICE_MESON || EARLY_UART_CHOICE_MVEBU || EARLY_UART_CHOICE_PL011 || EARLY_UART_CHOICE_SCIF)
> The split is going to cause confusion to the users because he/she may select
> the UART type first and then lose access to this list.
>
> Furthermore, the depends on !(...) is pretty horrible to have. This is also
> going to make more difficult to add new UART type (there are a few more
> existing...).
>
> So I would prefer if we have one list.
That probably can be done. I'll need to add more help, and maybe better
descriptions.
> > + optional
> > + help
> > + Those are platform specific options for early printk. This are
> > + deprecated and will soon be removed.
> > +
> > + Select a UART driver instead.
> > +
> > + config EARLY_PRINTK_BRCM
> > + bool "printk with 8250 on Broadcom 7445D0 boards with A15 processors"
> > + select EARLY_UART_8250
[..]
> > + config EARLY_PRINTK_ZYNQMP
> > + bool "printk with Cadence UART for Xilinx ZynqMP SoCs"
> > + select EARLY_UART_CADENCE
> > + depends on ARM_64
> > + help
> > + Say Y here if you want the early printk support on Xilinx
> > + ZynQMP platform.
>
> This is a bit odd to add a description for one Kconfig and not all the
> other. My preference would be to describe all of them, but I understand this
> will require extra work.
I just kept the description from your patch and didn't bother to write
help messages for the other. :-)
I think I can take the time now to rework the prompts and help messages
of all configuration options.
> > +
> > +endchoice
> > +
> > +
> > +config EARLY_UART_8250
> > + bool
> > +config EARLY_UART_CADENCE
> > + bool
> > +config EARLY_UART_EXYNOS4210
> > + bool
> > +config EARLY_UART_MESON
> > + bool
> > +config EARLY_UART_MVEBU
> > + bool
> > +config EARLY_UART_PL011
> > + bool
> > +config EARLY_UART_SCIF
> > + bool
> > +
> > +config EARLY_PRINTK
> > + depends on EARLY_UART_8250 || EARLY_UART_CADENCE || EARLY_UART_EXYNOS4210 || EARLY_UART_MESON || EARLY_UART_MVEBU || EARLY_UART_PL011 || EARLY_UART_SCIF
>
> Please rework this and let each EARLY_UART_* to select EARLY_PRINTK.
I though that wasn't possible, but it seems to work. I didn't understand
well enough how select worked. But having:
config EARLY_UART_8250
select EARLY_PRINTK
works, so I do that, and remove the long list of dependencies on other
config options.
> > +config EARLY_UART_INIT
> > + depends on EARLY_UART_PL011
> > + bool "Initialize UART early"
> > + default y if EARLY_PRINTK_FASTMODEL
> > + help
> > + Select N to keep the settings that the bootloader or firmware
> > + have selected, for maximum compatibility.
> > +
> > + Select Y to initialize the UART with a new baud rate.
>
> At the moment, we rely on the firmware to initialize the UART correctly (and
> not only the baud rate...). But it may be possible that it was done
> incorrectly. So the earlyprintk code may require to reset the UART. In the
> case, the user should have no choice as this as a pretty low impact.
>
> Can we instead select EARLY_UART_INIT based on whether the BAUD_RATE has
> been selected?
I had issue trying to have _INIT depends on BAUD_RATE != 0. That why I
did this.
But trying again with:
config EARLY_UART_INIT
depends on EARLY_UART_PL011 && EARLY_UART_PL011_BAUD_RATE != 0
def_bool y
seems to work fine. So I'll do that stop exposing _INIT to users.
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] 10+ messages in thread
end of thread, other threads:[~2020-03-09 15:05 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-06 17:42 [Xen-devel] [XEN PATCH v2 0/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
2020-03-06 17:42 ` [Xen-devel] [XEN PATCH v2 1/2] xen/arm: Rename all early printk macro Anthony PERARD
2020-03-06 23:57 ` Stefano Stabellini
2020-03-09 11:34 ` Anthony PERARD
2020-03-08 17:57 ` Julien Grall
2020-03-09 12:06 ` Anthony PERARD
2020-03-06 17:42 ` [Xen-devel] [XEN PATCH v2 2/2] xen/arm: Configure early printk via Kconfig Anthony PERARD
2020-03-06 23:57 ` Stefano Stabellini
2020-03-08 18:29 ` Julien Grall
2020-03-09 15:04 ` Anthony PERARD
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).