[Qemu-devel] Re: more about serial ports: do they even work?

[Qemu-devel] Re: more about serial ports: do they even work?

[David S. Ahern]

I don't recall seeing a followup to this post.

To put Michael's second suggestion into patch form, the following fixes
the problem for me:

--- kvm-81.orig/qemu/qemu-char.c        2008-12-14 06:16:27.000000000 -0700
+++ kvm-81/qemu/qemu-char.c     2009-02-02 14:12:20.000000000 -0700
@@ -1078,20 +1078,21 @@
             if (sarg | TIOCM_DTR)
                 *targ |= CHR_TIOCM_DTR;
             if (sarg | TIOCM_RTS)
                 *targ |= CHR_TIOCM_RTS;
         }
         break;
     case CHR_IOCTL_SERIAL_SET_TIOCM:
         {
             int sarg = *(int *)arg;
             int targ = 0;
+            ioctl(s->fd_in, TIOCMGET, &targ);
             if (sarg | CHR_TIOCM_DTR)
                 targ |= TIOCM_DTR;
             if (sarg | CHR_TIOCM_RTS)
                 targ |= TIOCM_RTS;
             ioctl(s->fd_in, TIOCMSET, &targ);
         }
         break;
     default:
         return -ENOTSUP;
     }

Is this approach palatable to folks?

david


Michael Tokarev wrote:
> Michael Tokarev ?????:
>> 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 .
>>
>> 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])
> 
> Here's the possible solution (NotAPAtch(tm)):
> 
> In kvm-xx/qemu/qemu-char.c:
> 
>     case CHR_IOCTL_SERIAL_SET_TIOCM:
>         {
>             int sarg = *(int *)arg;
>             int targ = 0;        <==== change this 0 to 0x4000
>             if (sarg | CHR_TIOCM_DTR)
>                 targ |= TIOCM_DTR;
>             if (sarg | CHR_TIOCM_RTS)
>                 targ |= TIOCM_RTS;
>             ioctl(s->fd_in, TIOCMSET, &targ);
>         }
>         break;
> 
> This is obviously a hack, esp. since this bit is not
> always present even on linux (after reading 8250.c
> driver).
> 
> Real fix will be, I guess, to read the full set
> first, and combine it with DTR|RTS received from
> guest, something like this:
> 
>     case
>         {
>             int sarg = *(int *)arg;
>             int targ = 0;
>             ioctl(s->fd_in, TIOCMGET, &targ);
>             if (!(sarg | CHR_TIOCM_DTR))
>                 targ &= ~TIOCM_DTR;
>             if (!(sarg | CHR_TIOCM_RTS))
>                 targ ~= ~TIOCM_RTS;
>             ioctl(s->fd_in, TIOCMSET, &targ);
>         }
>         break;
> 
> I.e., to always keep all the other bits, but
> allow changing DTR and RTS.
> 
> Again, I don't know how it's linux-specific, but
> it seems the solution above should work on other
> platforms just fine.
> 
> /mjt

[Qemu-devel] Re: more about serial ports: do they even work?

[Michael Tokarev]

David S. Ahern wrote:
> I don't recall seeing a followup to this post.
> 
> To put Michael's second suggestion into patch form, the following fixes
> the problem for me:
> 
> --- kvm-81.orig/qemu/qemu-char.c        2008-12-14 06:16:27.000000000 -0700
> +++ kvm-81/qemu/qemu-char.c     2009-02-02 14:12:20.000000000 -0700
> @@ -1078,20 +1078,21 @@
>              if (sarg | TIOCM_DTR)
>                  *targ |= CHR_TIOCM_DTR;
>              if (sarg | TIOCM_RTS)
>                  *targ |= CHR_TIOCM_RTS;
>          }
>          break;
>      case CHR_IOCTL_SERIAL_SET_TIOCM:
>          {
>              int sarg = *(int *)arg;
>              int targ = 0;
> +            ioctl(s->fd_in, TIOCMGET, &targ);

here, one more operation is necessary:
               targ &= ~(TIOCM_DTR|TIOCM_RTS);

>              if (sarg | CHR_TIOCM_DTR)
>                  targ |= TIOCM_DTR;
>              if (sarg | CHR_TIOCM_RTS)
>                  targ |= TIOCM_RTS;
>              ioctl(s->fd_in, TIOCMSET, &targ);
>          }
>          break;
>      default:
>          return -ENOTSUP;
>      }
> 
> Is this approach palatable to folks?
> 
> david

