Re: [Qemu-devel] [PATCH] keyboard: handle ps2 typing buffer overrun

[Michael Tokarev]

Okay, while the patch has been criticised by Eric already, mostly
due to the commit message and stylistic errors in the code, but
what about the code changed in this patch, are the changes needed,
correct?

I remember a discussion on a related topic,
http://thread.gmane.org/gmane.comp.emulators.qemu/292614 ,
and for example this reply by Gerd (Cc'd):
http://thread.gmane.org/gmane.comp.emulators.qemu/292614/focus=292921
Maybe the patch below can help?

Thanks,

/mjt

[Quoting original message in full]

17.05.2015 19:00, address@hidden wrote:
> Subject: [PATCH] keyboard: handle ps2 typing buffer overrun
> 
> Starting a linux guest with ps2 keyboard, if you type many times during 
> leaving
> grub and into linux kernel,then you can't use keyboard after linux 
> initialization finished.
> Specally when you setup linux guest from iso file,you will type in grub.
> During grub,the work method of ps2 keyboard is like this:
> First, ps2 keyboard driver send command KBD_CCMD_KBD_ENABLE.
> Second, if there is a keyboard input, then ps2 keyboard driver read data.
> Third, ps2 keyboard driver send command KBD_CCMD_KBD_ENABLE again.
> 
> After leaving grub and before finishing linux kernel ps2 driver 
> initialization,
> if you type many times, the input data keep saving in ps2 queue of qemu.
> Before linux kernel initialize ps2 keyboard,linux call i8042_controller_check,
> if i8042_controller_check return fail, then ps2 keyboard driver will never 
> initialize.
> (i8042.c in kernel 2.6.32 )
> static int i8042_controller_check(void)
> {
>     if (i8042_flush() == I8042_BUFFER_SIZE)
>         return -ENODEV;
>     return 0;
> }
> static int i8042_flush(void)
> {
>   ...
>     while (((str = i8042_read_status()) & I8042_STR_OBF) && (i < 
> I8042_BUFFER_SIZE)) {
>         udelay(50);
>         data = i8042_read_data();
>         i++;
>      }
>     return i;
> }
> During calling i8042_flush it is full in ps2 queue of qemu. ps_read_data will 
> execute
> kbd_update_irq(s->update_arg, q->count != 0). Because q->count!=0, 
> kbd_update_irq can set
> I8042_STR_OBF. Then i8042_flush() will return I8042_BUFFER_SIZE.
> 
> Signed-off-by: Hao Peng <address@hidden <mailto:address@hidden>>
> ---
>  hw/input/pckbd.c       | 11 +++++++++--
>  hw/input/ps2.c         |  7 +++++++
>  include/hw/input/ps2.h |  1 +
>  3 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index 9b9a7d7..1253b04 100644
> --- a/hw/input/pckbd.c
> +++ b/hw/input/pckbd.c
> @@ -207,6 +207,8 @@ static uint64_t kbd_read_status(void *opaque, hwaddr addr,
>      KBDState *s = opaque;
>      int val;
>      val = s->status;
> +    if(s->write_cmd == KBD_CCMD_KBD_ENABLE)
> +        val &= ~KBD_STAT_OBF;
>      DPRINTF("kbd: read status=0x%02x\n", val);
>      return val;
>  }
> @@ -251,9 +253,10 @@ static void kbd_write_command(void *opaque, hwaddr addr,
>          else
>              val = KBD_CCMD_NO_OP;
>      }
> -
> +    s->write_cmd = 0;
>      switch(val) {
>      case KBD_CCMD_READ_MODE:
> +        ps2_clear_queue(s->kbd);
>          kbd_queue(s, s->mode, 0);
>          break;
>      case KBD_CCMD_WRITE_MODE:
> @@ -284,6 +287,7 @@ static void kbd_write_command(void *opaque, hwaddr addr,
>          kbd_update_irq(s);
>          break;
>      case KBD_CCMD_KBD_ENABLE:
> +        s->write_cmd = KBD_CCMD_KBD_ENABLE;
>          s->mode &= ~KBD_MODE_DISABLE_KBD;
>          kbd_update_irq(s);
>          break;
> @@ -364,7 +368,10 @@ static void kbd_write_data(void *opaque, hwaddr addr,
>      default:
>          break;
>      }
> -    s->write_cmd = 0;
> +    if(s->write_cmd == KBD_CCMD_WRITE_MODE && s->mode == 0x61)
> +        s->write_cmd = KBD_CCMD_KBD_ENABLE;
> +    else
> +        s->write_cmd = 0;
>  }
>  
>  static void kbd_reset(void *opaque)
> diff --git a/hw/input/ps2.c b/hw/input/ps2.c
> index 4baeea2..b7c72bb 100644
> --- a/hw/input/ps2.c
> +++ b/hw/input/ps2.c
> @@ -151,6 +151,13 @@ void ps2_queue(void *opaque, int b)
>      s->update_irq(s->update_arg, 1);
>  }
>  
> +void ps2_clear_queue(void *opaque)
> +{
> +    PS2State *s = (PS2State *)opaque;
> +    PS2Queue *q = &s->queue;
> +    q->wptr = q->rptr = q->count = 0;
> +}
> +
>  /*
>     keycode is expressed as follow:
>     bit 7    - 0 key pressed, 1 = key released
> diff --git a/include/hw/input/ps2.h b/include/hw/input/ps2.h
> index 7c45ce7..7bd9158 100644
> --- a/include/hw/input/ps2.h
> +++ b/include/hw/input/ps2.h
> @@ -32,6 +32,7 @@ void ps2_write_mouse(void *, int val);
>  void ps2_write_keyboard(void *, int val);
>  uint32_t ps2_read_data(void *);
>  void ps2_queue(void *, int b);
> +void ps2_clear_queue(void *opaque);
>  void ps2_keyboard_set_translation(void *opaque, int mode);
>  void ps2_mouse_fake_event(void *opaque);
>  
> --