From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1LS5Ul-0000tm-2M for qemu-devel@nongnu.org; Wed, 28 Jan 2009 03:09:59 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1LS5Uj-0000tB-IM for qemu-devel@nongnu.org; Wed, 28 Jan 2009 03:09:58 -0500 Received: from [199.232.76.173] (port=47396 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1LS5Uj-0000t6-Dg for qemu-devel@nongnu.org; Wed, 28 Jan 2009 03:09:57 -0500 Received: from norkia.v3.sk ([92.240.234.41]:39410) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1LS5Ui-0004pL-EK for qemu-devel@nongnu.org; Wed, 28 Jan 2009 03:09:56 -0500 From: Lubomir Rintel In-Reply-To: <20081210180036.GG19379@volta.aurel32.net> References: <44179.89.102.207.186.1228258258.squirrel@mail.netbsd.sk> <20081210180036.GG19379@volta.aurel32.net> Content-Type: multipart/mixed; boundary="=-4yjhACJz3jfRQjkOW7fv" Date: Wed, 28 Jan 2009 09:10:04 +0100 Message-Id: <1233130204.5389.7.camel@localhost.localdomain> Mime-Version: 1.0 Subject: [Qemu-devel] [RESEND] [PATCH] Support for serial mouse emulation Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aurelien Jarno Cc: qemu-devel@nongnu.org --=-4yjhACJz3jfRQjkOW7fv Content-Type: text/plain Content-Transfer-Encoding: 7bit Hi, On Wed Dec 24 19:41:53 CET 2008, Aurelien Jarno wrote: > Please find my comments inline. Thanks a lot for your review. I am attaching the revised version, addressing most of your concerns. > This should be replaced by a Signed-off-by: line. Done >> Index: Makefile.target >> =================================================================== ... >> -OBJS+= ide.o pckbd.o ps2.o vga.o $(SOUND_HW) dma.o >> +OBJS+= ide.o pckbd.o ps2.o msmouse.o vga.o $(SOUND_HW) dma.o > ... (Lots of msmouse.o additions) ... > > Is it really necessary to enable that in all targets, especially when > this is inited only in hw/pc.c? Well, given it's a character device, it's generally possible to use everywhere a serial port can be used. I limited this to a single occurence of adding to OBJS in the new version of patch though. >> +CharDriverState *msmouse_chr = NULL; > > A global variable is not a good idea. You should instead create > structure containing this variable (and maybe others like > CharDriverState) called for example VMMouseState. Originally I created a global variable because the serial mouse did not work for me when attached at the serial port initialization time. Now I figured out multiple mouse can be attached at once, though only one can be selected at the time from the console. I am wondering if it would make sense to somehow make the serial mouse have preference over the PS/2 mouse by default, since it's most likely the way one would expect it to work? I did not create the structure with the state yet, since the only data I keep for the instance of serial mouse is its serial port character device. It's also consistent with original two-button mouse hardware using MS protocol, since it was basically stateless. On the other hand, three button mouses keep state about the past state of the third button, and use the two-button compatible protocol if the third button is depressed and its state did not change, to retain compatibility with two-button drivers, which is what this emulation doesn't support. (See the comment around the relevant part of the code.) I'll create a separate structure for serial mice and support this backward-compatible behavior if you want me to. >> + /* Only one serial mouse per instance is allowed */ > I don't think it is a good idea. I guess this is due to the use of a > global variable. Right. Fixed. ... (the character device methods)... > I really think all this part that you add in vl.c should be moved into > hw/vmmouse.c. The qemu_chr_open_msmouse should be merged into > msmouse_init and called from vl.c. Then the structure should be > allocated, and passed instead of the NULL argument of > qemu_add_mouse_event_handler(). I think you meant "qemu-char.c", it hat split from "vl.c". And I guess it shouldn't be moved to "vmmouse.c", since that's a VMWare mouse, but it could have been a typo as well. If you meant "msmouse.c", then that was done in the new revision of the patch. I'm not completely sure it is right though -- see, qemu-char.c contains a lot of similar examples of complete implementation of character devices. I find separation to msmouse.c a lot cleaner tough. >> + /* Buttons */ >> + bytes[0] |= (buttons_state & 0x01 ? 0x20 : 0x00); >> + bytes[0] |= (buttons_state & 0x02 ? 0x10 : 0x00); >> + bytes[3] |= (buttons_state & 0x04 ? 0x20 : 0x00); > > There is probably a copy&paste error in the indices here. It's actually correct. It is just that the serial mouse protocol evolved into being a little bit weird. -- Lubomir Rintel --=-4yjhACJz3jfRQjkOW7fv Content-Disposition: attachment; filename="qemu-20081224-msmouse.patch" Content-Type: text/x-patch; name=qemu-20081224-msmouse.patch; charset=UTF-8 Content-Transfer-Encoding: 7bit QEMU Microsoft serial mouse emulation Adds "msmouse" character device, which emulates a serial mouse. Use it with -serial msmouse. Signed-Off-By: Lubomir Rintel Index: Makefile.target =================================================================== --- Makefile.target (revision 6107) +++ Makefile.target (working copy) @@ -633,6 +633,9 @@ OBJS += rtl8139.o OBJS += e1000.o +# Serial mouse +OBJS += msmouse.o + ifeq ($(TARGET_BASE_ARCH), i386) # Hardware support OBJS+= ide.o pckbd.o ps2.o vga.o $(SOUND_HW) dma.o Index: qemu-char.c =================================================================== --- qemu-char.c (revision 6107) +++ qemu-char.c (working copy) @@ -30,6 +30,7 @@ #include "block.h" #include "hw/usb.h" #include "hw/baum.h" +#include "hw/msmouse.h" #include #include @@ -2153,6 +2154,8 @@ } else { printf("Unable to open driver: %s\n", p); } + } else if (!strcmp(filename, "msmouse")) { + chr = qemu_chr_open_msmouse(); } else #ifndef _WIN32 if (strstart(filename, "unix:", &p)) { Index: qemu-doc.texi =================================================================== --- qemu-doc.texi (revision 6107) +++ qemu-doc.texi (working copy) @@ -914,6 +914,8 @@ When @var{remote_host} or @var{src_ip} are not specified they default to @code{0.0.0.0}. When not using a specified @var{src_port} a random port is automatically chosen. +@item msmouse +Three button serial mouse. Configure the guest to use Microsoft protocol. If you just want a simple readonly console you can use @code{netcat} or @code{nc}, by starting qemu with: @code{-serial udp::4555} and nc as: Index: hw/msmouse.c =================================================================== --- hw/msmouse.c (revision 0) +++ hw/msmouse.c (revision 0) @@ -0,0 +1,78 @@ +/* + * QEMU Microsoft serial mouse emulation + * + * Copyright (c) 2008 Lubomir Rintel + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#include +#include "../qemu-common.h" +#include "../qemu-char.h" +#include "../console.h" +#include "msmouse.h" + +#define MSMOUSE_LO6(n) ((n) & 0x3f) +#define MSMOUSE_HI2(n) (((n) & 0xc0) >> 6) + +static void msmouse_event(void *opaque, + int dx, int dy, int dz, int buttons_state) +{ + CharDriverState *chr = (CharDriverState *)opaque; + + char bytes[4] = { 0x40, 0x00, 0x00, 0x00 }; + + /* Movement deltas */ + bytes[0] |= (MSMOUSE_HI2(dy) << 2) | MSMOUSE_HI2(dx); + bytes[1] |= MSMOUSE_LO6(dx); + bytes[2] |= MSMOUSE_LO6(dy); + + /* Buttons */ + bytes[0] |= (buttons_state & 0x01 ? 0x20 : 0x00); + bytes[0] |= (buttons_state & 0x02 ? 0x10 : 0x00); + bytes[3] |= (buttons_state & 0x04 ? 0x20 : 0x00); + + /* We always send the packet of, so that we do not have to keep track + of previous state of the middle button. This can potentially confuse + some very old drivers for two button mice though. */ + qemu_chr_read(chr, bytes, 4); +} + +static int msmouse_chr_write (struct CharDriverState *s, const uint8_t *buf, int len) +{ + /* Ignore writes to mouse port */ + return len; +} + +static void msmouse_chr_close (struct CharDriverState *chr) +{ + qemu_free (chr); +} + +CharDriverState *qemu_chr_open_msmouse(void) +{ + CharDriverState *chr; + + chr = qemu_mallocz(sizeof(CharDriverState)); + chr->chr_write = msmouse_chr_write; + chr->chr_close = msmouse_chr_close; + + qemu_add_mouse_event_handler(msmouse_event, chr, 0, "QEMU Microsoft Mouse"); + + return chr; +} Index: hw/msmouse.h =================================================================== --- hw/msmouse.h (revision 0) +++ hw/msmouse.h (revision 0) @@ -0,0 +1,2 @@ +/* hw/msmouse.c */ +CharDriverState *qemu_chr_open_msmouse(void); --=-4yjhACJz3jfRQjkOW7fv--