/mjt

[Qemu-devel] Re: more about serial ports: do they even work?

[David S. Ahern]

Michael Tokarev wrote:
> David S. Ahern wrote:
>> I don't recall seeing a followup to this post.
>>
>> To put Michael's second suggestion into patch form, the following fixes
>> the problem for me:
>>
>> --- kvm-81.orig/qemu/qemu-char.c        2008-12-14 06:16:27.000000000 -0700
>> +++ kvm-81/qemu/qemu-char.c     2009-02-02 14:12:20.000000000 -0700
>> @@ -1078,20 +1078,21 @@
>>              if (sarg | TIOCM_DTR)
>>                  *targ |= CHR_TIOCM_DTR;
>>              if (sarg | TIOCM_RTS)
>>                  *targ |= CHR_TIOCM_RTS;
>>          }
>>          break;
>>      case CHR_IOCTL_SERIAL_SET_TIOCM:
>>          {
>>              int sarg = *(int *)arg;
>>              int targ = 0;
>> +            ioctl(s->fd_in, TIOCMGET, &targ);
> 
> here, one more operation is necessary:
>                targ &= ~(TIOCM_DTR|TIOCM_RTS);
> 


Interesting. that change was not needed to fix my problem.

david


>>              if (sarg | CHR_TIOCM_DTR)
>>                  targ |= TIOCM_DTR;
>>              if (sarg | CHR_TIOCM_RTS)
>>                  targ |= TIOCM_RTS;
>>              ioctl(s->fd_in, TIOCMSET, &targ);
>>          }
>>          break;
>>      default:
>>          return -ENOTSUP;
>>      }
>>
>> Is this approach palatable to folks?
>>
>> david
> 
> /mjt

[Qemu-devel] Re: more about serial ports: do they even work?

[Michael Tokarev]

David S. Ahern wrote:
> Michael Tokarev wrote:
>> David S. Ahern wrote:
>>> I don't recall seeing a followup to this post.
>>>
>>> To put Michael's second suggestion into patch form, the following fixes
>>> the problem for me:
>>>
>>> --- kvm-81.orig/qemu/qemu-char.c        2008-12-14 06:16:27.000000000 -0700
>>> +++ kvm-81/qemu/qemu-char.c     2009-02-02 14:12:20.000000000 -0700
>>> @@ -1078,20 +1078,21 @@
>>>              if (sarg | TIOCM_DTR)
>>>                  *targ |= CHR_TIOCM_DTR;
>>>              if (sarg | TIOCM_RTS)
>>>                  *targ |= CHR_TIOCM_RTS;
>>>          }
>>>          break;
>>>      case CHR_IOCTL_SERIAL_SET_TIOCM:
>>>          {
>>>              int sarg = *(int *)arg;
>>>              int targ = 0;
>>> +            ioctl(s->fd_in, TIOCMGET, &targ);
>> here, one more operation is necessary:
>>                targ &= ~(TIOCM_DTR|TIOCM_RTS);
> 
> Interesting. that change was not needed to fix my problem.

It just means you (or, rather, your guests) never really needed to
DROP those signal lines, only to raise them.

>>>              if (sarg | CHR_TIOCM_DTR)
>>>                  targ |= TIOCM_DTR;
>>>              if (sarg | CHR_TIOCM_RTS)
>>>                  targ |= TIOCM_RTS;

Without that line above, the code never drops the two bits, once
set they can't be "removed" anymore.

By the way, this is upstream qemu issue, not kvm one, and has to be
pushed as such.  Good you CC'd qemu list.

/mjt

[Qemu-devel] Re: more about serial ports: do they even work?

[Mark Marshall]

Michael Tokarev wrote:
> David S. Ahern wrote:
>> Michael Tokarev wrote:
>>> David S. Ahern wrote:
>>>> I don't recall seeing a followup to this post.
>>>>
>>>> To put Michael's second suggestion into patch form, the following fixes
>>>> the problem for me:
>>>>
>>>> --- kvm-81.orig/qemu/qemu-char.c        2008-12-14 06:16:27.000000000 -0700
>>>> +++ kvm-81/qemu/qemu-char.c     2009-02-02 14:12:20.000000000 -0700
>>>> @@ -1078,20 +1078,21 @@
>>>>              if (sarg | TIOCM_DTR)
>>>>                  *targ |= CHR_TIOCM_DTR;
>>>>              if (sarg | TIOCM_RTS)
>>>>                  *targ |= CHR_TIOCM_RTS;
>>>>          }
>>>>          break;
>>>>      case CHR_IOCTL_SERIAL_SET_TIOCM:
>>>>          {
>>>>              int sarg = *(int *)arg;
>>>>              int targ = 0;
>>>> +            ioctl(s->fd_in, TIOCMGET, &targ);
>>> here, one more operation is necessary:
>>>                targ &= ~(TIOCM_DTR|TIOCM_RTS);
>> Interesting. that change was not needed to fix my problem.
> 
> It just means you (or, rather, your guests) never really needed to
> DROP those signal lines, only to raise them.
> 
>>>>              if (sarg | CHR_TIOCM_DTR)
>>>>                  targ |= TIOCM_DTR;
>>>>              if (sarg | CHR_TIOCM_RTS)
>>>>                  targ |= TIOCM_RTS;
Is this code really correct.  If it is is there a comment somewhere 
describing why it's correct?  I would have expected the lines above to be:

               if (sarg & CHR_TIOCM_DTR)
                   targ |= TIOCM_DTR;
               if (sarg & CHR_TIOCM_RTS)
                   targ |= TIOCM_RTS;

(Just an observation as this went past).

MM

> 
> Without that line above, the code never drops the two bits, once
> set they can't be "removed" anymore.
> 
> By the way, this is upstream qemu issue, not kvm one, and has to be
> pushed as such.  Good you CC'd qemu list.
> 
> /mjt
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[Qemu-devel] Re: more about serial ports: do they even work?

[Marcelo Tosatti]

On Tue, Feb 03, 2009 at 08:41:14AM +0000, Mark Marshall wrote:
> Michael Tokarev wrote:
>> David S. Ahern wrote:
>>> Michael Tokarev wrote:
>>>> David S. Ahern wrote:
>>>>> I don't recall seeing a followup to this post.
>>>>>
>>>>> To put Michael's second suggestion into patch form, the following fixes
>>>>> the problem for me:
>>>>>
>>>>> --- kvm-81.orig/qemu/qemu-char.c        2008-12-14 06:16:27.000000000 -0700
>>>>> +++ kvm-81/qemu/qemu-char.c     2009-02-02 14:12:20.000000000 -0700
>>>>> @@ -1078,20 +1078,21 @@
>>>>>              if (sarg | TIOCM_DTR)
>>>>>                  *targ |= CHR_TIOCM_DTR;
>>>>>              if (sarg | TIOCM_RTS)
>>>>>                  *targ |= CHR_TIOCM_RTS;
>>>>>          }
>>>>>          break;
>>>>>      case CHR_IOCTL_SERIAL_SET_TIOCM:
>>>>>          {
>>>>>              int sarg = *(int *)arg;
>>>>>              int targ = 0;
>>>>> +            ioctl(s->fd_in, TIOCMGET, &targ);
>>>> here, one more operation is necessary:
>>>>                targ &= ~(TIOCM_DTR|TIOCM_RTS);
>>> Interesting. that change was not needed to fix my problem.
>>
>> It just means you (or, rather, your guests) never really needed to
>> DROP those signal lines, only to raise them.
>>
>>>>>              if (sarg | CHR_TIOCM_DTR)
>>>>>                  targ |= TIOCM_DTR;
>>>>>              if (sarg | CHR_TIOCM_RTS)
>>>>>                  targ |= TIOCM_RTS;
> Is this code really correct.  If it is is there a comment somewhere  
> describing why it's correct?  I would have expected the lines above to 
> be:
>
>               if (sarg & CHR_TIOCM_DTR)
>                   targ |= TIOCM_DTR;
>               if (sarg & CHR_TIOCM_RTS)
>                   targ |= TIOCM_RTS;
>
> (Just an observation as this went past).

Right. I don't understand the point of converting to an "internal"
representation of TIOCM control bits. 

CHR_IOCTL_SERIAL_SET_TIOCM is clearly broken as mentioned. It should at 
least preserve the bits it does not control.

diff --git a/qemu/qemu-char.c b/qemu/qemu-char.c
index ac431c7..66971e1 100644
--- a/qemu/qemu-char.c
+++ b/qemu/qemu-char.c
@@ -1063,33 +1063,12 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
         break;
     case CHR_IOCTL_SERIAL_GET_TIOCM:
         {
-            int sarg = 0;
-            int *targ = (int *)arg;
-            ioctl(s->fd_in, TIOCMGET, &sarg);
-            *targ = 0;
-            if (sarg | TIOCM_CTS)
-                *targ |= CHR_TIOCM_CTS;
-            if (sarg | TIOCM_CAR)
-                *targ |= CHR_TIOCM_CAR;
-            if (sarg | TIOCM_DSR)
-                *targ |= CHR_TIOCM_DSR;
-            if (sarg | TIOCM_RI)
-                *targ |= CHR_TIOCM_RI;
-            if (sarg | TIOCM_DTR)
-                *targ |= CHR_TIOCM_DTR;
-            if (sarg | TIOCM_RTS)
-                *targ |= CHR_TIOCM_RTS;
+            ioctl(s->fd_in, TIOCMGET, arg);
         }
         break;
     case CHR_IOCTL_SERIAL_SET_TIOCM:
         {
-            int sarg = *(int *)arg;
-            int targ = 0;
-            if (sarg | CHR_TIOCM_DTR)
-                targ |= TIOCM_DTR;
-            if (sarg | CHR_TIOCM_RTS)
-                targ |= TIOCM_RTS;
-            ioctl(s->fd_in, TIOCMSET, &targ);
+            ioctl(s->fd_in, TIOCMSET, arg);
         }
         break;
     default:

[Qemu-devel] Re: more about serial ports: do they even work?

[Michael Tokarev]

Marcelo Tosatti wrote:
> On Tue, Feb 03, 2009 at 08:41:14AM +0000, Mark Marshall wrote:
[]
> Right. I don't understand the point of converting to an "internal"
> representation of TIOCM control bits. 

<fun mode>
Well, the same goes for the IOCTL values themselves too -- like
this CHR_IOCTL_SERIAL_SET_TIOCM itself.  I mean, TIOCMSET is
the right name for it ;)  But see below.

