From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [PATCH] Support for QEMU's CVS Date: Thu, 21 Dec 2006 10:20:35 -0600 Message-ID: <458AB453.3030701@cs.utexas.edu> References: <458A18F5.10108@cs.utexas.edu> <458A4AA7.1000705@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Return-path: To: Avi Kivity In-Reply-To: <458A4AA7.1000705-atKUWr5tajBWk0Htik3J/w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Avi Kivity wrote: > There are four issues I can see: > > - if there are multiple sources of kvm userspace, there are bound to > be problems with incompatible kernel API and userspace, especially as > I have plans for extensive changes to the API. I'll go and implement > a version check now, so that doesn't become a barrier to anything. A > submission to qemu-devel should include this check. With KQEMU, you have to pair a version of KQEMU with the appropriate version of QEMU. I don't think there's much of a problem doing something like this for KVM for the short term until API gets more stable (provided there's a version check so the error is useful to the user). > - the API changes are bound to cause more churn than necessary. I > guess we can live with that though. Yeah, I don't think this is a huge problem. > - the patch contains some qemu changes that date from the days we did > V2E, and are therefore unnecessary. I'll point them out below. Right, I noticed a few things that seemed unnecessary. > - where does libkvm live? an external package? I would think an external package. There are a couple more issues I know of: - kqemu and kvm don't play very well together at the moment. We need to revisit some of the logic to ensure that kqemu and kvm are never enabled at the same time. - the USE_KVM guards are probably unnecessary. We should be able to isolate all of the libkvmctl specific stuff to qemu-kvm.c and then stub things out as needed. >> diff -r 2eac80033ea0 hw/usb-hid.c >> --- a/hw/usb-hid.c Tue Dec 19 09:31:34 2006 +0000 >> +++ b/hw/usb-hid.c Wed Dec 20 23:01:25 2006 -0600 >> @@ -169,7 +169,9 @@ static const uint8_t qemu_tablet_config_ >> 0x81, /* u8 ep_bEndpointAddress; IN Endpoint 1 */ >> 0x03, /* u8 ep_bmAttributes; Interrupt */ >> 0x08, 0x00, /* u16 ep_wMaxPacketSize; */ >> - 0x03, /* u8 ep_bInterval; (255ms -- usb 2.0 spec) */ >> + /* Temporarily increase usb polling interval to prevent cpu >> + * saturation (3 ms is way too often for kvm) */ >> + 0x33, /* u8 ep_bInterval; (255ms -- usb 2.0 spec) */ >> > > This might be a good change for qemu, but on the other hand this is a > kvm deficiency that is forced on non-kvm users. Brad Campbell (who wrote the original HID description) has recently posted a patch that changes the interval to 0x20. 0x03 was causing a performance problems with Windows guests (there's a pretty recent thread on qemu-devel about it). How did you arrive at 0x33 btw? IIRC, Brad stopped at 0x20 as he saw no performance improvement beyond it. If KVM needs at least 0x33 then provided there is no negative impact to QEMU I don't really see a problem with it. kqemu is bound to have the same issue FWIW. Regards, Anthony Liguori ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV