From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] Support for QEMU's CVS Date: Thu, 21 Dec 2006 10:49:43 +0200 Message-ID: <458A4AA7.1000705@qumranet.com> References: <458A18F5.10108@cs.utexas.edu> 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: Anthony Liguori In-Reply-To: <458A18F5.10108-NZpS4cJIG2HvQtjrzfazuQ@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 Anthony Liguori wrote: > Howdy, > > The attached patch forward ports the KVM patch to QEMU's CVS. The > only significant change needed was hacking the Bochs BIOS to > dynamically disable SMM support if KVM is enabled. This was done by > using one of the Bochs DEBUG ports. Probably not the best long term > solution but it works. > > The patch is minimal (no VMDK changes, no migration, etc.). This is > to ease upstream integration. > > I am able to boot both a Windows and FC5 guest (under SVM). > > You'll need the BIOS (binary file can't be included in diff) from: > > http://www.cs.utexas.edu/~aliguori/bios.bin > > I reckon some changes still need to be made for CVS inclusion but I > think it's at least appropriate to send to qemu-devel and begin the > review process. 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. - the API changes are bound to cause more churn than necessary. I guess we can live with that though. - the patch contains some qemu changes that date from the days we did V2E, and are therefore unnecessary. I'll point them out below. - where does libkvm live? an external package? > 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. > diff -r 2eac80033ea0 target-i386/helper.c > --- a/target-i386/helper.c Tue Dec 19 09:31:34 2006 +0000 > +++ b/target-i386/helper.c Wed Dec 20 23:01:25 2006 -0600 > @@ -184,7 +186,15 @@ static inline void get_ss_esp_from_tss(u > if (!(env->tr.flags & DESC_P_MASK)) > cpu_abort(env, "invalid tss"); > type = (env->tr.flags >> DESC_TYPE_SHIFT) & 0xf; > +#ifdef USE_KVM > + /* > + * Bit 1 is the Busy bit. We believe it is legal to interrupt into a busy > + * segment > + */ > + if ((kvm_allowed && (type & 5) != 1) || (type & 7) != 1) > +#else > if ((type & 7) != 1) > +#endif > This is unnecessary now. However I also think it is correct (e.g. the #else part can be eliminated) > +#ifdef USE_KVM > + /* Probable qemu bug: 11 is a valid segment type */ > + ((env->tr.flags >> DESC_TYPE_SHIFT) & 0xd) != 9 || > +#else > ((env->tr.flags >> DESC_TYPE_SHIFT) & 0xf) != 9 || > +#endif > Ditto. > env->tr.limit < 103) > goto fail; > io_offset = lduw_kernel(env->tr.base + 0x66); > @@ -839,6 +854,13 @@ static void do_interrupt64(int intno, in > uint32_t e1, e2, e3, ss; > target_ulong old_eip, esp, offset; > > +#ifdef USE_KVM > + if (kvm_allowed) { > + printf("%s: unexpect\n", __FUNCTION__); > + exit(-1); > + } > +#endif > + > Unnecessary, I think. > has_error_code = 0; > if (!is_int && !is_hw) { > switch(intno) { > @@ -1122,6 +1144,12 @@ void do_interrupt_user(int intno, int is > int dpl, cpl; > uint32_t e2; > > +#ifdef USE_KVM > + if (kvm_allowed) { > + printf("%s: unexpect\n", __FUNCTION__); > + exit(-1); > + } > +#endif > Ditto. > dt = &env->idt; > ptr = dt->base + (intno * 8); > e2 = ldl_kernel(ptr + 4); > @@ -1147,6 +1175,12 @@ void do_interrupt(int intno, int is_int, > void do_interrupt(int intno, int is_int, int error_code, > target_ulong next_eip, int is_hw) > { > +#ifdef USE_KVM > + if (kvm_allowed) { > + printf("%s: unexpect\n", __FUNCTION__); > + exit(-1); > + } > +#endif > Ditto. > if (loglevel & CPU_LOG_INT) { > if ((env->cr[0] & CR0_PE_MASK)) { > static int count; > @@ -1958,6 +1992,12 @@ void helper_ljmp_protected_T0_T1(int nex > cpu_x86_load_seg_cache(env, R_CS, (new_cs & 0xfffc) | cpl, > get_seg_base(e1, e2), limit, e2); > EIP = new_eip; > +#ifdef USE_KVM > + if (kvm_allowed && (e2 & DESC_L_MASK)) { > + env->exception_index = -1; > + cpu_loop_exit(); > + } > +#endif > Ditto. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- 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