| Message ID | fe730863e2af01b68c7086f2f1f74220@hostfission.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] ps2: Clear the queue on PS/2 mouse reset and obey device disable | expand |
On Mon, May 07, 2018 at 10:00:22PM +1000, geoff--- via Qemu-devel wrote: > This allows guest's to correctly reinitialize and identify the mouse > should the guest decide to re-scan or reset during mouse input events. > > Signed-off-by: Geoffrey McRae <geoff@hostfission.com> > --- > hw/input/ps2.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/hw/input/ps2.c b/hw/input/ps2.c > index 06f5d2ac4a..6edf046820 100644 > --- a/hw/input/ps2.c > +++ b/hw/input/ps2.c > @@ -673,6 +673,9 @@ static void ps2_mouse_sync(DeviceState *dev) > { > PS2MouseState *s = (PS2MouseState *)dev; > > + if (!(s->mouse_status & MOUSE_STATUS_ENABLED)) > + return; > + Why this is needed? > @@ -776,6 +779,7 @@ void ps2_write_mouse(void *opaque, int val) > s->mouse_resolution = 2; > s->mouse_status = 0; > s->mouse_type = 0; > + ps2_reset_queue(&s->common); Looks good. cheers, Gerd
On 2018-05-07 22:21, Gerd Hoffmann wrote: > On Mon, May 07, 2018 at 10:00:22PM +1000, geoff--- via Qemu-devel > wrote: >> This allows guest's to correctly reinitialize and identify the mouse >> should the guest decide to re-scan or reset during mouse input events. >> >> Signed-off-by: Geoffrey McRae <geoff@hostfission.com> >> --- >> hw/input/ps2.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/hw/input/ps2.c b/hw/input/ps2.c >> index 06f5d2ac4a..6edf046820 100644 >> --- a/hw/input/ps2.c >> +++ b/hw/input/ps2.c >> @@ -673,6 +673,9 @@ static void ps2_mouse_sync(DeviceState *dev) >> { >> PS2MouseState *s = (PS2MouseState *)dev; >> >> + if (!(s->mouse_status & MOUSE_STATUS_ENABLED)) >> + return; >> + > > Why this is needed? To quote: https://wiki.osdev.org/%228042%22_PS/2_Controller#Detecting_PS.2F2_Device_Types The device should respond to the "identify" command by sending a sequence of none, one or two identification bytes. However, if you just send the "identify" command you can't prevent the response from the "identify" command from being mixed up with keyboard/mouse data. To fix this problem, you need to send the "disable scanning" command first. Disabling scanning means that the device ignores the user (e.g. keyboards ignore keypresses, mice ignore mouse movement and button presses, etc) and won't send data to mess your device identification code up. > >> @@ -776,6 +779,7 @@ void ps2_write_mouse(void *opaque, int val) >> s->mouse_resolution = 2; >> s->mouse_status = 0; >> s->mouse_type = 0; >> + ps2_reset_queue(&s->common); > > Looks good. > > cheers, > Gerd
On Mon, May 07, 2018 at 10:26:24PM +1000, geoff--- via Qemu-devel wrote: > On 2018-05-07 22:21, Gerd Hoffmann wrote: > > On Mon, May 07, 2018 at 10:00:22PM +1000, geoff--- via Qemu-devel wrote: > > > This allows guest's to correctly reinitialize and identify the mouse > > > should the guest decide to re-scan or reset during mouse input events. > > > > > > Signed-off-by: Geoffrey McRae <geoff@hostfission.com> > > > --- > > > hw/input/ps2.c | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/hw/input/ps2.c b/hw/input/ps2.c > > > index 06f5d2ac4a..6edf046820 100644 > > > --- a/hw/input/ps2.c > > > +++ b/hw/input/ps2.c > > > @@ -673,6 +673,9 @@ static void ps2_mouse_sync(DeviceState *dev) > > > { > > > PS2MouseState *s = (PS2MouseState *)dev; > > > > > > + if (!(s->mouse_status & MOUSE_STATUS_ENABLED)) > > > + return; > > > + > > > > Why this is needed? > > To quote: https://wiki.osdev.org/%228042%22_PS/2_Controller#Detecting_PS.2F2_Device_Types > > The device should respond to the "identify" command by sending a sequence of > none, one or two identification bytes. However, if you just send the > "identify" command you can't prevent the response from the "identify" > command from being mixed up with keyboard/mouse data. To fix this problem, > you need to send the "disable scanning" command first. Disabling scanning > means that the device ignores the user (e.g. keyboards ignore keypresses, > mice ignore mouse movement and button presses, etc) and won't send data to > mess your device identification code up. Ok. Same check should be added to ps2_keyboard_event() then I guess? cheers, Gerd
On 2018-05-07 22:41, Gerd Hoffmann wrote: > On Mon, May 07, 2018 at 10:26:24PM +1000, geoff--- via Qemu-devel > wrote: >> On 2018-05-07 22:21, Gerd Hoffmann wrote: >> > On Mon, May 07, 2018 at 10:00:22PM +1000, geoff--- via Qemu-devel wrote: >> > > This allows guest's to correctly reinitialize and identify the mouse >> > > should the guest decide to re-scan or reset during mouse input events. >> > > >> > > Signed-off-by: Geoffrey McRae <geoff@hostfission.com> >> > > --- >> > > hw/input/ps2.c | 4 ++++ >> > > 1 file changed, 4 insertions(+) >> > > >> > > diff --git a/hw/input/ps2.c b/hw/input/ps2.c >> > > index 06f5d2ac4a..6edf046820 100644 >> > > --- a/hw/input/ps2.c >> > > +++ b/hw/input/ps2.c >> > > @@ -673,6 +673,9 @@ static void ps2_mouse_sync(DeviceState *dev) >> > > { >> > > PS2MouseState *s = (PS2MouseState *)dev; >> > > >> > > + if (!(s->mouse_status & MOUSE_STATUS_ENABLED)) >> > > + return; >> > > + >> > >> > Why this is needed? >> >> To quote: >> https://wiki.osdev.org/%228042%22_PS/2_Controller#Detecting_PS.2F2_Device_Types >> >> The device should respond to the "identify" command by sending a >> sequence of >> none, one or two identification bytes. However, if you just send the >> "identify" command you can't prevent the response from the "identify" >> command from being mixed up with keyboard/mouse data. To fix this >> problem, >> you need to send the "disable scanning" command first. Disabling >> scanning >> means that the device ignores the user (e.g. keyboards ignore >> keypresses, >> mice ignore mouse movement and button presses, etc) and won't send >> data to >> mess your device identification code up. > > Ok. Same check should be added to ps2_keyboard_event() then I guess? Quite correct, I will include this in the next patch set. > > cheers, > Gerd
diff --git a/hw/input/ps2.c b/hw/input/ps2.c index 06f5d2ac4a..6edf046820 100644 --- a/hw/input/ps2.c +++ b/hw/input/ps2.c @@ -673,6 +673,9 @@ static void ps2_mouse_sync(DeviceState *dev) { PS2MouseState *s = (PS2MouseState *)dev; + if (!(s->mouse_status & MOUSE_STATUS_ENABLED)) + return; + if (s->mouse_buttons) { qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER); } @@ -776,6 +779,7 @@ void ps2_write_mouse(void *opaque, int val) s->mouse_resolution = 2; s->mouse_status = 0; s->mouse_type = 0; + ps2_reset_queue(&s->common); ps2_queue(&s->common, AUX_ACK); ps2_queue(&s->common, 0xaa); ps2_queue(&s->common, s->mouse_type);
This allows guest's to correctly reinitialize and identify the mouse should the guest decide to re-scan or reset during mouse input events. Signed-off-by: Geoffrey McRae <geoff@hostfission.com> --- hw/input/ps2.c | 4 ++++ 1 file changed, 4 insertions(+)