| Message ID | 20200306174250.291503-2-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN,v2,1/2] xen/arm: Rename all early printk macro | expand |
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 >
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,
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,
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,
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 */
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(-)