[Qemu-devel] [PATCH 1/2] ps2: Clear the queue on PS/2 mouse reset and obey device disable

[Qemu-devel] [PATCH 1/2] ps2: Clear the queue on PS/2 mouse reset and obey device disable

[geoff]

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;
+
      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);
-- 
2.14.2

Re: [Qemu-devel] [PATCH 1/2] ps2: Clear the queue on PS/2 mouse reset and obey device disable

[Gerd Hoffmann]

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

Re: [Qemu-devel] [PATCH 1/2] ps2: Clear the queue on PS/2 mouse reset and obey device disable

[geoff]

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

Re: [Qemu-devel] [PATCH 1/2] ps2: Clear the queue on PS/2 mouse reset and obey device disable

[Gerd Hoffmann]

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

Re: [Qemu-devel] [PATCH 1/2] ps2: Clear the queue on PS/2 mouse reset and obey device disable

[geoff]

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