[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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.