> CHR_IOCTL_SERIAL_SET_TIOCM is clearly broken as mentioned. It should at 
> least preserve the bits it does not control.
> 
> diff --git a/qemu/qemu-char.c b/qemu/qemu-char.c
> index ac431c7..66971e1 100644
> --- a/qemu/qemu-char.c
> +++ b/qemu/qemu-char.c
> @@ -1063,33 +1063,12 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
>          break;
>      case CHR_IOCTL_SERIAL_GET_TIOCM:
>          {
> +            ioctl(s->fd_in, TIOCMGET, arg);
>          }
>          break;

And those parens too, let it die, die! ;)
</fun mode>

Other than that, an.. excellent idea, I wanted to propose
just that when I first saw all this stuff, but was somewhat
afraid.  And I *think* there's at least *some* sense.  Qemu
is a CPU emulator and may work on another arch where those
bits are defined differently.  Maybe that was the reason for
all this converting - to be safe than sorry, so to say.  No?

/mjt

[Qemu-devel] Re: more about serial ports: do they even work?

[Marcelo Tosatti]

On Wed, Feb 04, 2009 at 11:37:14AM +0300, Michael Tokarev wrote:
> Marcelo Tosatti wrote:
> > On Tue, Feb 03, 2009 at 08:41:14AM +0000, Mark Marshall wrote:
> []
> > Right. I don't understand the point of converting to an "internal"
> > representation of TIOCM control bits. 
> 
> <fun mode>
> Well, the same goes for the IOCTL values themselves too -- like
> this CHR_IOCTL_SERIAL_SET_TIOCM itself.  I mean, TIOCMSET is
> the right name for it ;)  But see below.
> 
> > CHR_IOCTL_SERIAL_SET_TIOCM is clearly broken as mentioned. It should at 
> > least preserve the bits it does not control.
> > 
> > diff --git a/qemu/qemu-char.c b/qemu/qemu-char.c
> > index ac431c7..66971e1 100644
> > --- a/qemu/qemu-char.c
> > +++ b/qemu/qemu-char.c
> > @@ -1063,33 +1063,12 @@ static int tty_serial_ioctl(CharDriverState *chr, int cmd, void *arg)
> >          break;
> >      case CHR_IOCTL_SERIAL_GET_TIOCM:
> >          {
> > +            ioctl(s->fd_in, TIOCMGET, arg);
> >          }
> >          break;
> 
> And those parens too, let it die, die! ;)

