[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Support for serial mouse emulation
From: |
Aurelien Jarno |
Subject: |
Re: [Qemu-devel] [PATCH] Support for serial mouse emulation |
Date: |
Wed, 10 Dec 2008 19:00:36 +0100 |
User-agent: |
Mutt/1.5.18 (2008-05-17) |
On Tue, Dec 02, 2008 at 05:50:58PM -0500, address@hidden wrote:
> Attaching (sorry for not posting it inline, but my e-mail client mangles
> whitespace) a patch for mouse emulation on a serial port. Emulates a
> three-button mouse that uses Microsoft protocol.
>
> Tested with 386BSD 0.1 and XFree86 2.1. It seems that my guest is not the
> only one that lacks PS/2 mouse support though [1] :)
Please find my comments inline.
> [1] http://www.archivum.info/address@hidden/2005-09/msg00055.html
> QEMU Microsoft serial mouse emulation
> Lubomir Rintel <address@hidden>
This should be replaced by a Signed-off-by: line.
> Index: Makefile.target
> ===================================================================
> --- Makefile.target (revision 5799)
> +++ Makefile.target (working copy)
> @@ -659,7 +659,7 @@
>
> ifeq ($(TARGET_BASE_ARCH), i386)
> # Hardware support
> -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
> OBJS+= fdc.o mc146818rtc.o serial.o i8259.o i8254.o pcspk.o pc.o
> OBJS+= cirrus_vga.o apic.o parallel.o acpi.o piix_pci.o
> OBJS+= usb-uhci.o vmmouse.o vmport.o vmware_vga.o
> @@ -670,7 +670,7 @@
> # shared objects
> OBJS+= ppc.o ide.o vga.o $(SOUND_HW) dma.o openpic.o
> # PREP target
> -OBJS+= pckbd.o ps2.o serial.o i8259.o i8254.o fdc.o m48t59.o mc146818rtc.o
> +OBJS+= pckbd.o ps2.o msmouse.o serial.o i8259.o i8254.o fdc.o m48t59.o
> mc146818rtc.o
> OBJS+= prep_pci.o ppc_prep.o
> # Mac shared devices
> OBJS+= macio.o cuda.o adb.o mac_nvram.o mac_dbdma.o
> @@ -685,7 +685,7 @@
> OBJS+= mips_r4k.o mips_jazz.o mips_malta.o mips_mipssim.o
> OBJS+= mips_timer.o mips_int.o dma.o vga.o serial.o i8254.o i8259.o rc4030.o
> OBJS+= g364fb.o jazz_led.o
> -OBJS+= ide.o gt64xxx.o pckbd.o ps2.o fdc.o mc146818rtc.o usb-uhci.o acpi.o
> ds1225y.o
> +OBJS+= ide.o gt64xxx.o pckbd.o ps2.o msmouse.o fdc.o mc146818rtc.o
> usb-uhci.o acpi.o ds1225y.o
> OBJS+= piix_pci.o parallel.o cirrus_vga.o pcspk.o $(SOUND_HW)
> OBJS+= mipsnet.o
> OBJS+= pflash_cfi01.o
> @@ -704,7 +704,7 @@
> endif
> ifeq ($(TARGET_BASE_ARCH), sparc)
> ifeq ($(TARGET_ARCH), sparc64)
> -OBJS+= sun4u.o ide.o pckbd.o ps2.o vga.o apb_pci.o
> +OBJS+= sun4u.o ide.o pckbd.o ps2.o msmouse.o vga.o apb_pci.o
> OBJS+= fdc.o mc146818rtc.o serial.o m48t59.o
> OBJS+= cirrus_vga.o parallel.o ptimer.o
> else
> @@ -714,7 +714,7 @@
> endif
> endif
> ifeq ($(TARGET_BASE_ARCH), arm)
> -OBJS+= integratorcp.o versatilepb.o ps2.o smc91c111.o arm_pic.o arm_timer.o
> +OBJS+= integratorcp.o versatilepb.o ps2.o msmouse.o smc91c111.o arm_pic.o
> arm_timer.o
> OBJS+= arm_boot.o pl011.o pl031.o pl050.o pl080.o pl110.o pl181.o pl190.o
> OBJS+= versatile_pci.o ptimer.o
> OBJS+= realview_gic.o realview.o arm_sysctl.o mpcore.o
Is it really necessary to enable that in all targets, especially when
this is inited only in hw/pc.c?
> Index: qemu-char.c
> ===================================================================
> --- qemu-char.c (revision 5799)
> +++ 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 <unistd.h>
> #include <fcntl.h>
> @@ -2120,6 +2121,41 @@
> return NULL;
> }
>
> +/***********************************************************/
> +/* Microsoft serial mouse */
> +
> +/* This is written to by hw/msmouse.c if non-NULL */
> +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.
> +int msmouse_chr_write (struct CharDriverState *s, const uint8_t *buf, int
> len)
> +{
> + /* Ignore writes to mouse port */
> + return len;
> +}
> +
> +void msmouse_chr_close (struct CharDriverState *s)
> +{
> + qemu_free (s);
> + msmouse_chr = NULL;
> +}
> +
> +static CharDriverState *qemu_chr_open_msmouse(void)
> +{
> + CharDriverState *chr;
> +
> + /* Only one serial mouse per instance is allowed */
> + if (msmouse_chr != NULL) {
> + return NULL;
> + }
I don't think it is a good idea. I guess this is due to the use of a
global variable.
> + chr = qemu_mallocz(sizeof(CharDriverState));
> + chr->chr_write = msmouse_chr_write;
> + chr->chr_close = msmouse_chr_close;
> + msmouse_chr = chr;
> +
> + return chr;
> +}
> +
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().
> static TAILQ_HEAD(CharDriverStateHead, CharDriverState) chardevs
> = TAILQ_HEAD_INITIALIZER(chardevs);
>
> @@ -2154,6 +2190,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 5799)
> +++ qemu-doc.texi (working copy)
> @@ -904,6 +904,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.
> address@hidden 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,61 @@
> +/*
> + * 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 <stdlib.h>
> +#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)
> +{
> + if (!msmouse_chr)
> + return;
> +
> + 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);
There is probably a copy&paste error in the indices here.
> +
> + /* 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(msmouse_chr, bytes, 4);
> +}
> +
> +void *msmouse_init()
> +{
> + qemu_add_mouse_event_handler(msmouse_event, NULL, 0, "QEMU Microsoft
> Mouse");
> + return NULL;
> +}
> Index: hw/msmouse.h
> ===================================================================
> --- hw/msmouse.h (revision 0)
> +++ hw/msmouse.h (revision 0)
> @@ -0,0 +1,2 @@
> +extern CharDriverState *msmouse_chr;
> +void *msmouse_init(void);
> Index: hw/pc.c
> ===================================================================
> --- hw/pc.c (revision 5799)
> +++ hw/pc.c (working copy)
> @@ -33,6 +33,7 @@
> #include "boards.h"
> #include "console.h"
> #include "fw_cfg.h"
> +#include "msmouse.h"
>
> /* output Bochs bios info messages */
> //#define DEBUG_BIOS
> @@ -1092,6 +1093,8 @@
> }
> }
> }
> +
> + msmouse_init ();
> }
>
> static void pc_init_pci(ram_addr_t ram_size, int vga_ram_size,
--
.''`. Aurelien Jarno | GPG: 1024D/F1BCDB73
: :' : Debian developer | Electrical Engineer
`. `' address@hidden | address@hidden
`- people.debian.org/~aurel32 | www.aurel32.net