Re: "KVM internal error. Suberror: 1" with ancient 2.4 kernel as guest

[Avi Kivity <avi@redhat.com>]

On 03/26/2011 12:12 AM, Wei Xu wrote:
> Jiri&  Avi:
>
> I attached the patched I did for movq and movdqa emulation. Please note:
> (1) I only implemented those two. Other instructions like addq may be
> following same way.
> (2) I use same guest_fx_image to hold value and fxsave/fxrstor to copy
> to/from registers. This is not very efficient I admit.
> Any suggestions let me know.
>

Patch is severely whitespace damaged.  Please observe the kernel 
whitespace style.

I just remembered that I implemented this once - see the (very old) 
branch sse-mmio in kvm.git.


> Index: linux/contents/arch/x86/include/asm/kvm_emulate.h
> ===================================================================
> --- linux.orig/contents/arch/x86/include/asm/kvm_emulate.h	2010-07-19 06:42:26.000000000 -0700
> +++ linux/contents/arch/x86/include/asm/kvm_emulate.h	2011-03-21 09:16:39.000000000 -0700
> @@ -116,6 +116,7 @@
>   	enum { OP_REG, OP_MEM, OP_IMM, OP_NONE } type;
>   	unsigned int bytes;
>   	unsigned long val, orig_val, *ptr;
> +    unsigned long val_simd[2];
>   };

Breaks on i386 (ulong is 32-bit).

>
>   	if (c->src.type == OP_MEM) {
> +        void *val;
>   		c->src.ptr = (unsigned long *)memop;
>   		c->src.val = 0;
> +        if (c->src.bytes>  8) { /* movdq case */
> +            c->src.val_simd[0] = c->src.val_simd[1] = 0;
> +            val = c->src.val_simd;
> +        } else {
> +            val =&c->src.val;
> +        }

We have a union there for that purpose.

> @@ -2506,6 +2529,55 @@
>   		if (!test_cc(c->b, ctxt->eflags))
>   			c->dst.type = OP_NONE; /* no writeback */
>   		break;
> +    case 0x6f: /* movq from mm/m64 to mm; movdqa from xmm/m128 to xmm */
> +        if (c->op_bytes == 8){
> +            ctxt->vcpu->arch.guest_fx_image.st_space[c->modrm_reg<<2] =
> +                (c->src.val&  0x0ffffffff);
> +            ctxt->vcpu->arch.guest_fx_image.st_space[(c->modrm_reg<<2)+1] =
> +                (c->src.val>>  32);
> +            kvm_fx_restore(&ctxt->vcpu->arch.guest_fx_image);
> +            c->dst.type = OP_NONE; /* Disable writeback. */
> +            break;
> +        } else { /* movdqa */
> +            ctxt->vcpu->arch.guest_fx_image.xmm_space[c->modrm_reg<<2] =
> +                (c->src.val_simd[0]&  0x0ffffffff);
> +            ctxt->vcpu->arch.guest_fx_image.xmm_space[(c->modrm_reg<<2)+1] =
> +                (c->src.val_simd[0]>>  32);
> +            ctxt->vcpu->arch.guest_fx_image.xmm_space[(c->modrm_reg<<2)+2] =
> +                (c->src.val_simd[1]&  0x0ffffffff);
> +            ctxt->vcpu->arch.guest_fx_image.xmm_space[(c->modrm_reg<<2)+3] =
> +                (c->src.val_simd[1]>>  32);
> +            kvm_fx_restore(&ctxt->vcpu->arch.guest_fx_image);
> +            c->dst.type = OP_NONE; /* Disable writeback. */
> +            break;
> +        }
> +    case 0x7f: /* movq from mm to mm/m64; movdqa from xmm to xmm/m128 */
> +        if (c->op_bytes == 8) { /* movq */
> +            kvm_fx_save(&ctxt->vcpu->arch.guest_fx_image);
> +            if (c->dst.type == OP_MEM) {
> +                unsigned long lval,uval;
> +                lval = ctxt->vcpu->arch.guest_fx_image.st_space[c->modrm_reg<<2];
> +                uval = ctxt->vcpu->arch.guest_fx_image.st_space[(c->modrm_reg<<2)+1];
> +                c->dst.val = (uval<<32) + lval;
> +            } else {
> +                c->dst.type = OP_NONE; /* Disable writeback. */
> +            }
> +            break;
> +        } else { /* movdqa */
> +            kvm_fx_save(&ctxt->vcpu->arch.guest_fx_image);
> +            if (c->dst.type == OP_MEM) {
> +                unsigned long lval,uval;
> +                lval = ctxt->vcpu->arch.guest_fx_image.xmm_space[c->modrm_reg<<2];
> +                uval = ctxt->vcpu->arch.guest_fx_image.xmm_space[(c->modrm_reg<<2)+1];
> +                c->dst.val_simd[0] = (uval<<32) + lval;
> +                lval = ctxt->vcpu->arch.guest_fx_image.xmm_space[(c->modrm_reg<<2)+2];
> +                uval = ctxt->vcpu->arch.guest_fx_image.xmm_space[(c->modrm_reg<<2)+3];
> +                c->dst.val_simd[1] = (uval<<32) + lval;
> +            } else {
> +                c->dst.type = OP_NONE; /* Disable writeback. */
> +            }
> +            break;
> +        }

In my implementation, I just forced the guest mmu to be active, and used 
the sse instructions directly.
> Index: linux/contents/include/linux/kvm.h
> ===================================================================
> --- linux.orig/contents/include/linux/kvm.h	2010-07-19 06:42:23.000000000 -0700
> +++ linux/contents/include/linux/kvm.h	2011-03-21 09:16:39.000000000 -0700
> @@ -152,7 +152,7 @@
>   		/* KVM_EXIT_MMIO */
>   		struct {
>   			__u64 phys_addr;
> -			__u8  data[8];
> +			__u8  data[16];
>   			__u32 len;
>   			__u8  is_write;
>   		} mmio;

This breaks the userspace interface.  My implementation split the I/O 
into two separate 64-bit writes.

I guess I'll have to rebase it.

-- 
error compiling committee.c: too many arguments to function