Kill it!

> </fun mode>
> 
> Other than that, an.. excellent idea, I wanted to propose
> just that when I first saw all this stuff, but was somewhat
> afraid.  And I *think* there's at least *some* sense.  Qemu
> is a CPU emulator and may work on another arch where those
> bits are defined differently.  Maybe that was the reason for
> all this converting - to be safe than sorry, so to say.  No?

Probably, yes.

Does it work for you?

[Qemu-devel] Re: more about serial ports: do they even work?

[Michael Tokarev]

Marcelo Tosatti wrote:
[]
>> Other than that, an.. excellent idea, I wanted to propose
>> just that when I first saw all this stuff, but was somewhat
>> afraid.  And I *think* there's at least *some* sense.  Qemu
>> is a CPU emulator and may work on another arch where those
>> bits are defined differently.  Maybe that was the reason for
>> all this converting - to be safe than sorry, so to say.  No?
> 
> Probably, yes.

It's a question for qemu folks.

> Does it work for you?

Sure it does, since I only use it with kvm with the same arch
in guest and host, even with the same operating system.

Wug.  So much for so tiny issue.... Well, I'd not call non-working
serial port a "tiny issue".  Oh well.

/mjt

[Qemu-devel] Re: more about serial ports: do they even work?

[Stefano Stabellini]

Michael Tokarev wrote:

> Other than that, an.. excellent idea, I wanted to propose
> just that when I first saw all this stuff, but was somewhat
> afraid.  And I *think* there's at least *some* sense.  Qemu
> is a CPU emulator and may work on another arch where those
> bits are defined differently.  Maybe that was the reason for
> all this converting - to be safe than sorry, so to say.  No?
> 

Yes, this is exactly the reason why they were introduced in the first place.
Let's suppose that the guest defines those constants differently: we
need to parse them and covert them appropriately to the host format.
CHR_IOCTL_SERIAL_SET_TIOCM and CHR_TIOCM_DTR correspond to the guest
version of TIOCMSET and TIOCM_DTR and can be defined differently
depending on the particular guest arch.

Re: [Qemu-devel] Re: more about serial ports: do they even work?

[Stefano Stabellini]

Michael Tokarev wrote:

> David S. Ahern wrote:
>> Michael Tokarev wrote:
>>> David S. Ahern wrote:
>>>>      case CHR_IOCTL_SERIAL_SET_TIOCM:
>>>>          {
>>>>              int sarg = *(int *)arg;
>>>>              int targ = 0;
>>>> +            ioctl(s->fd_in, TIOCMGET, &targ);
>>> here, one more operation is necessary:
>>>                targ &= ~(TIOCM_DTR|TIOCM_RTS);
>> Interesting. that change was not needed to fix my problem.
> 
> It just means you (or, rather, your guests) never really needed to
> DROP those signal lines, only to raise them.
> 
>>>>              if (sarg | CHR_TIOCM_DTR)
>>>>                  targ |= TIOCM_DTR;
>>>>              if (sarg | CHR_TIOCM_RTS)
>>>>                  targ |= TIOCM_RTS;
> 
> Without that line above, the code never drops the two bits, once
> set they can't be "removed" anymore.
> 
> By the way, this is upstream qemu issue, not kvm one, and has to be
> pushed as such.  Good you CC'd qemu list.
> 


Either the two lines above or we could parse the whole set of possible
flags, like we do in the CHR_IOCTL_SERIAL_GET_TIOCM case.

[Qemu-devel] Re: more about serial ports: do they even work?

[David S. Ahern]

Stefano Stabellini wrote:
> Michael Tokarev wrote:
> 
>> Other than that, an.. excellent idea, I wanted to propose
>> just that when I first saw all this stuff, but was somewhat
>> afraid.  And I *think* there's at least *some* sense.  Qemu
>> is a CPU emulator and may work on another arch where those
>> bits are defined differently.  Maybe that was the reason for
>> all this converting - to be safe than sorry, so to say.  No?
>>
> 
> Yes, this is exactly the reason why they were introduced in the first place.
> Let's suppose that the guest defines those constants differently: we
> need to parse them and covert them appropriately to the host format.
> CHR_IOCTL_SERIAL_SET_TIOCM and CHR_TIOCM_DTR correspond to the guest
> version of TIOCMSET and TIOCM_DTR and can be defined differently
> depending on the particular guest arch.

The following works for me.  It fixes the existing checks in place for
the GET and replicates that for the SET. The ioctl initialization is
needed in the SET is needed.


Index: trunk/qemu-char.c
===================================================================
--- trunk/qemu-char.c   (revision 6519)
+++ trunk/qemu-char.c   (working copy)
@@ -1067,17 +1067,17 @@
             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,20 @@
         {
             int sarg = *(int *)arg;
             int targ = 0;
-            if (sarg | CHR_TIOCM_DTR)
+            ioctl(s->fd_in, TIOCMGET, &targ);
+            targ &= ~(CHR_TIOCM_CTS | CHR_TIOCM_CAR | CHR_TIOCM_DSR
+                     | CHR_TIOCM_RI | CHR_TIOCM_DTR | CHR_TIOCM_RTS);
+            if (sarg & CHR_TIOCM_CTS)
+                targ |= TIOCM_CTS;
+            if (sarg & CHR_TIOCM_CAR)
+                targ |= TIOCM_CAR;
+            if (sarg & CHR_TIOCM_DSR)
+                targ |= TIOCM_DSR;
+            if (sarg & CHR_TIOCM_RI)
+                targ |= TIOCM_RI;
+            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);
         }


david

[Qemu-devel] Re: more about serial ports: do they even work?

[Stefano Stabellini]

David S. Ahern wrote:

> 
> Stefano Stabellini wrote:
>> Michael Tokarev wrote:
>>
>>> Other than that, an.. excellent idea, I wanted to propose
>>> just that when I first saw all this stuff, but was somewhat
>>> afraid.  And I *think* there's at least *some* sense.  Qemu
>>> is a CPU emulator and may work on another arch where those
>>> bits are defined differently.  Maybe that was the reason for
>>> all this converting - to be safe than sorry, so to say.  No?
>>>
>> Yes, this is exactly the reason why they were introduced in the first place.
>> Let's suppose that the guest defines those constants differently: we
>> need to parse them and covert them appropriately to the host format.
>> CHR_IOCTL_SERIAL_SET_TIOCM and CHR_TIOCM_DTR correspond to the guest
>> version of TIOCMSET and TIOCM_DTR and can be defined differently
>> depending on the particular guest arch.
> 
> The following works for me.  It fixes the existing checks in place for
> the GET and replicates that for the SET. The ioctl initialization is
> needed in the SET is needed.

It looks pretty good to me.

Acked-by: stefano.stabellini@eu.citrix.com