[PATCH 0/4] Make serial msmouse work

[PATCH 0/4] Make serial msmouse work

[Arwed Meyer]

This series of patches makes `-serial msmouse` work in practice.

I tested with FreeDOS/CTMouse driver `ctmouse /V` which identifies a
Logitech compatible 3 button mouse.
It will probably run as well with any other compatible serial mouse
driver on Windows 9x etc.

Arwed Meyer (4):
  msmouse: Handle mouse reset
  chardev: src buffer const for write functions
  msmouse: Add pnp data
  serial: Allow unaligned i/o access

 chardev/char.c          |   4 +-
 chardev/msmouse.c       | 150 ++++++++++++++++++++++++++++++++--------
 hw/char/serial.c        |   3 +
 include/chardev/char.h  |   4 +-
 include/sysemu/replay.h |   2 +-
 replay/replay-char.c    |   2 +-
 stubs/replay-tools.c    |   2 +-
 7 files changed, 133 insertions(+), 34 deletions(-)

--
2.34.1

[PATCH 1/4] msmouse: Handle mouse reset

[Arwed Meyer]

Detect mouse reset via RTS or DTR line:
Don't send or process anything while in reset.
When coming out of reset, send ID sequence first thing.
This allows msmouse to be detected by common mouse drivers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
---
 chardev/msmouse.c | 65 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 63 insertions(+), 2 deletions(-)

diff --git a/chardev/msmouse.c b/chardev/msmouse.c
index eb9231dcdb..0ecf26a436 100644
--- a/chardev/msmouse.c
+++ b/chardev/msmouse.c
@@ -25,17 +25,20 @@
 #include "qemu/osdep.h"
 #include "qemu/module.h"
 #include "chardev/char.h"
+#include "chardev/char-serial.h"
 #include "ui/console.h"
 #include "ui/input.h"
 #include "qom/object.h"

