All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel]  [PATCH] keyboard:fix ps2 keyboard can't use
@ 2015-05-06 16:20 penghao122
  2015-05-06 17:03 ` Eric Blake
  0 siblings, 1 reply; 3+ messages in thread
From: penghao122 @ 2015-05-06 16:20 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-stable

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<penghao122@sina.com>
----
diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
index 9b9a7d7..f4fbcfc 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,11 @@ 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..f3580c1 100644
--- a/hw/input/ps2.c
+++ b/hw/input/ps2.c
@@ -150,7 +150,12 @@ void ps2_queue(void *opaque, int b)
q->count++;
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);

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] keyboard:fix ps2 keyboard can't use
  2015-05-06 16:20 [Qemu-devel] [PATCH] keyboard:fix ps2 keyboard can't use penghao122
@ 2015-05-06 17:03 ` Eric Blake
  2015-05-06 17:05   ` Eric Blake
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Blake @ 2015-05-06 17:03 UTC (permalink / raw)
  To: penghao122, qemu-devel; +Cc: qemu-stable

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

On 05/06/2015 10:20 AM, penghao122@sina.com wrote:

Missing a 'v2' in the subject line (hint: git send-email -v2)

Subject line doesn't make sense, and has incorrect spacing after colon.
 What was wrong with my suggestion that I gave on your v1?

keyboard: handle ps2 typing buffer overrun

> Starting a linux guest with ps2 keyboard, if you type many times during leaving 

Trailing whitespace in your commit message.

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

s/grub,the/grub, the/ (in general, space after comma in English prose;
I'll quit pointing it out)

> 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<penghao122@sina.com>
> ----

Incorrect separator. Git relies on exactly '---' as the division between
commit message and explanatory text, and then ignores any garbage until
the first 'diff' line.  Also missing a diffstat, which 'git
format-patch' will automatically insert as part of the explanatory text.

> diff --git a/hw/input/pckbd.c b/hw/input/pckbd.c
> index 9b9a7d7..f4fbcfc 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)

Your patch is fatally flawed:

$ git am ../\[Qemu-devel\]\ \[PATCH\]\ keyboard\:fix\ ps2\ keyboard\
can\'t\ use.eml
Applying: keyboard:fix ps2 keyboard can't use
fatal: corrupt patch at line 6
Patch failed at 0001 keyboard:fix ps2 keyboard can't use
The copy of the patch that failed is found in:
   /home/eblake/qemu/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

You probably pasted the contents of your patch into an email composer
that adjusted whitespace, and in the process made your patch useless.
PLEASE read http://wiki.qemu.org/Contribute/SubmitAPatch, and follow the
advice there for sending a patch.  While it is possible to send a patch
without using 'git send-email', it is so tricky to get it right that we
strongly encourage new contributors to just use what git already
provides.  In particular, I advise that you FIRST send a patch to
yourself using 'git send-email', and then use 'git am' to apply the
patch locally, to make sure that your configuration is correct and that
your patches will survive a round trip through email.  Once you have
that working, then it is okay to send to the list.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] keyboard:fix ps2 keyboard can't use
  2015-05-06 17:03 ` Eric Blake
@ 2015-05-06 17:05   ` Eric Blake
  0 siblings, 0 replies; 3+ messages in thread
From: Eric Blake @ 2015-05-06 17:05 UTC (permalink / raw)
  To: penghao122, qemu-devel; +Cc: qemu-stable

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

On 05/06/2015 11:03 AM, Eric Blake wrote:
> On 05/06/2015 10:20 AM, penghao122@sina.com wrote:
> 
> Missing a 'v2' in the subject line (hint: git send-email -v2)
> 

>> +++ 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)
> 
> Your patch is fatally flawed:

Oh, and also your patch fails scripts/checkpatch.pl due to missing {}
around the 'if' body.  Also something we mention on:

> PLEASE read http://wiki.qemu.org/Contribute/SubmitAPatch

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-05-06 18:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-06 16:20 [Qemu-devel] [PATCH] keyboard:fix ps2 keyboard can't use penghao122
2015-05-06 17:03 ` Eric Blake
2015-05-06 17:05   ` Eric Blake

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.