Patchwork [1/3] kvm: move KVM_GET_LAPIC/KVM_SET_LAPIC to hw/kvm/apic.c

login
register
mail settings
Submitter Paolo Bonzini
Date Oct. 30, 2012, 12:16 p.m.
Message ID <1351599394-24876-2-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/195471/
State New
Headers show

Comments

Paolo Bonzini - Oct. 30, 2012, 12:16 p.m.
Leave knowledge of the KVM in-kernel LAPIC ioctls to hw/kvm/apic.c.
The CPU doesn't need to know anything about kvm_lapic_state.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/kvm/apic.c     |   76 ++++++++++++++++++++++++++++++-----------------------
 kvm.h             |    4 +-
 target-i386/kvm.c |   14 +--------
 3 files changed, 47 insertions(+), 47 deletions(-)
Jan Kiszka - Oct. 30, 2012, 6:13 p.m.
On 2012-10-30 13:16, Paolo Bonzini wrote:
> Leave knowledge of the KVM in-kernel LAPIC ioctls to hw/kvm/apic.c.
> The CPU doesn't need to know anything about kvm_lapic_state.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/kvm/apic.c     |   76 ++++++++++++++++++++++++++++++-----------------------
>  kvm.h             |    4 +-
>  target-i386/kvm.c |   14 +--------
>  3 files changed, 47 insertions(+), 47 deletions(-)
> 
> diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c
> index dbac7ff..ddf6b7d 100644
> --- a/hw/kvm/apic.c
> +++ b/hw/kvm/apic.c
> @@ -25,62 +25,72 @@ static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
>      return *((uint32_t *)(kapic->regs + (reg_id << 4)));
>  }
>  
> -void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
> +int kvm_put_apic_state(DeviceState *d)
>  {
>      APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> +    struct kvm_lapic_state kapic;
>      int i;
>  
> -    memset(kapic, 0, sizeof(*kapic));
> -    kvm_apic_set_reg(kapic, 0x2, s->id << 24);
> -    kvm_apic_set_reg(kapic, 0x8, s->tpr);
> -    kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24);
> -    kvm_apic_set_reg(kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
> -    kvm_apic_set_reg(kapic, 0xf, s->spurious_vec);
> +    memset(&kapic, 0, sizeof(kapic));
> +    kvm_apic_set_reg(&kapic, 0x2, s->id << 24);
> +    kvm_apic_set_reg(&kapic, 0x8, s->tpr);
> +    kvm_apic_set_reg(&kapic, 0xd, s->log_dest << 24);
> +    kvm_apic_set_reg(&kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
> +    kvm_apic_set_reg(&kapic, 0xf, s->spurious_vec);
>      for (i = 0; i < 8; i++) {
> -        kvm_apic_set_reg(kapic, 0x10 + i, s->isr[i]);
> -        kvm_apic_set_reg(kapic, 0x18 + i, s->tmr[i]);
> -        kvm_apic_set_reg(kapic, 0x20 + i, s->irr[i]);
> +        kvm_apic_set_reg(&kapic, 0x10 + i, s->isr[i]);
> +        kvm_apic_set_reg(&kapic, 0x18 + i, s->tmr[i]);
> +        kvm_apic_set_reg(&kapic, 0x20 + i, s->irr[i]);
>      }
> -    kvm_apic_set_reg(kapic, 0x28, s->esr);
> -    kvm_apic_set_reg(kapic, 0x30, s->icr[0]);
> -    kvm_apic_set_reg(kapic, 0x31, s->icr[1]);
> +    kvm_apic_set_reg(&kapic, 0x28, s->esr);
> +    kvm_apic_set_reg(&kapic, 0x30, s->icr[0]);
> +    kvm_apic_set_reg(&kapic, 0x31, s->icr[1]);
>      for (i = 0; i < APIC_LVT_NB; i++) {
> -        kvm_apic_set_reg(kapic, 0x32 + i, s->lvt[i]);
> +        kvm_apic_set_reg(&kapic, 0x32 + i, s->lvt[i]);
>      }
> -    kvm_apic_set_reg(kapic, 0x38, s->initial_count);
> -    kvm_apic_set_reg(kapic, 0x3e, s->divide_conf);
> +    kvm_apic_set_reg(&kapic, 0x38, s->initial_count);
> +    kvm_apic_set_reg(&kapic, 0x3e, s->divide_conf);
> +
> +    return kvm_vcpu_ioctl(s->cpu_env, KVM_SET_LAPIC, &kapic);
>  }
>  
> -void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
> +int kvm_get_apic_state(DeviceState *d)
>  {
>      APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> -    int i, v;
> -
> -    s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
> -    s->tpr = kvm_apic_get_reg(kapic, 0x8);
> -    s->arb_id = kvm_apic_get_reg(kapic, 0x9);
> -    s->log_dest = kvm_apic_get_reg(kapic, 0xd) >> 24;
> -    s->dest_mode = kvm_apic_get_reg(kapic, 0xe) >> 28;
> -    s->spurious_vec = kvm_apic_get_reg(kapic, 0xf);
> +    struct kvm_lapic_state kapic;
> +    int i, v, ret;
> +
> +    ret = kvm_vcpu_ioctl(s->cpu_env, KVM_GET_LAPIC, &kapic);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    s->id = kvm_apic_get_reg(&kapic, 0x2) >> 24;
> +    s->tpr = kvm_apic_get_reg(&kapic, 0x8);
> +    s->arb_id = kvm_apic_get_reg(&kapic, 0x9);
> +    s->log_dest = kvm_apic_get_reg(&kapic, 0xd) >> 24;
> +    s->dest_mode = kvm_apic_get_reg(&kapic, 0xe) >> 28;
> +    s->spurious_vec = kvm_apic_get_reg(&kapic, 0xf);
>      for (i = 0; i < 8; i++) {
> -        s->isr[i] = kvm_apic_get_reg(kapic, 0x10 + i);
> -        s->tmr[i] = kvm_apic_get_reg(kapic, 0x18 + i);
> -        s->irr[i] = kvm_apic_get_reg(kapic, 0x20 + i);
> +        s->isr[i] = kvm_apic_get_reg(&kapic, 0x10 + i);
> +        s->tmr[i] = kvm_apic_get_reg(&kapic, 0x18 + i);
> +        s->irr[i] = kvm_apic_get_reg(&kapic, 0x20 + i);
>      }
> -    s->esr = kvm_apic_get_reg(kapic, 0x28);
> -    s->icr[0] = kvm_apic_get_reg(kapic, 0x30);
> -    s->icr[1] = kvm_apic_get_reg(kapic, 0x31);
> +    s->esr = kvm_apic_get_reg(&kapic, 0x28);
> +    s->icr[0] = kvm_apic_get_reg(&kapic, 0x30);
> +    s->icr[1] = kvm_apic_get_reg(&kapic, 0x31);
>      for (i = 0; i < APIC_LVT_NB; i++) {
> -        s->lvt[i] = kvm_apic_get_reg(kapic, 0x32 + i);
> +        s->lvt[i] = kvm_apic_get_reg(&kapic, 0x32 + i);
>      }
> -    s->initial_count = kvm_apic_get_reg(kapic, 0x38);
> -    s->divide_conf = kvm_apic_get_reg(kapic, 0x3e);
> +    s->initial_count = kvm_apic_get_reg(&kapic, 0x38);
> +    s->divide_conf = kvm_apic_get_reg(&kapic, 0x3e);
>  
>      v = (s->divide_conf & 3) | ((s->divide_conf >> 1) & 4);
>      s->count_shift = (v + 1) & 7;
>  
>      s->initial_count_load_time = qemu_get_clock_ns(vm_clock);
>      apic_next_timer(s, s->initial_count_load_time);
> +    return 0;
>  }
>  
>  static void kvm_apic_set_base(APICCommonState *s, uint64_t val)
> diff --git a/kvm.h b/kvm.h
> index 2b26dcb..0056f92 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -191,8 +191,8 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg);
>  
>  void kvm_irqchip_add_irq_route(KVMState *s, int gsi, int irqchip, int pin);
>  
> -void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic);
> -void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic);
> +int kvm_put_apic_state(DeviceState *d);
> +int kvm_get_apic_state(DeviceState *d);
>  
>  struct kvm_guest_debug;
>  struct kvm_debug_exit_arch;
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 3aa62b2..092d4f1 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -1384,16 +1384,9 @@ static int kvm_get_mp_state(CPUX86State *env)
>  static int kvm_get_apic(CPUX86State *env)
>  {
>      DeviceState *apic = env->apic_state;
> -    struct kvm_lapic_state kapic;
> -    int ret;
>  
>      if (apic && kvm_irqchip_in_kernel()) {
> -        ret = kvm_vcpu_ioctl(env, KVM_GET_LAPIC, &kapic);
> -        if (ret < 0) {
> -            return ret;
> -        }
> -
> -        kvm_get_apic_state(apic, &kapic);
> +        return kvm_get_apic_state(apic);
>      }
>      return 0;
>  }
> @@ -1401,12 +1394,9 @@ static int kvm_get_apic(CPUX86State *env)
>  static int kvm_put_apic(CPUX86State *env)
>  {
>      DeviceState *apic = env->apic_state;
> -    struct kvm_lapic_state kapic;
>  
>      if (apic && kvm_irqchip_in_kernel()) {
> -        kvm_put_apic_state(apic, &kapic);
> -
> -        return kvm_vcpu_ioctl(env, KVM_SET_LAPIC, &kapic);
> +        return kvm_put_apic_state(apic);
>      }
>      return 0;
>  }
> 

This refactoring is fine with me.

Acked-by: Jan Kiszka <jan.kiszka@siemens.com>

Jan

Patch

diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c
index dbac7ff..ddf6b7d 100644
--- a/hw/kvm/apic.c
+++ b/hw/kvm/apic.c
@@ -25,62 +25,72 @@  static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
     return *((uint32_t *)(kapic->regs + (reg_id << 4)));
 }
 
-void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+int kvm_put_apic_state(DeviceState *d)
 {
     APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
+    struct kvm_lapic_state kapic;
     int i;
 
-    memset(kapic, 0, sizeof(*kapic));
-    kvm_apic_set_reg(kapic, 0x2, s->id << 24);
-    kvm_apic_set_reg(kapic, 0x8, s->tpr);
-    kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24);
-    kvm_apic_set_reg(kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
-    kvm_apic_set_reg(kapic, 0xf, s->spurious_vec);
+    memset(&kapic, 0, sizeof(kapic));
+    kvm_apic_set_reg(&kapic, 0x2, s->id << 24);
+    kvm_apic_set_reg(&kapic, 0x8, s->tpr);
+    kvm_apic_set_reg(&kapic, 0xd, s->log_dest << 24);
+    kvm_apic_set_reg(&kapic, 0xe, s->dest_mode << 28 | 0x0fffffff);
+    kvm_apic_set_reg(&kapic, 0xf, s->spurious_vec);
     for (i = 0; i < 8; i++) {
-        kvm_apic_set_reg(kapic, 0x10 + i, s->isr[i]);
-        kvm_apic_set_reg(kapic, 0x18 + i, s->tmr[i]);
-        kvm_apic_set_reg(kapic, 0x20 + i, s->irr[i]);
+        kvm_apic_set_reg(&kapic, 0x10 + i, s->isr[i]);
+        kvm_apic_set_reg(&kapic, 0x18 + i, s->tmr[i]);
+        kvm_apic_set_reg(&kapic, 0x20 + i, s->irr[i]);
     }
-    kvm_apic_set_reg(kapic, 0x28, s->esr);
-    kvm_apic_set_reg(kapic, 0x30, s->icr[0]);
-    kvm_apic_set_reg(kapic, 0x31, s->icr[1]);
+    kvm_apic_set_reg(&kapic, 0x28, s->esr);
+    kvm_apic_set_reg(&kapic, 0x30, s->icr[0]);
+    kvm_apic_set_reg(&kapic, 0x31, s->icr[1]);
     for (i = 0; i < APIC_LVT_NB; i++) {
-        kvm_apic_set_reg(kapic, 0x32 + i, s->lvt[i]);
+        kvm_apic_set_reg(&kapic, 0x32 + i, s->lvt[i]);
     }
-    kvm_apic_set_reg(kapic, 0x38, s->initial_count);
-    kvm_apic_set_reg(kapic, 0x3e, s->divide_conf);
+    kvm_apic_set_reg(&kapic, 0x38, s->initial_count);
+    kvm_apic_set_reg(&kapic, 0x3e, s->divide_conf);
+
+    return kvm_vcpu_ioctl(s->cpu_env, KVM_SET_LAPIC, &kapic);
 }
 
-void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+int kvm_get_apic_state(DeviceState *d)
 {
     APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
-    int i, v;
-
-    s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
-    s->tpr = kvm_apic_get_reg(kapic, 0x8);
-    s->arb_id = kvm_apic_get_reg(kapic, 0x9);
-    s->log_dest = kvm_apic_get_reg(kapic, 0xd) >> 24;
-    s->dest_mode = kvm_apic_get_reg(kapic, 0xe) >> 28;
-    s->spurious_vec = kvm_apic_get_reg(kapic, 0xf);
+    struct kvm_lapic_state kapic;
+    int i, v, ret;
+
+    ret = kvm_vcpu_ioctl(s->cpu_env, KVM_GET_LAPIC, &kapic);
+    if (ret < 0) {
+        return ret;
+    }
+
+    s->id = kvm_apic_get_reg(&kapic, 0x2) >> 24;
+    s->tpr = kvm_apic_get_reg(&kapic, 0x8);
+    s->arb_id = kvm_apic_get_reg(&kapic, 0x9);
+    s->log_dest = kvm_apic_get_reg(&kapic, 0xd) >> 24;
+    s->dest_mode = kvm_apic_get_reg(&kapic, 0xe) >> 28;
+    s->spurious_vec = kvm_apic_get_reg(&kapic, 0xf);
     for (i = 0; i < 8; i++) {
-        s->isr[i] = kvm_apic_get_reg(kapic, 0x10 + i);
-        s->tmr[i] = kvm_apic_get_reg(kapic, 0x18 + i);
-        s->irr[i] = kvm_apic_get_reg(kapic, 0x20 + i);
+        s->isr[i] = kvm_apic_get_reg(&kapic, 0x10 + i);
+        s->tmr[i] = kvm_apic_get_reg(&kapic, 0x18 + i);
+        s->irr[i] = kvm_apic_get_reg(&kapic, 0x20 + i);
     }
-    s->esr = kvm_apic_get_reg(kapic, 0x28);
-    s->icr[0] = kvm_apic_get_reg(kapic, 0x30);
-    s->icr[1] = kvm_apic_get_reg(kapic, 0x31);
+    s->esr = kvm_apic_get_reg(&kapic, 0x28);
+    s->icr[0] = kvm_apic_get_reg(&kapic, 0x30);
+    s->icr[1] = kvm_apic_get_reg(&kapic, 0x31);
     for (i = 0; i < APIC_LVT_NB; i++) {
-        s->lvt[i] = kvm_apic_get_reg(kapic, 0x32 + i);
+        s->lvt[i] = kvm_apic_get_reg(&kapic, 0x32 + i);
     }
-    s->initial_count = kvm_apic_get_reg(kapic, 0x38);
-    s->divide_conf = kvm_apic_get_reg(kapic, 0x3e);
+    s->initial_count = kvm_apic_get_reg(&kapic, 0x38);
+    s->divide_conf = kvm_apic_get_reg(&kapic, 0x3e);
 
     v = (s->divide_conf & 3) | ((s->divide_conf >> 1) & 4);
     s->count_shift = (v + 1) & 7;
 
     s->initial_count_load_time = qemu_get_clock_ns(vm_clock);
     apic_next_timer(s, s->initial_count_load_time);
+    return 0;
 }
 
 static void kvm_apic_set_base(APICCommonState *s, uint64_t val)