-#define MSMOUSE_LO6(n) ((n) & 0x3f)
-#define MSMOUSE_HI2(n) (((n) & 0xc0) >> 6)
+#define MSMOUSE_LO6(n)  ((n) & 0x3f)
+#define MSMOUSE_HI2(n)  (((n) & 0xc0) >> 6)
+#define MSMOUSE_PWR(cm) (cm & (CHR_TIOCM_RTS | CHR_TIOCM_DTR))

 struct MouseChardev {
     Chardev parent;

     QemuInputHandlerState *hs;
+    int tiocm;
     int axis[INPUT_AXIS__MAX];
     bool btns[INPUT_BUTTON__MAX];
     bool btnc[INPUT_BUTTON__MAX];
@@ -109,6 +112,11 @@ static void msmouse_input_event(DeviceState *dev, QemuConsole *src,
     InputMoveEvent *move;
     InputBtnEvent *btn;

+    /* Ignore events if serial mouse powered down. */
+    if (!MSMOUSE_PWR(mouse->tiocm)) {
+        return;
+    }
+
     switch (evt->type) {
     case INPUT_EVENT_KIND_REL:
         move = evt->u.rel.data;
@@ -132,6 +140,11 @@ static void msmouse_input_sync(DeviceState *dev)
     MouseChardev *mouse = MOUSE_CHARDEV(dev);
     Chardev *chr = CHARDEV(dev);

+    /* Ignore events if serial mouse powered down. */
+    if (!MSMOUSE_PWR(mouse->tiocm)) {
+        return;
+    }
+
     msmouse_queue_event(mouse);
     msmouse_chr_accept_input(chr);
 }
@@ -142,6 +155,52 @@ static int msmouse_chr_write(struct Chardev *s, const uint8_t *buf, int len)
     return len;
 }

+static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
+{
+    MouseChardev *mouse = MOUSE_CHARDEV(chr);
+    int c;
+    int *targ = (int *)arg;
+
+    switch (cmd) {
+    case CHR_IOCTL_SERIAL_SET_TIOCM:
+        c = mouse->tiocm;
+        mouse->tiocm = *(int *)arg;
+        if (MSMOUSE_PWR(mouse->tiocm)) {
+            if (!MSMOUSE_PWR(c)) {
+                /*
+                 * Power on after reset: send "M3"
+                 * cause we behave like a 3 button logitech
+                 * mouse.
+                 */
+                mouse->outbuf[0] = 'M';
+                mouse->outbuf[1] = '3';
+                mouse->outlen = 2;
+                /* Start sending data to serial. */
+                msmouse_chr_accept_input(chr);
+            }
+            break;
+        }
+        /*
+         * Reset mouse buffers on power down.
+         * Mouse won't send anything without power.
+         */
+        mouse->outlen = 0;
+        memset(mouse->axis, 0, sizeof(mouse->axis));
+        for (c = INPUT_BUTTON__MAX - 1; c >= 0; c--) {
+            mouse->btns[c] = false;
+            mouse->btnc[c] = false;
+        }
+        break;
+    case CHR_IOCTL_SERIAL_GET_TIOCM:
+        /* Remember line control status. */
+        *targ = mouse->tiocm;
+        break;
+    default:
+        return -ENOTSUP;
+    }
+    return 0;
+}
+
 static void char_msmouse_finalize(Object *obj)
 {
     MouseChardev *mouse = MOUSE_CHARDEV(obj);
@@ -166,6 +225,7 @@ static void msmouse_chr_open(Chardev *chr,
     *be_opened = false;
     mouse->hs = qemu_input_handler_register((DeviceState *)mouse,
                                             &msmouse_handler);
+    mouse->tiocm = 0;
 }

 static void char_msmouse_class_init(ObjectClass *oc, void *data)
@@ -175,6 +235,7 @@ static void char_msmouse_class_init(ObjectClass *oc, void *data)
     cc->open = msmouse_chr_open;
     cc->chr_write = msmouse_chr_write;
     cc->chr_accept_input = msmouse_chr_accept_input;
+    cc->chr_ioctl = msmouse_ioctl;
 }

 static const TypeInfo char_msmouse_type_info = {
--
2.34.1

Re: [PATCH 1/4] msmouse: Handle mouse reset

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 4743 bytes --]

Hi

On Wed, Sep 7, 2022 at 2:03 AM Arwed Meyer <arwed.meyer@gmx.de> wrote:

> Detect mouse reset via RTS or DTR line:
> Don't send or process anything while in reset.
> When coming out of reset, send ID sequence first thing.
> This allows msmouse to be detected by common mouse drivers.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
> Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
> ---
>  chardev/msmouse.c | 65 +++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 63 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/msmouse.c b/chardev/msmouse.c
> index eb9231dcdb..0ecf26a436 100644
> --- a/chardev/msmouse.c
> +++ b/chardev/msmouse.c
> @@ -25,17 +25,20 @@
>  #include "qemu/osdep.h"
>  #include "qemu/module.h"
>  #include "chardev/char.h"
> +#include "chardev/char-serial.h"
>  #include "ui/console.h"
>  #include "ui/input.h"
>  #include "qom/object.h"
>
> -#define MSMOUSE_LO6(n) ((n) & 0x3f)
> -#define MSMOUSE_HI2(n) (((n) & 0xc0) >> 6)
> +#define MSMOUSE_LO6(n)  ((n) & 0x3f)
> +#define MSMOUSE_HI2(n)  (((n) & 0xc0) >> 6)
> +#define MSMOUSE_PWR(cm) (cm & (CHR_TIOCM_RTS | CHR_TIOCM_DTR))
>
>  struct MouseChardev {
>      Chardev parent;
>
>      QemuInputHandlerState *hs;
> +    int tiocm;
>      int axis[INPUT_AXIS__MAX];
>      bool btns[INPUT_BUTTON__MAX];
>      bool btnc[INPUT_BUTTON__MAX];
> @@ -109,6 +112,11 @@ static void msmouse_input_event(DeviceState *dev,
> QemuConsole *src,
>      InputMoveEvent *move;
>      InputBtnEvent *btn;
>
> +    /* Ignore events if serial mouse powered down. */
> +    if (!MSMOUSE_PWR(mouse->tiocm)) {
> +        return;
> +    }
> +
>      switch (evt->type) {
>      case INPUT_EVENT_KIND_REL:
>          move = evt->u.rel.data;
> @@ -132,6 +140,11 @@ static void msmouse_input_sync(DeviceState *dev)
>      MouseChardev *mouse = MOUSE_CHARDEV(dev);
>      Chardev *chr = CHARDEV(dev);
>
> +    /* Ignore events if serial mouse powered down. */
> +    if (!MSMOUSE_PWR(mouse->tiocm)) {
> +        return;
> +    }
> +
>      msmouse_queue_event(mouse);
>      msmouse_chr_accept_input(chr);
>  }
> @@ -142,6 +155,52 @@ static int msmouse_chr_write(struct Chardev *s, const
> uint8_t *buf, int len)
>      return len;
>  }
>
> +static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
> +{
> +    MouseChardev *mouse = MOUSE_CHARDEV(chr);
> +    int c;
> +    int *targ = (int *)arg;
> +
> +    switch (cmd) {
> +    case CHR_IOCTL_SERIAL_SET_TIOCM:
> +        c = mouse->tiocm;
> +        mouse->tiocm = *(int *)arg;
> +        if (MSMOUSE_PWR(mouse->tiocm)) {
> +            if (!MSMOUSE_PWR(c)) {
> +                /*
> +                 * Power on after reset: send "M3"
> +                 * cause we behave like a 3 button logitech
> +                 * mouse.
> +                 */
> +                mouse->outbuf[0] = 'M';
> +                mouse->outbuf[1] = '3';
> +                mouse->outlen = 2;
> +                /* Start sending data to serial. */
> +                msmouse_chr_accept_input(chr);
> +            }
> +            break;
> +        }
> +        /*
> +         * Reset mouse buffers on power down.
> +         * Mouse won't send anything without power.
> +         */
> +        mouse->outlen = 0;
> +        memset(mouse->axis, 0, sizeof(mouse->axis));
> +        for (c = INPUT_BUTTON__MAX - 1; c >= 0; c--) {
> +            mouse->btns[c] = false;
> +            mouse->btnc[c] = false;
> +        }
>

Why not memset those fields as well?


> +        break;
> +    case CHR_IOCTL_SERIAL_GET_TIOCM:
> +        /* Remember line control status. */
> +        *targ = mouse->tiocm;
> +        break;
> +    default:
> +        return -ENOTSUP;
> +    }
> +    return 0;
> +}
> +
>  static void char_msmouse_finalize(Object *obj)
>  {
>      MouseChardev *mouse = MOUSE_CHARDEV(obj);
> @@ -166,6 +225,7 @@ static void msmouse_chr_open(Chardev *chr,
>      *be_opened = false;
>      mouse->hs = qemu_input_handler_register((DeviceState *)mouse,
>                                              &msmouse_handler);
> +    mouse->tiocm = 0;
>  }
>
>  static void char_msmouse_class_init(ObjectClass *oc, void *data)
> @@ -175,6 +235,7 @@ static void char_msmouse_class_init(ObjectClass *oc,
> void *data)
>      cc->open = msmouse_chr_open;
>      cc->chr_write = msmouse_chr_write;
>      cc->chr_accept_input = msmouse_chr_accept_input;
> +    cc->chr_ioctl = msmouse_ioctl;
>  }
>
>  static const TypeInfo char_msmouse_type_info = {
> --
> 2.34.1
>
>
>
lgtm otherwise,
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 6332 bytes --]

Re: [PATCH 1/4] msmouse: Handle mouse reset

[Arwed Meyer]

Am 08.09.22 um 11:45 schrieb Marc-André Lureau:
> Hi
>
> On Wed, Sep 7, 2022 at 2:03 AM Arwed Meyer <arwed.meyer@gmx.de
> <mailto:arwed.meyer@gmx.de>> wrote:
>
>     Detect mouse reset via RTS or DTR line:
>     Don't send or process anything while in reset.
>     When coming out of reset, send ID sequence first thing.
>     This allows msmouse to be detected by common mouse drivers.
>
>     Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
>     <https://gitlab.com/qemu-project/qemu/-/issues/77>
>     Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de
>     <mailto:arwed.meyer@gmx.de>>
>     ---
>       chardev/msmouse.c | 65 +++++++++++++++++++++++++++++++++++++++++++++--
>       1 file changed, 63 insertions(+), 2 deletions(-)
>
>     diff --git a/chardev/msmouse.c b/chardev/msmouse.c
>     index eb9231dcdb..0ecf26a436 100644
>     --- a/chardev/msmouse.c
>     +++ b/chardev/msmouse.c
>     @@ -25,17 +25,20 @@
>       #include "qemu/osdep.h"
>       #include "qemu/module.h"
>       #include "chardev/char.h"
>     +#include "chardev/char-serial.h"
>       #include "ui/console.h"
>       #include "ui/input.h"
>       #include "qom/object.h"
>
>     -#define MSMOUSE_LO6(n) ((n) & 0x3f)
>     -#define MSMOUSE_HI2(n) (((n) & 0xc0) >> 6)
>     +#define MSMOUSE_LO6(n)  ((n) & 0x3f)
>     +#define MSMOUSE_HI2(n)  (((n) & 0xc0) >> 6)
>     +#define MSMOUSE_PWR(cm) (cm & (CHR_TIOCM_RTS | CHR_TIOCM_DTR))
>
>       struct MouseChardev {
>           Chardev parent;
>
>           QemuInputHandlerState *hs;
>     +    int tiocm;
>           int axis[INPUT_AXIS__MAX];
>           bool btns[INPUT_BUTTON__MAX];
>           bool btnc[INPUT_BUTTON__MAX];
>     @@ -109,6 +112,11 @@ static void msmouse_input_event(DeviceState
>     *dev, QemuConsole *src,
>           InputMoveEvent *move;
>           InputBtnEvent *btn;
>
>     +    /* Ignore events if serial mouse powered down. */
>     +    if (!MSMOUSE_PWR(mouse->tiocm)) {
>     +        return;
>     +    }
>     +
>           switch (evt->type) {
>           case INPUT_EVENT_KIND_REL:
>               move = evt->u.rel.data;
>     @@ -132,6 +140,11 @@ static void msmouse_input_sync(DeviceState *dev)
>           MouseChardev *mouse = MOUSE_CHARDEV(dev);
>           Chardev *chr = CHARDEV(dev);
>
>     +    /* Ignore events if serial mouse powered down. */
>     +    if (!MSMOUSE_PWR(mouse->tiocm)) {
>     +        return;
>     +    }
>     +
>           msmouse_queue_event(mouse);
>           msmouse_chr_accept_input(chr);
>       }
>     @@ -142,6 +155,52 @@ static int msmouse_chr_write(struct Chardev *s,
>     const uint8_t *buf, int len)
>           return len;
>       }
>
>     +static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
>     +{
>     +    MouseChardev *mouse = MOUSE_CHARDEV(chr);
>     +    int c;
>     +    int *targ = (int *)arg;
>     +
>     +    switch (cmd) {
>     +    case CHR_IOCTL_SERIAL_SET_TIOCM:
>     +        c = mouse->tiocm;
>     +        mouse->tiocm = *(int *)arg;
>     +        if (MSMOUSE_PWR(mouse->tiocm)) {
>     +            if (!MSMOUSE_PWR(c)) {
>     +                /*
>     +                 * Power on after reset: send "M3"
>     +                 * cause we behave like a 3 button logitech
>     +                 * mouse.
>     +                 */
>     +                mouse->outbuf[0] = 'M';
>     +                mouse->outbuf[1] = '3';
>     +                mouse->outlen = 2;
>     +                /* Start sending data to serial. */
>     +                msmouse_chr_accept_input(chr);
>     +            }
>     +            break;
>     +        }
>     +        /*
>     +         * Reset mouse buffers on power down.
>     +         * Mouse won't send anything without power.
>     +         */
>     +        mouse->outlen = 0;
>     +        memset(mouse->axis, 0, sizeof(mouse->axis));
>     +        for (c = INPUT_BUTTON__MAX - 1; c >= 0; c--) {
>     +            mouse->btns[c] = false;
>     +            mouse->btnc[c] = false;
>     +        }
>
>
> Why not memset those fields as well?
>
>     +        break;
>     +    case CHR_IOCTL_SERIAL_GET_TIOCM:
>     +        /* Remember line control status. */
>     +        *targ = mouse->tiocm;
>     +        break;
>     +    default:
>     +        return -ENOTSUP;
>     +    }
>     +    return 0;
>     +}
>     +
>       static void char_msmouse_finalize(Object *obj)
>       {
>           MouseChardev *mouse = MOUSE_CHARDEV(obj);
>     @@ -166,6 +225,7 @@ static void msmouse_chr_open(Chardev *chr,
>           *be_opened = false;
>           mouse->hs = qemu_input_handler_register((DeviceState *)mouse,
>                                                   &msmouse_handler);
>     +    mouse->tiocm = 0;
>       }
>
>       static void char_msmouse_class_init(ObjectClass *oc, void *data)
>     @@ -175,6 +235,7 @@ static void char_msmouse_class_init(ObjectClass
>     *oc, void *data)
>           cc->open = msmouse_chr_open;
>           cc->chr_write = msmouse_chr_write;
>           cc->chr_accept_input = msmouse_chr_accept_input;
>     +    cc->chr_ioctl = msmouse_ioctl;
>       }
>
>       static const TypeInfo char_msmouse_type_info = {
>     --
>     2.34.1
>
>
>
> lgtm otherwise,
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com
> <mailto:marcandre.lureau@redhat.com>>
>
> --
> Marc-André Lureau
Hi,

old reflexes. Throwing memset at bool didn't really feel well. I
reconsidered in v2 patchset.

[PATCH 2/4] chardev: src buffer const for write functions

[Arwed Meyer]

Make source buffers const for char be write functions.
This allows using buffers returned by fifo as buf parameter and source buffer
should not be changed by write functions anyway.

Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
---
 chardev/char.c          | 4 ++--
 include/chardev/char.h  | 4 ++--
 include/sysemu/replay.h | 2 +-
 replay/replay-char.c    | 2 +-
 stubs/replay-tools.c    | 2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/chardev/char.c b/chardev/char.c
index 0169d8dde4..b005df3ccf 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -193,7 +193,7 @@ int qemu_chr_be_can_write(Chardev *s)
     return be->chr_can_read(be->opaque);
 }

-void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len)
+void qemu_chr_be_write_impl(Chardev *s, const uint8_t *buf, int len)
 {
     CharBackend *be = s->be;

@@ -202,7 +202,7 @@ void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len)
     }
 }

-void qemu_chr_be_write(Chardev *s, uint8_t *buf, int len)
+void qemu_chr_be_write(Chardev *s, const uint8_t *buf, int len)
 {
     if (qemu_chr_replay(s)) {
         if (replay_mode == REPLAY_MODE_PLAY) {
diff --git a/include/chardev/char.h b/include/chardev/char.h
index a319b5fdff..44cd82e405 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -186,7 +186,7 @@ int qemu_chr_be_can_write(Chardev *s);
  * the caller should call @qemu_chr_be_can_write to determine how much data
  * the front end can currently accept.
  */
-void qemu_chr_be_write(Chardev *s, uint8_t *buf, int len);
+void qemu_chr_be_write(Chardev *s, const uint8_t *buf, int len);

 /**
  * qemu_chr_be_write_impl:
@@ -195,7 +195,7 @@ void qemu_chr_be_write(Chardev *s, uint8_t *buf, int len);
  *
  * Implementation of back end writing. Used by replay module.
  */
-void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len);
+void qemu_chr_be_write_impl(Chardev *s, const uint8_t *buf, int len);

 /**
  * qemu_chr_be_update_read_handlers:
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 73dee9ccdf..7ec0882b50 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -198,7 +198,7 @@ uint64_t blkreplay_next_id(void);
 /*! Registers char driver to save it's events */
 void replay_register_char_driver(struct Chardev *chr);
 /*! Saves write to char device event to the log */
-void replay_chr_be_write(struct Chardev *s, uint8_t *buf, int len);
+void replay_chr_be_write(struct Chardev *s, const uint8_t *buf, int len);
 /*! Writes char write return value to the replay log. */
 void replay_char_write_event_save(int res, int offset);
 /*! Reads char write return value from the replay log. */
diff --git a/replay/replay-char.c b/replay/replay-char.c
index d2025948cf..a31aded032 100644
--- a/replay/replay-char.c
+++ b/replay/replay-char.c
@@ -48,7 +48,7 @@ void replay_register_char_driver(Chardev *chr)
     char_drivers[drivers_count++] = chr;
 }

-void replay_chr_be_write(Chardev *s, uint8_t *buf, int len)
+void replay_chr_be_write(Chardev *s, const uint8_t *buf, int len)
 {
     CharEvent *event = g_new0(CharEvent, 1);

diff --git a/stubs/replay-tools.c b/stubs/replay-tools.c
index f2e72bb225..3e8ca3212d 100644
--- a/stubs/replay-tools.c
+++ b/stubs/replay-tools.c
@@ -53,7 +53,7 @@ void replay_register_char_driver(struct Chardev *chr)
 {
 }

-void replay_chr_be_write(struct Chardev *s, uint8_t *buf, int len)
+void replay_chr_be_write(struct Chardev *s, const uint8_t *buf, int len)
 {
     abort();
 }
--
2.34.1

Re: [PATCH 2/4] chardev: src buffer const for write functions

[Philippe Mathieu-Daudé via]

On Wed, Sep 7, 2022 at 12:07 AM Arwed Meyer <arwed.meyer@gmx.de> wrote:
>
> Make source buffers const for char be write functions.
> This allows using buffers returned by fifo as buf parameter and source buffer
> should not be changed by write functions anyway.
>
> Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
> ---
>  chardev/char.c          | 4 ++--
>  include/chardev/char.h  | 4 ++--
>  include/sysemu/replay.h | 2 +-
>  replay/replay-char.c    | 2 +-
>  stubs/replay-tools.c    | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)

> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index a319b5fdff..44cd82e405 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -186,7 +186,7 @@ int qemu_chr_be_can_write(Chardev *s);
>   * the caller should call @qemu_chr_be_can_write to determine how much data
>   * the front end can currently accept.
>   */
> -void qemu_chr_be_write(Chardev *s, uint8_t *buf, int len);
> +void qemu_chr_be_write(Chardev *s, const uint8_t *buf, int len);
>
>  /**
>   * qemu_chr_be_write_impl:
> @@ -195,7 +195,7 @@ void qemu_chr_be_write(Chardev *s, uint8_t *buf, int len);
>   *
>   * Implementation of back end writing. Used by replay module.
>   */
> -void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len);
> +void qemu_chr_be_write_impl(Chardev *s, const uint8_t *buf, int len);

Doh this reminds me of this series:
https://lore.kernel.org/qemu-devel/20190220010232.18731-1-philmd@redhat.com/

Re: [PATCH 2/4] chardev: src buffer const for write functions

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 3977 bytes --]

Hi


On Wed, Sep 7, 2022 at 2:05 AM Arwed Meyer <arwed.meyer@gmx.de> wrote:

> Make source buffers const for char be write functions.
> This allows using buffers returned by fifo as buf parameter and source
> buffer
> should not be changed by write functions anyway.
>
> Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---
>  chardev/char.c          | 4 ++--
>  include/chardev/char.h  | 4 ++--
>  include/sysemu/replay.h | 2 +-
>  replay/replay-char.c    | 2 +-
>  stubs/replay-tools.c    | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 0169d8dde4..b005df3ccf 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -193,7 +193,7 @@ int qemu_chr_be_can_write(Chardev *s)
>      return be->chr_can_read(be->opaque);
>  }
>
> -void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len)
> +void qemu_chr_be_write_impl(Chardev *s, const uint8_t *buf, int len)
>  {
>      CharBackend *be = s->be;
>
> @@ -202,7 +202,7 @@ void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf,
> int len)
>      }
>  }
>
> -void qemu_chr_be_write(Chardev *s, uint8_t *buf, int len)
> +void qemu_chr_be_write(Chardev *s, const uint8_t *buf, int len)
>  {
>      if (qemu_chr_replay(s)) {
>          if (replay_mode == REPLAY_MODE_PLAY) {
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index a319b5fdff..44cd82e405 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -186,7 +186,7 @@ int qemu_chr_be_can_write(Chardev *s);
>   * the caller should call @qemu_chr_be_can_write to determine how much
> data
>   * the front end can currently accept.
>   */
> -void qemu_chr_be_write(Chardev *s, uint8_t *buf, int len);
> +void qemu_chr_be_write(Chardev *s, const uint8_t *buf, int len);
>
>  /**
>   * qemu_chr_be_write_impl:
> @@ -195,7 +195,7 @@ void qemu_chr_be_write(Chardev *s, uint8_t *buf, int
> len);
>   *
>   * Implementation of back end writing. Used by replay module.
>   */
> -void qemu_chr_be_write_impl(Chardev *s, uint8_t *buf, int len);
> +void qemu_chr_be_write_impl(Chardev *s, const uint8_t *buf, int len);
>
>  /**
>   * qemu_chr_be_update_read_handlers:
> diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
> index 73dee9ccdf..7ec0882b50 100644
> --- a/include/sysemu/replay.h
> +++ b/include/sysemu/replay.h
> @@ -198,7 +198,7 @@ uint64_t blkreplay_next_id(void);
>  /*! Registers char driver to save it's events */
>  void replay_register_char_driver(struct Chardev *chr);
>  /*! Saves write to char device event to the log */
> -void replay_chr_be_write(struct Chardev *s, uint8_t *buf, int len);
> +void replay_chr_be_write(struct Chardev *s, const uint8_t *buf, int len);
>  /*! Writes char write return value to the replay log. */
>  void replay_char_write_event_save(int res, int offset);
>  /*! Reads char write return value from the replay log. */
> diff --git a/replay/replay-char.c b/replay/replay-char.c
> index d2025948cf..a31aded032 100644
> --- a/replay/replay-char.c
> +++ b/replay/replay-char.c
> @@ -48,7 +48,7 @@ void replay_register_char_driver(Chardev *chr)
>      char_drivers[drivers_count++] = chr;
>  }
>
> -void replay_chr_be_write(Chardev *s, uint8_t *buf, int len)
> +void replay_chr_be_write(Chardev *s, const uint8_t *buf, int len)
>  {
>      CharEvent *event = g_new0(CharEvent, 1);
>
> diff --git a/stubs/replay-tools.c b/stubs/replay-tools.c
> index f2e72bb225..3e8ca3212d 100644
> --- a/stubs/replay-tools.c
> +++ b/stubs/replay-tools.c
> @@ -53,7 +53,7 @@ void replay_register_char_driver(struct Chardev *chr)
>  {
>  }
>
> -void replay_chr_be_write(struct Chardev *s, uint8_t *buf, int len)
> +void replay_chr_be_write(struct Chardev *s, const uint8_t *buf, int len)
>  {
>      abort();
>  }
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 4913 bytes --]

[PATCH 3/4] msmouse: Add pnp data

[Arwed Meyer]

Make msmouse send serial pnp data.
Enables you to see nice qemu device name in Win9x.

Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
---
 chardev/msmouse.c | 101 +++++++++++++++++++++++++++++++---------------
 1 file changed, 68 insertions(+), 33 deletions(-)

diff --git a/chardev/msmouse.c b/chardev/msmouse.c
index 0ecf26a436..b4ddaee778 100644
--- a/chardev/msmouse.c
+++ b/chardev/msmouse.c
@@ -24,6 +24,7 @@

 #include "qemu/osdep.h"
 #include "qemu/module.h"
+#include "qemu/fifo8.h"
 #include "chardev/char.h"
 #include "chardev/char-serial.h"
 #include "ui/console.h"
@@ -34,6 +35,25 @@
 #define MSMOUSE_HI2(n)  (((n) & 0xc0) >> 6)
 #define MSMOUSE_PWR(cm) (cm & (CHR_TIOCM_RTS | CHR_TIOCM_DTR))

+/* Serial PnP for 6 bit devices/mice sends all ASCII chars - 0x20 */
+#define M(c) (c - 0x20)
+/* Serial fifo size. */
+#define MSMOUSE_BUF_SZ 64
+
+/* Mouse ID: Send "M3" cause we behave like a 3 button logitech mouse. */
+const uint8_t mouse_id[] = {'M', '3'};
+/*
+ * PnP start "(", PnP version (1.0), vendor ID, product ID, '\\',
+ * serial ID (omitted), '\\', MS class name, '\\', driver ID (omitted), '\\',
+ * product description, checksum, ")"
+ * Missing parts are inserted later.
+ */
+const uint8_t pnp_data[] = {M('('), 1, '$', M('Q'), M('M'), M('U'),
+                         M('0'), M('0'), M('0'), M('1'),
+                         M('\\'), M('\\'),
+                         M('M'), M('O'), M('U'), M('S'), M('E'),
+                         M('\\'), M('\\')};
+
 struct MouseChardev {
     Chardev parent;

@@ -42,8 +62,7 @@ struct MouseChardev {
     int axis[INPUT_AXIS__MAX];
     bool btns[INPUT_BUTTON__MAX];
     bool btnc[INPUT_BUTTON__MAX];
-    uint8_t outbuf[32];
-    int outlen;
+    Fifo8 outbuf;
 };
 typedef struct MouseChardev MouseChardev;

@@ -54,21 +73,15 @@ DECLARE_INSTANCE_CHECKER(MouseChardev, MOUSE_CHARDEV,
 static void msmouse_chr_accept_input(Chardev *chr)
 {
     MouseChardev *mouse = MOUSE_CHARDEV(chr);
-    int len;
+    uint32_t len_out, len;

-    len = qemu_chr_be_can_write(chr);
-    if (len > mouse->outlen) {
-        len = mouse->outlen;
-    }
-    if (!len) {
+    len_out = qemu_chr_be_can_write(chr);
+    if (!len_out || fifo8_is_empty(&mouse->outbuf)) {
         return;
     }
-
-    qemu_chr_be_write(chr, mouse->outbuf, len);
-    mouse->outlen -= len;
-    if (mouse->outlen) {
-        memmove(mouse->outbuf, mouse->outbuf + len, mouse->outlen);
-    }
+    len = MIN(fifo8_num_used(&mouse->outbuf), len_out);
+    qemu_chr_be_write(chr, fifo8_pop_buf(&mouse->outbuf, len, &len_out),
+            len_out);
 }

 static void msmouse_queue_event(MouseChardev *mouse)
@@ -94,12 +107,11 @@ static void msmouse_queue_event(MouseChardev *mouse)
         mouse->btnc[INPUT_BUTTON_MIDDLE]) {
         bytes[3] |= (mouse->btns[INPUT_BUTTON_MIDDLE] ? 0x20 : 0x00);
         mouse->btnc[INPUT_BUTTON_MIDDLE] = false;
-        count = 4;
+        count++;
     }

-    if (mouse->outlen <= sizeof(mouse->outbuf) - count) {
-        memcpy(mouse->outbuf + mouse->outlen, bytes, count);
-        mouse->outlen += count;
+    if (fifo8_num_free(&mouse->outbuf) >= count) {
+        fifo8_push_all(&mouse->outbuf, bytes, count);
     } else {
         /* queue full -> drop event */
     }
@@ -155,11 +167,22 @@ static int msmouse_chr_write(struct Chardev *s, const uint8_t *buf, int len)
     return len;
 }

+static QemuInputHandler msmouse_handler = {
+    .name  = "QEMU Microsoft Mouse",
+    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
+    .event = msmouse_input_event,
+    .sync  = msmouse_input_sync,
+};
+
 static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
 {
     MouseChardev *mouse = MOUSE_CHARDEV(chr);
-    int c;
+    int c, i, j;
+    uint8_t bytes[MSMOUSE_BUF_SZ / 2];
     int *targ = (int *)arg;
+    const uint8_t hexchr[16] = {M('0'), M('1'), M('2'), M('3'), M('4'), M('5'),
+                             M('6'), M('7'), M('8'), M('9'), M('A'), M('B'),
+                             M('C'), M('D'), M('E'), M('F')};

     switch (cmd) {
     case CHR_IOCTL_SERIAL_SET_TIOCM:
@@ -168,13 +191,30 @@ static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
         if (MSMOUSE_PWR(mouse->tiocm)) {
             if (!MSMOUSE_PWR(c)) {
                 /*
-                 * Power on after reset: send "M3"
-                 * cause we behave like a 3 button logitech
-                 * mouse.
+                 * Power on after reset: Send ID and PnP data
+                 * No need to check fifo space as it is empty at this point.
+                 */
+                fifo8_push_all(&mouse->outbuf, mouse_id, sizeof(mouse_id));
+                /* Add PnP data: */
+                fifo8_push_all(&mouse->outbuf, pnp_data, sizeof(pnp_data));
+                /*
+                 * Add device description from qemu handler name.
+                 * Make sure this all fits into the queue beforehand!
                  */
-                mouse->outbuf[0] = 'M';
-                mouse->outbuf[1] = '3';
-                mouse->outlen = 2;
+                c = M(')');
+                for (i = 0; msmouse_handler.name[i]; i++) {
+                    bytes[i] = M(msmouse_handler.name[i]);
+                    c += bytes[i];
+                }
+                /* Calc more of checksum */
+                for (j = 0; j < sizeof(pnp_data); j++) {
+                    c += pnp_data[j];
+                }
+                c &= 0xff;
+                bytes[i++] = hexchr[c >> 4];
+                bytes[i++] = hexchr[c & 0x0f];
+                bytes[i++] = M(')');
+                fifo8_push_all(&mouse->outbuf, bytes, i);
                 /* Start sending data to serial. */
                 msmouse_chr_accept_input(chr);
             }
@@ -184,7 +224,7 @@ static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
          * Reset mouse buffers on power down.
          * Mouse won't send anything without power.
          */
-        mouse->outlen = 0;
+        fifo8_reset(&mouse->outbuf);
         memset(mouse->axis, 0, sizeof(mouse->axis));
         for (c = INPUT_BUTTON__MAX - 1; c >= 0; c--) {
             mouse->btns[c] = false;
@@ -206,15 +246,9 @@ static void char_msmouse_finalize(Object *obj)
     MouseChardev *mouse = MOUSE_CHARDEV(obj);

     qemu_input_handler_unregister(mouse->hs);
+    fifo8_destroy(&mouse->outbuf);
 }

-static QemuInputHandler msmouse_handler = {
-    .name  = "QEMU Microsoft Mouse",
-    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
-    .event = msmouse_input_event,
-    .sync  = msmouse_input_sync,
-};
-
 static void msmouse_chr_open(Chardev *chr,
                              ChardevBackend *backend,
                              bool *be_opened,
@@ -226,6 +260,7 @@ static void msmouse_chr_open(Chardev *chr,
     mouse->hs = qemu_input_handler_register((DeviceState *)mouse,
                                             &msmouse_handler);
     mouse->tiocm = 0;
+    fifo8_create(&mouse->outbuf, MSMOUSE_BUF_SZ);
 }

 static void char_msmouse_class_init(ObjectClass *oc, void *data)
--
2.34.1

Re: [PATCH 3/4] msmouse: Add pnp data

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 7857 bytes --]

Hi

On Wed, Sep 7, 2022 at 2:05 AM Arwed Meyer <arwed.meyer@gmx.de> wrote:

> Make msmouse send serial pnp data.
> Enables you to see nice qemu device name in Win9x.
>
> Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
> ---
>  chardev/msmouse.c | 101 +++++++++++++++++++++++++++++++---------------
>  1 file changed, 68 insertions(+), 33 deletions(-)
>
> diff --git a/chardev/msmouse.c b/chardev/msmouse.c
> index 0ecf26a436..b4ddaee778 100644
> --- a/chardev/msmouse.c
> +++ b/chardev/msmouse.c
> @@ -24,6 +24,7 @@
>
>  #include "qemu/osdep.h"
>  #include "qemu/module.h"
> +#include "qemu/fifo8.h"
>  #include "chardev/char.h"
>  #include "chardev/char-serial.h"
>  #include "ui/console.h"
> @@ -34,6 +35,25 @@
>  #define MSMOUSE_HI2(n)  (((n) & 0xc0) >> 6)
>  #define MSMOUSE_PWR(cm) (cm & (CHR_TIOCM_RTS | CHR_TIOCM_DTR))
>
> +/* Serial PnP for 6 bit devices/mice sends all ASCII chars - 0x20 */
> +#define M(c) (c - 0x20)
> +/* Serial fifo size. */
> +#define MSMOUSE_BUF_SZ 64
> +
> +/* Mouse ID: Send "M3" cause we behave like a 3 button logitech mouse. */
> +const uint8_t mouse_id[] = {'M', '3'};
> +/*
> + * PnP start "(", PnP version (1.0), vendor ID, product ID, '\\',
> + * serial ID (omitted), '\\', MS class name, '\\', driver ID (omitted),
> '\\',
> + * product description, checksum, ")"
> + * Missing parts are inserted later.
> + */
> +const uint8_t pnp_data[] = {M('('), 1, '$', M('Q'), M('M'), M('U'),
> +                         M('0'), M('0'), M('0'), M('1'),
> +                         M('\\'), M('\\'),
> +                         M('M'), M('O'), M('U'), M('S'), M('E'),
> +                         M('\\'), M('\\')};
> +
>  struct MouseChardev {
>      Chardev parent;
>
> @@ -42,8 +62,7 @@ struct MouseChardev {
>      int axis[INPUT_AXIS__MAX];
>      bool btns[INPUT_BUTTON__MAX];
>      bool btnc[INPUT_BUTTON__MAX];
> -    uint8_t outbuf[32];
> -    int outlen;
> +    Fifo8 outbuf;
>

Could you make this outbuf replacement a different patch?


>  };
>  typedef struct MouseChardev MouseChardev;
>
> @@ -54,21 +73,15 @@ DECLARE_INSTANCE_CHECKER(MouseChardev, MOUSE_CHARDEV,
>  static void msmouse_chr_accept_input(Chardev *chr)
>  {
>      MouseChardev *mouse = MOUSE_CHARDEV(chr);
> -    int len;
> +    uint32_t len_out, len;
>
> -    len = qemu_chr_be_can_write(chr);
> -    if (len > mouse->outlen) {
> -        len = mouse->outlen;
> -    }
> -    if (!len) {
> +    len_out = qemu_chr_be_can_write(chr);
> +    if (!len_out || fifo8_is_empty(&mouse->outbuf)) {
>          return;
>      }
> -
> -    qemu_chr_be_write(chr, mouse->outbuf, len);
> -    mouse->outlen -= len;
> -    if (mouse->outlen) {
> -        memmove(mouse->outbuf, mouse->outbuf + len, mouse->outlen);
> -    }
> +    len = MIN(fifo8_num_used(&mouse->outbuf), len_out);
> +    qemu_chr_be_write(chr, fifo8_pop_buf(&mouse->outbuf, len, &len_out),
> +            len_out);
>  }
>
>  static void msmouse_queue_event(MouseChardev *mouse)
> @@ -94,12 +107,11 @@ static void msmouse_queue_event(MouseChardev *mouse)
>          mouse->btnc[INPUT_BUTTON_MIDDLE]) {
>          bytes[3] |= (mouse->btns[INPUT_BUTTON_MIDDLE] ? 0x20 : 0x00);
>          mouse->btnc[INPUT_BUTTON_MIDDLE] = false;
> -        count = 4;
> +        count++;
>      }
>
> -    if (mouse->outlen <= sizeof(mouse->outbuf) - count) {
> -        memcpy(mouse->outbuf + mouse->outlen, bytes, count);
> -        mouse->outlen += count;
> +    if (fifo8_num_free(&mouse->outbuf) >= count) {
> +        fifo8_push_all(&mouse->outbuf, bytes, count);
>      } else {
>          /* queue full -> drop event */
>      }
> @@ -155,11 +167,22 @@ static int msmouse_chr_write(struct Chardev *s,
> const uint8_t *buf, int len)
>      return len;
>  }
>
> +static QemuInputHandler msmouse_handler = {
> +    .name  = "QEMU Microsoft Mouse",
> +    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
> +    .event = msmouse_input_event,
> +    .sync  = msmouse_input_sync,
> +};
> +
>  static int msmouse_ioctl(Chardev *chr, int cmd, void *arg)
>  {
>      MouseChardev *mouse = MOUSE_CHARDEV(chr);
> -    int c;
> +    int c, i, j;
> +    uint8_t bytes[MSMOUSE_BUF_SZ / 2];
>      int *targ = (int *)arg;
> +    const uint8_t hexchr[16] = {M('0'), M('1'), M('2'), M('3'), M('4'),
> M('5'),
> +                             M('6'), M('7'), M('8'), M('9'), M('A'),
> M('B'),
> +                             M('C'), M('D'), M('E'), M('F')};
>
>      switch (cmd) {
>      case CHR_IOCTL_SERIAL_SET_TIOCM:
> @@ -168,13 +191,30 @@ static int msmouse_ioctl(Chardev *chr, int cmd, void
> *arg)
>          if (MSMOUSE_PWR(mouse->tiocm)) {
>              if (!MSMOUSE_PWR(c)) {
>                  /*
> -                 * Power on after reset: send "M3"
> -                 * cause we behave like a 3 button logitech
> -                 * mouse.
> +                 * Power on after reset: Send ID and PnP data
> +                 * No need to check fifo space as it is empty at this
> point.
> +                 */
> +                fifo8_push_all(&mouse->outbuf, mouse_id,
> sizeof(mouse_id));
> +                /* Add PnP data: */
> +                fifo8_push_all(&mouse->outbuf, pnp_data,
> sizeof(pnp_data));
> +                /*
> +                 * Add device description from qemu handler name.
> +                 * Make sure this all fits into the queue beforehand!
>                   */
> -                mouse->outbuf[0] = 'M';
> -                mouse->outbuf[1] = '3';
> -                mouse->outlen = 2;
> +                c = M(')');
> +                for (i = 0; msmouse_handler.name[i]; i++) {
> +                    bytes[i] = M(msmouse_handler.name[i]);
> +                    c += bytes[i];
> +                }
> +                /* Calc more of checksum */
> +                for (j = 0; j < sizeof(pnp_data); j++) {
> +                    c += pnp_data[j];
> +                }
> +                c &= 0xff;
> +                bytes[i++] = hexchr[c >> 4];
> +                bytes[i++] = hexchr[c & 0x0f];
> +                bytes[i++] = M(')');
> +                fifo8_push_all(&mouse->outbuf, bytes, i);
>                  /* Start sending data to serial. */
>                  msmouse_chr_accept_input(chr);
>              }
> @@ -184,7 +224,7 @@ static int msmouse_ioctl(Chardev *chr, int cmd, void
> *arg)
>           * Reset mouse buffers on power down.
>           * Mouse won't send anything without power.
>           */
> -        mouse->outlen = 0;
> +        fifo8_reset(&mouse->outbuf);
>          memset(mouse->axis, 0, sizeof(mouse->axis));
>          for (c = INPUT_BUTTON__MAX - 1; c >= 0; c--) {
>              mouse->btns[c] = false;
> @@ -206,15 +246,9 @@ static void char_msmouse_finalize(Object *obj)
>      MouseChardev *mouse = MOUSE_CHARDEV(obj);
>
>      qemu_input_handler_unregister(mouse->hs);
> +    fifo8_destroy(&mouse->outbuf);
>  }
>
> -static QemuInputHandler msmouse_handler = {
> -    .name  = "QEMU Microsoft Mouse",
> -    .mask  = INPUT_EVENT_MASK_BTN | INPUT_EVENT_MASK_REL,
> -    .event = msmouse_input_event,
> -    .sync  = msmouse_input_sync,
> -};
> -
>
 static void msmouse_chr_open(Chardev *chr,
>                               ChardevBackend *backend,
>                               bool *be_opened,
> @@ -226,6 +260,7 @@ static void msmouse_chr_open(Chardev *chr,
>      mouse->hs = qemu_input_handler_register((DeviceState *)mouse,
>                                              &msmouse_handler);
>      mouse->tiocm = 0;
> +    fifo8_create(&mouse->outbuf, MSMOUSE_BUF_SZ);
>
 }
>
>  static void char_msmouse_class_init(ObjectClass *oc, void *data)
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 10625 bytes --]

[PATCH 4/4] serial: Allow unaligned i/o access

[Arwed Meyer]

Unaligned i/o access on serial UART works on real PCs.
This is used for example by FreeDOS CTMouse driver. Without this it
can't reset and detect serial mice.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
---
 hw/char/serial.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/char/serial.c b/hw/char/serial.c
index 7061aacbce..41b5e61977 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -961,6 +961,9 @@ void serial_set_frequency(SerialState *s, uint32_t frequency)
 const MemoryRegionOps serial_io_ops = {
     .read = serial_ioport_read,
     .write = serial_ioport_write,
+    .valid = {
+        .unaligned = 1,
+    },
     .impl = {
         .min_access_size = 1,
         .max_access_size = 1,
--
2.34.1

Re: [PATCH 4/4] serial: Allow unaligned i/o access

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

Hi

On Wed, Sep 7, 2022 at 2:03 AM Arwed Meyer <arwed.meyer@gmx.de> wrote:

> Unaligned i/o access on serial UART works on real PCs.
> This is used for example by FreeDOS CTMouse driver. Without this it
> can't reset and detect serial mice.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
> Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
> ---
>  hw/char/serial.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/hw/char/serial.c b/hw/char/serial.c
> index 7061aacbce..41b5e61977 100644
> --- a/hw/char/serial.c
> +++ b/hw/char/serial.c
> @@ -961,6 +961,9 @@ void serial_set_frequency(SerialState *s, uint32_t
> frequency)
>  const MemoryRegionOps serial_io_ops = {
>      .read = serial_ioport_read,
>      .write = serial_ioport_write,
> +    .valid = {
> +        .unaligned = 1,
> +    },
>

I don't get how this can help if both min_access_size & max_access_size are
1.


>      .impl = {
>          .min_access_size = 1,
>          .max_access_size = 1,
> --
> 2.34.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1863 bytes --]

Re: [PATCH 4/4] serial: Allow unaligned i/o access

[Michael S. Tsirkin]

On Thu, Sep 08, 2022 at 02:11:28PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Sep 7, 2022 at 2:03 AM Arwed Meyer <arwed.meyer@gmx.de> wrote:
> 
>     Unaligned i/o access on serial UART works on real PCs.
>     This is used for example by FreeDOS CTMouse driver. Without this it
>     can't reset and detect serial mice.
> 
>     Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
>     Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
>     ---
>      hw/char/serial.c | 3 +++
>      1 file changed, 3 insertions(+)
> 
>     diff --git a/hw/char/serial.c b/hw/char/serial.c
>     index 7061aacbce..41b5e61977 100644
>     --- a/hw/char/serial.c
>     +++ b/hw/char/serial.c
>     @@ -961,6 +961,9 @@ void serial_set_frequency(SerialState *s, uint32_t
>     frequency)
>      const MemoryRegionOps serial_io_ops = {
>          .read = serial_ioport_read,
>          .write = serial_ioport_write,
>     +    .valid = {
>     +        .unaligned = 1,
>     +    },
> 
> 
> I don't get how this can help if both min_access_size & max_access_size are 1.
>  
> 
>          .impl = {
>              .min_access_size = 1,
>              .max_access_size = 1,
>     --
>     2.34.1


Because that's .impl. If access is invalid we don't get as far
as breaking it up to chunks.

> 
> 
> 
> 
> --
> Marc-André Lureau

Re: [PATCH 4/4] serial: Allow unaligned i/o access

[Arwed Meyer]

Am 08.09.22 um 13:15 schrieb Michael S. Tsirkin:
> On Thu, Sep 08, 2022 at 02:11:28PM +0400, Marc-André Lureau wrote:
>> Hi
>>
>> On Wed, Sep 7, 2022 at 2:03 AM Arwed Meyer <arwed.meyer@gmx.de> wrote:
>>
>>      Unaligned i/o access on serial UART works on real PCs.
>>      This is used for example by FreeDOS CTMouse driver. Without this it
>>      can't reset and detect serial mice.
>>
>>      Resolves: https://gitlab.com/qemu-project/qemu/-/issues/77
>>      Signed-off-by: Arwed Meyer <arwed.meyer@gmx.de>
>>      ---
>>       hw/char/serial.c | 3 +++
>>       1 file changed, 3 insertions(+)
>>
>>      diff --git a/hw/char/serial.c b/hw/char/serial.c
>>      index 7061aacbce..41b5e61977 100644
>>      --- a/hw/char/serial.c
>>      +++ b/hw/char/serial.c
>>      @@ -961,6 +961,9 @@ void serial_set_frequency(SerialState *s, uint32_t
>>      frequency)
>>       const MemoryRegionOps serial_io_ops = {
>>           .read = serial_ioport_read,
>>           .write = serial_ioport_write,
>>      +    .valid = {
>>      +        .unaligned = 1,
>>      +    },
>>
>>
>> I don't get how this can help if both min_access_size & max_access_size are 1.
>>
>>
>>           .impl = {
>>               .min_access_size = 1,
>>               .max_access_size = 1,
>>      --
>>      2.34.1
>
>
> Because that's .impl. If access is invalid we don't get as far
> as breaking it up to chunks.
>
>>
>>
>>
>>
>> --
>> Marc-André Lureau
>
Exactly. Not really knowing the serial/chardev code much it took me a
while to figure out why calling FreeDOS CTMouse/protocol.com would never
execute the ioctl mouse reset code in msmouse.c.