[RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[Sachin Sant]

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

The following patch will allow a user to use sysrq keys over a serial 
console using the ctrl-o key sequence. This is similar to functionality 
provided by the hvc console drivers on PPC boxes.



[-- Attachment #2: 8250-magic-sysrq-support.patch --]
[-- Type: text/plain, Size: 927 bytes --]

Signed-off-by: Sachin Sant <sachinp@in.ibm.com>

diff -Naurp linux-2.6.14.3/drivers/serial/8250.c linux-2.6.14.3-new/drivers/serial/8250.c
--- linux-2.6.14.3/drivers/serial/8250.c	2005-11-11 11:03:12.000000000 +0530
+++ linux-2.6.14.3-new/drivers/serial/8250.c	2005-11-17 15:12:42.000000000 +0530
@@ -1084,6 +1084,23 @@ receive_chars(struct uart_8250_port *up,
 			 */
 		}
 		ch = serial_inp(up, UART_RX);
+
+#if defined(CONFIG_MAGIC_SYSRQ) && defined(CONFIG_SERIAL_CORE_CONSOLE)
+		/* Handle the SysRq ^O Hack */
+		if (ch == '\x0f') {
+			up->port.sysrq = jiffies + HZ*5;
+			goto ignore_char; 
+		}
+		if (up->port.sysrq) {
+			int swallow;
+			spin_unlock(&up->port.lock);
+			swallow = uart_handle_sysrq_char(&up->port, ch, regs);
+			spin_lock(&up->port.lock);
+			if (swallow)
+				goto ignore_char;
+		}
+#endif /* CONFIG_MAGIC_SYSRQ && CONFIG_SERIAL_CORE_CONSOLE */
+
 		flag = TTY_NORMAL;
 		up->port.icount.rx++;
 

Re: [RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[Arjan van de Ven]

On Wed, 2005-11-30 at 16:47 +0530, Sachin Sant wrote:
> The following patch will allow a user to use sysrq keys over a serial 
> console using the ctrl-o key sequence. This is similar to functionality 
> provided by the hvc console drivers on PPC boxes.

is sending a BRK over serial not enough? (the existing method)
minicom for sure can send that.. I'm sure all other terminal emulator
apps can as well.

Re: [RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[Russell King]

On Wed, Nov 30, 2005 at 04:47:14PM +0530, Sachin Sant wrote:
> The following patch will allow a user to use sysrq keys over a serial 
> console using the ctrl-o key sequence. This is similar to functionality 
> provided by the hvc console drivers on PPC boxes.

Several problems:
1. ^O is normally the "flush" control code, for discarding unsent output.
2. ^O might be inadvertantly received due to a connected PC trying to
   auto-detect connected devices.
3. see review below.

> diff -Naurp linux-2.6.14.3/drivers/serial/8250.c linux-2.6.14.3-new/drivers/serial/8250.c
> --- linux-2.6.14.3/drivers/serial/8250.c	2005-11-11 11:03:12.000000000 +0530
> +++ linux-2.6.14.3-new/drivers/serial/8250.c	2005-11-17 15:12:42.000000000 +0530
> @@ -1084,6 +1084,23 @@ receive_chars(struct uart_8250_port *up,
>  			 */
>  		}
>  		ch = serial_inp(up, UART_RX);
> +
> +#if defined(CONFIG_MAGIC_SYSRQ) && defined(CONFIG_SERIAL_CORE_CONSOLE)
> +		/* Handle the SysRq ^O Hack */
> +		if (ch == '\x0f') {
> +			up->port.sysrq = jiffies + HZ*5;

Why are you doing this and not using uart_handle_break()?

Do you realise that this enables sysrq support for _any_ 8250 serial
port, be it console or not?

> +			goto ignore_char; 
> +		}
> +		if (up->port.sysrq) {
> +			int swallow;
> +			spin_unlock(&up->port.lock);
> +			swallow = uart_handle_sysrq_char(&up->port, ch, regs);
> +			spin_lock(&up->port.lock);

We don't drop the lock when calling uart_handle_sysrq_char() further
down in this function.  Why is this needed?  Also, why is this needed
to be duplicated?

> +			if (swallow)
> +				goto ignore_char;
> +		}
> +#endif /* CONFIG_MAGIC_SYSRQ && CONFIG_SERIAL_CORE_CONSOLE */
> +
>  		flag = TTY_NORMAL;
>  		up->port.icount.rx++;
>  

Sorry, NACK.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[Sachin Sant]

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

> Why are you doing this and not using uart_handle_break()?
> 
> Do you realise that this enables sysrq support for _any_ 8250 serial
> port, be it console or not?
> 
At present we have ctrl-break that works over a serial connection. But 
there are few instances where the above does not works. Consider the 
following senarios

p615, power4, no-hmc configuration.

I attached an IBM 3153 (a "real" tty) to the serial port.I then observed
that neither ctrl-o or the tty's ctrl-SysRq_key worked. (but ctrl-o did 
still work with the patched kernel, of course).  However, the 
ctrl-Break_key does work on the native tty, taking the place of the 
ctrl-o on vterms.So in summary, ctrl-Break works. I then re-attached the 
cu cable (both are defined alike as ttyS0 and give login's) and 
re-verified that ctrl-Break stopped working.

In the future, if this configuration is found hung, then (provided sysrq 
was enabled), attach a physical tty and use ctrl-Break to debug.

2nd configuration: p630, hmc-attached, booted in "full-system partition" 
mode.Everything in the first configuration applied here too.There is 
only a slight diff in the way the console works.Since the hmc is 
attached, the "vterm" on the hmc will be the console.However, when it is 
booted in "full-system partition" mode, the OS sees that console as 
ttyS0 - just like a non-hmc attached connection.Unfortunately, neither 
the ctrl-o nor the ctrl-Break works on this either (just like the cu 
session above).The only way to get ctrl-Break to work is to "close the 
terminal" operation on the hmc, and then attach and turn on a real tty 
onto the lowest serial port (not the "HMC" port) on the p630.  You can't 
have both running at the same time, because they both configure as 
ttyS0.  Then with the real tty attached, you can use ctrl-Break to debug.

So the general idea behind this patch was to make the behaviour of 
invoking sysrq key more consistent over virtual and real consoles. It's 
not a must but would be nice to have this functionality.

> 
> We don't drop the lock when calling uart_handle_sysrq_char() further
> down in this function.  Why is this needed?  Also, why is this needed
> to be duplicated?
> 
You are correct. This is not needed. I have removed this piece of code.

How about the following patch.

[-- Attachment #2: 8250-magic-sysrq-support.patch --]
[-- Type: text/plain, Size: 657 bytes --]

Signed-off-by: Sachin Sant <sachinp@in.ibm.com>

diff -Naurp a/drivers/serial/8250.c b/drivers/serial/8250.c
--- a/drivers/serial/8250.c	2005-11-11 11:03:12.000000000 +0530
+++ b/drivers/serial/8250.c	2005-12-06 11:52:40.000000000 +0530
@@ -1084,6 +1084,15 @@ receive_chars(struct uart_8250_port *up,
 			 */
 		}
 		ch = serial_inp(up, UART_RX);
+
+#if defined(CONFIG_MAGIC_SYSRQ) && defined(CONFIG_SERIAL_CORE_CONSOLE)
+		/* Handle the SysRq ^O Hack */
+		if (ch == '\x0f') {
+			up->port.sysrq = jiffies + HZ*5;
+			goto ignore_char; 
+		}
+#endif /* CONFIG_MAGIC_SYSRQ && CONFIG_SERIAL_CORE_CONSOLE */
+
 		flag = TTY_NORMAL;
 		up->port.icount.rx++;
 

Re: [RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[Russell King]

On Tue, Dec 06, 2005 at 12:18:32PM +0530, Sachin Sant wrote:
> >Why are you doing this and not using uart_handle_break()?
> >
> >Do you realise that this enables sysrq support for _any_ 8250 serial
> >port, be it console or not?
>
> At present we have ctrl-break that works over a serial connection. But 
> there are few instances where the above does not works. Consider the 
> following senarios
> 
> p615, power4, no-hmc configuration.
> 
> I attached an IBM 3153 (a "real" tty) to the serial port.I then observed
> that neither ctrl-o or the tty's ctrl-SysRq_key worked.  (but ctrl-o did 
> still work with the patched kernel, of course).  However, the 
> ctrl-Break_key does work on the native tty, taking the place of the 
> ctrl-o on vterms.

Frown.  Sorry, I'm not sure what "p615", "power4" and "no-hmc" is.  I
also don't know what an IBM 3153 is.

However, you seem to be suggesting that a terminal application somehow
forwards the ctrl-sysrq (and it's actually alt-sysrq).  Maybe it does,
maybe it doesn't.  Probably depends on the terminal application itself.

Eg, with minicom, you need to ask minicom to create the serial break
event itself, normally by (assuming default configuration) <ctrl-a> <f>.

> So in summary, ctrl-Break works. I then re-attached the 
> cu cable (both are defined alike as ttyS0 and give login's) and 
> re-verified that ctrl-Break stopped working.
> 
> In the future, if this configuration is found hung, then (provided sysrq 
> was enabled), attach a physical tty and use ctrl-Break to debug.
> 
> 2nd configuration: p630, hmc-attached, booted in "full-system partition" 
> mode.Everything in the first configuration applied here too.

At around this point, I'm starting to loose track of this email -
please can you format the message better, eg use at least one space
(preferably two) after a full stop and separate up what you want to
say into easy to read paragraphs.

> There is 
> only a slight diff in the way the console works.Since the hmc is 
> attached, the "vterm" on the hmc will be the console.However, when it is 
> booted in "full-system partition" mode, the OS sees that console as 
> ttyS0 - just like a non-hmc attached connection.Unfortunately, neither 
> the ctrl-o nor the ctrl-Break works on this either (just like the cu 
> session above).The only way to get ctrl-Break to work is to "close the 
> terminal" operation on the hmc, and then attach and turn on a real tty 
> onto the lowest serial port (not the "HMC" port) on the p630.  You can't 
> have both running at the same time, because they both configure as 
> ttyS0.  Then with the real tty attached, you can use ctrl-Break to debug.
> 
> So the general idea behind this patch was to make the behaviour of 
> invoking sysrq key more consistent over virtual and real consoles. It's 
> not a must but would be nice to have this functionality.

I don't think you've addressed my concern... but I'm afraid I haven't
been able to properly follow what you're saying.

In any case, applying this patch means that you _permanently_ prevent
the reception of ^O on _ANY_ 8250 serial port, whether it be a serial
console or not.

With this patch, I guess it's tough luck if you have a modem connected
to your PC and you want to run ppp or x,y,z modem protocols.

> >We don't drop the lock when calling uart_handle_sysrq_char() further
> >down in this function.  Why is this needed?  Also, why is this needed
> >to be duplicated?
>
> You are correct. This is not needed. I have removed this piece of code.
> 
> How about the following patch.

I'm still highly concerned about this whole idea.  Applying this patch
_will_ without doubt inconvenience a lot of people who expect ^O to be
received as normal.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[Sachin Sant]

>
>
>Frown.  Sorry, I'm not sure what "p615", "power4" and "no-hmc" is.  I
>also don't know what an IBM 3153 is.
>
>However, you seem to be suggesting that a terminal application somehow
>forwards the ctrl-sysrq (and it's actually alt-sysrq).  Maybe it does,
>maybe it doesn't.  Probably depends on the terminal application itself.
>
>Eg, with minicom, you need to ask minicom to create the serial break
>event itself, normally by (assuming default configuration) <ctrl-a> <f>.
>
>  
>
Sorry about using all those cryptic terms. I was trying to explain with
the help of few test scenario's. I had tested on couple of PowerPC processor
based machines which can be configured either as a standalone machine 
[ Without any Logical Partitions ] or can be logically partitioned.
One can use a Hardware Management console [HMC] to manage the machine.


>I don't think you've addressed my concern... but I'm afraid I haven't
>been able to properly follow what you're saying.
>
>In any case, applying this patch means that you _permanently_ prevent
>the reception of ^O on _ANY_ 8250 serial port, whether it be a serial
>console or not.
>
>With this patch, I guess it's tough luck if you have a modem connected
>to your PC and you want to run ppp or x,y,z modem protocols.
>
>  
>
The point i was trying to make was to have a consistent behavior across
virtual and real consoles connected to certain kind of machines. 
But as stated by you this patch might cause lot of inconvenience to 
other user. So it's best not to introduce this patch.

>  
>

Re: [RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[Olaf Hering]

 On Tue, Dec 06, Russell King wrote:

> I'm still highly concerned about this whole idea.  Applying this patch
> _will_ without doubt inconvenience a lot of people who expect ^O to be
> received as normal.

If one boots with 'console=ttyS0', the 'ctrl o' should be handled only
on ttyS0. However, I'm not sure if anyone uses ^O in this situation via
the system console. In our case, ttyS0 is automatically activated via
add_preferred_console in arch/powerpc/kernel/setup-common.c.
If there is a clever way to handle ^O only for the system console, would
such a patch be accepted? I'm currently looking through the code to see
how it could be done.

-- 
short story of a lazy sysadmin:
 alias appserv=wotan

Re: [RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[Russell King]

On Wed, Dec 07, 2005 at 11:22:46PM +0100, Olaf Hering wrote:
>  On Tue, Dec 06, Russell King wrote:
> 
> > I'm still highly concerned about this whole idea.  Applying this patch
> > _will_ without doubt inconvenience a lot of people who expect ^O to be
> > received as normal.
> 
> If one boots with 'console=ttyS0', the 'ctrl o' should be handled only
> on ttyS0. However, I'm not sure if anyone uses ^O in this situation via
> the system console. In our case, ttyS0 is automatically activated via
> add_preferred_console in arch/powerpc/kernel/setup-common.c.
> If there is a clever way to handle ^O only for the system console, would
> such a patch be accepted? I'm currently looking through the code to see
> how it could be done.

Easily.  Have a look at the internals of uart_handle_break() in
include/linux/serial_core.h

However, please be aware that ^O is the default control character for
"flush output" which I think is something you may want to use with a
serial console.  Eg:

speed 38400 baud; rows 0; columns 0; line = 1;
intr = ^C; quit = ^\; erase = ^?; kill = ^U; eof = ^D; eol = <undef>;
eol2 = <undef>; start = ^Q; stop = ^S; susp = ^Z; rprnt = ^R; werase = ^W;
lnext = ^V; flush = ^O; min = 1; time = 0;
            ^^^^^^^^^^^

Hence it's a poor choice.  Maybe picking a character which isn't
already used by default for another purpose would be appropriate?
'^]', the classic telnet escape character maybe?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[Olaf Hering]

 On Wed, Dec 07, Russell King wrote:

> Easily.  Have a look at the internals of uart_handle_break() in
> include/linux/serial_core.h

This one works for me, tested with 2.6.5.


 drivers/serial/8250.c |    7 +++++++
 1 files changed, 7 insertions(+)

Index: linux-2.6.15-rc5-olh/drivers/serial/8250.c
===================================================================
--- linux-2.6.15-rc5-olh.orig/drivers/serial/8250.c
+++ linux-2.6.15-rc5-olh/drivers/serial/8250.c
@@ -1154,6 +1154,13 @@ receive_chars(struct uart_8250_port *up,
 			 */
 		}
 		ch = serial_inp(up, UART_RX);
+
+#if defined(CONFIG_MAGIC_SYSRQ) && defined(CONFIG_SERIAL_CORE_CONSOLE)
+		/* Handle the SysRq ^O Hack, but only on the system console */
+		if (ch == '\x0f' && uart_handle_break(&up->port))
+			goto ignore_char;
+#endif
+
 		flag = TTY_NORMAL;
 		up->port.icount.rx++;
 

-- 
short story of a lazy sysadmin:
 alias appserv=wotan

Re: [RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

[Milton Miller]

On Wed Dec 07 2005 - 18:38:41 EST, Russell King wrote:
> On Wed, Dec 07, 2005 at 11:22:46PM +0100, Olaf Hering wrote:
> > On Tue, Dec 06, Russell King wrote:
> >
> > > I'm still highly concerned about this whole idea. Applying this 
> patch
> > > _will_ without doubt inconvenience a lot of people who expect ^O 
> to be
> > > received as normal.
> >
> > If one boots with 'console=ttyS0', the 'ctrl o' should be handled 
> only
> > on ttyS0. However, I'm not sure if anyone uses ^O in this situation 
> via
> > the system console. In our case, ttyS0 is automatically activated via
> > add_preferred_console in arch/powerpc/kernel/setup-common.c.
> > If there is a clever way to handle ^O only for the system console, 
> would
> > such a patch be accepted? I'm currently looking through the code to 
> see
> > how it could be done.
>
> Easily. Have a look at the internals of uart_handle_break() in
> include/linux/serial_core.h
>
> However, please be aware that ^O is the default control character for
> "flush output" which I think is something you may want to use with a
> serial console. Eg:
>
> speed 38400 baud; rows 0; columns 0; line = 1;
> intr = ^C; quit = ^\; erase = ^?; kill = ^U; eof = ^D; eol = <undef>;
> eol2 = <undef>; start = ^Q; stop = ^S; susp = ^Z; rprnt = ^R; werase = 
> ^W;
> lnext = ^V; flush = ^O; min = 1; time = 0;
> ^^^^^^^^^^^
>
> Hence it's a poor choice. Maybe picking a character which isn't
> already used by default for another purpose would be appropriate?
> '^]', the classic telnet escape character maybe?

Aaarrrrhh NO!

Don't you ever login to a box to telent somewhere else?

I don't want to way 5 seconds to disconnect my telnet from the
hung remote machine (or do a saK either).

If this goes in, perhaps a parameter (which automatically shows
up in sysfs) to change it on the fly?  (ok, something tied to
the class device rather than the serial module would be nicer).

milton

[Hopefully I got all the cc's, I'm not subscribed]