Skip to content

Commit

Permalink
um: line: Use separate IRQs per line
Browse files Browse the repository at this point in the history 
Today, all possible serial lines (ssl*=) as well as all
possible consoles (con*=) each share a single interrupt
(with a fixed number) with others of the same type.

Now, if you have two lines, say ssl0 and ssl1, and one
of them is connected to an fd you cannot read (e.g. a
file), but the other gets a read interrupt, then both
of them get the interrupt since it's shared. Then, the
read() call will return EOF, since it's a file being
written and there's nothing to read (at least not at
the current offset, at the end).

Unfortunately, this is treated as a read error, and we
close this line, losing all the possible output.

It might be possible to work around this and make the
IRQ sharing work, however, now that we have dynamically
allocated IRQs that are easy to use, simply use that to
achieve separating between the events; then there's no
interrupt for that line and we never attempt the read
in the first place, thus not closing the line.

This manifested itself in the wifi hostap/hwsim tests
where the parallel script communicates via one serial
console and the kernel messages go to another (a file)
and sending data on the communication console caused
the kernel messages to stop flowing into the file.

Reported-by: Jouni Malinen <j@w1.fi>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Acked-By: anton ivanov <anton.ivanov@cambridgegreys.com>
Signed-off-by: Richard Weinberger <richard@nod.at>
  • Loading branch information
@jmberg-intel @richardweinberger
jmberg-intel authored and richardweinberger committed May 27, 2022
1 parent 2419ac3 commit d5a9597
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 33 deletions.
10 changes: 5 additions & 5 deletions arch/um/drivers/chan_kern.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ static void line_timer_cb(struct work_struct *work)
struct line *line = container_of(work, struct line, task.work);

if (!line->throttled)
chan_interrupt(line, line->driver->read_irq);
chan_interrupt(line, line->read_irq);
}

int enable_chan(struct line *line)
Expand Down Expand Up @@ -195,9 +195,9 @@ void free_irqs(void)
chan = list_entry(ele, struct chan, free_list);

if (chan->input && chan->enabled)
um_free_irq(chan->line->driver->read_irq, chan);
um_free_irq(chan->line->read_irq, chan);
if (chan->output && chan->enabled)
um_free_irq(chan->line->driver->write_irq, chan);
um_free_irq(chan->line->write_irq, chan);
chan->enabled = 0;
}
}
Expand All @@ -215,9 +215,9 @@ static void close_one_chan(struct chan *chan, int delay_free_irq)
spin_unlock_irqrestore(&irqs_to_free_lock, flags);
} else {
if (chan->input && chan->enabled)
um_free_irq(chan->line->driver->read_irq, chan);
um_free_irq(chan->line->read_irq, chan);
if (chan->output && chan->enabled)
um_free_irq(chan->line->driver->write_irq, chan);
um_free_irq(chan->line->write_irq, chan);
chan->enabled = 0;
}
if (chan->ops->close != NULL)
Expand Down
22 changes: 13 additions & 9 deletions arch/um/drivers/line.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ static int flush_buffer(struct line *line)
count = line->buffer + LINE_BUFSIZE - line->head;

n = write_chan(line->chan_out, line->head, count,
line->driver->write_irq);
line->write_irq);
if (n < 0)
return n;
if (n == count) {
Expand All @@ -156,7 +156,7 @@ static int flush_buffer(struct line *line)

count = line->tail - line->head;
n = write_chan(line->chan_out, line->head, count,
line->driver->write_irq);
line->write_irq);

if (n < 0)
return n;
Expand Down Expand Up @@ -195,7 +195,7 @@ int line_write(struct tty_struct *tty, const unsigned char *buf, int len)
ret = buffer_data(line, buf, len);
else {
n = write_chan(line->chan_out, buf, len,
line->driver->write_irq);
line->write_irq);
if (n < 0) {
ret = n;
goto out_up;
Expand All @@ -215,7 +215,7 @@ void line_throttle(struct tty_struct *tty)
{
struct line *line = tty->driver_data;

deactivate_chan(line->chan_in, line->driver->read_irq);
deactivate_chan(line->chan_in, line->read_irq);
line->throttled = 1;
}

Expand All @@ -224,7 +224,7 @@ void line_unthrottle(struct tty_struct *tty)
struct line *line = tty->driver_data;

line->throttled = 0;
chan_interrupt(line, line->driver->read_irq);
chan_interrupt(line, line->read_irq);
}

static irqreturn_t line_write_interrupt(int irq, void *data)
Expand Down Expand Up @@ -260,19 +260,23 @@ int line_setup_irq(int fd, int input, int output, struct line *line, void *data)
int err;

if (input) {
err = um_request_irq(driver->read_irq, fd, IRQ_READ,
line_interrupt, IRQF_SHARED,
err = um_request_irq(UM_IRQ_ALLOC, fd, IRQ_READ,
line_interrupt, 0,
driver->read_irq_name, data);
if (err < 0)
return err;

line->read_irq = err;
}

if (output) {
err = um_request_irq(driver->write_irq, fd, IRQ_WRITE,
line_write_interrupt, IRQF_SHARED,
err = um_request_irq(UM_IRQ_ALLOC, fd, IRQ_WRITE,
line_write_interrupt, 0,
driver->write_irq_name, data);
if (err < 0)
return err;

line->write_irq = err;
}

return 0;
Expand Down
4 changes: 2 additions & 2 deletions arch/um/drivers/line.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ struct line_driver {
const short minor_start;
const short type;
const short subtype;
const int read_irq;
const char *read_irq_name;
const int write_irq;
const char *write_irq_name;
struct mc_device mc;
struct tty_driver *driver;
Expand All @@ -35,6 +33,8 @@ struct line {
struct tty_port port;
int valid;

int read_irq, write_irq;

char *init_str;
struct list_head chan_list;
struct chan *chan_in, *chan_out;
Expand Down
2 changes: 0 additions & 2 deletions arch/um/drivers/ssl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ static struct line_driver driver = {
.minor_start = 64,
.type = TTY_DRIVER_TYPE_SERIAL,
.subtype = 0,
.read_irq = SSL_IRQ,
.read_irq_name = "ssl",
.write_irq = SSL_WRITE_IRQ,
.write_irq_name = "ssl-write",
.mc = {
.list = LIST_HEAD_INIT(driver.mc.list),
Expand Down
2 changes: 0 additions & 2 deletions arch/um/drivers/stdio_console.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ static struct line_driver driver = {
.minor_start = 0,
.type = TTY_DRIVER_TYPE_CONSOLE,
.subtype = SYSTEM_TYPE_CONSOLE,
.read_irq = CONSOLE_IRQ,
.read_irq_name = "console",
.write_irq = CONSOLE_WRITE_IRQ,
.write_irq_name = "console-write",
.mc = {
.list = LIST_HEAD_INIT(driver.mc.list),
Expand Down
22 changes: 9 additions & 13 deletions arch/um/include/asm/irq.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,15 @@

#define TIMER_IRQ 0
#define UMN_IRQ 1
#define CONSOLE_IRQ 2
#define CONSOLE_WRITE_IRQ 3
#define UBD_IRQ 4
#define UM_ETH_IRQ 5
#define SSL_IRQ 6
#define SSL_WRITE_IRQ 7
#define ACCEPT_IRQ 8
#define MCONSOLE_IRQ 9
#define WINCH_IRQ 10
#define SIGIO_WRITE_IRQ 11
#define TELNETD_IRQ 12
#define XTERM_IRQ 13
#define RANDOM_IRQ 14
#define UBD_IRQ 2
#define UM_ETH_IRQ 3
#define ACCEPT_IRQ 4
#define MCONSOLE_IRQ 5
#define WINCH_IRQ 6
#define SIGIO_WRITE_IRQ 7
#define TELNETD_IRQ 8
#define XTERM_IRQ 9
#define RANDOM_IRQ 10

#ifdef CONFIG_UML_NET_VECTOR

Expand Down

0 comments on commit d5a9597

Please sign in to comment.