diff mbox

[RFC,24/29] xen/arm: Don't use pl011 UART by default for early printk

Message ID 29c9b83854141ee04ee872e7477b8963e538be54.1367188423.git.julien.grall@linaro.org
State Changes Requested, archived
Headers show

Commit Message

Julien Grall April 28, 2013, 11:02 p.m. UTC
Add CONFIG_EARLY_PRINTK options in configs/arm{32,64}.mk to let the user
to choose if he wants to have early output, ie before the console is initialized.

This code is specific for each UART. When CONFIG_EARLY_PRINTK is enabled,
Xen will only be able to run on a board with this UART.

If a developper wants to add support for a new UART, he must implement the
following function/variable:
   - early_uart_paddr: variable which contains the physical base address
   for the UART
   - early_uart_init: initialize the UART
   - early_uart_ready: check and wait until the UART can transmit a new
   character
   - early_uart_transmit: transmit a character

For more details about the parameters of each function,
see arm{32,64}/debug-pl011.S comments.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 config/arm32.mk                    |   11 ++++++
 config/arm64.mk                    |   11 ++++++
 xen/arch/arm/Makefile              |    2 +-
 xen/arch/arm/Rules.mk              |    9 +++++
 xen/arch/arm/arm32/Makefile        |    5 ++-
 xen/arch/arm/arm32/debug-pl011.S   |   64 +++++++++++++++++++++++++++++++++
 xen/arch/arm/arm32/debug.S         |   33 +++++++++++++++++
 xen/arch/arm/arm32/head.S          |   66 ++++++++++++++++------------------
 xen/arch/arm/arm64/Makefile        |    3 ++
 xen/arch/arm/arm64/debug-pl011.S   |   64 +++++++++++++++++++++++++++++++++
 xen/arch/arm/arm64/debug.S         |   33 +++++++++++++++++
 xen/arch/arm/arm64/head.S          |   69 +++++++++++++++++-------------------
 xen/arch/arm/early_printk.c        |   16 +--------
 xen/include/asm-arm/config.h       |    2 --
 xen/include/asm-arm/early_printk.h |    2 +-
 15 files changed, 298 insertions(+), 92 deletions(-)
 create mode 100644 xen/arch/arm/arm32/debug-pl011.S
 create mode 100644 xen/arch/arm/arm32/debug.S
 create mode 100644 xen/arch/arm/arm64/debug-pl011.S
 create mode 100644 xen/arch/arm/arm64/debug.S

Comments

Ian Campbell April 29, 2013, 4:45 p.m. UTC | #1
On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote:
> Add CONFIG_EARLY_PRINTK options in configs/arm{32,64}.mk to let the user
> to choose if he wants to have early output, ie before the console is initialized.

These shouldn't go in config/arm*.mk but should be handed in
xen/arch/arm/
> 
> This code is specific for each UART. When CONFIG_EARLY_PRINTK is enabled,
> Xen will only be able to run on a board with this UART.
> 
> If a developper wants to add support for a new UART, he must implement the
> following function/variable:
>    - early_uart_paddr: variable which contains the physical base address
>    for the UART
>    - early_uart_init: initialize the UART
>    - early_uart_ready: check and wait until the UART can transmit a new
>    character
>    - early_uart_transmit: transmit a character
> 
> For more details about the parameters of each function,
> see arm{32,64}/debug-pl011.S comments.

It's a damned shame the asm isn't compatible, oh well...

> diff --git a/config/arm32.mk b/config/arm32.mk
> index f64f0c1..83a7767 100644
> --- a/config/arm32.mk
> +++ b/config/arm32.mk
> @@ -7,6 +7,17 @@ CONFIG_ARM_$(XEN_OS) := y
>  # Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb:
>  CFLAGS += -marm
> 
> +# Xen early debugging function
> +# This is helpful 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.
> +# Possible value:
> +#   - none: no early printk

Blank/unset would represent none? Or you mean literal "none"?

> +#   - pl011: printk with PL011 UART
> +CONFIG_EARLY_PRINTK := none

I guess you mean literal none...

Can this be overriden on command line or in .config? You may need to
use ?= so it can be.

>  HAS_PL011 := y
> 
>  # Use only if calling $(LD) directly.
> diff --git a/config/arm64.mk b/config/arm64.mk
> index b2457eb..6187df8 100644
> --- a/config/arm64.mk
> +++ b/config/arm64.mk
> @@ -4,6 +4,17 @@ CONFIG_ARM_$(XEN_OS) := y
> 
>  CFLAGS += #-marm -march= -mcpu= etc
> 
> +# Xen early debugging function
> +# This is helpful 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.
> +# Possible value:
> +#   - none: no early printk
> +#   - pl011: printk with PL011 UART
> +CONFIG_EARLY_PRINTK := none

Shall we create config/arm.mk, included from arm32 and arm64 and reduce
the duplication?

> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
> index 1ad3364..6af8ca3 100644
> --- a/xen/arch/arm/arm32/Makefile
> +++ b/xen/arch/arm/arm32/Makefile
> @@ -5,4 +5,7 @@ obj-y += mode_switch.o
>  obj-y += proc-ca15.o
> 
>  obj-y += traps.o
> -obj-y += domain.o
> \ No newline at end of file
> +obj-y += domain.o
> +
> +obj-$(EARLY_PRINTK) += debug.o
> +obj-$(CONFIG_EARLY_PL011) += debug-pl011.o

This could become
	obj-$(EARLY_PRINTK) += debug-$(CONFIG_EARLY_PRINTK).o 
and save adding a new one for each name? And if you create a stub
debug-none.S you could just make it obj-y ? Should we gate this on
debug=y?

> @@ -106,8 +106,10 @@ past_zImage:
>          bne   1b
> 
>  boot_cpu:
> -#ifdef EARLY_UART_ADDRESS
> -        ldr   r11, =EARLY_UART_ADDRESS  /* r11 := UART base address */
> +#ifdef EARLY_PRINTK
> +        ldr   r0, =early_uart_paddr     /* VA of early_uart_paddr */
> +        add   r0, r0, r10               /* PA of early_uart_paddr */
> +        ldr   r11, [r0]                 /* r11 := UART base address */

If head.S were to #include debug-pl011.inc would that simplify some of
this stuff?

Ian.
Julien Grall April 29, 2013, 6:12 p.m. UTC | #2
On 04/29/2013 05:45 PM, Ian Campbell wrote:

> On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote:
>> Add CONFIG_EARLY_PRINTK options in configs/arm{32,64}.mk to let the user
>> to choose if he wants to have early output, ie before the console is initialized.
> 
> These shouldn't go in config/arm*.mk but should be handed in
> xen/arch/arm/


I don't understand why, it's a config option and the user can modify
arm32.mk

>>
>> This code is specific for each UART. When CONFIG_EARLY_PRINTK is enabled,
>> Xen will only be able to run on a board with this UART.
>>
>> If a developper wants to add support for a new UART, he must implement the
>> following function/variable:
>>    - early_uart_paddr: variable which contains the physical base address
>>    for the UART
>>    - early_uart_init: initialize the UART
>>    - early_uart_ready: check and wait until the UART can transmit a new
>>    character
>>    - early_uart_transmit: transmit a character
>>
>> For more details about the parameters of each function,
>> see arm{32,64}/debug-pl011.S comments.
> 
> It's a damned shame the asm isn't compatible, oh well...
> 
>> diff --git a/config/arm32.mk b/config/arm32.mk
>> index f64f0c1..83a7767 100644
>> --- a/config/arm32.mk
>> +++ b/config/arm32.mk
>> @@ -7,6 +7,17 @@ CONFIG_ARM_$(XEN_OS) := y
>>  # Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb:
>>  CFLAGS += -marm
>>
>> +# Xen early debugging function
>> +# This is helpful 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.
>> +# Possible value:
>> +#   - none: no early printk
> 
> Blank/unset would represent none? Or you mean literal "none"?

I choose to use literal "none", but finally I think it's better to have
blank/unset.

>> +#   - pl011: printk with PL011 UART
>> +CONFIG_EARLY_PRINTK := none
> 
> I guess you mean literal none...
> 
> Can this be overriden on command line or in .config? You may need to
> use ?= so it can be.

Yes. Make overrides all "local" variables with the ones on the command line.

>>  HAS_PL011 := y
>>
>>  # Use only if calling $(LD) directly.
>> diff --git a/config/arm64.mk b/config/arm64.mk
>> index b2457eb..6187df8 100644
>> --- a/config/arm64.mk
>> +++ b/config/arm64.mk
>> @@ -4,6 +4,17 @@ CONFIG_ARM_$(XEN_OS) := y
>>
>>  CFLAGS += #-marm -march= -mcpu= etc
>>
>> +# Xen early debugging function
>> +# This is helpful 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.
>> +# Possible value:
>> +#   - none: no early printk
>> +#   - pl011: printk with PL011 UART
>> +CONFIG_EARLY_PRINTK := none
> 
> Shall we create config/arm.mk, included from arm32 and arm64 and reduce
> the duplication?


I think so.

>> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
>> index 1ad3364..6af8ca3 100644
>> --- a/xen/arch/arm/arm32/Makefile
>> +++ b/xen/arch/arm/arm32/Makefile
>> @@ -5,4 +5,7 @@ obj-y += mode_switch.o
>>  obj-y += proc-ca15.o
>>
>>  obj-y += traps.o
>> -obj-y += domain.o
>> \ No newline at end of file
>> +obj-y += domain.o
>> +
>> +obj-$(EARLY_PRINTK) += debug.o
>> +obj-$(CONFIG_EARLY_PL011) += debug-pl011.o
> 
> This could become
> 	obj-$(EARLY_PRINTK) += debug-$(CONFIG_EARLY_PRINTK).o 
> and save adding a new one for each name? And if you create a stub
> debug-none.S you could just make it obj-y ? Should we gate this on
> debug=y?


What does debug=y do?

I think we don't need debug-none.S if CONFIG_EARLY_PRINTK is unset when
early printk is disabled. We just need to check in Rules.mk is
CONFIG_EARLY_PRINTK is set or not and defined EARLY_PRINTK.

>> @@ -106,8 +106,10 @@ past_zImage:
>>          bne   1b
>>
>>  boot_cpu:
>> -#ifdef EARLY_UART_ADDRESS
>> -        ldr   r11, =EARLY_UART_ADDRESS  /* r11 := UART base address */
>> +#ifdef EARLY_PRINTK
>> +        ldr   r0, =early_uart_paddr     /* VA of early_uart_paddr */
>> +        add   r0, r0, r10               /* PA of early_uart_paddr */
>> +        ldr   r11, [r0]                 /* r11 := UART base address */
> 
> If head.S were to #include debug-pl011.inc would that simplify some of
> this stuff?


It's also used in debug.s, which is a wrapper for the C early printk.

If we use macros instead of functions the code will be simplified. All
link register used will be removed.

I will rework this patch.
Ian Campbell April 30, 2013, 9 a.m. UTC | #3
On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote:
> diff --git a/config/arm32.mk b/config/arm32.mk
> index f64f0c1..83a7767 100644
> --- a/config/arm32.mk
> +++ b/config/arm32.mk
> @@ -7,6 +7,17 @@ CONFIG_ARM_$(XEN_OS) := y
>  # Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb:
>  CFLAGS += -marm
> 
> +# Xen early debugging function
> +# This is helpful 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.
> +# Possible value:
> +#   - none: no early printk
> +#   - pl011: printk with PL011 UART
> +CONFIG_EARLY_PRINTK := none

I'm wondering if this is the right place for this "config", other than
the load address (which is also arguably not supposed to be here), they
are mostly per-platform variables rather than user focussed config
options.

This new option seems more akin to the debug=y or CONFIG_DTB_FILE
buildtime options, which are mostly contained xen/Makefile and
xen/arch/arm/Makefile etc.

I'm not really sure about this though -- the hypervisor doesn't really
have much in the way of user tunable CONFIG options -- anyone else have
any opinions?
Ian Campbell April 30, 2013, 9:18 a.m. UTC | #4
On Mon, 2013-04-29 at 19:12 +0100, Julien Grall wrote:
> On 04/29/2013 05:45 PM, Ian Campbell wrote:
> 
> > On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote:
> >> Add CONFIG_EARLY_PRINTK options in configs/arm{32,64}.mk to let the user
> >> to choose if he wants to have early output, ie before the console is initialized.
> > 
> > These shouldn't go in config/arm*.mk but should be handed in
> > xen/arch/arm/
> 
> 
> I don't understand why, it's a config option and the user can modify
> arm32.mk

Heh, I forgot I'd said this already and repeated myself this morning,
oops!

The thing is that arm*.mk (and the other *.mk) tend towards things which
are statically different between platforms, not actual user configurable
stuff.

The problem is that we don't have much on the way of user configurable
stuff in the hypervisor, so we don't have much infrastructure and or
precedent.

> >> +# Xen early debugging function
> >> +# This is helpful 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.
> >> +# Possible value:
> >> +#   - none: no early printk
> > 
> > Blank/unset would represent none? Or you mean literal "none"?
> 
> I choose to use literal "none", but finally I think it's better to have
> blank/unset.

It would be more customary to have the variable be not set, I think.

> 
> >> +#   - pl011: printk with PL011 UART
> >> +CONFIG_EARLY_PRINTK := none
> > 
> > I guess you mean literal none...
> > 
> > Can this be overriden on command line or in .config? You may need to
> > use ?= so it can be.
> 
> Yes. Make overrides all "local" variables with the ones on the command line.

What about .config ?

> >> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
> >> index 1ad3364..6af8ca3 100644
> >> --- a/xen/arch/arm/arm32/Makefile
> >> +++ b/xen/arch/arm/arm32/Makefile
> >> @@ -5,4 +5,7 @@ obj-y += mode_switch.o
> >>  obj-y += proc-ca15.o
> >>
> >>  obj-y += traps.o
> >> -obj-y += domain.o
> >> \ No newline at end of file
> >> +obj-y += domain.o
> >> +
> >> +obj-$(EARLY_PRINTK) += debug.o
> >> +obj-$(CONFIG_EARLY_PL011) += debug-pl011.o
> > 
> > This could become
> > 	obj-$(EARLY_PRINTK) += debug-$(CONFIG_EARLY_PRINTK).o 
> > and save adding a new one for each name? And if you create a stub
> > debug-none.S you could just make it obj-y ? Should we gate this on
> > debug=y?
> 
> 
> What does debug=y do?

It turns on ASSERTS and undefs the NDEBUG define which is used to
surround bits of debugging code and stuff like that etc. It also implies
other things like verbose=y and debug symbols in the hypervisor binary
(i.e. passes -g to gcc/ld).

> I think we don't need debug-none.S if CONFIG_EARLY_PRINTK is unset when
> early printk is disabled. We just need to check in Rules.mk is
> CONFIG_EARLY_PRINTK is set or not and defined EARLY_PRINTK.

That sounds workable, will wait to see how it ends up looking

Ian.
Julien Grall April 30, 2013, 11:21 a.m. UTC | #5
Missing Xen devel mailing list.

On 30 April 2013 12:19, Julien Grall <julien.grall@linaro.org> wrote:

>
>
>
> On 30 April 2013 10:18, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>
>> On Mon, 2013-04-29 at 19:12 +0100, Julien Grall wrote:
>> > On 04/29/2013 05:45 PM, Ian Campbell wrote:
>> >
>> > > On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote:
>> > >> Add CONFIG_EARLY_PRINTK options in configs/arm{32,64}.mk to let the
>> user
>> > >> to choose if he wants to have early output, ie before the console is
>> initialized.
>> > >
>> > > These shouldn't go in config/arm*.mk but should be handed in
>> > > xen/arch/arm/
>> >
>> >
>> > I don't understand why, it's a config option and the user can modify
>> > arm32.mk
>>
>> Heh, I forgot I'd said this already and repeated myself this morning,
>> oops!
>>
>> The thing is that arm*.mk (and the other *.mk) tend towards things which
>> are statically different between platforms, not actual user configurable
>> stuff.
>>
>> The problem is that we don't have much on the way of user configurable
>> stuff in the hypervisor, so we don't have much infrastructure and or
>> precedent.
>>
>> > >> +# Xen early debugging function
>> > >> +# This is helpful 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.
>> > >> +# Possible value:
>> > >> +#   - none: no early printk
>> > >
>> > > Blank/unset would represent none? Or you mean literal "none"?
>> >
>> > I choose to use literal "none", but finally I think it's better to have
>> > blank/unset.
>>
>> It would be more customary to have the variable be not set, I think.
>>
>> >
>> > >> +#   - pl011: printk with PL011 UART
>> > >> +CONFIG_EARLY_PRINTK := none
>> > >
>> > > I guess you mean literal none...
>> > >
>> > > Can this be overriden on command line or in .config? You may need to
>> > > use ?= so it can be.
>> >
>> > Yes. Make overrides all "local" variables with the ones on the command
>> line.
>>
>> What about .config ?
>>
>
> I didn't know that Xen can have a .config file. So I will use ?=.
>
>
>>
>> > >> diff --git a/xen/arch/arm/arm32/Makefile
>> b/xen/arch/arm/arm32/Makefile
>> > >> index 1ad3364..6af8ca3 100644
>> > >> --- a/xen/arch/arm/arm32/Makefile
>> > >> +++ b/xen/arch/arm/arm32/Makefile
>> > >> @@ -5,4 +5,7 @@ obj-y += mode_switch.o
>> > >>  obj-y += proc-ca15.o
>> > >>
>> > >>  obj-y += traps.o
>> > >> -obj-y += domain.o
>> > >> \ No newline at end of file
>> > >> +obj-y += domain.o
>> > >> +
>> > >> +obj-$(EARLY_PRINTK) += debug.o
>> > >> +obj-$(CONFIG_EARLY_PL011) += debug-pl011.o
>> > >
>> > > This could become
>> > >     obj-$(EARLY_PRINTK) += debug-$(CONFIG_EARLY_PRINTK).o
>> > > and save adding a new one for each name? And if you create a stub
>> > > debug-none.S you could just make it obj-y ? Should we gate this on
>> > > debug=y?
>> >
>> >
>> > What does debug=y do?
>>
>> It turns on ASSERTS and undefs the NDEBUG define which is used to
>> surround bits of debugging code and stuff like that etc. It also implies
>> other things like verbose=y and debug symbols in the hypervisor binary
>> (i.e. passes -g to gcc/ld).
>>
>
> Thanks. Even if we gate CONFIG_EARLY_PRINTK to debug=y, we should let
> the developper the option to disable/enable early printk. Otherwise we will
> lost debug when Xen can boot on multiple board.
>
> --
> Julien Grall
>
Julien Grall April 30, 2013, 11:24 a.m. UTC | #6
On 30 April 2013 10:00, Ian Campbell <Ian.Campbell@citrix.com> wrote:

