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

[Eric Blake <eblake@redhat.com>]

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

On 05/17/2015 10:00 AM, penghao122@sina.com wrote:
> Subject: [PATCH] keyboard: handle ps2 typing buffer overrun

This line is redundant.

Also, you have tried to send this patch more than once.  Please be sure
to include a proper 'v3' or 'v4' in the subject line, as appropriate.

> 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.

s/Specally/Specifically,/
s/file,you/file, you/

> 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,

s/initialize/initializes/
s/keyboard,linux call/keyboard, linux calls/

> I8042_STR_OBF. Then i8042_flush() will return I8042_BUFFER_SIZE.
> Signed-off-by: Hao Peng <penghao122@sina.com>

Usually a blank line between the main commit body and the trailing tags
like Signed-off-by.

> ---
>  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;

Missing {}, and please use space after keywords like 'if'.  Please run
./scripts/checkpatch.pl on your submission.

>      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) {

Pre-existing, but as long as you are touching this code, it is worth
putting a space after the keyword 'switch'.


> @@ -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;

More missing {}, and another missing space after 'if'.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]