Michael Tokarev wrote:
> After some debugging and debugging, with a help
> Hollis Blanchard on #kvm@freenode, I discovered
> that kvm (or, rather, qemu) does not work correctly
> with serial ports, at least on linux. One problem
> report has already here, author Cc'd -- see e.g.
> http://marc.info/?l=kvm&m=122995568009533&w=2 .
... [quoted in full below]...
Ok, It's a real shame to see SO many wrong attempts to
do it all, with so many idiotic mistakes all over...
But c'est la vie, it seems... ;)
So here we go. Attached is a patch that fixes two
problems with serial ports &qemu (yes it's a qemu
issue, and, as far as I can see, both probs were
here for a long time).
First is completely f*cked up flags reporting and
setup for TIOCMGET and TIOCMSET ioctls, where ALL
known flags were reported and set in case at least
one flag is set, misusing "if(a|b) foo" instead of
"if(a&b) foo" -- just a typo I assume, but heck of
a typo... ;)
And second - for TIOCMSET it preserves other, unknown
flags. Which fixes the problem that started it all,
since there was a bit set internally in kernel which,
when unset, makes serial port non-working, but TIOCMSET
dropped all "other" bits on the floor.
And for this second one, I'm still unsure. The patch
I'm sending only tries to remove TIOCM_DTR and _RTS
bits (RTS is useless since it's controlled by the connected
device, isn't it?), leaving all others, incl., say,
CAR, RI, CTS etc, in place. The question is -- some
of those bits are "input" lines, i.e., the ones controlled
by the attached device, and I don't know if all platforms
will ignore those instead of reporting error. I.e, maybe
filter also those who are known "inputs"? And while we're
at it, still, how about RTS?
Signed-off-By: Michael Tokarev <mjt@xxxxxxxxxx>
Thanks!
/ashamed mjt
--- original content follows ---
> Here's what's going on.
>
> When opening a host's port, kvm resets the status
> lines, doing this:
>
> ioctl(13, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS|TIOCM_DSR|0x4000])
> ioctl(13, TIOCMSET, [TIOCM_DTR|TIOCM_RTS])
>
> which results in the following set
>
> ioctl(13, TIOCMGET, [TIOCM_DTR|TIOCM_RTS|TIOCM_CTS|TIOCM_DSR])
>
> Note the difference between the default set and new one: the
> missing bit, 0x4000, which is unknown to strace(1) but is defined
> as TIOCM_OUT2 in linux headers.
>
> After that change (resetting the TIOCM_OUT2 bit), no writes
> to the serial port works anymore, they're all gets accepted
> by host kernel and are buffered in the kernel.
>
> After some time, when the kernel buffer fills up, and since
> the port (on host side) is opened in non-blocking mode, the
> writes starts returning EAGAIN, and kvm process starts
> endless loop, which were seen by David.
>
> Here's the trivial program to demonstrate the idea:
>
> ---- cut ---
> #include <sys/types.h>
> #include <unistd.h>
> #include <fcntl.h>
> #include <sys/ioctl.h>
> #include <termios.h>
>
> int main(int argc, char **argv) {
> fd = open("/dev/ttyS0", O_RDWR|O_NONBLOCK);
> fcntl(fd, F_SETFL, O_RDWR);
> x = TIOCM_DTR|TIOCM_RTS|TIOCM_CTS|TIOCM_DSR // |0x4000
> ;
> ioctl(fd, TIOCMSET, &x);
> ioctl(fd, TIOCMGET, &x);
> write(fd, "at\r", 3);
> read(fd, buf, 20);
> close(fd);
>
> return 0;
> }
> --- cut ---
>
>
> Run it under strace while a dialup modem is connected to /dev/ttyS0
> (i used this way for testing). It will stuck at read, and nothing
> will be written, even when write() will happily return 3. Un-comment
> the |0x4000 thing, and it will work.
>
> I'm not sure what should be done with this, and how much this is
> linux-specific. But it is obvious that bit (TIOCM_OUT2) should
> be left in-place (after which the thing works), at least on linux.
>
> Note that this bit is NOT shown in /proc/tty/driver/serial file
> (which shows other bits).
>
> Note also that this file (/proc/tty/driver/serial) helps to see
> if any write were performed: compare the counters. In 'tx'
> there's number of bytes actually sent to device, as opposed to
> accepted by the kernel. When you write something to /dev/ttyS0,
> that number increases, IF that something actually reached the
> device.
>
> Thanks.
>
> /mjt
--- kvm-83/qemu/qemu-char.c.orig 2009-01-13 16:29:42.000000000 +0300
+++ kvm-83/qemu/qemu-char.c 2009-02-05 21:19:35.972015110 +0300
@@ -1067,17 +1067,17 @@ static int tty_serial_ioctl(CharDriverSt
int *targ = (int *)arg;
ioctl(s->fd_in, TIOCMGET, &sarg);
*targ = 0;
- if (sarg | TIOCM_CTS)
+ if (sarg & TIOCM_CTS)
*targ |= CHR_TIOCM_CTS;
- if (sarg | TIOCM_CAR)
+ if (sarg & TIOCM_CAR)
*targ |= CHR_TIOCM_CAR;
- if (sarg | TIOCM_DSR)
+ if (sarg & TIOCM_DSR)
*targ |= CHR_TIOCM_DSR;
- if (sarg | TIOCM_RI)
+ if (sarg & TIOCM_RI)
*targ |= CHR_TIOCM_RI;
- if (sarg | TIOCM_DTR)
+ if (sarg & TIOCM_DTR)
*targ |= CHR_TIOCM_DTR;
- if (sarg | TIOCM_RTS)
+ if (sarg & TIOCM_RTS)
*targ |= CHR_TIOCM_RTS;
}
break;
@@ -1085,9 +1085,11 @@ static int tty_serial_ioctl(CharDriverSt
{
int sarg = *(int *)arg;
int targ = 0;
- if (sarg | CHR_TIOCM_DTR)
+ ioctl(s->fd_in, TIOCMGET, &targ);
+ targ &= ~(TIOCM_DTR|TIOCM_RTS);
+ if (sarg & CHR_TIOCM_DTR)
targ |= TIOCM_DTR;
- if (sarg | CHR_TIOCM_RTS)
+ if (sarg & CHR_TIOCM_RTS)
targ |= TIOCM_RTS;
ioctl(s->fd_in, TIOCMSET, &targ);
}