> On Mon, 2013-04-29 at 00:02 +0100, Julien Grall wrote:
> > diff --git a/config/arm32.mk b/config/arm32.mk
> > index f64f0c1..83a7767 100644
> > --- a/config/arm32.mk
> > +++ b/config/arm32.mk
> > @@ -7,6 +7,17 @@ CONFIG_ARM_$(XEN_OS) := y
> >  # Explicitly specifiy 32-bit ARM ISA since toolchain default can be
> -mthumb:
> >  CFLAGS += -marm
> >
> > +# Xen early debugging function
> > +# This is helpful 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.
> > +# Possible value:
> > +#   - none: no early printk
> > +#   - pl011: printk with PL011 UART
> > +CONFIG_EARLY_PRINTK := none
>
> I'm wondering if this is the right place for this "config", other than
> the load address (which is also arguably not supposed to be here), they
> are mostly per-platform variables rather than user focussed config
> options.
> This new option seems more akin to the debug=y or CONFIG_DTB_FILE
> buildtime options, which are mostly contained xen/Makefile and
> xen/arch/arm/Makefile etc.
>
> I'm not really sure about this though -- the hypervisor doesn't really
> have much in the way of user tunable CONFIG options -- anyone else have
> any opinions?
>
>
>
I think we need to have a "high" level way to choose the early printk
driver. The user
should not take care about the base address. Is .config is suitable for
this kind of
options?
Ian Campbell April 30, 2013, 12:44 p.m. UTC | #7
On Tue, 2013-04-30 at 12:21 +0100, Julien Grall wrote:
> Missing Xen devel mailing list.

Ignore my private reply, I've inserted it below...

>         
>         Thanks. Even if we gate CONFIG_EARLY_PRINTK to debug=y, we
>         should let the developper the option to disable/enable early
>         printk. Otherwise we will lost debug when Xen can boot on
>         multiple board.

I'm not sure I follow, these are both valid
        debug=y CONFIG_EARLY_PRINTK=pl011
        debug=y /* No early printk */
In the latter case you have debug on a system which can boot on multiple
boards.

Or are you concerned about not being able to build a production
(non-debug) hypervisor with early-printk? i.e.
        debug=n CONFIG_EARLY_PRINTK=pl011
?
Julien Grall April 30, 2013, 1:39 p.m. UTC | #8
On 04/30/2013 01:44 PM, Ian Campbell wrote:

> On Tue, 2013-04-30 at 12:21 +0100, Julien Grall wrote:
>> Missing Xen devel mailing list.
> 
> Ignore my private reply, I've inserted it below...
> 
>>         
>>         Thanks. Even if we gate CONFIG_EARLY_PRINTK to debug=y, we
>>         should let the developper the option to disable/enable early
>>         printk. Otherwise we will lost debug when Xen can boot on
>>         multiple board.
> 
> I'm not sure I follow, these are both valid
>         debug=y CONFIG_EARLY_PRINTK=pl011
>         debug=y /* No early printk */
> In the latter case you have debug on a system which can boot on multiple
> boards.
> 
> Or are you concerned about not being able to build a production
> (non-debug) hypervisor with early-printk? i.e.
>         debug=n CONFIG_EARLY_PRINTK=pl011
> ?

It's the first one. CONFIG_EARLY_PRINTK should not be enabled on a
production build.
Ian Campbell April 30, 2013, 1:51 p.m. UTC | #9
On Tue, 2013-04-30 at 14:39 +0100, Julien Grall wrote:
> On 04/30/2013 01:44 PM, Ian Campbell wrote:
> 
> > On Tue, 2013-04-30 at 12:21 +0100, Julien Grall wrote:
> >> Missing Xen devel mailing list.
> > 
> > Ignore my private reply, I've inserted it below...
> > 
> >>         
> >>         Thanks. Even if we gate CONFIG_EARLY_PRINTK to debug=y, we
> >>         should let the developper the option to disable/enable early
> >>         printk. Otherwise we will lost debug when Xen can boot on
> >>         multiple board.
> > 
> > I'm not sure I follow, these are both valid
> >         debug=y CONFIG_EARLY_PRINTK=pl011
> >         debug=y /* No early printk */
> > In the latter case you have debug on a system which can boot on multiple
> > boards.
> > 
> > Or are you concerned about not being able to build a production
> > (non-debug) hypervisor with early-printk? i.e.
> >         debug=n CONFIG_EARLY_PRINTK=pl011
> > ?
> 
> It's the first one. CONFIG_EARLY_PRINTK should not be enabled on a
> production build.

I'm still confused what your concern is.

A build with debug=y is not a production build.

Ian.
Julien Grall April 30, 2013, 1:57 p.m. UTC | #10
On 04/30/2013 02:51 PM, Ian Campbell wrote:

> On Tue, 2013-04-30 at 14:39 +0100, Julien Grall wrote:
>> On 04/30/2013 01:44 PM, Ian Campbell wrote:
>>
>>> On Tue, 2013-04-30 at 12:21 +0100, Julien Grall wrote:
>>>> Missing Xen devel mailing list.
>>>
>>> Ignore my private reply, I've inserted it below...
>>>
>>>>         
>>>>         Thanks. Even if we gate CONFIG_EARLY_PRINTK to debug=y, we
>>>>         should let the developper the option to disable/enable early
>>>>         printk. Otherwise we will lost debug when Xen can boot on
>>>>         multiple board.
>>>
>>> I'm not sure I follow, these are both valid
>>>         debug=y CONFIG_EARLY_PRINTK=pl011
>>>         debug=y /* No early printk */
>>> In the latter case you have debug on a system which can boot on multiple
>>> boards.
>>>
>>> Or are you concerned about not being able to build a production
>>> (non-debug) hypervisor with early-printk? i.e.
>>>         debug=n CONFIG_EARLY_PRINTK=pl011
>>> ?
>>
>> It's the first one. CONFIG_EARLY_PRINTK should not be enabled on a
>> production build.
> 
> I'm still confused what your concern is.
> 
> A build with debug=y is not a production build.


I was not sure whether debug=y should imply CONFIG_EARLY_PRINTK or not.

