[PATCH] serial COM: windows serial COM PollingFunc don't sleep

Werner de Carne posted 1 patch 9 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230807201443.2668-1-werner@carne.de
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
chardev/char-win.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
[PATCH] serial COM: windows serial COM PollingFunc don't sleep
Posted by Werner de Carne 9 months, 2 weeks ago
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1802
Signed-off-by: Werner de Carne <werner@carne.de>
---
 chardev/char-win.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/chardev/char-win.c b/chardev/char-win.c
index d4fb44c4dc..92a7016105 100644
--- a/chardev/char-win.c
+++ b/chardev/char-win.c
@@ -28,7 +28,7 @@
 #include "qapi/error.h"
 #include "chardev/char-win.h"
 
-static void win_chr_read(Chardev *chr, DWORD len)
+static int win_chr_read(Chardev *chr, DWORD len)
 {
     WinChardev *s = WIN_CHARDEV(chr);
     int max_size = qemu_chr_be_can_write(chr);
@@ -40,7 +40,7 @@ static void win_chr_read(Chardev *chr, DWORD len)
         len = max_size;
     }
     if (len == 0) {
-        return;
+        return 0;
     }
 
     ZeroMemory(&s->orecv, sizeof(s->orecv));
@@ -56,6 +56,8 @@ static void win_chr_read(Chardev *chr, DWORD len)
     if (size > 0) {
         qemu_chr_be_write(chr, buf, size);
     }
+    
+    return size > 0 ? 1 : 0;
 }
 
 static int win_chr_serial_poll(void *opaque)
@@ -67,8 +69,9 @@ static int win_chr_serial_poll(void *opaque)
 
     ClearCommError(s->file, &comerr, &status);
     if (status.cbInQue > 0) {
-        win_chr_read(chr, status.cbInQue);
-        return 1;
+        if (win_chr_read(chr, status.cbInQue)) {
+			return 1;
+		}
     }
     return 0;
 }
@@ -147,8 +150,9 @@ int win_chr_pipe_poll(void *opaque)
 
     PeekNamedPipe(s->file, NULL, 0, NULL, &size, NULL);
     if (size > 0) {
-        win_chr_read(chr, size);
-        return 1;
+    	if (win_chr_read(chr, size)) {
+			return 1;
+		}
     }
     return 0;
 }
-- 
2.28.0.windows.1
Re: [PATCH] serial COM: windows serial COM PollingFunc don't sleep
Posted by Marc-André Lureau 9 months, 2 weeks ago
Hi Werner

On Tue, Aug 8, 2023 at 12:46 AM Werner de Carne <werner@carne.de> wrote:
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1802
> Signed-off-by: Werner de Carne <werner@carne.de>

This changes the polling callback to return 0 when I/O can't be
processed.  in util/main-loop.c, it results in an early break of
os_host_main_loop_wait().

How does that help?

thanks

> ---
>  chardev/char-win.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/chardev/char-win.c b/chardev/char-win.c
> index d4fb44c4dc..92a7016105 100644
> --- a/chardev/char-win.c
> +++ b/chardev/char-win.c
> @@ -28,7 +28,7 @@
>  #include "qapi/error.h"
>  #include "chardev/char-win.h"
>
> -static void win_chr_read(Chardev *chr, DWORD len)
> +static int win_chr_read(Chardev *chr, DWORD len)
>  {
>      WinChardev *s = WIN_CHARDEV(chr);
>      int max_size = qemu_chr_be_can_write(chr);
> @@ -40,7 +40,7 @@ static void win_chr_read(Chardev *chr, DWORD len)
>          len = max_size;
>      }
>      if (len == 0) {
> -        return;
> +        return 0;
>      }
>
>      ZeroMemory(&s->orecv, sizeof(s->orecv));
> @@ -56,6 +56,8 @@ static void win_chr_read(Chardev *chr, DWORD len)
>      if (size > 0) {
>          qemu_chr_be_write(chr, buf, size);
>      }
> +
> +    return size > 0 ? 1 : 0;
>  }
>
>  static int win_chr_serial_poll(void *opaque)
> @@ -67,8 +69,9 @@ static int win_chr_serial_poll(void *opaque)
>
>      ClearCommError(s->file, &comerr, &status);
>      if (status.cbInQue > 0) {
> -        win_chr_read(chr, status.cbInQue);
> -        return 1;
> +        if (win_chr_read(chr, status.cbInQue)) {
> +                       return 1;
> +               }
>      }
>      return 0;
>  }
> @@ -147,8 +150,9 @@ int win_chr_pipe_poll(void *opaque)
>
>      PeekNamedPipe(s->file, NULL, 0, NULL, &size, NULL);
>      if (size > 0) {
> -        win_chr_read(chr, size);
> -        return 1;
> +       if (win_chr_read(chr, size)) {
> +                       return 1;
> +               }
>      }
>      return 0;
>  }
> --
> 2.28.0.windows.1
>
>


