From mboxrd@z Thu Jan  1 00:00:00 1970
From: Borislav Petkov <bp@alien8.de>
Subject: Re: [PATCH -v2] kvm: Emulate MOVBE
Date: Sun, 21 Apr 2013 13:46:50 +0200
Message-ID: <20130421114649.GC4594@pd.tnic>
References: <20130409234602.GI5077@pd.tnic>
 <20130410112942.07dfc167@slackpad>
 <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>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: Gleb Natapov <gleb@redhat.com>, Andre Przywara <andre@andrep.de>,
	kvm@vger.kernel.org,
	=?utf-8?B?SsO2cmcgUsO2ZGVs?= <joro@8bytes.org>,
	"H. Peter Anvin" <hpa@zytor.com>, x86-ml <x86@kernel.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail.skyhub.de ([78.46.96.112]:42861 "EHLO mail.skyhub.de"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752445Ab3DULqf (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sun, 21 Apr 2013 07:46:35 -0400
Content-Disposition: inline
In-Reply-To: <516D37A0.2070706@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Tue, Apr 16, 2013 at 01:36:00PM +0200, Paolo Bonzini wrote:
> Il 14/04/2013 23:02, Borislav Petkov ha scritto:
> > 	*(u16 *)&ctxt->dst.val =3D swab16((u16)ctxt->src.val);
> >=20
> > 	movzwl  112(%rdi), %eax # ctxt_5(D)->src.D.27823.val, tmp82
> > 	rolw    $8, %ax #, tmp82
> > 	movw    %ax, 240(%rdi)  # tmp82, MEM[(u16 *)ctxt_5(D) + 240B]
>=20
> I think this breaks the C aliasing rules.
>=20
> Using valptr is okay because it is a char.

Yep, good catch.

We normally build with -fno-strict-aliasing but when I change that, gcc
catches it:

arch/x86/kvm/emulate.c: In function =E2=80=98em_movbe=E2=80=99:
arch/x86/kvm/emulate.c:3121:3: warning: dereferencing type-punned point=
er will break strict-aliasing rules [-Wstrict-aliasing]

However, it screams even louder about the valptr variant

	*(u16 *)ctxt->dst.valptr =3D swab16(*(u16 *)ctxt->src.valptr);

too:

arch/x86/kvm/emulate.c: In function =E2=80=98em_movbe=E2=80=99:
arch/x86/kvm/emulate.c:3117:3: warning: dereferencing type-punned point=
er will break strict-aliasing rules [-Wstrict-aliasing]
arch/x86/kvm/emulate.c:3117:3: warning: dereferencing type-punned point=
er will break strict-aliasing rules [-Wstrict-aliasing]
arch/x86/kvm/emulate.c:3117:3: warning: dereferencing type-punned point=
er will break strict-aliasing rules [-Wstrict-aliasing]
arch/x86/kvm/emulate.c:3117:3: warning: dereferencing type-punned point=
er will break strict-aliasing rules [-Wstrict-aliasing]
arch/x86/kvm/emulate.c:3117:3: warning: dereferencing type-punned point=
er will break strict-aliasing rules [-Wstrict-aliasing]

We probably need something with copying values to a temp variable or so=
=2E

Thanks.

--=20
Regards/Gruss,
    Boris.

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