Sorry for the misreading.
Ian Campbell April 30, 2013, 2:09 p.m. UTC | #11
On Tue, 2013-04-30 at 14:57 +0100, Julien Grall wrote:
> On 04/30/2013 02:51 PM, Ian Campbell wrote:
> 
> > On Tue, 2013-04-30 at 14:39 +0100, Julien Grall wrote:
> >> On 04/30/2013 01:44 PM, Ian Campbell wrote:
> >>
> >>> On Tue, 2013-04-30 at 12:21 +0100, Julien Grall wrote:
> >>>> Missing Xen devel mailing list.
> >>>
> >>> Ignore my private reply, I've inserted it below...
> >>>
> >>>>         
> >>>>         Thanks. Even if we gate CONFIG_EARLY_PRINTK to debug=y, we
> >>>>         should let the developper the option to disable/enable early
> >>>>         printk. Otherwise we will lost debug when Xen can boot on
> >>>>         multiple board.
> >>>
> >>> I'm not sure I follow, these are both valid
> >>>         debug=y CONFIG_EARLY_PRINTK=pl011
> >>>         debug=y /* No early printk */
> >>> In the latter case you have debug on a system which can boot on multiple
> >>> boards.
> >>>
> >>> Or are you concerned about not being able to build a production
> >>> (non-debug) hypervisor with early-printk? i.e.
> >>>         debug=n CONFIG_EARLY_PRINTK=pl011
> >>> ?
> >>
> >> It's the first one. CONFIG_EARLY_PRINTK should not be enabled on a
> >> production build.
> > 
> > I'm still confused what your concern is.
> > 
> > A build with debug=y is not a production build.
> 
> 
> I was not sure whether debug=y should imply CONFIG_EARLY_PRINTK or not.

Ah, no, rather the other way round -- earlyprintk is allowed only if
debug=y.

I'm not 100% sure that's a good idea, no doubt someone will soon come up
with an early boot failure which only happens on production builds...

Ian.
diff mbox

Patch

diff --git a/config/arm32.mk b/config/arm32.mk
index f64f0c1..83a7767 100644
--- a/config/arm32.mk
+++ b/config/arm32.mk
@@ -7,6 +7,17 @@  CONFIG_ARM_$(XEN_OS) := y
 # Explicitly specifiy 32-bit ARM ISA since toolchain default can be -mthumb:
 CFLAGS += -marm
 
+# Xen early debugging function
+# This is helpful 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.
+# Possible value:
+#   - none: no early printk
+#   - pl011: printk with PL011 UART
+CONFIG_EARLY_PRINTK := none
 HAS_PL011 := y
 
 # Use only if calling $(LD) directly.
diff --git a/config/arm64.mk b/config/arm64.mk
index b2457eb..6187df8 100644
--- a/config/arm64.mk
+++ b/config/arm64.mk
@@ -4,6 +4,17 @@  CONFIG_ARM_$(XEN_OS) := y
 
 CFLAGS += #-marm -march= -mcpu= etc
 
+# Xen early debugging function
+# This is helpful 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.
+# Possible value:
+#   - none: no early printk
+#   - pl011: printk with PL011 UART
+CONFIG_EARLY_PRINTK := none
 HAS_PL011 := y
 
 # Use only if calling $(LD) directly.
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 981ad78..498777b 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -2,7 +2,7 @@  subdir-$(arm32) += arm32
 subdir-$(arm64) += arm64
 subdir-y += platforms
 
-obj-y += early_printk.o
+obj-$(EARLY_PRINTK) += early_printk.o
 obj-y += cpu.o
 obj-y += domain.o
 obj-y += domctl.o
diff --git a/xen/arch/arm/Rules.mk b/xen/arch/arm/Rules.mk
index a0a14e0..2053b1e 100644
--- a/xen/arch/arm/Rules.mk
+++ b/xen/arch/arm/Rules.mk
@@ -36,3 +36,12 @@  endif
 ifneq ($(call cc-option,$(CC),-fvisibility=hidden,n),n)
 CFLAGS += -DGCC_HAS_VISIBILITY_ATTRIBUTE
 endif
+
+EARLY_PRINTK := n
+
+ifeq ($(CONFIG_EARLY_PRINTK), pl011)
+EARLY_PRINTK := y
+CONFIG_EARLY_PL011 := y
+endif
+
+CFLAGS-$(EARLY_PRINTK) += -DEARLY_PRINTK
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index 1ad3364..6af8ca3 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -5,4 +5,7 @@  obj-y += mode_switch.o
 obj-y += proc-ca15.o
 
 obj-y += traps.o
-obj-y += domain.o
\ No newline at end of file
+obj-y += domain.o
+
+obj-$(EARLY_PRINTK) += debug.o
+obj-$(CONFIG_EARLY_PL011) += debug-pl011.o
diff --git a/xen/arch/arm/arm32/debug-pl011.S b/xen/arch/arm/arm32/debug-pl011.S
new file mode 100644
index 0000000..b00bb1f
--- /dev/null
+++ b/xen/arch/arm/arm32/debug-pl011.S
@@ -0,0 +1,64 @@ 
+/*
+ * xen/arch/arm/arm32/debug-pl011.S
+ *
+ * PL011 specific debug code
+ *
+ * Copyright (c) 2013 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/asm_defns.h>
+
+#define PL011_UART_BASE_ADDRESS 0x1c090000
+
+.globl early_uart_paddr
+early_uart_paddr: .word PL011_UART_BASE_ADDRESS
+
+/* PL011 UART initialization
+ * r11: UART base address
+ * Clobber r1 */
+.globl early_uart_init
+early_uart_init:
+        mov   r1, #0x0
+        str   r1, [r11, #0x28]       /* -> UARTFBRD (Baud divisor fraction) */
+        mov   r1, #0x4               /* 7.3728MHz / 0x4 == 16 * 115200 */
+        str   r1, [r11, #0x24]       /* -> UARTIBRD (Baud divisor integer) */
+        mov   r1, #0x60              /* 8n1 */
+        str   r1, [r11, #0x2C]       /* -> UARTLCR_H (Line control) */
+        ldr   r1, =0x00000301        /* RXE | TXE | UARTEN */
+        str   r1, [r11, #0x30]       /* -> UARTCR (Control Register) */
+        mov   pc, lr
+
+/* PL011 UART wait UART to be ready to transmit
+ * r11: UART base address
+ * Clobber r2 */
+.globl early_uart_ready
+early_uart_ready:
+        ldr   r2, [r11, #0x18]       /* <- UARTFR (Flag register) */
+        tst   r2, #0x8               /* Check BUSY bit */
+        bne   early_uart_ready       /* Wait for the UART to be ready */
+        mov   pc, lr
+
+/* PL011 UART transmit character
+ * r2: character to transmit
+ * r11: UART base address */
+.globl early_uart_transmit
+early_uart_transmit:
+        str   r2, [r11]              /* -> UARTDR (Data Register) */
+        mov   pc, lr
+
+/*
+ * Local variables:
+ * mode: ASM
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/arm32/debug.S b/xen/arch/arm/arm32/debug.S
new file mode 100644
index 0000000..af1b412
--- /dev/null
+++ b/xen/arch/arm/arm32/debug.S
@@ -0,0 +1,33 @@ 
+/*
+ * xen/arch/arm/arm32/debug.S
+ *
+ * Wrapper for early printk
+ *
+ * Julien Grall <julien.grall@linaro.org>
+ * Copyright (c) 2013 Linaro Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/config.h>
+
+.globl early_putch
+/* Print a character on the UART - this function is called by C
+ * r0: character to print */
+early_putch:
+        mov   r1, fp                            /* Preserve fp aka r11 */
+        mov   r3, lr                            /* Save link register */
+        ldr   r11, =FIXMAP_ADDR(FIXMAP_CONSOLE)
+        bl    early_uart_ready
+        mov   r2, r0
+        bl    early_uart_transmit
+        mov   fp, r1                            /* Restore fp */
+        mov   pc, r3
diff --git a/xen/arch/arm/arm32/head.S b/xen/arch/arm/arm32/head.S
index 0b4cfde..55781cd 100644
--- a/xen/arch/arm/arm32/head.S
+++ b/xen/arch/arm/arm32/head.S
@@ -34,7 +34,7 @@ 
 
 /* Macro to print a string to the UART, if there is one.
  * Clobbers r0-r3. */
-#ifdef EARLY_UART_ADDRESS
+#ifdef EARLY_PRINTK
 #define PRINT(_s)       \
         adr   r0, 98f ; \
         bl    puts    ; \
@@ -42,9 +42,9 @@ 
 98:     .asciz _s     ; \
         .align 2      ; \
 99:
-#else
+#else /* EARLY_PRINTK */
 #define PRINT(s)
-#endif
+#endif /* !EARLY_PRINTK */
 
         .arm
 
@@ -106,8 +106,10 @@  past_zImage:
         bne   1b
 
 boot_cpu:
-#ifdef EARLY_UART_ADDRESS
-        ldr   r11, =EARLY_UART_ADDRESS  /* r11 := UART base address */
+#ifdef EARLY_PRINTK
+        ldr   r0, =early_uart_paddr     /* VA of early_uart_paddr */
+        add   r0, r0, r10               /* PA of early_uart_paddr */
+        ldr   r11, [r0]                 /* r11 := UART base address */
         teq   r12, #0                   /* CPU 0 sets up the UART too */
         bleq  init_uart
         PRINT("- CPU ")
@@ -216,7 +218,7 @@  skip_bss:
         bne   pt_ready
 
         /* console fixmap */
-#ifdef EARLY_UART_ADDRESS
+#if defined(EARLY_PRINTK)
         ldr   r1, =xen_fixmap
         add   r1, r1, r10            /* r1 := paddr (xen_fixmap) */
         mov   r3, #0
@@ -279,7 +281,7 @@  pt_ready:
 paging:
 
 
-#ifdef EARLY_UART_ADDRESS
+#ifdef EARLY_PRINTK
         /* Use a virtual address to access the UART. */
         ldr   r11, =FIXMAP_ADDR(FIXMAP_CONSOLE)
 #endif
@@ -345,58 +347,52 @@  fail:   PRINT("- Boot failed -\r\n")
 1:      wfe
         b     1b
 
-#ifdef EARLY_UART_ADDRESS
 
-/* Bring up the UART. Specific to the PL011 UART.
- * Clobbers r0-r2 */
+#ifdef EARLY_PRINTK
+/* Bring up the UART.
+ * Clobbers r0-r3 */
 init_uart:
-        mov   r1, #0x0
-        str   r1, [r11, #0x28]       /* -> UARTFBRD (Baud divisor fraction) */
-        mov   r1, #0x4               /* 7.3728MHz / 0x4 == 16 * 115200 */
-        str   r1, [r11, #0x24]       /* -> UARTIBRD (Baud divisor integer) */
-        mov   r1, #0x60              /* 8n1 */
-        str   r1, [r11, #0x2C]       /* -> UARTLCR_H (Line control) */
-        ldr   r1, =0x00000301        /* RXE | TXE | UARTEN */
-        str   r1, [r11, #0x30]       /* -> UARTCR (Control Register) */
+        mov   r3, lr                /* Save link register */
+        bl    early_uart_init
         adr   r0, 1f
-        b     puts
+        b     2f                    /* Jump to puts */
 1:      .asciz "- UART enabled -\r\n"
         .align 4
 
-/* Print early debug messages.  Specific to the PL011 UART.
+/* Print early debug messages.
  * r0: Nul-terminated string to print.
- * Clobbers r0-r2 */
+ * Clobbers r0-r3 */
 puts:
-        ldr   r2, [r11, #0x18]       /* <- UARTFR (Flag register) */
-        tst   r2, #0x8               /* Check BUSY bit */
-        bne   puts                   /* Wait for the UART to be ready */
+        mov   r3, lr                 /* Save link register */
+2:
+        bl    early_uart_ready
         ldrb  r2, [r0], #1           /* Load next char */
         teq   r2, #0                 /* Exit on nul */
-        moveq pc, lr
-        str   r2, [r11]              /* -> UARTDR (Data Register) */
-        b     puts
+        moveq pc, r3
+        bl    early_uart_transmit
+        b     2b
 
 /* Print a 32-bit number in hex.  Specific to the PL011 UART.
  * r0: Number to print.
- * clobbers r0-r3 */
+ * clobbers r0-r4 */
 putn:
+        mov   r4, lr                 /* Save link register */
         adr   r1, hex
         mov   r3, #8
-1:      ldr   r2, [r11, #0x18]       /* <- UARTFR (Flag register) */
-        tst   r2, #0x8               /* Check BUSY bit */
-        bne   1b                     /* Wait for the UART to be ready */
+1:
+        bl    early_uart_ready
         and   r2, r0, #0xf0000000    /* Mask off the top nybble */
         ldrb  r2, [r1, r2, lsr #28]  /* Convert to a char */
-        str   r2, [r11]              /* -> UARTDR (Data Register) */
+        bl    early_uart_transmit
         lsl   r0, #4                 /* Roll it through one nybble at a time */
         subs  r3, r3, #1
         bne   1b
-        mov   pc, lr
+        mov   pc, r4
 
 hex:    .ascii "0123456789abcdef"
         .align 2
 
-#else  /* EARLY_UART_ADDRESS */
+#else  /* EARLY_PRINTK */
 
 init_uart:
 .global early_puts
@@ -404,7 +400,7 @@  early_puts:
 puts:
 putn:   mov   pc, lr
 
-#endif /* EARLY_UART_ADDRESS */
+#endif /* !EARLY_PRINTK */
 
 /*
  * Local variables:
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index be41f43..d612f87 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -5,3 +5,6 @@  obj-y += mode_switch.o
 
 obj-y += traps.o
 obj-y += domain.o
+
+obj-$(EARLY_PRINTK) += debug.o
+obj-$(CONFIG_EARLY_PL011) += debug-pl011.o
diff --git a/xen/arch/arm/arm64/debug-pl011.S b/xen/arch/arm/arm64/debug-pl011.S
new file mode 100644
index 0000000..1f55fcd
--- /dev/null
+++ b/xen/arch/arm/arm64/debug-pl011.S
@@ -0,0 +1,64 @@ 
+/*
+ * xen/arch/arm/arm64/debug-pl011.S
+ *
+ * PL011 specific debug code
+ *
+ * Copyright (c) 2013 Citrix Systems.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/asm_defns.h>
+
+#define PL011_UART_BASE_ADDRESS 0x1c090000
+
+.globl early_uart_paddr
+early_uart_paddr: .dword PL011_UART_BASE_ADDRESS
+
+/* PL011 UART initialization
+ * x23: UART base address
+ * Clobber x1 */
+.globl early_uart_init
+early_uart_init:
+        mov   x1, #0x0
+        strh  w1, [x23, #0x28]       /* -> UARTFBRD (Baud divisor fraction) */
+        mov   x1, #0x4               /* 7.3728MHz / 0x4 == 16 * 115200 */
+        strh  w1, [x23, #0x24]       /* -> UARTIBRD (Baud divisor integer) */
+        mov   x1, #0x60              /* 8n1 */
+        str   w1, [x23, #0x2C]       /* -> UARTLCR_H (Line control) */
+        ldr   x1, =0x00000301        /* RXE | TXE | UARTEN */
+        str   w1, [x23, #0x30]       /* -> UARTCR (Control Register) */
+        ret
+
+/* PL011 UART wait UART to be ready to transmit
+ * x23: UART base address
+ * Clobber x2 */
+.globl early_uart_ready
+early_uart_ready:
+        ldrh  w2, [x23, #0x18]       /* <- UARTFR (Flag register) */
+        tst   w2, #0x8               /* Check BUSY bit */
+        b.ne  early_uart_ready       /* Wait for the UART to be ready */
+        ret
+
+/* PL011 UART transmit character
+ * x2: character to transmit
+ * x23: UART base address */
+.globl early_uart_transmit
+early_uart_transmit:
+        strb  w2, [x23]              /* -> UARTDR (Data Register) */
+        ret
+
+/*
+ * Local variables:
+ * mode: ASM
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/arm64/debug.S b/xen/arch/arm/arm64/debug.S
new file mode 100644
index 0000000..d1d5b36
--- /dev/null
+++ b/xen/arch/arm/arm64/debug.S
@@ -0,0 +1,33 @@ 
+/*
+ * xen/arch/arm/arm64/debug.S
+ *
+ * Wrapper for early printk
+ *
+ * Julien Grall <julien.grall@linaro.org>
+ * Copyright (c) 2013 Linaro Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <asm/config.h>
+
+.globl early_putch
+/* Print a charact on the UART - this function is called by C
+ * x0: character to print */
+early_putch:
+        mov   x3, x30                           /* Save link register */
+        ldr   x23, =FIXMAP_ADDR(FIXMAP_CONSOLE)
+        bl    early_uart_ready
+        mov   x2, x0
+        bl    early_uart_transmit
+
+        mov   x30, x3                           /* Restore link register */
+        ret
diff --git a/xen/arch/arm/arm64/head.S b/xen/arch/arm/arm64/head.S
index ef02899..9908a12 100644
--- a/xen/arch/arm/arm64/head.S
+++ b/xen/arch/arm/arm64/head.S
@@ -30,8 +30,8 @@ 
 #define PT_DEV_L3 0xe73 /* nG=1 AF=1 SH=10 AP=01 NS=1 ATTR=100 T=1 P=1 */
 
 /* Macro to print a string to the UART, if there is one.
- * Clobbers r0-r3. */
-#ifdef EARLY_UART_ADDRESS
+ * Clobbers x0-x3. */
+#ifdef EARLY_PRINTK
 #define PRINT(_s)       \
         adr   x0, 98f ; \
         bl    puts    ; \
@@ -39,9 +39,9 @@ 
 98:     .asciz _s     ; \
         .align 2      ; \
 99:
-#else
+#else /* EARLY_PRINTK */
 #define PRINT(s)
-#endif
+#endif /* !EARLY_PRINTK */
 
         /*.aarch64*/
 
@@ -109,8 +109,10 @@  real_start:
 2:
 
 boot_cpu:
-#ifdef EARLY_UART_ADDRESS
-        ldr   x23, =EARLY_UART_ADDRESS  /* x23 := UART base address */
+#ifdef EARLY_PRINTK
+        ldr   x0, =early_uart_paddr     /* VA of early_uart_paddr */
+        add   x0, x0, x20               /* PA of early_uart_paddr */
+        ldr   x23, [x0]                 /* x23 := UART base address */
         cbnz  x22, 1f
         bl    init_uart                 /* CPU 0 sets up the UART too */
 1:      PRINT("- CPU ")
@@ -206,7 +208,6 @@  skip_bss:
         mov   x4, x1                 /* Next level into xen_first */
 
        /* console fixmap */
-#ifdef EARLY_UART_ADDRESS
         ldr   x1, =xen_fixmap
         add   x1, x1, x20            /* x1 := paddr (xen_fixmap) */
         lsr   x2, x23, #12
@@ -214,7 +215,6 @@  skip_bss:
         mov   x3, #PT_DEV_L3
         orr   x2, x2, x3             /* x2 := 4K dev map including UART */
         str   x2, [x1, #(FIXMAP_CONSOLE*8)] /* Map it in the first fixmap's slot */
-#endif
 
         /* Build the baseline idle pagetable's first-level entries */
         ldr   x1, =xen_second
@@ -266,10 +266,8 @@  pt_ready:
         br    x1                     /* Get a proper vaddr into PC */
 paging:
 
-#ifdef EARLY_UART_ADDRESS
         /* Use a virtual address to access the UART. */
         ldr   x23, =FIXMAP_ADDR(FIXMAP_CONSOLE)
-#endif
 
         PRINT("- Ready -\r\n")
 
@@ -329,60 +327,57 @@  fail:   PRINT("- Boot failed -\r\n")
 1:      wfe
         b     1b
 
-#ifdef EARLY_UART_ADDRESS
+#ifdef EARLY_PRINTK
 
-/* Bring up the UART. Specific to the PL011 UART.
- * Clobbers r0-r2 */
+/* Bring up the UART.
+ * Clobbers x0-x3 */
 init_uart:
-        mov   x1, #0x0
-        strh  w1, [x23, #0x24]       /* -> UARTIBRD (Baud divisor fraction) */
-        mov   x1, #0x4               /* 7.3728MHz / 0x4 == 16 * 115200 */
-        strh  w1, [x23, #0x24]       /* -> UARTIBRD (Baud divisor integer) */
-        mov   x1, #0x60              /* 8n1 */
-        strh  w1, [x23, #0x24]       /* -> UARTLCR_H (Line control) */
-        ldr   x1, =0x00000301        /* RXE | TXE | UARTEN */
-        strh  w1, [x23, #0x30]       /* -> UARTCR (Control Register) */
+        mov   x3, x30                /* Save link register */
+        bl    early_uart_init
         adr   x0, 1f
-        b     puts
+        b     2f                     /* Jump to puts */
 1:      .asciz "- UART enabled -\r\n"
         .align 4
 
-/* Print early debug messages.  Specific to the PL011 UART.
- * r0: Nul-terminated string to print.
- * Clobbers r0-r2 */
+/* Print early debug messages.
+ * x0: Nul-terminated string to print.
+ * Clobbers x0-x3 */
 puts:
-        ldrh  w2, [x23, #0x18]       /* <- UARTFR (Flag register) */
-        tst   w2, #0x8               /* Check BUSY bit */
-        b.ne  puts                   /* Wait for the UART to be ready */
+        mov   x3, x30                /* Save link register */
+2:
+        bl    early_uart_ready
         ldrb  w2, [x0], #1           /* Load next char */
         cbz   w2, 1f                 /* Exit on nul */
-        str   w2, [x23]              /* -> UARTDR (Data Register) */
+        bl    early_uart_transmit
         b     puts
+
 1:
+        mov   x30, x3                /* Restore link register */
         ret
 
 /* Print a 32-bit number in hex.  Specific to the PL011 UART.
- * r0: Number to print.
- * clobbers r0-r3 */
+ * x0: Number to print.
+ * clobbers x0-x4 */
 putn:
+        mov   x4, x30                /* Save link register */
         adr   x1, hex
         mov   x3, #8
-1:      ldrh  w2, [x23, #0x18]       /* <- UARTFR (Flag register) */
-        tst   w2, #0x8               /* Check BUSY bit */
-        b.ne  1b                     /* Wait for the UART to be ready */
+1:
+        bl early_uart_ready
         and   x2, x0, #0xf0000000    /* Mask off the top nybble */
         lsr   x2, x2, #28
         ldrb  w2, [x1, x2]           /* Convert to a char */
-        strb  w2, [x23]              /* -> UARTDR (Data Register) */
+        bl    early_uart_transmit
         lsl   x0, x0, #4             /* Roll it through one nybble at a time */
         subs  x3, x3, #1
         b.ne  1b
+        mov   x30, x3                /* Restore link register */
         ret
 
 hex:    .ascii "0123456789abcdef"
         .align 2
 
-#else  /* EARLY_UART_ADDRESS */
+#else  /* EARLY_PRINTK */
 
 init_uart:
 .global early_puts
@@ -390,4 +385,4 @@  early_puts:
 puts:
 putn:   ret
 
-#endif /* EARLY_UART_ADDRESS */
+#endif /* EARLY_PRINTK */
diff --git a/xen/arch/arm/early_printk.c b/xen/arch/arm/early_printk.c
index b21f572..7df511a 100644
--- a/xen/arch/arm/early_printk.c
+++ b/xen/arch/arm/early_printk.c
@@ -15,19 +15,7 @@ 
 #include <xen/string.h>
 #include <asm/early_printk.h>
 
-#ifdef EARLY_UART_ADDRESS
-
-void __init early_putch(char c)
-{
-    volatile uint32_t *r;
-
-    r = (uint32_t *)(XEN_VIRT_START + (1 << 21));
-
-    /* XXX: assuming a PL011 UART. */
-    while(*(r + 0x6) & 0x8)
-        ;
-    *r = c;
-}
+void early_putch(char c);
 
 static void __init early_puts(const char *s)
 {
@@ -67,5 +55,3 @@  early_panic(const char *fmt, ...)
 
     while(1);
 }
-
-#endif /* #ifdef EARLY_UART_ADDRESS */
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 1f327c7..943175f 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -141,8 +141,6 @@  extern unsigned long frametable_virt_end;
 #define watchdog_disable() ((void)0)
 #define watchdog_enable()  ((void)0)
 
-/* Board-specific: base address of PL011 UART */
-#define EARLY_UART_ADDRESS 0x1c090000
 /* Board-specific: base address of GIC + its regs */
 #define GIC_BASE_ADDRESS 0x2c000000
 #define GIC_DR_OFFSET 0x1000
diff --git a/xen/include/asm-arm/early_printk.h b/xen/include/asm-arm/early_printk.h
index a0297a7..b72fce7 100644
--- a/xen/include/asm-arm/early_printk.h
+++ b/xen/include/asm-arm/early_printk.h
@@ -12,7 +12,7 @@ 
 
 #include <xen/config.h>
 
-#ifdef EARLY_UART_ADDRESS
+#ifdef EARLY_PRINTK
 
 void early_printk(const char *fmt, ...);
 void early_panic(const char *fmt, ...) __attribute__((noreturn));