-- 
Marc-André Lureau
Re: [PATCH] serial COM: windows serial COM PollingFunc don't sleep
Posted by Werner de Carne 9 months, 2 weeks ago
Hi Marc-André,

when processing 2 or more characters, the guest machine mps2-an386 (uart 
has a 1 character fifo) will only process one character. Without the 
loop break, the guest gets no computing time and no further character 
can be processed. The guest gets no more computing time and so 
everything freezes. It is also sufficient to connect the uart0 from the 
guest to the Windows host with -serial COMx, but do not activate the 
uart0 in the guest. The receipt of a single character at the Windows 
host also results in the deadlock.

greetings

Am 07.08.2023 um 22:59 schrieb Marc-André Lureau:
> Hi Werner
>
> On Tue, Aug 8, 2023 at 12:46 AM Werner de Carne<werner@carne.de>  wrote:
>> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1802
>> Signed-off-by: Werner de Carne<werner@carne.de>
> This changes the polling callback to return 0 when I/O can't be
> processed.  in util/main-loop.c, it results in an early break of
> os_host_main_loop_wait().
>
> How does that help?
>
> thanks
>
>> ---
>>   chardev/char-win.c | 16 ++++++++++------
>>   1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/chardev/char-win.c b/chardev/char-win.c
>> index d4fb44c4dc..92a7016105 100644
>> --- a/chardev/char-win.c
>> +++ b/chardev/char-win.c
>> @@ -28,7 +28,7 @@
>>   #include "qapi/error.h"
>>   #include "chardev/char-win.h"
>>
>> -static void win_chr_read(Chardev *chr, DWORD len)
>> +static int win_chr_read(Chardev *chr, DWORD len)
>>   {
>>       WinChardev *s = WIN_CHARDEV(chr);
>>       int max_size = qemu_chr_be_can_write(chr);
>> @@ -40,7 +40,7 @@ static void win_chr_read(Chardev *chr, DWORD len)
>>           len = max_size;
>>       }
>>       if (len == 0) {
>> -        return;
>> +        return 0;
>>       }
>>
>>       ZeroMemory(&s->orecv, sizeof(s->orecv));
>> @@ -56,6 +56,8 @@ static void win_chr_read(Chardev *chr, DWORD len)
>>       if (size > 0) {
>>           qemu_chr_be_write(chr, buf, size);
>>       }
>> +
>> +    return size > 0 ? 1 : 0;
>>   }
>>
>>   static int win_chr_serial_poll(void *opaque)
>> @@ -67,8 +69,9 @@ static int win_chr_serial_poll(void *opaque)
>>
>>       ClearCommError(s->file, &comerr, &status);
>>       if (status.cbInQue > 0) {
>> -        win_chr_read(chr, status.cbInQue);
>> -        return 1;
>> +        if (win_chr_read(chr, status.cbInQue)) {
>> +                       return 1;
>> +               }
>>       }
>>       return 0;
>>   }
>> @@ -147,8 +150,9 @@ int win_chr_pipe_poll(void *opaque)
>>
>>       PeekNamedPipe(s->file, NULL, 0, NULL, &size, NULL);
>>       if (size > 0) {
>> -        win_chr_read(chr, size);
>> -        return 1;
>> +       if (win_chr_read(chr, size)) {
>> +                       return 1;
>> +               }
>>       }
>>       return 0;
>>   }
>> --
>> 2.28.0.windows.1
>>
>>
>