2005-11-30 11:14:24

by Sachin Sant

[permalink] [raw]
Subject: [RFC] [PATCH] Adding ctrl-o sysrq hack support to 8250 driver

Signed-off-by: Sachin Sant <[email protected]>

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++;


Attachments:
8250-magic-sysrq-support.patch (927.00 B)

2005-11-30 12:12:29

by Arjan van de Ven

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

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.


2005-11-30 13:04:36

by Russell King

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

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

2005-12-06 06:45:43

by Sachin Sant

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

Signed-off-by: Sachin Sant <[email protected]>

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++;


Attachments:
8250-magic-sysrq-support.patch (657.00 B)

2005-12-06 17:16:43

by Russell King

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

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

2005-12-07 06:28:57

by Sachin Sant

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

>
>
>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.

>
>

2005-12-07 22:22:55

by Olaf Hering

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

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

2005-12-07 23:39:18

by Russell King

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

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

2005-12-08 01:38:40

by Milton Miller

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

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]

2005-12-08 14:58:23

by Olaf Hering

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

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