[PATCH] early console registration

[PATCH] early console registration

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

Here's another patch that just sets the bit corresponding to the boot CPU in 
the cpu_online_map if we successfully register an early console.  The 
early_console_setup routine is simple, and I'd expect additions of the form:

...
#ifdef CONFIG_SGI_L1_SERIAL_CONSOLE
	{
		extern int sn_serial_console_early_setup(void);
		if (!sn_serial_console_early_setup())
			return 0;
	}
#endif
...
#ifdef CONFIG_FOO_CONSOLE
	{
		extern int foo_early_setup(void);
		if (!foo_early_setup())
			return 0;
	}
#endif
...
etc.

Is this reasonable?  I've tested it on a couple of machines here and it works 
well.  It's really useful for debugging setup_arch problems.

Thanks,
Jesse


[-- Attachment #2: early-printk-sn-3.patch --]
[-- Type: text/plain, Size: 3734 bytes --]

===== arch/ia64/kernel/setup.c 1.70 vs edited =====
--- 1.70/arch/ia64/kernel/setup.c	Wed Mar 17 04:46:59 2004
+++ edited/arch/ia64/kernel/setup.c	Fri May 14 14:11:29 2004
@@ -280,6 +280,28 @@
 }
 #endif
 
+/**
+ * early_console_setup - setup debugging console
+ *
+ * Consoles started here require little enough setup that we can start using
+ * them very early in the boot process, either right after the machine
+ * vector initialization, or even before if the drivers can detect their hw.
+ *
+ * Returns non-zero if a console couldn't be setup.
+ */
+static int __init early_console_setup(void)
+{
+#ifdef CONFIG_SGI_L1_SERIAL_CONSOLE
+	{
+		extern int sn_serial_console_early_setup(void);
+		if(!sn_serial_console_early_setup())
+			return 0;
+	}
+#endif
+
+	return -1;
+}
+
 void __init
 setup_arch (char **cmdline_p)
 {
@@ -296,6 +318,10 @@
 #ifdef CONFIG_IA64_GENERIC
 	machvec_init(acpi_get_sysname());
 #endif
+
+	/* If we register an early console, allow CPU 0 to printk */
+	if (!early_console_setup())
+		cpu_set(smp_processor_id(), cpu_online_map);
 
 #ifdef CONFIG_ACPI_BOOT
 	/* Initialize the ACPI boot-time table parser */
===== drivers/char/sn_serial.c 1.11 vs edited =====
--- 1.11/drivers/char/sn_serial.c	Mon Mar 29 02:48:14 2004
+++ edited/drivers/char/sn_serial.c	Fri May 14 14:00:05 2004
@@ -229,24 +229,6 @@
 	return ia64_sn_console_intr_status() & SAL_CONSOLE_INTR_RECV;
 }
 
-/* The early printk (possible setup) and function call */
-
-void
-early_printk_sn_sal(const char *s, unsigned count)
-{
-	extern void early_sn_setup(void);
-
-	if (!sn_func) {
-		if (IS_RUNNING_ON_SIMULATOR())
-			sn_func = &sim_ops;
-		else
-			sn_func = &poll_ops;
-
-		early_sn_setup();
-	}
-	sn_func->sal_puts(s, count);
-}
-
 #ifdef DEBUG
 /* this is as "close to the metal" as we can get, used when the driver
  * itself may be broken */
@@ -259,7 +241,15 @@
 
 	va_start(args, fmt);
 	printed_len = vscnprintf(printk_buf, sizeof(printk_buf), fmt, args);
-	early_printk_sn_sal(printk_buf, printed_len);
+	if (!sn_func) {
+		if (IS_RUNNING_ON_SIMULATOR())
+			sn_func = &sim_ops;
+		else
+			sn_func = &poll_ops;
+
+		early_sn_setup();
+	}
+	sn_func->sal_puts(printk_buf, printed_len);
 	va_end(args);
 	return printed_len;
 }
@@ -941,6 +931,45 @@
  */
 
 #ifdef CONFIG_SGI_L1_SERIAL_CONSOLE
+
+/*
+ * Very simple early output routine.  Assumes sn_func operation structure
+ * has arlready been setup by sn_serial_console_early_setup().
+ */
+static void __init
+sn_sal_console_write_early(struct console *co, const char *s, unsigned count)
+{
+	sn_func->sal_puts(s, count);
+}
+
+static struct console sal_console_early __initdata = {
+	.name = "sn_sal",
+	.write = sn_sal_console_write_early,
+	.flags = CON_PRINTBUFFER,
+	.index  = -1,
+};
+
+/*
+ * Register a console early on...
+ */
+int __init
+sn_serial_console_early_setup(void)
+{
+	if (!ia64_platform_is("sn2"))
+		return -1;
+
+	if (IS_RUNNING_ON_SIMULATOR())
+		sn_func = &sim_ops;
+	else
+		sn_func = &poll_ops;
+
+	early_sn_setup(); /* Find SAL entry points */
+
+	register_console(&sal_console_early);
+
+	return 0;
+}
+
 /*
  * Print a string to the SAL console.  The console_lock must be held
  * when we get here.
@@ -998,18 +1027,10 @@
 	return sn_sal_driver;
 }
 
-static int __init
-sn_sal_console_setup(struct console *co, char *options)
-{
-	return 0;
-}
-
-
 static struct console sal_console = {
 	.name = "ttyS",
 	.write = sn_sal_console_write,
 	.device = sn_sal_console_device,
-	.setup = sn_sal_console_setup,
 	.index = -1
 };
 
@@ -1020,6 +1041,7 @@
 		sn_sal_switch_to_asynch();
 		DPRINTF("sn_sal_serial_console_init : register console\n");
 		register_console(&sal_console);
+		unregister_console(&sal_console_early);
 	}
 	return 0;
 }

Re: [PATCH] early console registration

[David Mosberger]

>>>>> On Fri, 14 May 2004 14:25:59 -0700, Jesse Barnes <jbarnes@engr.sgi.com> said:

  Jesse> Is this reasonable?  I've tested it on a couple of machines
  Jesse> here and it works well.  It's really useful for debugging
  Jesse> setup_arch problems.

I don't like the #ifdef's very much, but I suppose I can live with
them in this case.  IIRC, Bjorn also has been working on some
early-printk changes so I'd be interested in his opinion.

	--david

Re: [PATCH] early console registration

[Jesse Barnes]

On Wednesday, May 19, 2004 9:17 pm, David Mosberger wrote:
> >>>>> On Fri, 14 May 2004 14:25:59 -0700, Jesse Barnes
> >>>>> <jbarnes@engr.sgi.com> said:
>
>   Jesse> Is this reasonable?  I've tested it on a couple of machines
>   Jesse> here and it works well.  It's really useful for debugging
>   Jesse> setup_arch problems.
>
> I don't like the #ifdef's very much, but I suppose I can live with
> them in this case.  IIRC, Bjorn also has been working on some
> early-printk changes so I'd be interested in his opinion.

Yeah, me neither, but I like the alternative even less--ia64_platform_is 
and .weak symbols or something.  I *think* this approach will work for Bjorn 
too (and any other driver that's ok with being called early), but it would be 
nice to hear it. :)

Thanks,
Jesse

Re: [PATCH] early console registration

[Bjorn Helgaas]

On Thursday 20 May 2004 7:03 am, Jesse Barnes wrote:
> On Wednesday, May 19, 2004 9:17 pm, David Mosberger wrote:
> > >>>>> On Fri, 14 May 2004 14:25:59 -0700, Jesse Barnes
> > >>>>> <jbarnes@engr.sgi.com> said:
> >
> >   Jesse> Is this reasonable?  I've tested it on a couple of machines
> >   Jesse> here and it works well.  It's really useful for debugging
> >   Jesse> setup_arch problems.
> >
> > I don't like the #ifdef's very much, but I suppose I can live with
> > them in this case.  IIRC, Bjorn also has been working on some
> > early-printk changes so I'd be interested in his opinion.
> 
> Yeah, me neither, but I like the alternative even less--ia64_platform_is 
> and .weak symbols or something.  I *think* this approach will work for Bjorn 
> too (and any other driver that's ok with being called early), but it would be 
> nice to hear it. :)

Well, you could always implement a PCDP (primary console & debug
port device table -- an extension of HCDP).  That would remove the
need for the ia64_platform_is() in your driver and move the #ifdefs
into the pcdp setup routine.  That wouldn't solve the problem for
legacy firmware, though, so what you've got is probably the best
we can do for now.

The interesting bit to me is setting CPU 0 online early.  We've
already set up I/O port space and the machine vector, so that
should be OK.  We haven't done cpu_init() yet, so the per-CPU
data mapping isn't set up yet.  I guess it's probably reasonable
to prohibit early console drivers from using per-CPU data.

Bjorn

Re: [PATCH] early console registration

[Jesse Barnes]

On Thursday, May 20, 2004 11:18 am, Bjorn Helgaas wrote:
> The interesting bit to me is setting CPU 0 online early.  We've
> already set up I/O port space and the machine vector, so that
> should be OK.  We haven't done cpu_init() yet, so the per-CPU
> data mapping isn't set up yet.  I guess it's probably reasonable
> to prohibit early console drivers from using per-CPU data.

Yeah, that's a good point.  In the comments above the early console setup 
function we should outline the requirements and restrictions for a console 
registered there.  So far we've got:
  o cannot allocate memory
  o cannot use per-cpu data
  o cannot use interrupts

What else?

Jesse

[PATCH] early console registration

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 482 bytes --]

Here's an updated early console registration patch, how does it look?  There's 
another piece to update the sn2 console driver with support for it, but I can 
send that straight to Andrew.  I think Bjorn has a similar patch for the zx1 
console.

Allow consoles to be registered early if they support doing output very early 
in the boot process.  Consoles that allow this are subject to the limitations 
outlined in the comments for the early_console_setup routine.

Thanks,
Jesse

[-- Attachment #2: early-printk-2.patch --]
[-- Type: text/plain, Size: 1479 bytes --]

===== arch/ia64/kernel/setup.c 1.71 vs edited =====
--- 1.71/arch/ia64/kernel/setup.c	Fri May 14 19:00:11 2004
+++ edited/arch/ia64/kernel/setup.c	Thu Jun  3 14:39:40 2004
@@ -280,6 +280,36 @@
 }
 #endif
 
+/**
+ * early_console_setup - setup debugging console
+ *
+ * Consoles started here require little enough setup that we can start using
+ * them very early in the boot process, either right after the machine
+ * vector initialization, or even before if the drivers can detect their hw.
+ *
+ * Since these drivers are initialized and called very early on in the boot
+ * process, some services are unavailable until the system is up:
+ *   o memory allocation
+ *   o per-cpu data
+ *   o interrupts
+ * The driver is free to have an initcall that switches the console ops
+ * though, as is done in the sn2 console driver.
+ *
+ * Returns non-zero if a console couldn't be setup.
+ */
+static int __init early_console_setup(void)
+{
+#ifdef CONFIG_SGI_L1_SERIAL_CONSOLE
+	{
+		extern int sn_serial_console_early_setup(void);
+		if(!sn_serial_console_early_setup())
+			return 0;
+	}
+#endif
+
+	return -1;
+}
+
 void __init
 setup_arch (char **cmdline_p)
 {
@@ -296,6 +326,10 @@
 #ifdef CONFIG_IA64_GENERIC
 	machvec_init(acpi_get_sysname());
 #endif
+
+	/* If we register an early console, allow CPU 0 to printk */
+	if (!early_console_setup())
+		cpu_set(smp_processor_id(), cpu_online_map);
 
 #ifdef CONFIG_ACPI_BOOT
 	/* Initialize the ACPI boot-time table parser */

Re: [PATCH] early console registration

[Bjorn Helgaas]

On Thursday 03 June 2004 3:49 pm, Jesse Barnes wrote:
> Allow consoles to be registered early if they support doing output very early 
> in the boot process.  Consoles that allow this are subject to the limitations 
> outlined in the comments for the early_console_setup routine.

(See original mail with patch here:
    http://www.gelato.unsw.edu.au/linux-ia64/0406/9894.html)

I'm still a little uneasy about setting cpu_online for CPU 0 so early.
What if somebody comes along in the future and assumes the original
semantics for cpu_online(0)?

As another possibility, the following is a little more intrusive to
printk.c, but also a little more explicit about what we're doing
(untested).

=== drivers/char/sn_serial.c 1.14 vs edited ==--- 1.14/drivers/char/sn_serial.c	2004-05-11 09:25:13 -06:00
+++ edited/drivers/char/sn_serial.c	2004-06-14 16:24:05 -06:00
@@ -1007,6 +1007,7 @@
 
 static struct console sal_console = {
 	.name = "ttyS",
+	.flags = CON_EARLY,
 	.write = sn_sal_console_write,
 	.device = sn_sal_console_device,
 	.setup = sn_sal_console_setup,
=== include/linux/console.h 1.13 vs edited ==--- 1.13/include/linux/console.h	2004-05-14 20:00:12 -06:00
+++ edited/include/linux/console.h	2004-06-14 12:42:16 -06:00
@@ -80,6 +80,7 @@
 #define CON_PRINTBUFFER	(1)
 #define CON_CONSDEV	(2) /* Last on the command line */
 #define CON_ENABLED	(4)
+#define CON_EARLY	(8) /* Works before CPU is online */
 
 struct console
 {
=== kernel/printk.c 1.38 vs edited ==--- 1.38/kernel/printk.c	2004-05-25 03:53:03 -06:00
+++ edited/kernel/printk.c	2004-06-14 16:12:17 -06:00
@@ -113,6 +113,9 @@
 /* Flag: console code may call schedule() */
 static int console_may_schedule;
 
+/* Early consoles work before CPU is online */
+static int early_console_registered;
+
 /*
  *	Setup a list of consoles. Called from init/main.c
  */
@@ -384,7 +387,8 @@
 	struct console *con;
 
 	for (con = console_drivers; con; con = con->next) {
-		if ((con->flags & CON_ENABLED) && con->write)
+		if ((con->flags & CON_ENABLED) && con->write &&
+		    (cpu_online(smp_processor_id()) || con->flags & CON_EARLY))
 			con->write(con, &LOG_BUF(start), end - start);
 	}
 }
@@ -544,7 +548,8 @@
 	}
 
 	if (!cpu_online(smp_processor_id()) &&
-	    system_state != SYSTEM_RUNNING) {
+	    system_state != SYSTEM_RUNNING &&
+	    !early_console_registered) {
 		/*
 		 * Some console drivers may assume that per-cpu resources have
 		 * been allocated.  So don't allow them to be called by this
@@ -733,6 +738,8 @@
 
 	if (!(console->flags & CON_ENABLED))
 		return;
+
+	early_console_registered |= console->flags & CON_EARLY;
 
 	/*
 	 *	Put this console in the list - keep the

Re: [PATCH] early console registration

[Jesse Barnes]

On Monday, June 14, 2004 6:27 pm, Bjorn Helgaas wrote:
> On Thursday 03 June 2004 3:49 pm, Jesse Barnes wrote:
> > Allow consoles to be registered early if they support doing output very
> > early in the boot process.  Consoles that allow this are subject to the
> > limitations outlined in the comments for the early_console_setup routine.
>
> (See original mail with patch here:
>     http://www.gelato.unsw.edu.au/linux-ia64/0406/9894.html)
>
> I'm still a little uneasy about setting cpu_online for CPU 0 so early.
> What if somebody comes along in the future and assumes the original
> semantics for cpu_online(0)?
>
> As another possibility, the following is a little more intrusive to
> printk.c, but also a little more explicit about what we're doing
> (untested).

Yeah, that's fine.  In fact, I've tested something similar and sent it to 
Andrew, but he came back and asked why not just set CPU 0 online early?  And 
given that CPU 0 is 'online' as soon as the kernel starts executing head.S, 
it seems like it *should* keep working, and we should probably keep it that 
way.

But either way is fine with me, so long as it gets upstream somehow.

Jesse

Re: [PATCH] early console registration

[Bjorn Helgaas]

On Tuesday 15 June 2004 7:12 am, Jesse Barnes wrote:
> Yeah, that's fine.  In fact, I've tested something similar and sent it to 
> Andrew, but he came back and asked why not just set CPU 0 online early?  And 
> given that CPU 0 is 'online' as soon as the kernel starts executing head.S, 
> it seems like it *should* keep working, and we should probably keep it that 
> way.

It's *not* online as soon as it starts head.S, though -- that's what I
meant about preserving the original cpu_online semantics.  It currently
means that at least cpu_init() has been called, so memory allocation,
per-cpu data, and interrupts should work.

If we mark CPU 0 as 'online' earlier, some hypothetical new code could
test cpu_online(0) and incorrectly assume per-cpu data works.

But AFAIK, there is no such code now, and I don't have any better ideas,
so maybe we should just go with your ia64-specific patch for now.

Re: [PATCH] early console registration

[Jesse Barnes]

On Wednesday, June 16, 2004 4:22 pm, Bjorn Helgaas wrote:
> On Tuesday 15 June 2004 7:12 am, Jesse Barnes wrote:
> > Yeah, that's fine.  In fact, I've tested something similar and sent it to
> > Andrew, but he came back and asked why not just set CPU 0 online early? 
> > And given that CPU 0 is 'online' as soon as the kernel starts executing
> > head.S, it seems like it *should* keep working, and we should probably
> > keep it that way.
>
> It's *not* online as soon as it starts head.S, though -- that's what I
> meant about preserving the original cpu_online semantics.  It currently
> means that at least cpu_init() has been called, so memory allocation,
> per-cpu data, and interrupts should work.

Well, online in the sense that it can execute instructions, but yes, other 
limitations apply, I tried to spell them out in the comments for the 
function.

> If we mark CPU 0 as 'online' earlier, some hypothetical new code could
> test cpu_online(0) and incorrectly assume per-cpu data works.

Yeah, that's possible, but the window is pretty small.

> But AFAIK, there is no such code now, and I don't have any better ideas,
> so maybe we should just go with your ia64-specific patch for now.

Sounds good to me.

Jesse

[PATCH] early console registration

[Jesse Barnes]

[-- Attachment #1: Type: text/plain, Size: 1113 bytes --]

> > But AFAIK, there is no such code now, and I don't have any better ideas,
> > so maybe we should just go with your ia64-specific patch for now.
>
> Sounds good to me.

Bjorn is ok with it, so here it is again.  Pat, can you fix up the #ifdef 
check for your new driver when it gets in?  You should be able to reuse the 
function name...

Allow consoles to register early if they're capable of early output.  If one 
of them registers successfully, set CPU 0 online so that printk() will 
actually call the console drivers.

Drivers that are capable of early output can add themselves to the 
early_console_setup routine in the following form:

...
#ifdef CONFIG_SGI_L1_SERIAL_CONSOLE
        {
                extern int sn_serial_console_early_setup(void);
                if (!sn_serial_console_early_setup())
                        return 0;
        }
#endif
...
#ifdef CONFIG_FOO_CONSOLE
        {
                extern int foo_early_setup(void);
                if (!foo_early_setup())
                        return 0;
        }
#endif
...

Signed-off-by: Jesse Barnes <jbarnes@sgi.com>

Thanks,
Jesse

[-- Attachment #2: early-console-setup.patch --]
[-- Type: text/x-diff, Size: 1146 bytes --]

===== arch/ia64/kernel/setup.c 1.74 vs edited =====
--- 1.74/arch/ia64/kernel/setup.c	2004-06-05 01:15:10 -04:00
+++ edited/arch/ia64/kernel/setup.c	2004-06-22 13:51:27 -04:00
@@ -280,6 +280,28 @@
 }
 #endif
 
+/**
+ * early_console_setup - setup debugging console
+ *
+ * Consoles started here require little enough setup that we can start using
+ * them very early in the boot process, either right after the machine
+ * vector initialization, or even before if the drivers can detect their hw.
+ *
+ * Returns non-zero if a console couldn't be setup.
+ */
+static int __init early_console_setup(void)
+{
+#ifdef CONFIG_SGI_L1_SERIAL_CONSOLE
+	{
+		extern int sn_serial_console_early_setup(void);
+		if(!sn_serial_console_early_setup())
+			return 0;
+	}
+#endif
+
+	return -1;
+}
+
 void __init
 setup_arch (char **cmdline_p)
 {
@@ -296,6 +318,10 @@
 #ifdef CONFIG_IA64_GENERIC
 	machvec_init(acpi_get_sysname());
 #endif
+
+	/* If we register an early console, allow CPU 0 to printk */
+	if (!early_console_setup())
+		cpu_set(smp_processor_id(), cpu_online_map);
 
 #ifdef CONFIG_ACPI_BOOT
 	/* Initialize the ACPI boot-time table parser */