Re: [PATCH] kvm tools: Clean up LINT assignment code

[Matt Evans <matt@ozlabs.org>]

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@gmail.com>
> ---
>  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;
 
-	lapic->lvt_lint0.delivery_mode = APIC_MODE_EXTINT;
-	lapic->lvt_lint1.delivery_mode = APIC_MODE_NMI;
+	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);
+	return ioctl(vcpu->vcpu_fd, KVM_SET_LAPIC, &lapic);
 }
 
 struct kvm_cpu *kvm_cpu__init(struct kvm *kvm, unsigned long cpu_id)