|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH for-4.13] xen/arm: Add Skeleton for using configuring early printk using Kconfig
On Fri, 13 Sep 2019, Julien Grall wrote:
> At the moment, early printk can only be configured on the make command
> line. It is not very handy because a user has to remove the option
> everytime it is using another command other than compiling the
> hypervisor.
>
> Furthermore, early printk is one of the few odds one that are not using
> Kconfig.
>
> So this is about time to move it to Kconfig. For now, a skeleton is
> added with one example based on Cadence UART. Follow-up will continue to
> convert all the options to Kconfig.
>
> Because Kconfig will prefix all the config by CONFIG_, it is necessary
> to adapt the define within the code.
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
>
> ---
>
> I have sent it as RFC because this is not complete. I will convert the
> rest once we agree the approach is correct.
> ---
> xen/Kconfig.debug | 2 ++
> xen/arch/arm/Kconfig.debug | 40
> ++++++++++++++++++++++++++++++++++++++
> xen/arch/arm/Rules.mk | 5 ++---
> xen/arch/arm/arm32/head.S | 8 ++++----
> xen/arch/arm/arm64/debug.S | 4 ++--
> xen/arch/arm/arm64/head.S | 8 ++++----
> xen/arch/x86/Kconfig.debug | 0
> xen/include/asm-arm/early_printk.h | 2 +-
> 8 files changed, 55 insertions(+), 14 deletions(-)
> create mode 100644 xen/arch/arm/Kconfig.debug
> create mode 100644 xen/arch/x86/Kconfig.debug
>
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index e10e314e25..d0806dba32 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -112,6 +112,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 0000000000..bc3b622695
> --- /dev/null
> +++ b/xen/arch/arm/Kconfig.debug
> @@ -0,0 +1,40 @@
> +choice
> + prompt "Enable early printk"
> +
> + optional
> + config EARLY_PRINTK_ZYNQMP
> + bool "Enable early printk on Xilinx ZynQMP"
> + select EARLY_UART_CADENCE
> + help
> + Say Y here if you want the early printk support on Xilinx
> + ZynQMP platform.
> +
> + config EARLY_PRINTK_CADENCE
> + bool "Enable early printk via Cadence UART"
> + select EARLY_UART_CADENCE
Why not select EARLY_PRINTK directly? Is EARLY_UART_CADENCE actually
needed?
> + help
> + Say Y here if you wish the early printk to direct their
> + output to a Cadence UART. You can use this option to provide
> + the parameters for the Cadence UART rather than selecting
> + one of the platform specific options above if you know the
> + parameters for the port.
> +
> + This option is preferred over the platform specific options;
> + the platform specific options are deprecated and will soon
> + be removed.
> +endchoice
> +
> +config EARLY_PRINTK
> + bool
> +
> +config EARLY_UART_CADENCE
> + bool
> + select EARLY_PRINTK
> +
> +config EARLY_UART_BASE_ADDRESS
> + hex "Physical base address of debug UART" if EARLY_PRINTK
> + default 0xff000000 if EARLY_PRINTK_ZYNQMP
This only works for EARLY_PRINTK_ZYNQMP and not for
EARLY_PRINTK_CADENCE. I imagine we need a default line for each of the
possible options above.
> +config EARLY_PRINTK_INC
> + string
> + default "debug-cadence.inc" if EARLY_UART_CADENCE
> diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
> index 3d9a0ed357..084f1725a8 100644
> --- a/xen/arch/arm/Rules.mk
> +++ b/xen/arch/arm/Rules.mk
> @@ -46,7 +46,6 @@ 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)))
> @@ -82,9 +81,9 @@ 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) +=
> -DCONFIG_EARLY_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) +=
> -DCONFIG_EARLY_UART_BASE_ADDRESS=$(EARLY_UART_BASE_ADDRESS)
I don't have an opinion on the naming, the following is a suggestion to
make the patch easier to write. It looks like we could get away without
any of the renaming below if we choose to export
EARLY_UART_BASE_ADDRESS and keep using EARLY_UART_BASE_ADDRESS directly
in the code below.
> CFLAGS-$(EARLY_PRINTK) += -DEARLY_UART_REG_SHIFT=$(EARLY_UART_REG_SHIFT)
>
> else # !CONFIG_DEBUG
> diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
> index 8f945d318a..0c7e405299 100644
> --- a/xen/arch/arm/arm32/head.S
> +++ b/xen/arch/arm/arm32/head.S
> @@ -32,8 +32,8 @@
> #define PT_UPPER(x) (PT_##x & 0xf00)
> #define PT_LOWER(x) (PT_##x & 0x0ff)
>
> -#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
>
> /*
> @@ -190,7 +190,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")
> @@ -580,7 +580,7 @@ ENTRY(switch_ttbr)
> * Clobbers r0 - r3
> */
> init_uart:
> - mov_w r11, EARLY_UART_BASE_ADDRESS
> + mov_w r11, CONFIG_EARLY_UART_BASE_ADDRESS
> #ifdef EARLY_PRINTK_INIT_UART
> early_uart_init r11, r1, r2
> #endif
> diff --git a/xen/arch/arm/arm64/debug.S b/xen/arch/arm/arm64/debug.S
> index b7f53ac051..71cad9d762 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 790b485f04..32b895ecea 100644
> --- a/xen/arch/arm/arm64/head.S
> +++ b/xen/arch/arm/arm64/head.S
> @@ -40,8 +40,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
>
> /*
> @@ -351,7 +351,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")
> @@ -753,7 +753,7 @@ ENTRY(switch_ttbr)
> * Clobbers x0 - x1
> */
> init_uart:
> - ldr x23, =EARLY_UART_BASE_ADDRESS
> + ldr x23, =CONFIG_EARLY_UART_BASE_ADDRESS
> #ifdef EARLY_PRINTK_INIT_UART
> early_uart_init x23, 0
> #endif
> diff --git a/xen/arch/x86/Kconfig.debug b/xen/arch/x86/Kconfig.debug
> new file mode 100644
> index 0000000000..e69de29bb2
> diff --git a/xen/include/asm-arm/early_printk.h
> b/xen/include/asm-arm/early_printk.h
> index 078cf701dc..d5485decfa 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 */
>
> --
> 2.11.0
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |