subject:"\[PATCH\] kvm tools\: Clean up LINT assignment code"

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

2011-12-14 Thread Matt Evans

On 14 Dec 2011, at 17:13, Sasha Levin  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 
>>> ---
>>> 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 

Pekka, as I forgot the SOB:

Signed-off-by: Matt Evans 

(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

2011-12-13 Thread Sasha Levin

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 
> > ---
> >  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 

-- 

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

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

2011-12-13 Thread Matt Evans

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 
> ---
>  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

[PATCH] kvm tools: Clean up LINT assignment code

2011-12-11 Thread Sasha Levin

Just set delivery mode directly without going through ugly casting.

This cleans up and simplifies the code.

Signed-off-by: Sasha Levin 
---
 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

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

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

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

[PATCH] kvm tools: Clean up LINT assignment code

4 matches

Site Navigation

Mail list logo

Footer information