From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sasha Levin <levinsasha928@gmail.com>
Subject: Re: [PATCH] kvm tools: Clean up LINT assignment code
Date: Wed, 14 Dec 2011 08:13:07 +0200
Message-ID: <1323843187.3992.11.camel@lappy>
References: <1323633019-8424-1-git-send-email-levinsasha928@gmail.com>
	 <4EE804B4.5020904@ozlabs.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1251
Content-Transfer-Encoding: QUOTED-PRINTABLE
Cc: penberg@kernel.org, mingo@elte.hu, gorcunov@gmail.com,
	asias.hejun@gmail.com, kvm@vger.kernel.org
To: Matt Evans <matt@ozlabs.org>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-ee0-f46.google.com ([74.125.83.46]:43091 "EHLO
	mail-ee0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1750774Ab1LNGNN (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 14 Dec 2011 01:13:13 -0500
Received: by eekc4 with SMTP id c4so421216eek.19
        for <kvm@vger.kernel.org>; Tue, 13 Dec 2011 22:13:12 -0800 (PST)
In-Reply-To: <4EE804B4.5020904@ozlabs.org>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, 2011-12-14 at 13:06 +1100, Matt Evans wrote:
> Hi Sasha,
>=20
> On 12/12/11 06:50, Sasha Levin wrote:
> > Just set delivery mode directly without going through ugly casting.
> >=20
> > This cleans up and simplifies the code.
> >=20
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> > ---
> >  tools/kvm/x86/kvm-cpu.c |   10 ++--------
> >  1 files changed, 2 insertions(+), 8 deletions(-)
> >=20
> > diff --git a/tools/kvm/x86/kvm-cpu.c b/tools/kvm/x86/kvm-cpu.c
> > index 27b7a8f..cc1f560 100644
> > --- a/tools/kvm/x86/kvm-cpu.c
> > +++ b/tools/kvm/x86/kvm-cpu.c
> > @@ -81,18 +81,12 @@ static int kvm_cpu__set_lint(struct kvm_cpu *vc=
pu)
> >  {
> >  	struct kvm_lapic_state klapic;
> >  	struct local_apic *lapic =3D (void *)&klapic;
> > -	u32 lvt;
> > =20
> >  	if (ioctl(vcpu->vcpu_fd, KVM_GET_LAPIC, &klapic))
> >  		return -1;
> > =20
> > -	lvt =3D *(u32 *)&lapic->lvt_lint0;
> > -	lvt =3D SET_APIC_DELIVERY_MODE(lvt, APIC_MODE_EXTINT);
> > -	*(u32 *)&lapic->lvt_lint0 =3D lvt;
> > -
> > -	lvt =3D *(u32 *)&lapic->lvt_lint1;
> > -	lvt =3D SET_APIC_DELIVERY_MODE(lvt, APIC_MODE_NMI);
> > -	*(u32 *)&lapic->lvt_lint1 =3D lvt;
> > +	lapic->lvt_lint0.delivery_mode =3D APIC_MODE_EXTINT;
> > +	lapic->lvt_lint1.delivery_mode =3D APIC_MODE_NMI;
> > =20
> >  	return ioctl(vcpu->vcpu_fd, KVM_SET_LAPIC, &klapic);
> >  }
>=20
> I'm getting this on x86-32, gcc 4.4.3:
>=20
>   CC       x86/kvm-cpu.o
> cc1: warnings being treated as errors
> x86/kvm-cpu.c: In function =91kvm_cpu__set_lint=92:
> x86/kvm-cpu.c:89: error: dereferencing pointer =91lapic=92 does break=
 strict-aliasing rules
> x86/kvm-cpu.c:88: error: dereferencing pointer =91lapic=92 does break=
 strict-aliasing rules
> x86/kvm-cpu.c:83: note: initialized from here
> make: *** [x86/kvm-cpu.o] Error 1
>=20
> Removing the nasty aliasing (patch below) seems to be a good way to g=
o.  What do
> you think?
>=20
>=20
> Cheers,
>=20
>=20
> Matt
>=20
> ---
>=20
> diff --git a/tools/kvm/x86/kvm-cpu.c b/tools/kvm/x86/kvm-cpu.c
> index cc1f560..30f1ad6 100644
> --- a/tools/kvm/x86/kvm-cpu.c
> +++ b/tools/kvm/x86/kvm-cpu.c
> @@ -79,16 +79,15 @@ void kvm_cpu__delete(struct kvm_cpu *vcpu)
> =20
>  static int kvm_cpu__set_lint(struct kvm_cpu *vcpu)
>  {
> -	struct kvm_lapic_state klapic;
> -	struct local_apic *lapic =3D (void *)&klapic;
> +	struct local_apic lapic;
> =20
> -	if (ioctl(vcpu->vcpu_fd, KVM_GET_LAPIC, &klapic))
> +	if (ioctl(vcpu->vcpu_fd, KVM_GET_LAPIC, &lapic))
>  		return -1;

It felt a bit wrong to me getting the ioctl to assign it directly to
local_apic since api.txt says it should be returning a kvm_lapic_state.
But on the other hand, api.txt also says "The data format and layout ar=
e
the same as documented in the architecture manual." so I guess we can g=
o
with that.

	Acked-by: Sasha Levin <levinsasha928@gmail.com>

--=20

Sasha.