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

2011-12-14 Thread Matt Evans
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

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

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

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