On 2026/08/25 1:58, Akihiko Odaki wrote:
On 2026/08/23 16:15, Christian Quante wrote:
ps2_write_mouse() ends its command switch with a bare "default: break;",
so an unknown command draws no reply at all. A real PS/2 device answers
every byte it is given -- ACK (0xFA) when it understood one, resend
(0xFE) when it did not -- and a guest that gets nothing back is left
waiting out its reply timeout. The keyboard path in the same file has
always answered unknown commands with KBD_REPLY_RESEND.

Two guests were measured on this.

OS/2 probes the mouse with the vendor command 0xBB, which QEMU does not
implement, and then polls the status port until its own timeout runs
out. On a Warp 3 guest that wait costs about 25 ms of every boot under
TCG, and 2.1 s under KVM, where each of those polls leaves the guest.
With this patch the wait ends on the first read: the guest takes the
same error path an unexpected reply would, and does not retry.

Linux probes for a TrackPoint with 0xE1 and fails the same way. Timing
the psmouse detection from a mark written to /dev/kmsg to the kernel's
"input:" line, three boots each of a 6.18.35 kernel under TCG:
426.7/428.8/441.6 ms without this patch, 21.4/21.6/21.2 ms with it. The
mouse is detected identically either way; libps2 caps its retries at
two attempts and ends in the same -EPROTO the timeout produced.

With no response, __ps2_command() in libps2 returns -EIO. With this patch, two 0xfe responses instead make it return -EPROTO. The two cases therefore do not return the same error.


The specification's second stage -- 0xFC (Error) when the byte after a
rejected one is invalid as well -- is deliberately left out. It would
need state that has to survive migration, no guest is known to test for
it, and the keyboard path has answered unknown commands with a bare
resend for twenty years.

Commit 06b3611fc2a3 ("ps2: reject unknown commands, instead of
blindly accepting them") changed unknown keyboard commands from ACK
to Resend in 2016, so this behavior is ten years old, not twenty.

Also, let's add Cc: [email protected]



Signed-off-by: Christian Quante <[email protected]>
---
  hw/input/ps2.c | 8 ++++++++
  1 file changed, 8 insertions(+)

diff --git a/hw/input/ps2.c b/hw/input/ps2.c
index 5516eb262d..1e8c6d983a 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -73,6 +73,7 @@
  #define AUX_SET_DEFAULT     0xF6
  #define AUX_RESET           0xFF    /* Reset aux device */
  #define AUX_ACK             0xFA    /* Command byte ACK. */
+#define AUX_RESEND          0xFE    /* Command NACK, send the cmd again */
  #define MOUSE_STATUS_REMOTE     0x40
  #define MOUSE_STATUS_ENABLED    0x20
@@ -955,6 +956,13 @@ void ps2_write_mouse(PS2MouseState *s, int val)
                  s->mouse_type);
              break;
          default:
+            /*
+             * A PS/2 device answers every command it is given; an unknown +             * one draws a resend. Staying silent leaves the guest waiting
+             * out its reply timeout. The keyboard path above answers
+             * unknown commands with KBD_REPLY_RESEND.
+             */

Mentioning the mismatch with the keyboard path in the patch message makes sense, but keeping it in the inline comment is somewhat extraneous since consistent behavior across both paths is an obvious expectation. I suggest dropping the final sentence.

Additionally, it would be better to ensure the inline rationale is documented consistently across both the mouse and keyboard paths.

Otherwise, the patch looks solid to me.

Regards,
Akihiko Odaki
+            ps2_queue(ps2, AUX_RESEND);
              break;
          }
          break;



Reply via email to