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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel