Re: [PATCH] Support for QEMU's CVS

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.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