From patchwork Thu Feb 5 18:33:30 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Tokarev X-Patchwork-Id: 5694 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n15IXpUD005728 for ; Thu, 5 Feb 2009 18:33:51 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751568AbZBESdh (ORCPT ); Thu, 5 Feb 2009 13:33:37 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754513AbZBESdh (ORCPT ); Thu, 5 Feb 2009 13:33:37 -0500 Received: from isrv.corpit.ru ([81.13.33.159]:58543 "EHLO isrv.corpit.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751568AbZBESdf (ORCPT ); Thu, 5 Feb 2009 13:33:35 -0500 Received: from [192.168.1.200] (unknown [192.168.1.200]) by isrv.corpit.ru (Postfix) with ESMTP id 92EB9B033; Thu, 5 Feb 2009 21:33:30 +0300 (MSK) (envelope-from mjt@tls.msk.ru) Message-ID: <498B30FA.40800@msgid.tls.msk.ru> Date: Thu, 05 Feb 2009 21:33:30 +0300 From: Michael Tokarev Organization: Telecom Service, JSC User-Agent: Mozilla-Thunderbird 2.0.0.19 (X11/20090103) MIME-Version: 1.0 To: KVM list CC: "David S. Ahern" , Mark Marshall , Marcelo Tosatti , Stefano Stabellini Subject: [PATCH, finally]: more about serial ports: do they even work? References: <497E1B15.2090908@msgid.tls.msk.ru> In-Reply-To: <497E1B15.2090908@msgid.tls.msk.ru> Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org 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 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 > #include > #include > #include > #include > > 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); }