diff --git a/kvm.h b/kvm.h
index 2b26dcb..0056f92 100644
--- a/kvm.h
+++ b/kvm.h
@@ -191,8 +191,8 @@  int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg);
 
 void kvm_irqchip_add_irq_route(KVMState *s, int gsi, int irqchip, int pin);
 
-void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic);
-void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic);
+int kvm_put_apic_state(DeviceState *d);
+int kvm_get_apic_state(DeviceState *d);
 
 struct kvm_guest_debug;
 struct kvm_debug_exit_arch;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 3aa62b2..092d4f1 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1384,16 +1384,9 @@  static int kvm_get_mp_state(CPUX86State *env)
 static int kvm_get_apic(CPUX86State *env)
 {
     DeviceState *apic = env->apic_state;
-    struct kvm_lapic_state kapic;
-    int ret;
 
     if (apic && kvm_irqchip_in_kernel()) {
-        ret = kvm_vcpu_ioctl(env, KVM_GET_LAPIC, &kapic);
-        if (ret < 0) {
-            return ret;
-        }
-
-        kvm_get_apic_state(apic, &kapic);
+        return kvm_get_apic_state(apic);
     }
     return 0;
 }
@@ -1401,12 +1394,9 @@  static int kvm_get_apic(CPUX86State *env)
 static int kvm_put_apic(CPUX86State *env)
 {
     DeviceState *apic = env->apic_state;
-    struct kvm_lapic_state kapic;
 
     if (apic && kvm_irqchip_in_kernel()) {
-        kvm_put_apic_state(apic, &kapic);
-
-        return kvm_vcpu_ioctl(env, KVM_SET_LAPIC, &kapic);
+        return kvm_put_apic_state(apic);
     }
     return 0;
 }