| Message ID | 20200306174250.291503-3-anthony.perard@citrix.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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.
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,
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,
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