* [PATCH] kvm tools: Clean up LINT assignment code @ 2011-12-11 19:50 Sasha Levin 2011-12-14 2:06 ` Matt Evans 0 siblings, 1 reply; 4+ messages in thread From: Sasha Levin @ 2011-12-11 19:50 UTC (permalink / raw) To: penberg; +Cc: mingo, gorcunov, asias.hejun, kvm, Sasha Levin 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); } -- 1.7.8 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] kvm tools: Clean up LINT assignment code 2011-12-11 19:50 [PATCH] kvm tools: Clean up LINT assignment code Sasha Levin @ 2011-12-14 2:06 ` Matt Evans 2011-12-14 6:13 ` Sasha Levin 0 siblings, 1 reply; 4+ messages in thread From: Matt Evans @ 2011-12-14 2:06 UTC (permalink / raw) To: Sasha Levin; +Cc: penberg, mingo, gorcunov, asias.hejun, kvm 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) ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] kvm tools: Clean up LINT assignment code 2011-12-14 2:06 ` Matt Evans @ 2011-12-14 6:13 ` Sasha Levin 2011-12-14 23:25 ` Matt Evans 0 siblings, 1 reply; 4+ messages in thread From: Sasha Levin @ 2011-12-14 6:13 UTC (permalink / raw) To: Matt Evans; +Cc: penberg, mingo, gorcunov, asias.hejun, kvm 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@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 <levinsasha928@gmail.com> -- Sasha. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] kvm tools: Clean up LINT assignment code 2011-12-14 6:13 ` Sasha Levin @ 2011-12-14 23:25 ` Matt Evans 0 siblings, 0 replies; 4+ messages in thread From: Matt Evans @ 2011-12-14 23:25 UTC (permalink / raw) To: Sasha Levin Cc: penberg@kernel.org, mingo@elte.hu, gorcunov@gmail.com, asias.hejun@gmail.com, kvm@vger.kernel.org On 14 Dec 2011, at 17:13, Sasha Levin <levinsasha928@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 <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; > > 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@gmail.com> Pekka, as I forgot the SOB: Signed-off-by: Matt Evans <matt@ozlabs.org> (Or would you prefer a repost so it's all in one?) Matt ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-12-14 23:25 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-12-11 19:50 [PATCH] kvm tools: Clean up LINT assignment code Sasha Levin 2011-12-14 2:06 ` Matt Evans 2011-12-14 6:13 ` Sasha Levin 2011-12-14 23:25 ` Matt Evans
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox