Re: [PATCH -v2] kvm: Emulate MOVBE

[Borislav Petkov <bp@alien8.de>]

On Mon, Apr 22, 2013 at 10:53:42AM +0200, Paolo Bonzini wrote:
> Il 21/04/2013 14:23, Borislav Petkov ha scritto:
> > On Sun, Apr 21, 2013 at 01:46:50PM +0200, Borislav Petkov wrote:
> >> We probably need something with copying values to a temp variable or so.
> > 
> > Basically something like that:
> > 
> >         case 2:
> >                 /*
> >                  * From MOVBE definition: "...When the operand size is 16 bits,
> >                  * the upper word of the destination register remains unchanged
> >                  * ..."
> >                  *
> >                  * Both casting ->valptr and ->val to u16 breaks strict aliasing
> >                  * rules so we have to do the operation almost per hand.
> >                  */
> >                 tmp = (u16)ctxt->src.val;
> >                 ctxt->dst.val &= ~0xffffUL;
> >                 ctxt->dst.val |= (unsigned long)swab16(tmp);
> > 		break;
> > 
> > This passes all gcc checks, even the stricter ones when building with W=3.
> 
> I thought the valptr one was ok.

Yep, it looked like that too. And, it could actually really be ok and
the gcc's warning here is bogus. I'll try to talk to gcc people about
it.

> I find this one more readable, too. How does the generated code look
> like?

Well, so so:

	movzwl	112(%rdi), %eax	# ctxt_5(D)->src.D.27823.val, tmp87
	movq	240(%rdi), %rdx	# ctxt_5(D)->dst.D.27823.val, tmp89
	xorw	%dx, %dx	# tmp89
	rolw	$8, %ax	#, tmp87
	movzwl	%ax, %eax	# tmp87, tmp91

I have hard time understanding why it is adding this insn here - it can
simply drop it and continue with the 64-bit OR. It's not like it changes
anything...

	orq	%rdx, %rax	# tmp89, tmp91
	movq	%rax, 240(%rdi)	# tmp91, ctxt_5(D)->dst.D.27823.val

Btw, I wanted to ask: when kvm commits the results, does it look at
ctxt->op_bytes to know exactly how many bytes to write to the guest?
Because if it does, we can save ourselves the trouble here.

Or does it simply write both the full sizeof(unsigned long) bytes of
->src.val and ->dst.val to the guest?

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--