From mboxrd@z Thu Jan 1 00:00:00 1970 From: Borislav Petkov Subject: Re: [PATCH -v2] kvm: Emulate MOVBE Date: Mon, 22 Apr 2013 11:38:10 +0200 Message-ID: <20130422093810.GC4637@pd.tnic> References: <20130410100845.GB17919@redhat.com> <20130410123901.46b65169@slackpad> <20130410121639.GE17919@redhat.com> <20130411001815.GA17544@pd.tnic> <20130414084303.GE17919@redhat.com> <20130414210218.GF20547@pd.tnic> <516D37A0.2070706@redhat.com> <20130421114649.GC4594@pd.tnic> <20130421122321.GD4594@pd.tnic> <5174FA96.1040807@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: Gleb Natapov , Andre Przywara , kvm@vger.kernel.org, =?utf-8?B?SsO2cmcgUsO2ZGVs?= , "H. Peter Anvin" , x86-ml To: Paolo Bonzini Return-path: Received: from mail.skyhub.de ([78.46.96.112]:45945 "EHLO mail.skyhub.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753157Ab3DVJiN (ORCPT ); Mon, 22 Apr 2013 05:38:13 -0400 Content-Disposition: inline In-Reply-To: <5174FA96.1040807@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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. --