[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Ard Biesheuvel]

This patch registers hvc0 as the preferred console if no console
has been specified explicitly on the kernel command line.

The purpose is to allow platform agnostic kernels and boot images
(such as distro installers) to boot in a Xen/ARM domU without the
need to modify the command line by hand.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

This applies on top of the patch Julien is cooking up to move the
call to xen_guest_init() to an earlier boot stage. This needs to
be at least before the call to console_init() in start_kernel()
for this patch to do anything meaningful.

 arch/arm/xen/enlighten.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index c7ca936ebd99..61382cafa877 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -24,6 +24,7 @@
 #include <linux/cpuidle.h>
 #include <linux/cpufreq.h>
 #include <linux/cpu.h>
+#include <linux/console.h>
 
 #include <linux/mm.h>
 
@@ -322,6 +323,9 @@ static int __init xen_guest_init(void)
 
 	register_cpu_notifier(&xen_cpu_notifier);
 
+	if (!console_set_on_cmdline)
+		add_preferred_console("hvc", 0, NULL);
+
 	return 0;
 }
 early_initcall(xen_guest_init);
-- 
1.8.3.2

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Stefano Stabellini]

On Fri, 30 Jan 2015, Ard Biesheuvel wrote:
> This patch registers hvc0 as the preferred console if no console
> has been specified explicitly on the kernel command line.
> 
> The purpose is to allow platform agnostic kernels and boot images
> (such as distro installers) to boot in a Xen/ARM domU without the
> need to modify the command line by hand.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> 
> This applies on top of the patch Julien is cooking up to move the
> call to xen_guest_init() to an earlier boot stage. This needs to
> be at least before the call to console_init() in start_kernel()
> for this patch to do anything meaningful.

This patch looks good but there isn't much point in queuing this up
without the prerequisite.

Julien, what patch exactly is that? What is the status of it?


>  arch/arm/xen/enlighten.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index c7ca936ebd99..61382cafa877 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -24,6 +24,7 @@
>  #include <linux/cpuidle.h>
>  #include <linux/cpufreq.h>
>  #include <linux/cpu.h>
> +#include <linux/console.h>
>  
>  #include <linux/mm.h>
>  
> @@ -322,6 +323,9 @@ static int __init xen_guest_init(void)
>  
>  	register_cpu_notifier(&xen_cpu_notifier);
>  
> +	if (!console_set_on_cmdline)
> +		add_preferred_console("hvc", 0, NULL);
> +
>  	return 0;
>  }
>  early_initcall(xen_guest_init);
> -- 
> 1.8.3.2
> 

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Ian Campbell]

On Tue, 2015-02-10 at 07:00 +0000, Stefano Stabellini wrote:
> On Fri, 30 Jan 2015, Ard Biesheuvel wrote:
> > This patch registers hvc0 as the preferred console if no console
> > has been specified explicitly on the kernel command line.
> > 
> > The purpose is to allow platform agnostic kernels and boot images
> > (such as distro installers) to boot in a Xen/ARM domU without the
> > need to modify the command line by hand.
> > 
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > 
> > This applies on top of the patch Julien is cooking up to move the
> > call to xen_guest_init() to an earlier boot stage. This needs to
> > be at least before the call to console_init() in start_kernel()
> > for this patch to do anything meaningful.
> 
> This patch looks good but there isn't much point in queuing this up
> without the prerequisite.
> 
> Julien, what patch exactly is that? What is the status of it?
> 
> 
> >  arch/arm/xen/enlighten.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> > index c7ca936ebd99..61382cafa877 100644
> > --- a/arch/arm/xen/enlighten.c
> > +++ b/arch/arm/xen/enlighten.c
> > @@ -24,6 +24,7 @@
> >  #include <linux/cpuidle.h>
> >  #include <linux/cpufreq.h>
> >  #include <linux/cpu.h>
> > +#include <linux/console.h>
> >  
> >  #include <linux/mm.h>
> >  
> > @@ -322,6 +323,9 @@ static int __init xen_guest_init(void)
> >  
> >  	register_cpu_notifier(&xen_cpu_notifier);
> >  
> > +	if (!console_set_on_cmdline)
> > +		add_preferred_console("hvc", 0, NULL);

FWIW on x86 this doesn't depend on console_set_on_cmdline, does it need
to here?

On x86 it does depend on !xen_initial_domain. I suppose on the principal
that a VT is normally available there. I suppose that doesn't apply to
ARM so much, although it could.

> > +
> >  	return 0;
> >  }
> >  early_initcall(xen_guest_init);
> > -- 
> > 1.8.3.2
> > 

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Ard Biesheuvel]

