[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [XEN PATCH v3 2/2] xen/arm: Configure early printk via Kconfig

Hi Anthony,

On 11/03/2020 15:26, Anthony PERARD wrote:
On Wed, Mar 11, 2020 at 02:18:20PM +0000, Julien Grall wrote:
diff --git a/docs/misc/arm/early-printk.txt b/docs/misc/arm/early-printk.txt
index 89e081e51eaf..c61973013097 100644
--- a/docs/misc/arm/early-printk.txt
+++ b/docs/misc/arm/early-printk.txt
@@ -1,64 +1,39 @@
   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

NIT: AFAICT, the file is using one space after full stop. I would like to
keep it like that for consistency :).

Sound good, I should look at how to fix my vim configuration so it stop
adding extra spaces :-(

:set nojoinspaces

Won't happen again :-).

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"

To double check, this means that earlyprintk can be selected in EXPERT mode
now. However, in the document early-printk.txt, the feature is said to only
be enabled with CONFIG_DEBUG=y.

I like the idea of allowing a user to enable earlyprintk in EXPERT mode
(some early boot bug may only occur in non-debug build). So I am happy to
keep the code like. Can you update the doc accordingly?

Will do.

   endif # DEBUG || EXPERT
diff --git a/xen/arch/arm/Kconfig.debug b/xen/arch/arm/Kconfig.debug
new file mode 100644
index 000000000000..ffb21e8ac20a
--- /dev/null
+++ b/xen/arch/arm/Kconfig.debug
@@ -0,0 +1,287 @@
+       bool "Early printk"
+       optional
+       help
+               You may want to enable early printk if you are debugging 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.
+               Choose one of the UART drivers for early printk, then you'll
+               have to specify the parameters, like the base address.
+               Alternatively, there are platform specific options which will
+               have default values for the various parameters.

Would it be worth to mention such options are deprecated?

Yes, I should mention that here. (And probably in the early-printk.txt
doc as well.)

How about:
     Alternatively, there are platform specific options which will
     have default values for the various parameters. But such option are
     deprecated and will soon be removed.

Or it would be better to highlight the fact that they are deprecated, so
maybe the following would be better:
     Deprecated: Alternatively, there are platform specific options which
     will have default values for the various parameters. But such option
     will soon be removed.

The second version looks better to me.

+       config EARLY_PRINTK_BRCM
+               bool "Early printk with 8250 on Broadcom 7445D0 boards with A15 
+               select EARLY_UART_8250

I noticed below you added "depends on ARM_64" on the Xilinx SoC. In general,
platform specific options are tied to either arm32 or arm64, even if the
UART "driver" is arch agnostic.

Those "depends" are only there because the early uart driver is only
available for one arch. "debug-cadence.inc" can only be found in
"arch/arm/arm64/", not in arm32, for example.

You could technically boot Xen on Arm 32-bit on Armv8 HW provided they
support 32-bit at the hypervisor level, but we never supported this case. So
I am wondering whether we should add depends on each earlyprintk. Stefano,
any opinions?

+       depends on EARLY_PRINTK
+       hex "Early printk, physical base address of debug UART"
+       default 0x87e024000000 if EARLY_PRINTK_THUNDERX

You are allowing EARLY_PRINTK_THUNDERX to be selected on Arm32 platform but
the address is above 4G. I suspect this would break randconfig build.

gcc doesn't seems to complain :-).

I was expecting GAS to throw an error because the 64-bit value does not fit in a 32-bit register. But... it looks like GAS will silently truncate the value to 0x24000000 :(.
(I mean "arm-none-eabi-gcc (Arch Repository) 9.2.0")

But I can have thunderx depends on arm_64.
Is there a way to constrainst the address in Kconfig?


Julien Grall

Xen-devel mailing list



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