Jeremy Katz
2006-Sep-02 19:59 UTC
[Xen-devel] [PATCH] Paravirt framebuffer console cleanups [5/5]
If we''re not set up to use a graphical console for the guest, then we need to ensure that the xvc console ends up as the primary console. This also should make it so that we can lose the console_use_vt bits. Changes from last time include adding the same bits for x86_64 and ia64 Signed-off-by: Jeremy Katz <katzj@redhat.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Steven Smith
2006-Sep-04 09:02 UTC
Re: [Xen-devel] [PATCH] Paravirt framebuffer console cleanups [5/5]
> If we''re not set up to use a graphical console for the guest, then we > need to ensure that the xvc console ends up as the primary console. > This also should make it so that we can lose the console_use_vt bits. > > Changes from last time include adding the same bits for x86_64 and ia64 > > Signed-off-by: Jeremy Katz <katzj@redhat.com>> diff -r c859588adc5e -r 72582b968445 linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c > --- a/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c Sat Sep 02 15:31:29 2006 -0400 > +++ b/linux-2.6-xen-sparse/arch/i386/kernel/setup-xen.c Sat Sep 02 15:32:38 2006 -0400 > @@ -1868,9 +1868,6 @@ void __init setup_arch(char **cmdline_p) > } else { > #if defined(CONFIG_VT) && defined(CONFIG_DUMMY_CONSOLE) > conswitchp = &dummy_con; > -#else > - extern int console_use_vt; > - console_use_vt = 0; > #endif > } > } > diff -r c859588adc5e -r 72582b968445 linux-2.6-xen-sparse/arch/ia64/kernel/setup.c > --- a/linux-2.6-xen-sparse/arch/ia64/kernel/setup.c Sat Sep 02 15:31:29 2006 -0400 > +++ b/linux-2.6-xen-sparse/arch/ia64/kernel/setup.c Sat Sep 02 15:32:38 2006 -0400 > @@ -546,9 +546,9 @@ setup_arch (char **cmdline_p) > xen_start_info->nr_pages, xen_start_info->flags); > > if (!is_initial_xendomain()) { > - extern int console_use_vt; > +#if !defined(CONFIG_VT) || !defined(CONFIG_DUMMY_CONSOLE) > conswitchp = NULL; > - console_use_vt = 0; > +#endif > } > } > #endif > diff -r c859588adc5e -r 72582b968445 linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c > --- a/linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c Sat Sep 02 15:31:29 2006 -0400 > +++ b/linux-2.6-xen-sparse/arch/x86_64/kernel/setup-xen.c Sat Sep 02 15:32:38 2006 -0400 > @@ -987,9 +987,10 @@ void __init setup_arch(char **cmdline_p) > #endif > #endif > } else { > - extern int console_use_vt; > - console_use_vt = 0; > - } > +#if defined(CONFIG_VT) && defined(CONFIG_DUMMY_CONSOLE) > + conswitchp = &dummy_con; > +#endif > + } > } > #else /* CONFIG_XEN */ > > diff -r c859588adc5e -r 72582b968445 linux-2.6-xen-sparse/drivers/char/tty_io.c > --- a/linux-2.6-xen-sparse/drivers/char/tty_io.c Sat Sep 02 15:31:29 2006 -0400 > +++ b/linux-2.6-xen-sparse/drivers/char/tty_io.c Sat Sep 02 15:32:38 2006 -0400It looks to me like this patch makes tty_io.c identical to the upstream version. Is this correct?> @@ -132,8 +132,6 @@ LIST_HEAD(tty_drivers); /* linked list > vt.c for deeply disgusting hack reasons */ > DECLARE_MUTEX(tty_sem); > > -int console_use_vt = 1; > - > #ifdef CONFIG_UNIX98_PTYS > extern struct tty_driver *ptm_driver; /* Unix98 pty masters; for /dev/ptmx */ > extern int pty_limit; /* Config limit on Unix98 ptys */ > @@ -2056,7 +2054,7 @@ retry_open: > goto got_driver; > } > #ifdef CONFIG_VT > - if (console_use_vt && (device == MKDEV(TTY_MAJOR,0))) { > + if (device == MKDEV(TTY_MAJOR,0)) { > extern struct tty_driver *console_driver; > driver = console_driver; > index = fg_console; > @@ -3243,8 +3241,6 @@ static int __init tty_init(void) > #endif > > #ifdef CONFIG_VT > - if (!console_use_vt) > - goto out_vt;You can also get rid of the out_vt label.> cdev_init(&vc0_cdev, &console_fops); > if (cdev_add(&vc0_cdev, MKDEV(TTY_MAJOR, 0), 1) || > register_chrdev_region(MKDEV(TTY_MAJOR, 0), 1, "/dev/vc/0") < 0) > diff -r c859588adc5e -r 72582b968445 linux-2.6-xen-sparse/drivers/xen/console/console.c > --- a/linux-2.6-xen-sparse/drivers/xen/console/console.c Sat Sep 02 15:31:29 2006 -0400 > +++ b/linux-2.6-xen-sparse/drivers/xen/console/console.c Sat Sep 02 15:32:38 2006 -0400 > @@ -663,6 +663,20 @@ static int __init xencons_init(void) > printk("Xen virtual console successfully installed as %s%d\n", > DRV(xencons_driver)->name, xc_num); > > + /* Don''t need to check about graphical fb for domain 0 */ > + if (is_initial_xendomain()) > + return 0; > + > + rc = 0; > + if (xenbus_scanf(XBT_NIL, "console", "use_graphics", "%d", &rc) < 0) > + printk(KERN_ERR "Unable to read console/use_graphics\n"); > + if (rc == 0) { > + /* FIXME: this is ugly */ > + unregister_console(&kcons_info); > + kcons_info.flags |= CON_CONSDEV; > + register_console(&kcons_info); > + } > +Could you pull the setting of CON_CONSDEV into xen_console_init and avoid this weirdness? This whole passage is a little peculiar, in that you''re only going to do anything if use_graphics is disabled. I''d expect that just leaving the current behaviour alone would be the correct thing to do in that case. Also, why do you need to set CON_CONSDEV at all?> return 0; > } >Steven. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Katz
2006-Sep-05 16:11 UTC
Re: [Xen-devel] [PATCH] Paravirt framebuffer console cleanups [5/5]
On Mon, 2006-09-04 at 10:02 +0100, Steven Smith wrote:> > diff -r c859588adc5e -r 72582b968445 linux-2.6-xen-sparse/drivers/char/tty_io.c > > --- a/linux-2.6-xen-sparse/drivers/char/tty_io.c Sat Sep 02 15:31:29 2006 -0400 > > +++ b/linux-2.6-xen-sparse/drivers/char/tty_io.c Sat Sep 02 15:32:38 2006 -0400 > > It looks to me like this patch makes tty_io.c identical to the > upstream version. Is this correct?Yep -- so actually could be removed from the sparse tree. I''m usually working in a non-sparse tree, so the diff is more relevant.> > diff -r c859588adc5e -r 72582b968445 linux-2.6-xen-sparse/drivers/xen/console/console.c > > --- a/linux-2.6-xen-sparse/drivers/xen/console/console.c Sat Sep 02 15:31:29 2006 -0400 > > +++ b/linux-2.6-xen-sparse/drivers/xen/console/console.c Sat Sep 02 15:32:38 2006 -0400 > > @@ -663,6 +663,20 @@ static int __init xencons_init(void) > > printk("Xen virtual console successfully installed as %s%d\n", > > DRV(xencons_driver)->name, xc_num); > > > > + /* Don''t need to check about graphical fb for domain 0 */ > > + if (is_initial_xendomain()) > > + return 0; > > + > > + rc = 0; > > + if (xenbus_scanf(XBT_NIL, "console", "use_graphics", "%d", &rc) < 0) > > + printk(KERN_ERR "Unable to read console/use_graphics\n"); > > + if (rc == 0) { > > + /* FIXME: this is ugly */ > > + unregister_console(&kcons_info); > > + kcons_info.flags |= CON_CONSDEV; > > + register_console(&kcons_info); > > + } > > + > Could you pull the setting of CON_CONSDEV into xen_console_init and > avoid this weirdness?Nope -- when consoles are registered, we don''t have enough infrastructure to check xenbus to see if console/use_graphics is set or not> This whole passage is a little peculiar, in that you''re only going to > do anything if use_graphics is disabled. I''d expect that just leaving > the current behaviour alone would be the correct thing to do in that > case. > > Also, why do you need to set CON_CONSDEV at all?Otherwise, you don''t get kernel console messages. Realistically, I think the better longer-term answer is to create an early xenconsole -- then, when we do this later initialization, we can deregister the early console and do the right registration for the "real" console. This mirrors pretty closely what''s done on a few other arches Jeremy _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Steven Smith
2006-Sep-06 09:18 UTC
Re: [Xen-devel] [PATCH] Paravirt framebuffer console cleanups [5/5]
> On Mon, 2006-09-04 at 10:02 +0100, Steven Smith wrote: > > > diff -r c859588adc5e -r 72582b968445 linux-2.6-xen-sparse/drivers/char/tty_io.c > > > --- a/linux-2.6-xen-sparse/drivers/char/tty_io.c Sat Sep 02 15:31:29 2006 -0400 > > > +++ b/linux-2.6-xen-sparse/drivers/char/tty_io.c Sat Sep 02 15:32:38 2006 -0400 > > > > It looks to me like this patch makes tty_io.c identical to the > > upstream version. Is this correct? > Yep -- so actually could be removed from the sparse tree. I''m usually > working in a non-sparse tree, so the diff is more relevant.Ack.> > > diff -r c859588adc5e -r 72582b968445 linux-2.6-xen-sparse/drivers/xen/console/console.c > > > --- a/linux-2.6-xen-sparse/drivers/xen/console/console.c Sat Sep 02 15:31:29 2006 -0400 > > > +++ b/linux-2.6-xen-sparse/drivers/xen/console/console.c Sat Sep 02 15:32:38 2006 -0400 > > > @@ -663,6 +663,20 @@ static int __init xencons_init(void) > > > printk("Xen virtual console successfully installed as %s%d\n", > > > DRV(xencons_driver)->name, xc_num); > > > > > > + /* Don''t need to check about graphical fb for domain 0 */ > > > + if (is_initial_xendomain()) > > > + return 0; > > > + > > > + rc = 0; > > > + if (xenbus_scanf(XBT_NIL, "console", "use_graphics", "%d", &rc) < 0) > > > + printk(KERN_ERR "Unable to read console/use_graphics\n"); > > > + if (rc == 0) { > > > + /* FIXME: this is ugly */ > > > + unregister_console(&kcons_info); > > > + kcons_info.flags |= CON_CONSDEV; > > > + register_console(&kcons_info); > > > + } > > > + > > Could you pull the setting of CON_CONSDEV into xen_console_init and > > avoid this weirdness? > Nope -- when consoles are registered, we don''t have enough > infrastructure to check xenbus to see if console/use_graphics is set or > notThat''s a pity.> > This whole passage is a little peculiar, in that you''re only going to > > do anything if use_graphics is disabled. I''d expect that just leaving > > the current behaviour alone would be the correct thing to do in that > > case. > > > > Also, why do you need to set CON_CONSDEV at all? > > Otherwise, you don''t get kernel console messages.Why not? None of the other console drivers seem to need to set it explicitly, and it seems like it would break setting console=blah on the kernel command line. It also strikes me as very odd that you need to do this iff use_graphics is switched off.> Realistically, I think the better longer-term answer is to create an > early xenconsole -- then, when we do this later initialization, we can > deregister the early console and do the right registration for the > "real" console. This mirrors pretty closely what''s done on a few other > archesAgreed, but not in this patch series. Steven. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jeremy Katz
2006-Sep-06 14:14 UTC
Re: [Xen-devel] [PATCH] Paravirt framebuffer console cleanups [5/5]
On Wed, 2006-09-06 at 10:18 +0100, Steven Smith wrote:> > On Mon, 2006-09-04 at 10:02 +0100, Steven Smith wrote: > > > Could you pull the setting of CON_CONSDEV into xen_console_init and > > > avoid this weirdness? > > Nope -- when consoles are registered, we don''t have enough > > infrastructure to check xenbus to see if console/use_graphics is set or > > not > That''s a pity.That''s what I thought as well.> > > This whole passage is a little peculiar, in that you''re only going to > > > do anything if use_graphics is disabled. I''d expect that just leaving > > > the current behaviour alone would be the correct thing to do in that > > > case. > > > > > > Also, why do you need to set CON_CONSDEV at all? > > > > Otherwise, you don''t get kernel console messages. > Why not? None of the other console drivers seem to need to set it > explicitly, and it seems like it would break setting console=blah on > the kernel command line.Nothing else sets it explicitly because it happens implicitly in the console registration code for the first thing registered with register_console(). Alternately, if you have a console=, then register_console ensures that your console device has CON_CONSDEV set. kernel/printk.c if you want to read all the gory details> > Realistically, I think the better longer-term answer is to create an > > early xenconsole -- then, when we do this later initialization, we can > > deregister the early console and do the right registration for the > > "real" console. This mirrors pretty closely what''s done on a few other > > arches > Agreed, but not in this patch series.Yep -- one step at a time. And I''ve got it on my todo list for after the xenfb stuff is merged Jeremy _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Steven Smith
2006-Sep-06 21:27 UTC
Re: [Xen-devel] [PATCH] Paravirt framebuffer console cleanups [5/5]
> > > > This whole passage is a little peculiar, in that you''re only going to > > > > do anything if use_graphics is disabled. I''d expect that just leaving > > > > the current behaviour alone would be the correct thing to do in that > > > > case. > > > > > > > > Also, why do you need to set CON_CONSDEV at all? > > > Otherwise, you don''t get kernel console messages. > > Why not? None of the other console drivers seem to need to set it > > explicitly, and it seems like it would break setting console=blah on > > the kernel command line. > Nothing else sets it explicitly because it happens implicitly in the > console registration code for the first thing registered with > register_console(). Alternately, if you have a console=, then > register_console ensures that your console device has CON_CONSDEV set. > kernel/printk.c if you want to read all the gory detailsThe intent of this code seems to be to make sure that CON_CONSDEV is definitely set on xenconsole if use_graphics is not set, yes? But as far as I can see, if use_graphics is not set, xenfb should never call register_console(), and so this is all redundant. What am I missing? Steven. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel