From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47184) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sgv7H-0006RG-87 for qemu-devel@nongnu.org; Tue, 19 Jun 2012 05:53:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sgv7C-0002Ly-Fn for qemu-devel@nongnu.org; Tue, 19 Jun 2012 05:52:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11148) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sgv7C-0002Ln-7l for qemu-devel@nongnu.org; Tue, 19 Jun 2012 05:52:50 -0400 Message-ID: <4FE04BED.4080407@redhat.com> Date: Tue, 19 Jun 2012 17:52:45 +0800 From: Amos Kong MIME-Version: 1.0 References: <433764b9-c58b-4b4e-8a4d-0f6cd84a25e7@zmail13.collab.prod.int.phx2.redhat.com> <4FD0326F.3010806@redhat.com> <20120611140642.06be2ee8@doriath.home> <4FD62827.4060900@us.ibm.com> <20120611142546.66871522@doriath.home> <4FD9BB06.3000509@redhat.com> <4FDAE857.6050106@redhat.com> <4FDAEAFD.9090406@redhat.com> <20120615103531.614ed1c7@doriath.home> <4FDF49AD.3040806@redhat.com> In-Reply-To: <4FDF49AD.3040806@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/6] qerror: add MAX_KEYCODES 16 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: Anthony Liguori , Gerd Hoffmann , qemu-devel@nongnu.org On 18/06/12 23:30, Amos Kong wrote: > On 06/15/2012 09:35 PM, Luiz Capitulino wrote: >> On Fri, 15 Jun 2012 09:57:49 +0200 >> Gerd Hoffmann wrote: >> >>> Hi, >>> >>>>> It seems we need to notice user when inputted keys are more than 16. >>>> >>>> Hi Gerd, >>>> >>>> When I use 'sendkey' command to send key-series to guest, some keyboard >>>> events will be send. There is a limitation (16) that was introduced by this >>>> old commit c8256f9d (without description). Do you know the reason? >>> >>> Probably hardware limitation, ps/2 keyboards can buffer up to 16 keys IIRC. >> >> Then the perfect thing to do would be to drop the MAX_KEYCODES check from >> the sendkey command and move bounds checking down to the device emulation code. >> >> >> However, this will require a bit of code churn if we do it for all devices, >> and won't buy us much, as the most likely reason for the error is a client/user >> trying to send too many keys in parallel to the guest, right? > > Agree, we can notice in stderr when the redundant keys are ignored as hid. > > > #define QUEUE_LENGTH 16 /* should be enough for a triple-click */ > > static void hid_keyboard_event(void *opaque, int keycode) > { > ... > if (hs->n == QUEUE_LENGTH) { > fprintf(stderr, "usb-kbd: warning: key event queue full\n"); > return; > } I dropped the limitation in sendkey command, and didn't change current ps2.c, executed some tests in different environments. environment max inputted key number ------------ ----------------------- win7 notepad 100 rhel6 grub 15 rhel6 pxe 15 rhel6 login window 10 rhel6 vim 16 rhel6 terminal(init 3) 200 It seems original 256 queue limitation in ps2.c is fine. I would only drop limitation(16) in old sendkey command, it's secure. >> If this is right, then I think that the best thing to do would be to drop the >> MAX_KEYCODES check from the sendkey command and document that devices can drop >> keys if too many of them are sent in parallel or too fast (we can mention ps/2 >> as an example of a 16 bytes limit). >> >>> >>> Likewise the usb hid devices can buffer up to 16 events. In that case >>> it is just a qemu implementation detail and not a property of the >>> hardware we are emulating, so it can be changed. Not trivially though >>> as the buffer is part of the migration data, so it is more work that >>> just changing a #define. -- Amos.