Re: [PATCH] kvm tools: Clean up LINT assignment code
On 14 Dec 2011, at 17:13, Sasha Levin levinsasha...@gmail.com wrote: 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 levinsasha...@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; 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 levinsasha...@gmail.com Pekka, as I forgot the SOB: Signed-off-by: Matt Evans m...@ozlabs.org (Or would you prefer a repost so it's all in one?) Matt-- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tools: Clean up LINT assignment code
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 levinsasha...@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) -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] kvm tools: Clean up LINT assignment code
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 levinsasha...@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; 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 levinsasha...@gmail.com -- Sasha. -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] kvm tools: Clean up LINT assignment code
Just set delivery mode directly without going through ugly casting. This cleans up and simplifies the code. Signed-off-by: Sasha Levin levinsasha...@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); } -- 1.7.8 -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html