* [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;
as well as URLs for NNTP newsgroup(s).