diff mbox

[v2,9/9] qemu-kvm: hpet: Add MSI support for in-kernel irqchip mode

Message ID 3d089633897515a214af032ed633342a7001945f.1303823975.git.jan.kiszka@siemens.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kiszka April 26, 2011, 1:19 p.m. UTC
Just like PCI devices, the HPET requires special care to route MSI
events to the in-kernel irqchip if enabled.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/hpet.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 70 insertions(+), 1 deletions(-)

Comments

Michael Tokarev April 26, 2011, 1:30 p.m. UTC | #1
26.04.2011 17:19, Jan Kiszka wrote:

>  hw/hpet.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-

> +static int modifying_bit(uint64_t old, uint64_t new, uint64_t mask)
> +{
> +    return (old ^ new) & mask;
> +}

Such constructs always look suspicious.  I'm not even sure anymore
(after using C for over 20 years ;) that this works... how about

 return (old ^ new) & mask  ? 1 : 0;

instead, or something along this?  I mean, if sizeof(int)==4, how
`return 1ULL<<32' will be interpreted in this context?  Tiny test
program tells me it will return 0...

/mjt
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka April 26, 2011, 1:55 p.m. UTC | #2
On 2011-04-26 15:30, Michael Tokarev wrote:
> 26.04.2011 17:19, Jan Kiszka wrote:
> 
>>  hw/hpet.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 
>> +static int modifying_bit(uint64_t old, uint64_t new, uint64_t mask)
>> +{
>> +    return (old ^ new) & mask;
>> +}
> 
> Such constructs always look suspicious.  I'm not even sure anymore
> (after using C for over 20 years ;) that this works... how about
> 
>  return (old ^ new) & mask  ? 1 : 0;
> 
> instead, or something along this?  I mean, if sizeof(int)==4, how
> `return 1ULL<<32' will be interpreted in this context?  Tiny test
> program tells me it will return 0...

Good catch, will fix (doesn't bite use here, flags fit into 32 bit, but
nevertheless).

Jan
Avi Kivity April 26, 2011, 1:56 p.m. UTC | #3
On 04/26/2011 04:55 PM, Jan Kiszka wrote:
> On 2011-04-26 15:30, Michael Tokarev wrote:
> >  26.04.2011 17:19, Jan Kiszka wrote:
> >
> >>   hw/hpet.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >
> >>  +static int modifying_bit(uint64_t old, uint64_t new, uint64_t mask)
> >>  +{
> >>  +    return (old ^ new)&  mask;
> >>  +}
> >
> >  Such constructs always look suspicious.  I'm not even sure anymore
> >  (after using C for over 20 years ;) that this works... how about
> >
> >   return (old ^ new)&  mask  ? 1 : 0;
> >
> >  instead, or something along this?  I mean, if sizeof(int)==4, how
> >  `return 1ULL<<32' will be interpreted in this context?  Tiny test
> >  program tells me it will return 0...
>
> Good catch, will fix (doesn't bite use here, flags fit into 32 bit, but
> nevertheless).
>

Note, a bool return type works here.
Jan Kiszka April 26, 2011, 1:58 p.m. UTC | #4
On 2011-04-26 15:56, Avi Kivity wrote:
> On 04/26/2011 04:55 PM, Jan Kiszka wrote:
>> On 2011-04-26 15:30, Michael Tokarev wrote:
>>>  26.04.2011 17:19, Jan Kiszka wrote:
>>>
>>>>   hw/hpet.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>
>>>>  +static int modifying_bit(uint64_t old, uint64_t new, uint64_t mask)
>>>>  +{
>>>>  +    return (old ^ new)&  mask;
>>>>  +}
>>>
>>>  Such constructs always look suspicious.  I'm not even sure anymore
>>>  (after using C for over 20 years ;) that this works... how about
>>>
>>>   return (old ^ new)&  mask  ? 1 : 0;
>>>
>>>  instead, or something along this?  I mean, if sizeof(int)==4, how
>>>  `return 1ULL<<32' will be interpreted in this context?  Tiny test
>>>  program tells me it will return 0...
>>
>> Good catch, will fix (doesn't bite use here, flags fit into 32 bit, but
>> nevertheless).
>>
> 
> Note, a bool return type works here.

Yes, that was my favorite as well.

Jan
diff mbox

Patch

diff --git a/hw/hpet.c b/hw/hpet.c
index 6ce07bc..e773ed8 100644
--- a/hw/hpet.c
+++ b/hw/hpet.c
@@ -31,6 +31,7 @@ 
 #include "hpet_emul.h"
 #include "sysbus.h"
 #include "mc146818rtc.h"
+#include "kvm.h"
 
 //#define HPET_DEBUG
 #ifdef HPET_DEBUG
@@ -55,6 +56,7 @@  typedef struct HPETTimer {  /* timers */
     uint8_t wrap_flag;      /* timer pop will indicate wrap for one-shot 32-bit
                              * mode. Next pop will be actual timer expiration.
                              */
+    KVMMsiMessage kmm;
 } HPETTimer;
 
 typedef struct HPETState {
@@ -141,11 +143,59 @@  static int deactivating_bit(uint64_t old, uint64_t new, uint64_t mask)
     return ((old & mask) && !(new & mask));
 }
 
+static int modifying_bit(uint64_t old, uint64_t new, uint64_t mask)
+{
+    return (old ^ new) & mask;
+}
+
 static uint64_t hpet_get_ticks(HPETState *s)
 {
     return ns_to_ticks(qemu_get_clock_ns(vm_clock) + s->hpet_offset);
 }
 
+static void kvm_hpet_msi_update(HPETTimer *t)
+{
+    KVMMsiMessage new_entry;
+    int ret = 0;
+
+    if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
+        return;
+    }
+
+    if (timer_fsb_route(t)) {
+        new_entry.addr_hi = 0;
+        new_entry.addr_lo = t->fsb >> 32;
+        new_entry.data = t->fsb & 0xffffffff;
+        if (t->kmm.gsi == -1) {
+            kvm_msi_message_add(&new_entry);
+            ret = 1;
+        } else {
+            ret = kvm_msi_message_update(&t->kmm, &new_entry);
+        }
+        if (ret > 0) {
+            t->kmm = new_entry;
+            kvm_commit_irq_routes();
+        }
+    } else if (t->kmm.gsi != -1) {
+        kvm_msi_message_del(&t->kmm);
+        t->kmm.gsi = -1;
+    }
+}
+
+static void kvm_hpet_msi_free(HPETState *s)
+{
+    int i;
+
+    for (i = 0; i < s->num_timers; i++) {
+        HPETTimer *timer = &s->timer[i];
+
+        if (timer->kmm.gsi != -1) {
+            kvm_msi_message_del(&timer->kmm);
+            timer->kmm.gsi = -1;
+        }
+    }
+}
+
 /*
  * calculate diff between comparator value and current ticks
  */
@@ -192,7 +242,11 @@  static void update_irq(struct HPETTimer *timer, int set)
             qemu_irq_lower(s->irqs[route]);
         }
     } else if (timer_fsb_route(timer)) {
-        stl_phys(timer->fsb >> 32, timer->fsb & 0xffffffff);
+        if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+            kvm_set_irq(timer->kmm.gsi, 1, NULL);
+        } else {
+            stl_phys(timer->fsb >> 32, timer->fsb & 0xffffffff);
+        }
     } else if (timer->config & HPET_TN_TYPE_LEVEL) {
         s->isr |= mask;
         qemu_irq_raise(s->irqs[route]);
@@ -214,6 +268,8 @@  static int hpet_pre_load(void *opaque)
 {
     HPETState *s = opaque;
 
+    kvm_hpet_msi_free(s);
+
     /* version 1 only supports 3, later versions will load the actual value */
     s->num_timers = HPET_MIN_TIMERS;
     return 0;
@@ -222,6 +278,7 @@  static int hpet_pre_load(void *opaque)
 static int hpet_post_load(void *opaque, int version_id)
 {
     HPETState *s = opaque;
+    int i;
 
     /* Recalculate the offset between the main counter and guest time */
     s->hpet_offset = ticks_to_ns(s->hpet_counter) - qemu_get_clock_ns(vm_clock);
@@ -241,6 +298,10 @@  static int hpet_post_load(void *opaque, int version_id)
         hpet_pit_disable();
     }
 
+    for (i = 0; i < s->num_timers; i++) {
+        kvm_hpet_msi_update(&s->timer[i]);
+    }
+
     return 0;
 }
 
@@ -485,6 +546,9 @@  static void hpet_ram_writel(void *opaque, target_phys_addr_t addr,
             } else if (deactivating_bit(old_val, new_val, HPET_TN_ENABLE)) {
                 hpet_del_timer(timer);
             }
+            if (modifying_bit(old_val, new_val, HPET_TN_FSB_ENABLE)) {
+                kvm_hpet_msi_update(timer);
+            }
             break;
         case HPET_TN_CFG + 4: // Interrupt capabilities
             DPRINTF("qemu: invalid HPET_TN_CFG+4 write\n");
@@ -533,9 +597,11 @@  static void hpet_ram_writel(void *opaque, target_phys_addr_t addr,
                 break;
         case HPET_TN_ROUTE:
             timer->fsb = (timer->fsb & 0xffffffff00000000ULL) | new_val;
+            kvm_hpet_msi_update(timer);
             break;
         case HPET_TN_ROUTE + 4:
             timer->fsb = (new_val << 32) | (timer->fsb & 0xffffffff);
+            kvm_hpet_msi_update(timer);
             break;
         default:
             DPRINTF("qemu: invalid hpet_ram_writel\n");
@@ -667,6 +733,8 @@  static void hpet_reset(DeviceState *d)
     hpet_cfg.hpet[s->hpet_id].event_timer_block_id = (uint32_t)s->capability;
     hpet_cfg.hpet[s->hpet_id].address = sysbus_from_qdev(d)->mmio[0].addr;
     count = 1;
+
+    kvm_hpet_msi_free(s);
 }
 
 static void hpet_handle_rtc_irq(void *opaque, int n, int level)
@@ -711,6 +779,7 @@  static int hpet_init(SysBusDevice *dev)
         timer->qemu_timer = qemu_new_timer_ns(vm_clock, hpet_timer, timer);
         timer->tn = i;
         timer->state = s;
+        timer->kmm.gsi = -1;
     }
 
     /* 64-bit main counter; LegacyReplacementRoute. */