On 10 February 2015 at 15:26, Ian Campbell <ian.campbell@citrix.com> wrote:
> On Tue, 2015-02-10 at 07:00 +0000, Stefano Stabellini wrote:
>> On Fri, 30 Jan 2015, Ard Biesheuvel wrote:
>> > This patch registers hvc0 as the preferred console if no console
>> > has been specified explicitly on the kernel command line.
>> >
>> > The purpose is to allow platform agnostic kernels and boot images
>> > (such as distro installers) to boot in a Xen/ARM domU without the
>> > need to modify the command line by hand.
>> >
>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > ---
>> >
>> > This applies on top of the patch Julien is cooking up to move the
>> > call to xen_guest_init() to an earlier boot stage. This needs to
>> > be at least before the call to console_init() in start_kernel()
>> > for this patch to do anything meaningful.
>>
>> This patch looks good but there isn't much point in queuing this up
>> without the prerequisite.
>>
>> Julien, what patch exactly is that? What is the status of it?
>>
>>
>> >  arch/arm/xen/enlighten.c | 4 ++++
>> >  1 file changed, 4 insertions(+)
>> >
>> > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
>> > index c7ca936ebd99..61382cafa877 100644
>> > --- a/arch/arm/xen/enlighten.c
>> > +++ b/arch/arm/xen/enlighten.c
>> > @@ -24,6 +24,7 @@
>> >  #include <linux/cpuidle.h>
>> >  #include <linux/cpufreq.h>
>> >  #include <linux/cpu.h>
>> > +#include <linux/console.h>
>> >
>> >  #include <linux/mm.h>
>> >
>> > @@ -322,6 +323,9 @@ static int __init xen_guest_init(void)
>> >
>> >     register_cpu_notifier(&xen_cpu_notifier);
>> >
>> > +   if (!console_set_on_cmdline)
>> > +           add_preferred_console("hvc", 0, NULL);
>
> FWIW on x86 this doesn't depend on console_set_on_cmdline, does it need
> to here?
>

I didn't check the code, but it seems inappropriate to add a preferred
console implicitly if the user has set 'console=' on the command line.

> On x86 it does depend on !xen_initial_domain. I suppose on the principal
> that a VT is normally available there. I suppose that doesn't apply to
> ARM so much, although it could.
>

OK, I got confused by the xen_guest_init(). So do you mean if if
(!xen_initial_domain) should be added?

-- 
Ard.

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Ian Campbell]

On Tue, 2015-02-10 at 15:51 +0800, Ard Biesheuvel wrote:
> > FWIW on x86 this doesn't depend on console_set_on_cmdline, does it need
> > to here?
> >
> 
> I didn't check the code, but it seems inappropriate to add a preferred
> console implicitly if the user has set 'console=' on the command line.

I had been thinking that add_preferred_console would DTRT, but it seems
not. Seems strange that most calls to it do not check if the console is
already set, but it does seem like the right thing in this case.

> > On x86 it does depend on !xen_initial_domain. I suppose on the principal
> > that a VT is normally available there. I suppose that doesn't apply to
> > ARM so much, although it could.
> >
> 
> OK, I got confused by the xen_guest_init(). So do you mean if if
> (!xen_initial_domain) should be added?

(dom0 is "Just A Guest" too ;-))

Adding it would be consistent with x86, I'm not precisely sure if that
is important or desirable in this case. I'd be inclined to start with
the if there.

Ian.

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Stefano Stabellini]

On Tue, 10 Feb 2015, Ian Campbell wrote:
> On Tue, 2015-02-10 at 15:51 +0800, Ard Biesheuvel wrote:
> > > FWIW on x86 this doesn't depend on console_set_on_cmdline, does it need
> > > to here?
> > >
> > 
> > I didn't check the code, but it seems inappropriate to add a preferred
> > console implicitly if the user has set 'console=' on the command line.
> 
> I had been thinking that add_preferred_console would DTRT, but it seems
> not. Seems strange that most calls to it do not check if the console is
> already set, but it does seem like the right thing in this case.
> 
> > > On x86 it does depend on !xen_initial_domain. I suppose on the principal
> > > that a VT is normally available there. I suppose that doesn't apply to
> > > ARM so much, although it could.
> > >
> > 
> > OK, I got confused by the xen_guest_init(). So do you mean if if
> > (!xen_initial_domain) should be added?
> 
> (dom0 is "Just A Guest" too ;-))
> 
> Adding it would be consistent with x86, I'm not precisely sure if that
> is important or desirable in this case. I'd be inclined to start with
> the if there.

The reasoning is that dom0 command line arguments come from its old
native grub stanza, therefore the console parameter is incorrect, right?
As opposed to regular domUs, that being freshly installed, are supposed
to have the correct console parameter?

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Stefano Stabellini]

On Thu, 12 Feb 2015, Stefano Stabellini wrote:
> On Tue, 10 Feb 2015, Ian Campbell wrote:
> > On Tue, 2015-02-10 at 15:51 +0800, Ard Biesheuvel wrote:
> > > > FWIW on x86 this doesn't depend on console_set_on_cmdline, does it need
> > > > to here?
> > > >
> > > 
> > > I didn't check the code, but it seems inappropriate to add a preferred
> > > console implicitly if the user has set 'console=' on the command line.
> > 
> > I had been thinking that add_preferred_console would DTRT, but it seems
> > not. Seems strange that most calls to it do not check if the console is
> > already set, but it does seem like the right thing in this case.
> > 
> > > > On x86 it does depend on !xen_initial_domain. I suppose on the principal
> > > > that a VT is normally available there. I suppose that doesn't apply to
> > > > ARM so much, although it could.
> > > >
> > > 
> > > OK, I got confused by the xen_guest_init(). So do you mean if if
> > > (!xen_initial_domain) should be added?
> > 
> > (dom0 is "Just A Guest" too ;-))
> > 
> > Adding it would be consistent with x86, I'm not precisely sure if that
> > is important or desirable in this case. I'd be inclined to start with
> > the if there.
> 
> The reasoning is that dom0 command line arguments come from its old
> native grub stanza, therefore the console parameter is incorrect, right?
> As opposed to regular domUs, that being freshly installed, are supposed
> to have the correct console parameter?

I got it the other way around, but the question remains: why the
difference between dom0 and domUs? I guess that on x86 might make sense
because dom0 has access to vga there.

It is also interesting to note that the actual code is:

if (!xen_initial_domain()) {
		add_preferred_console("xenboot", 0, NULL);
		add_preferred_console("tty", 0, NULL);
		add_preferred_console("hvc", 0, NULL);

that I am guessing it would prioritize a possible graphical console if
present, like fbcon on pvfb.

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Ian Campbell]

On Thu, 2015-02-12 at 04:35 +0000, Stefano Stabellini wrote:
> On Tue, 10 Feb 2015, Ian Campbell wrote:
> > On Tue, 2015-02-10 at 15:51 +0800, Ard Biesheuvel wrote:
> > > > FWIW on x86 this doesn't depend on console_set_on_cmdline, does it need
> > > > to here?
> > > >
> > > 
> > > I didn't check the code, but it seems inappropriate to add a preferred
> > > console implicitly if the user has set 'console=' on the command line.
> > 
> > I had been thinking that add_preferred_console would DTRT, but it seems
> > not. Seems strange that most calls to it do not check if the console is
> > already set, but it does seem like the right thing in this case.
> > 
> > > > On x86 it does depend on !xen_initial_domain. I suppose on the principal
> > > > that a VT is normally available there. I suppose that doesn't apply to
> > > > ARM so much, although it could.
> > > >
> > > 
> > > OK, I got confused by the xen_guest_init(). So do you mean if if
> > > (!xen_initial_domain) should be added?
> > 
> > (dom0 is "Just A Guest" too ;-))
> > 
> > Adding it would be consistent with x86, I'm not precisely sure if that
> > is important or desirable in this case. I'd be inclined to start with
> > the if there.
> 
> The reasoning is that dom0 command line arguments come from its old
> native grub stanza, therefore the console parameter is incorrect, right?
> As opposed to regular domUs, that being freshly installed, are supposed
> to have the correct console parameter?

Other way around I think, dom0 has the correct stuff from grub.cfg
whereas the guest may not.

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Julien Grall]

On 12/02/2015 12:54, Ian Campbell wrote:
> On Thu, 2015-02-12 at 04:35 +0000, Stefano Stabellini wrote:
>> On Tue, 10 Feb 2015, Ian Campbell wrote:
>>> On Tue, 2015-02-10 at 15:51 +0800, Ard Biesheuvel wrote:
>>>>> FWIW on x86 this doesn't depend on console_set_on_cmdline, does it need
>>>>> to here?
>>>>>
>>>>
>>>> I didn't check the code, but it seems inappropriate to add a preferred
>>>> console implicitly if the user has set 'console=' on the command line.
>>>
>>> I had been thinking that add_preferred_console would DTRT, but it seems
>>> not. Seems strange that most calls to it do not check if the console is
>>> already set, but it does seem like the right thing in this case.
>>>
>>>>> On x86 it does depend on !xen_initial_domain. I suppose on the principal
>>>>> that a VT is normally available there. I suppose that doesn't apply to
>>>>> ARM so much, although it could.
>>>>>
>>>>
>>>> OK, I got confused by the xen_guest_init(). So do you mean if if
>>>> (!xen_initial_domain) should be added?
>>>
>>> (dom0 is "Just A Guest" too ;-))
>>>
>>> Adding it would be consistent with x86, I'm not precisely sure if that
>>> is important or desirable in this case. I'd be inclined to start with
>>> the if there.
>>
>> The reasoning is that dom0 command line arguments come from its old
>> native grub stanza, therefore the console parameter is incorrect, right?
>> As opposed to regular domUs, that being freshly installed, are supposed
>> to have the correct console parameter?
>
> Other way around I think, dom0 has the correct stuff from grub.cfg
> whereas the guest may not.

Yes, currently for ARM guest you have to add 'extra="console=hvc0"' in 
the configuration file.

Regards,

-- 
Julien Grall

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Stefano Stabellini]

On Thu, 12 Feb 2015, Julien Grall wrote:
> On 12/02/2015 12:54, Ian Campbell wrote:
> > On Thu, 2015-02-12 at 04:35 +0000, Stefano Stabellini wrote:
> > > On Tue, 10 Feb 2015, Ian Campbell wrote:
> > > > On Tue, 2015-02-10 at 15:51 +0800, Ard Biesheuvel wrote:
> > > > > > FWIW on x86 this doesn't depend on console_set_on_cmdline, does it
> > > > > > need
> > > > > > to here?
> > > > > > 
> > > > > 
> > > > > I didn't check the code, but it seems inappropriate to add a preferred
> > > > > console implicitly if the user has set 'console=' on the command line.
> > > > 
> > > > I had been thinking that add_preferred_console would DTRT, but it seems
> > > > not. Seems strange that most calls to it do not check if the console is
> > > > already set, but it does seem like the right thing in this case.
> > > > 
> > > > > > On x86 it does depend on !xen_initial_domain. I suppose on the
> > > > > > principal
> > > > > > that a VT is normally available there. I suppose that doesn't apply
> > > > > > to
> > > > > > ARM so much, although it could.
> > > > > > 
> > > > > 
> > > > > OK, I got confused by the xen_guest_init(). So do you mean if if
> > > > > (!xen_initial_domain) should be added?
> > > > 
> > > > (dom0 is "Just A Guest" too ;-))
> > > > 
> > > > Adding it would be consistent with x86, I'm not precisely sure if that
> > > > is important or desirable in this case. I'd be inclined to start with
> > > > the if there.
> > > 
> > > The reasoning is that dom0 command line arguments come from its old
> > > native grub stanza, therefore the console parameter is incorrect, right?
> > > As opposed to regular domUs, that being freshly installed, are supposed
> > > to have the correct console parameter?
> > 
> > Other way around I think, dom0 has the correct stuff from grub.cfg
> > whereas the guest may not.
> 
> Yes, currently for ARM guest you have to add 'extra="console=hvc0"' in the
> configuration file.

I see. If this is the concern that we are trying to address, then yes,
doing the same that we are already doing on x86 might be best.

[PATCH] xen/arm: allow console=hvc0 to be omitted for guests

[Julien Grall]

Hi Stefano,

On 10/02/2015 15:00, Stefano Stabellini wrote:
> On Fri, 30 Jan 2015, Ard Biesheuvel wrote:
>> This patch registers hvc0 as the preferred console if no console
>> has been specified explicitly on the kernel command line.
>>
>> The purpose is to allow platform agnostic kernels and boot images
>> (such as distro installers) to boot in a Xen/ARM domU without the
>> need to modify the command line by hand.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>
>> This applies on top of the patch Julien is cooking up to move the
>> call to xen_guest_init() to an earlier boot stage. This needs to
>> be at least before the call to console_init() in start_kernel()
>> for this patch to do anything meaningful.
>
> This patch looks good but there isn't much point in queuing this up
> without the prerequisite.
>
> Julien, what patch exactly is that? What is the status of it?

Calling xen_guest_init from setup_arch. It's based on a patch you sent 
an year ago (http://osdir.com/ml/linux-kernel/2013-08/msg00675.html)

But I had some issue with the console. The splitting of xen_guest_init 
doesn't work. It seems that we decode the IRQ too early.

I haven't had time to rework it and sent it.

Regards,

-- 
Julien Grall