On Wed, 2011-12-14 at 13:06 +1100, Matt Evans wrote:
> Hi Sasha,
>
> On 12/12/11 06:50, Sasha Levin wrote:
> > Just set delivery mode directly without going through ugly casting.
> >
> > This cleans up and simplifies the code.
> >
> > Signed-off-by: Sasha Levin <levinsasha928@xxxxxxxxx>
> > ---
> > tools/kvm/x86/kvm-cpu.c | 10 ++--------
> > 1 files changed, 2 insertions(+), 8 deletions(-)
> >
> > 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 *vcpu)
> > {
> > struct kvm_lapic_state klapic;
> > struct local_apic *lapic = (void *)&klapic;
> > - u32 lvt;
> >
> > if (ioctl(vcpu->vcpu_fd, KVM_GET_LAPIC, &klapic))
> > return -1;
> >
> > - lvt = *(u32 *)&lapic->lvt_lint0;
> > - lvt = SET_APIC_DELIVERY_MODE(lvt, APIC_MODE_EXTINT);
> > - *(u32 *)&lapic->lvt_lint0 = lvt;
> > -
> > - lvt = *(u32 *)&lapic->lvt_lint1;
> > - lvt = SET_APIC_DELIVERY_MODE(lvt, APIC_MODE_NMI);
> > - *(u32 *)&lapic->lvt_lint1 = lvt;
> > + lapic->lvt_lint0.delivery_mode = APIC_MODE_EXTINT;
> > + lapic->lvt_lint1.delivery_mode = APIC_MODE_NMI;
> >
> > return ioctl(vcpu->vcpu_fd, KVM_SET_LAPIC, &klapic);
> > }
>
> I'm getting this on x86-32, gcc 4.4.3:
>
> CC x86/kvm-cpu.o
> cc1: warnings being treated as errors
> x86/kvm-cpu.c: In function ‘kvm_cpu__set_lint’:
> x86/kvm-cpu.c:89: error: dereferencing pointer ‘lapic’ does break strict-aliasing rules
> x86/kvm-cpu.c:88: error: dereferencing pointer ‘lapic’ does break strict-aliasing rules
> x86/kvm-cpu.c:83: note: initialized from here
> make: *** [x86/kvm-cpu.o] Error 1
>
> Removing the nasty aliasing (patch below) seems to be a good way to go. What do
> you think?
>
>
> Cheers,
>
>
> Matt
>
> ---
>
> 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)
>
> static int kvm_cpu__set_lint(struct kvm_cpu *vcpu)
> {
> - struct kvm_lapic_state klapic;
> - struct local_apic *lapic = (void *)&klapic;
> + struct local_apic lapic;
>
> - 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 are
the same as documented in the architecture manual." so I guess we can go
with that.
Acked-by: Sasha Levin <levinsasha928@xxxxxxxxx>
--
Sasha.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
