[Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Jan Kiszka]

Some half a year ago when I posted my first attempt to refactor MSI
for KVM support, we came to the conclusion that it might suffice to do
transparent dynamic routing for user-space injected MSI messages. These
two patches now implement such an approach for upstream.

As QEMU does not yet include irqfd support (for vhost) or pci device
assignment, this is already enough to enable MSI over the in-kernel
irqchip. Still, this is only RFC as it is just lightly tested and should
primarily collect feedback regarding the direction. If it's fine, I'd
like to base further qemu-kvm refactorings and upstream preparations on
top of such a series.

Also, I'd like to reanimate my KVM patch to provide direct MSI injection
in future kernels so that we do not need to take this long path here
forever.

Jan Kiszka (2):
  kvm: Introduce basic MSI support in-kernel irqchips
  KVM: x86: Wire up MSI support for in-kernel irqchip

 hw/apic.c     |    3 +
 hw/kvm/apic.c |   33 ++++++++++-
 hw/pc.c       |    5 --
 kvm-all.c     |  171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 kvm.h         |    1 +
 5 files changed, 205 insertions(+), 8 deletions(-)

-- 
1.7.3.4

[Qemu-devel] [RFC][PATCH 1/2] kvm: Introduce basic MSI support in-kernel irqchips

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

This patch basically adds kvm_irqchip_send_msi, a service for sending
arbitrary MSI messages to KVM's in-kernel irqchip models.

As the current KVI API requires us to establish a static route from a
pseudo GSI to the target MSI message and inject the MSI via toggling
that GSI, we need to play some tricks to make this unfortunately
interface transparent. We create those routes on demand and keep them
in a hash table. Succeeding messages can then search for an existing
route in the table first and reuse it whenever possible. If we should
run out of limited GSIs, we simply flush the table and rebuild it as
messages are sent.

This approach is rather simple and could be optimized further. However,
it is more efficient to enhance the KVM API so that we do not need this
clumsy dynamic routing over futures kernels.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |  171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 kvm.h     |    1 +
 2 files changed, 171 insertions(+), 1 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 3a1fa40..11ed19c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -48,6 +48,8 @@
     do { } while (0)
 #endif
 
+#define KVM_MSI_HASHTAB_SIZE    256
+
 typedef struct KVMSlot
 {
     target_phys_addr_t start_addr;
@@ -59,6 +61,11 @@ typedef struct KVMSlot
 
 typedef struct kvm_dirty_log KVMDirtyLog;
 
+typedef struct KVMMSIRoute {
+    struct kvm_irq_routing_entry kroute;
+    QTAILQ_ENTRY(KVMMSIRoute) entry;
+} KVMMSIRoute;
+
 struct KVMState
 {
     KVMSlot slots[32];
@@ -87,6 +94,7 @@ struct KVMState
     int nr_allocated_irq_routes;
     uint32_t *used_gsi_bitmap;
     unsigned int max_gsi;
+    QTAILQ_HEAD(msi_hashtab, KVMMSIRoute) msi_hashtab[KVM_MSI_HASHTAB_SIZE];
 #endif
 };
 
@@ -862,9 +870,16 @@ static void set_gsi(KVMState *s, unsigned int gsi)
     s->used_gsi_bitmap[gsi / 32] |= 1U << (gsi % 32);
 }
 
+static void clear_gsi(KVMState *s, unsigned int gsi)
+{
+    assert(gsi < s->max_gsi);
+
+    s->used_gsi_bitmap[gsi / 32] &= ~(1U << (gsi % 32));
+}
+
 static void kvm_init_irq_routing(KVMState *s)
 {
-    int gsi_count;
+    int gsi_count, i;
 
     gsi_count = kvm_check_extension(s, KVM_CAP_IRQ_ROUTING);
     if (gsi_count > 0) {
@@ -884,6 +899,10 @@ static void kvm_init_irq_routing(KVMState *s)
     s->irq_routes = g_malloc0(sizeof(*s->irq_routes));
     s->nr_allocated_irq_routes = 0;
 
+    for (i = 0; i < KVM_MSI_HASHTAB_SIZE; i++) {
+        QTAILQ_INIT(&s->msi_hashtab[i]);
+    }
+
     kvm_arch_init_irq_routing(s);
 }
 
@@ -914,6 +933,54 @@ static void kvm_add_routing_entry(KVMState *s,
     set_gsi(s, entry->gsi);
 }
 
+static void kvm_remove_routing_entry(KVMState *s,
+                                     struct kvm_irq_routing_entry *entry)
+{
+    struct kvm_irq_routing_entry *e;
+    int gsi = entry->gsi;
+    int i;
+
+    for (i = 0; i < s->irq_routes->nr; ++i) {
+        e = &s->irq_routes->entries[i];
+        if (e->type == entry->type && e->gsi == gsi) {
+            switch (e->type) {
+            case KVM_IRQ_ROUTING_IRQCHIP:
+                if (e->u.irqchip.irqchip == entry->u.irqchip.irqchip &&
+                    e->u.irqchip.pin == entry->u.irqchip.pin) {
+                    goto found;
+                }
+                break;
+            case KVM_IRQ_ROUTING_MSI:
+                if (e->u.msi.address_lo == entry->u.msi.address_lo &&
+                    e->u.msi.address_hi == entry->u.msi.address_hi &&
+                    e->u.msi.data == entry->u.msi.data) {
+                    goto found;
+                }
+                break;
+            default:
+                break;
+            }
+        }
+    }
+    /* route not found */
+    return;
+
+found:
+    s->irq_routes->nr--;
+    *e = s->irq_routes->entries[s->irq_routes->nr];
+
+    /* If there are no other users of this GSI, release it. */
+    for (i = 0; i < s->irq_routes->nr; i++) {
+        e = &s->irq_routes->entries[i];
+        if (e->gsi == gsi) {
+            break;
+        }
+    }
+    if (i == s->irq_routes->nr) {
+        clear_gsi(s, gsi);
+    }
+}
+
 void kvm_irqchip_add_route(KVMState *s, int irq, int irqchip, int pin)
 {
     struct kvm_irq_routing_entry e;
@@ -932,11 +999,113 @@ int kvm_irqchip_commit_routes(KVMState *s)
     return kvm_vm_ioctl(s, KVM_SET_GSI_ROUTING, s->irq_routes);
 }
 
+static unsigned int kvm_hash_msi(uint32_t data)
+{
+    /* This is optimized for IA32 MSI layout. However, no other arch shall
+     * repeat the mistake of not providing a direct MSI injection API. */
+    return data & 0xff;
+}
+
+static void kvm_flush_dynamic_msi_routes(KVMState *s)
+{
+    KVMMSIRoute *route, *next;
+    unsigned int hash;
+
+    for (hash = 0; hash < KVM_MSI_HASHTAB_SIZE; hash++) {
+        QTAILQ_FOREACH_SAFE(route, &s->msi_hashtab[hash], entry, next) {
+            kvm_remove_routing_entry(s, &route->kroute);
+            QTAILQ_REMOVE(&s->msi_hashtab[hash], route, entry);
+        }
+    }
+}
+
+static int kvm_get_pseudo_gsi(KVMState *s)
+{
+    uint32_t *word = s->used_gsi_bitmap;
+    int i, bit;
+    bool retry = true;
+
+again:
+    /* Return the lowest unused GSI in the bitmap */
+    for (i = 0; i < s->max_gsi / 32; i++) {
+        bit = ffs(~word[i]);
+        if (!bit) {
+            continue;
+        }
+
+        return bit - 1 + i * 32;
+    }
+    if (retry) {
+        retry = false;
+        kvm_flush_dynamic_msi_routes(s);
+        goto again;
+    }
+    return -ENOSPC;
+
+}
+
+static KVMMSIRoute *kvm_lookup_msi_route(KVMState *s, uint64_t addr,
+                                         uint32_t data)
+{
+    unsigned int hash = kvm_hash_msi(data);
+    KVMMSIRoute *route;
+
+    QTAILQ_FOREACH(route, &s->msi_hashtab[hash], entry) {
+        if (route->kroute.u.msi.address_lo == (uint32_t)addr &&
+            route->kroute.u.msi.address_hi == (addr >> 32) &&
+            route->kroute.u.msi.data == data) {
+            return route;
+        }
+    }
+    return NULL;
+}
+
+int kvm_irqchip_send_msi(KVMState *s, uint64_t addr, uint32_t data)
+{
+    KVMMSIRoute *route;
+
+    route = kvm_lookup_msi_route(s, addr, data);
+    if (!route) {
+        int gsi, ret;
+
+        gsi = kvm_get_pseudo_gsi(s);
+        if (gsi < 0) {
+            return gsi;
+        }
+
+        route = g_malloc(sizeof(KVMMSIRoute));
+        route->kroute.gsi = gsi;
+        route->kroute.type = KVM_IRQ_ROUTING_MSI;
+        route->kroute.flags = 0;
+        route->kroute.u.msi.address_lo = (uint32_t)addr;
+        route->kroute.u.msi.address_hi = addr >> 32;
+        route->kroute.u.msi.data = data;
+
+        kvm_add_routing_entry(s, &route->kroute);
+
+        QTAILQ_INSERT_TAIL(&s->msi_hashtab[kvm_hash_msi(data)], route, entry);
+
+        ret = kvm_irqchip_commit_routes(s);
+        if (ret < 0) {
+            return ret;
+        }
+    }
+
+    assert(route->kroute.type == KVM_IRQ_ROUTING_MSI);
+
+    return kvm_irqchip_set_irq(s, route->kroute.gsi, 1);
+}
+
 #else /* !KVM_CAP_IRQ_ROUTING */
 
 static void kvm_init_irq_routing(KVMState *s)
 {
 }
+
+int kvm_irqchip_send_msi(KVMState *s, uint64_t addr, uint32_t data)
+{
+    abort();
+}
 #endif /* !KVM_CAP_IRQ_ROUTING */
 
 static int kvm_irqchip_create(KVMState *s)
diff --git a/kvm.h b/kvm.h
index 55f107d..18ffb7b 100644
--- a/kvm.h
+++ b/kvm.h
@@ -132,6 +132,7 @@ int kvm_arch_on_sigbus(int code, void *addr);
 void kvm_arch_init_irq_routing(KVMState *s);
 
 int kvm_irqchip_set_irq(KVMState *s, int irq, int level);
+int kvm_irqchip_send_msi(KVMState *s, uint64_t addr, uint32_t data);
 
 void kvm_irqchip_add_route(KVMState *s, int gsi, int irqchip, int pin);
 int kvm_irqchip_commit_routes(KVMState *s);
-- 
1.7.3.4

Re: [Qemu-devel] [RFC][PATCH 1/2] kvm: Introduce basic MSI support in-kernel irqchips

[Avi Kivity]

On 03/22/2012 01:17 AM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> This patch basically adds kvm_irqchip_send_msi, a service for sending
> arbitrary MSI messages to KVM's in-kernel irqchip models.
>
> As the current KVI API requires us to establish a static route from a

s/KVI/KVM/

> pseudo GSI to the target MSI message and inject the MSI via toggling
> that GSI, we need to play some tricks to make this unfortunately

s/unfortunately/unfortunate/

> interface transparent. We create those routes on demand and keep them
> in a hash table. Succeeding messages can then search for an existing
> route in the table first and reuse it whenever possible. If we should
> run out of limited GSIs, we simply flush the table and rebuild it as
> messages are sent.
>
> This approach is rather simple and could be optimized further. However,
> it is more efficient to enhance the KVM API so that we do not need this
> clumsy dynamic routing over futures kernels.

Two APIs are clumsier than one.

wet the patch itself, suggest replacing the home grown hash with
http://developer.gnome.org/glib/2.30/glib-Caches.html.   

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [RFC][PATCH 1/2] kvm: Introduce basic MSI support in-kernel irqchips

[Michael S. Tsirkin]

On Wed, Mar 28, 2012 at 01:09:25PM +0200, Avi Kivity wrote:
> On 03/22/2012 01:17 AM, Jan Kiszka wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> >
> > This patch basically adds kvm_irqchip_send_msi, a service for sending
> > arbitrary MSI messages to KVM's in-kernel irqchip models.
> >
> > As the current KVI API requires us to establish a static route from a
> 
> s/KVI/KVM/
> 
> > pseudo GSI to the target MSI message and inject the MSI via toggling
> > that GSI, we need to play some tricks to make this unfortunately
> 
> s/unfortunately/unfortunate/
> 
> > interface transparent. We create those routes on demand and keep them
> > in a hash table. Succeeding messages can then search for an existing
> > route in the table first and reuse it whenever possible. If we should
> > run out of limited GSIs, we simply flush the table and rebuild it as
> > messages are sent.
> >
> > This approach is rather simple and could be optimized further. However,
> > it is more efficient to enhance the KVM API so that we do not need this
> > clumsy dynamic routing over futures kernels.
> 
> Two APIs are clumsier than one.
>
> wet the patch itself, suggest replacing the home grown hash with
> http://developer.gnome.org/glib/2.30/glib-Caches.html.   

I'd claim that the existing API is really not fit
for what this patch wants to do, specifically
support a huge number of MSI vectors.

GSI routing  was supposed to be mostly static. We do things
like RCU slow path when they are changed, so routing
changes during injection will be bad for real time guests.

So either we want to support unlimited number of MSI vectors,
in which case it makes sense to me to add kernel support to do
this efficiently, or not in which case we don't need it
in userspace either.

No?

> -- 
> error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [RFC][PATCH 1/2] kvm: Introduce basic MSI support in-kernel irqchips

[Jan Kiszka]

On 2012-03-28 13:09, Avi Kivity wrote:
> On 03/22/2012 01:17 AM, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> This patch basically adds kvm_irqchip_send_msi, a service for sending
>> arbitrary MSI messages to KVM's in-kernel irqchip models.
>>
>> As the current KVI API requires us to establish a static route from a
> 
> s/KVI/KVM/
> 
>> pseudo GSI to the target MSI message and inject the MSI via toggling
>> that GSI, we need to play some tricks to make this unfortunately
> 
> s/unfortunately/unfortunate/

Will fix these.

> 
>> interface transparent. We create those routes on demand and keep them
>> in a hash table. Succeeding messages can then search for an existing
>> route in the table first and reuse it whenever possible. If we should
>> run out of limited GSIs, we simply flush the table and rebuild it as
>> messages are sent.
>>
>> This approach is rather simple and could be optimized further. However,
>> it is more efficient to enhance the KVM API so that we do not need this
>> clumsy dynamic routing over futures kernels.
> 
> Two APIs are clumsier than one.

The current one is very clumsy for user-injected MSIs while the new one
won't be. It will also be very simple it implement if you recall the
patch. I think that is worth it.

> 
> wet the patch itself, suggest replacing the home grown hash with
> http://developer.gnome.org/glib/2.30/glib-Caches.html.   

Let's keep it simple :). We have no need for many of those features, and
it would not be possible to implement the logic as compact as it is
right now.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 1/2] kvm: Introduce basic MSI support in-kernel irqchips

[Avi Kivity]

On 03/28/2012 01:33 PM, Jan Kiszka wrote:
> On 2012-03-28 13:09, Avi Kivity wrote:
> > On 03/22/2012 01:17 AM, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> This patch basically adds kvm_irqchip_send_msi, a service for sending
> >> arbitrary MSI messages to KVM's in-kernel irqchip models.
> >>
> >> As the current KVI API requires us to establish a static route from a
> > 
> > s/KVI/KVM/
> > 
> >> pseudo GSI to the target MSI message and inject the MSI via toggling
> >> that GSI, we need to play some tricks to make this unfortunately
> > 
> > s/unfortunately/unfortunate/
>
> Will fix these.

Only needed if you end up reposting.

> > 
> >> interface transparent. We create those routes on demand and keep them
> >> in a hash table. Succeeding messages can then search for an existing
> >> route in the table first and reuse it whenever possible. If we should
> >> run out of limited GSIs, we simply flush the table and rebuild it as
> >> messages are sent.
> >>
> >> This approach is rather simple and could be optimized further. However,
> >> it is more efficient to enhance the KVM API so that we do not need this
> >> clumsy dynamic routing over futures kernels.
> > 
> > Two APIs are clumsier than one.
>
> The current one is very clumsy for user-injected MSIs while the new one
> won't be. It will also be very simple it implement if you recall the
> patch. I think that is worth it.

Don't see why.  The clumsiness will be retained.  The cpu doesn't care
how clumsy the API is, only the reader.

>
> > 
> > wet the patch itself, suggest replacing the home grown hash with
> > http://developer.gnome.org/glib/2.30/glib-Caches.html.   
>
> Let's keep it simple :). We have no need for many of those features, and
> it would not be possible to implement the logic as compact as it is
> right now.

Due to the callbacks?

What if the code grows?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [RFC][PATCH 1/2] kvm: Introduce basic MSI support in-kernel irqchips

[Jan Kiszka]

On 2012-03-28 13:44, Avi Kivity wrote:
> On 03/28/2012 01:33 PM, Jan Kiszka wrote:
>> On 2012-03-28 13:09, Avi Kivity wrote:
>>> On 03/22/2012 01:17 AM, Jan Kiszka wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> This patch basically adds kvm_irqchip_send_msi, a service for sending
>>>> arbitrary MSI messages to KVM's in-kernel irqchip models.
>>>>
>>>> As the current KVI API requires us to establish a static route from a
>>>
>>> s/KVI/KVM/
>>>
>>>> pseudo GSI to the target MSI message and inject the MSI via toggling
>>>> that GSI, we need to play some tricks to make this unfortunately
>>>
>>> s/unfortunately/unfortunate/
>>
>> Will fix these.
> 
> Only needed if you end up reposting.

I will have to, I spotted a memory leak.

> 
>>>
>>>> interface transparent. We create those routes on demand and keep them
>>>> in a hash table. Succeeding messages can then search for an existing
>>>> route in the table first and reuse it whenever possible. If we should
>>>> run out of limited GSIs, we simply flush the table and rebuild it as
>>>> messages are sent.
>>>>
>>>> This approach is rather simple and could be optimized further. However,
>>>> it is more efficient to enhance the KVM API so that we do not need this
>>>> clumsy dynamic routing over futures kernels.
>>>
>>> Two APIs are clumsier than one.
>>
>> The current one is very clumsy for user-injected MSIs while the new one
>> won't be. It will also be very simple it implement if you recall the
>> patch. I think that is worth it.
> 
> Don't see why.  The clumsiness will be retained.  The cpu doesn't care
> how clumsy the API is, only the reader.

We won't have to do any hashing/caching over the new API, just a plain
"deliver this MSI" IOCTL. Specifically all our upcoming archs like Power
and ARM will be able to take the shiny highway instead of the winding
countryside road.

> 
>>
>>>
>>> wet the patch itself, suggest replacing the home grown hash with
>>> http://developer.gnome.org/glib/2.30/glib-Caches.html.   
>>
>> Let's keep it simple :). We have no need for many of those features, and
>> it would not be possible to implement the logic as compact as it is
>> right now.
> 
> Due to the callbacks?

Yep. That API pays of if you have more iterations and insertions/removals.

> 
> What if the code grows?

It won't as it only has to emulate direct MSI injection over the
existing API. That's a static feature.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 1/2] kvm: Introduce basic MSI support in-kernel irqchips

[Avi Kivity]

On 03/28/2012 01:54 PM, Jan Kiszka wrote:
> > 
> >>>
> >>>> interface transparent. We create those routes on demand and keep them
> >>>> in a hash table. Succeeding messages can then search for an existing
> >>>> route in the table first and reuse it whenever possible. If we should
> >>>> run out of limited GSIs, we simply flush the table and rebuild it as
> >>>> messages are sent.
> >>>>
> >>>> This approach is rather simple and could be optimized further. However,
> >>>> it is more efficient to enhance the KVM API so that we do not need this
> >>>> clumsy dynamic routing over futures kernels.
> >>>
> >>> Two APIs are clumsier than one.
> >>
> >> The current one is very clumsy for user-injected MSIs while the new one
> >> won't be. It will also be very simple it implement if you recall the
> >> patch. I think that is worth it.
> > 
> > Don't see why.  The clumsiness will be retained.  The cpu doesn't care
> > how clumsy the API is, only the reader.
>
> We won't have to do any hashing/caching over the new API, just a plain
> "deliver this MSI" IOCTL. Specifically all our upcoming archs like Power
> and ARM will be able to take the shiny highway instead of the winding
> countryside road.

Upcoming archs are a good card to play.  However that code will remain
for x86, and there's nothing arch specific about it, is there?

> > 
> >>> wet the patch itself, suggest replacing the home grown hash with
> >>> http://developer.gnome.org/glib/2.30/glib-Caches.html.   
> >>
> >> Let's keep it simple :). We have no need for many of those features, and
> >> it would not be possible to implement the logic as compact as it is
> >> right now.
> > 
> > Due to the callbacks?
>
> Yep. That API pays of if you have more iterations and insertions/removals.

Okay, will wait for std::unordered_map<>.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [RFC][PATCH 1/2] kvm: Introduce basic MSI support in-kernel irqchips

[Jan Kiszka]

On 2012-03-28 14:32, Avi Kivity wrote:
> On 03/28/2012 01:54 PM, Jan Kiszka wrote:
>>>
>>>>>
>>>>>> interface transparent. We create those routes on demand and keep them
>>>>>> in a hash table. Succeeding messages can then search for an existing
>>>>>> route in the table first and reuse it whenever possible. If we should
>>>>>> run out of limited GSIs, we simply flush the table and rebuild it as
>>>>>> messages are sent.
>>>>>>
>>>>>> This approach is rather simple and could be optimized further. However,
>>>>>> it is more efficient to enhance the KVM API so that we do not need this
>>>>>> clumsy dynamic routing over futures kernels.
>>>>>
>>>>> Two APIs are clumsier than one.
>>>>
>>>> The current one is very clumsy for user-injected MSIs while the new one
>>>> won't be. It will also be very simple it implement if you recall the
>>>> patch. I think that is worth it.
>>>
>>> Don't see why.  The clumsiness will be retained.  The cpu doesn't care
>>> how clumsy the API is, only the reader.
>>
>> We won't have to do any hashing/caching over the new API, just a plain
>> "deliver this MSI" IOCTL. Specifically all our upcoming archs like Power
>> and ARM will be able to take the shiny highway instead of the winding
>> countryside road.
> 
> Upcoming archs are a good card to play.  However that code will remain
> for x86, and there's nothing arch specific about it, is there?

Other archs that support MSI will then always come with something like
KVM_CAP_SET_MSI, our signal to take the fast lane. x86 with be the only
arch to potentially miss this cap.

So, yes, we will always need the code for old x86 (as long as we support
it), but we should not enforce this logic on anyone else.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 1/2] kvm: Introduce basic MSI support in-kernel irqchips

[Michael S. Tsirkin]

On Wed, Mar 28, 2012 at 01:44:41PM +0200, Avi Kivity wrote:
> On 03/28/2012 01:33 PM, Jan Kiszka wrote:
> > On 2012-03-28 13:09, Avi Kivity wrote:
> > > On 03/22/2012 01:17 AM, Jan Kiszka wrote:
> > >> From: Jan Kiszka <jan.kiszka@siemens.com>
> > >>
> > >> This patch basically adds kvm_irqchip_send_msi, a service for sending
> > >> arbitrary MSI messages to KVM's in-kernel irqchip models.
> > >>
> > >> As the current KVI API requires us to establish a static route from a
> > > 
> > > s/KVI/KVM/
> > > 
> > >> pseudo GSI to the target MSI message and inject the MSI via toggling
> > >> that GSI, we need to play some tricks to make this unfortunately
> > > 
> > > s/unfortunately/unfortunate/
> >
> > Will fix these.
> 
> Only needed if you end up reposting.
> 
> > > 
> > >> interface transparent. We create those routes on demand and keep them
> > >> in a hash table. Succeeding messages can then search for an existing
> > >> route in the table first and reuse it whenever possible. If we should
> > >> run out of limited GSIs, we simply flush the table and rebuild it as
> > >> messages are sent.
> > >>
> > >> This approach is rather simple and could be optimized further. However,
> > >> it is more efficient to enhance the KVM API so that we do not need this
> > >> clumsy dynamic routing over futures kernels.
> > > 
> > > Two APIs are clumsier than one.
> >
> > The current one is very clumsy for user-injected MSIs while the new one
> > won't be. It will also be very simple it implement if you recall the
> > patch. I think that is worth it.
> 
> Don't see why.  The clumsiness will be retained.  The cpu doesn't care
> how clumsy the API is, only the reader.

It does care that the performance will be bad. GSIs were
supposed by design to be static, so routing changes are slow.

> >
> > > 
> > > wet the patch itself, suggest replacing the home grown hash with
> > > http://developer.gnome.org/glib/2.30/glib-Caches.html.   
> >
> > Let's keep it simple :). We have no need for many of those features, and
> > it would not be possible to implement the logic as compact as it is
> > right now.
> 
> Due to the callbacks?
> 
> What if the code grows?
> 
> -- 
> error compiling committee.c: too many arguments to function

[Qemu-devel] [RFC][PATCH 2/2] KVM: x86: Wire up MSI support for in-kernel irqchip

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

Catch writes to the MSI MMIO region in the KVM APIC and forward them to
the kernel. Provide the kernel support GSI routing, this allows to
enable MSI support also for in-kernel irqchip mode.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/apic.c     |    3 +++
 hw/kvm/apic.c |   33 +++++++++++++++++++++++++++++++--
 hw/pc.c       |    5 -----
 3 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 4eeaf88..5fbf01c 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -19,6 +19,7 @@
 #include "apic_internal.h"
 #include "apic.h"
 #include "ioapic.h"
+#include "msi.h"
 #include "host-utils.h"
 #include "trace.h"
 #include "pc.h"
@@ -862,6 +863,8 @@ static void apic_init(APICCommonState *s)
 
     s->timer = qemu_new_timer_ns(vm_clock, apic_timer, s);
     local_apics[s->idx] = s;
+
+    msi_supported = true;
 }
 
 static void apic_class_init(ObjectClass *klass, void *data)
diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c
index ffe7a52..7d83b1a 100644
--- a/hw/kvm/apic.c
+++ b/hw/kvm/apic.c
@@ -10,6 +10,7 @@
  * See the COPYING file in the top-level directory.
  */
 #include "hw/apic_internal.h"
+#include "hw/msi.h"
 #include "kvm.h"
 
 static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
@@ -145,10 +146,38 @@ static void kvm_apic_external_nmi(APICCommonState *s)
     run_on_cpu(s->cpu_env, do_inject_external_nmi, s);
 }
 
+static uint64_t kvm_apic_mem_read(void *opaque, target_phys_addr_t addr,
+                                  unsigned size)
+{
+    return -1U;
+}
+
+static void kvm_apic_mem_write(void *opaque, target_phys_addr_t addr,
+                               uint64_t data, unsigned size)
+{
+    int ret;
+
+    ret = kvm_irqchip_send_msi(kvm_state, addr, data);
+    if (ret < 0) {
+        fprintf(stderr, "KVM: injection failed, MSI lost (%s)\n",
+                strerror(-ret));
+    }
+}
+
+static const MemoryRegionOps kvm_apic_io_ops = {
+    .read = kvm_apic_mem_read,
+    .write = kvm_apic_mem_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
 static void kvm_apic_init(APICCommonState *s)
 {
-    memory_region_init_reservation(&s->io_memory, "kvm-apic-msi",
-                                   MSI_SPACE_SIZE);
+    memory_region_init_io(&s->io_memory, &kvm_apic_io_ops, s, "kvm-apic-msi",
+                          MSI_SPACE_SIZE);
+
+    if (kvm_has_gsi_routing()) {
+        msi_supported = true;
+    }
 }
 
 static void kvm_apic_class_init(ObjectClass *klass, void *data)
diff --git a/hw/pc.c b/hw/pc.c
index 83a1b5b..fab620a 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -907,11 +907,6 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
         apic_mapped = 1;
     }
 
-    /* KVM does not support MSI yet. */
-    if (!kvm_irqchip_in_kernel()) {
-        msi_supported = true;
-    }
-
     return dev;
 }
 
-- 
1.7.3.4

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1402 bytes --]

On 2012-03-22 00:17, Jan Kiszka wrote:
> Some half a year ago when I posted my first attempt to refactor MSI
> for KVM support, we came to the conclusion that it might suffice to do
> transparent dynamic routing for user-space injected MSI messages. These
> two patches now implement such an approach for upstream.
> 
> As QEMU does not yet include irqfd support (for vhost) or pci device
> assignment, this is already enough to enable MSI over the in-kernel
> irqchip. Still, this is only RFC as it is just lightly tested and should
> primarily collect feedback regarding the direction. If it's fine, I'd
> like to base further qemu-kvm refactorings and upstream preparations on
> top of such a series.
> 
> Also, I'd like to reanimate my KVM patch to provide direct MSI injection
> in future kernels so that we do not need to take this long path here
> forever.
> 
> Jan Kiszka (2):
>   kvm: Introduce basic MSI support in-kernel irqchips
>   KVM: x86: Wire up MSI support for in-kernel irqchip
> 
>  hw/apic.c     |    3 +
>  hw/kvm/apic.c |   33 ++++++++++-
>  hw/pc.c       |    5 --
>  kvm-all.c     |  171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  kvm.h         |    1 +
>  5 files changed, 205 insertions(+), 8 deletions(-)
> 

Anyone any comments? I think this series could open the door for
kernel_irqchip=on as default in QEMU 1.1.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Michael S. Tsirkin]

On Wed, Mar 28, 2012 at 09:13:22AM +0200, Jan Kiszka wrote:
> On 2012-03-22 00:17, Jan Kiszka wrote:
> > Some half a year ago when I posted my first attempt to refactor MSI
> > for KVM support, we came to the conclusion that it might suffice to do
> > transparent dynamic routing for user-space injected MSI messages. These
> > two patches now implement such an approach for upstream.
> > 
> > As QEMU does not yet include irqfd support (for vhost) or pci device
> > assignment, this is already enough to enable MSI over the in-kernel
> > irqchip. Still, this is only RFC as it is just lightly tested and should
> > primarily collect feedback regarding the direction. If it's fine, I'd
> > like to base further qemu-kvm refactorings and upstream preparations on
> > top of such a series.
> > 
> > Also, I'd like to reanimate my KVM patch to provide direct MSI injection
> > in future kernels so that we do not need to take this long path here
> > forever.
> > 
> > Jan Kiszka (2):
> >   kvm: Introduce basic MSI support in-kernel irqchips
> >   KVM: x86: Wire up MSI support for in-kernel irqchip
> > 
> >  hw/apic.c     |    3 +
> >  hw/kvm/apic.c |   33 ++++++++++-
> >  hw/pc.c       |    5 --
> >  kvm-all.c     |  171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  kvm.h         |    1 +
> >  5 files changed, 205 insertions(+), 8 deletions(-)
> > 
> 
> Anyone any comments? I think this series could open the door for
> kernel_irqchip=on as default in QEMU 1.1.
> 
> Jan
> 

For what this patch is trying to do, would adding a simple ioctl for
injecting a given message into guest be cleaner?
Also, how would this support irqfd in the future? Will we have to
rip it all out and replace with per-device tracking that we
have today?

-- 
MST

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Jan Kiszka]

On 2012-03-28 11:45, Michael S. Tsirkin wrote:
> On Wed, Mar 28, 2012 at 09:13:22AM +0200, Jan Kiszka wrote:
>> On 2012-03-22 00:17, Jan Kiszka wrote:
>>> Some half a year ago when I posted my first attempt to refactor MSI
>>> for KVM support, we came to the conclusion that it might suffice to do
>>> transparent dynamic routing for user-space injected MSI messages. These
>>> two patches now implement such an approach for upstream.
>>>
>>> As QEMU does not yet include irqfd support (for vhost) or pci device
>>> assignment, this is already enough to enable MSI over the in-kernel
>>> irqchip. Still, this is only RFC as it is just lightly tested and should
>>> primarily collect feedback regarding the direction. If it's fine, I'd
>>> like to base further qemu-kvm refactorings and upstream preparations on
>>> top of such a series.
>>>
>>> Also, I'd like to reanimate my KVM patch to provide direct MSI injection
>>> in future kernels so that we do not need to take this long path here
>>> forever.
>>>
>>> Jan Kiszka (2):
>>>   kvm: Introduce basic MSI support in-kernel irqchips
>>>   KVM: x86: Wire up MSI support for in-kernel irqchip
>>>
>>>  hw/apic.c     |    3 +
>>>  hw/kvm/apic.c |   33 ++++++++++-
>>>  hw/pc.c       |    5 --
>>>  kvm-all.c     |  171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>  kvm.h         |    1 +
>>>  5 files changed, 205 insertions(+), 8 deletions(-)
>>>
>>
>> Anyone any comments? I think this series could open the door for
>> kernel_irqchip=on as default in QEMU 1.1.
>>
>> Jan
>>
> 
> For what this patch is trying to do, would adding a simple ioctl for
> injecting a given message into guest be cleaner?

For sure, and I already proposed this in the past. I think we were only
discussing the extensibility of such an IOCTL.

Anyway, that won't help with existing kernels. That's why I'm proposing
this userspace approach as an interim solution.

> Also, how would this support irqfd in the future? Will we have to
> rip it all out and replace with per-device tracking that we
> have today?

Irqfd and kvm device assignment will require additional interfaces (of
the kvm core in QEMU) via which you will be able to request stable
routes from such sources to specified MSIs. That will be widely
orthogonal to what is done in these patches here. Upstream is not
affected yet as it neither supports device assignment nor irqfds up to now.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Michael S. Tsirkin]

On Wed, Mar 28, 2012 at 11:50:27AM +0200, Jan Kiszka wrote:
> On 2012-03-28 11:45, Michael S. Tsirkin wrote:
> > On Wed, Mar 28, 2012 at 09:13:22AM +0200, Jan Kiszka wrote:
> >> On 2012-03-22 00:17, Jan Kiszka wrote:
> >>> Some half a year ago when I posted my first attempt to refactor MSI
> >>> for KVM support, we came to the conclusion that it might suffice to do
> >>> transparent dynamic routing for user-space injected MSI messages. These
> >>> two patches now implement such an approach for upstream.
> >>>
> >>> As QEMU does not yet include irqfd support (for vhost) or pci device
> >>> assignment, this is already enough to enable MSI over the in-kernel
> >>> irqchip. Still, this is only RFC as it is just lightly tested and should
> >>> primarily collect feedback regarding the direction. If it's fine, I'd
> >>> like to base further qemu-kvm refactorings and upstream preparations on
> >>> top of such a series.
> >>>
> >>> Also, I'd like to reanimate my KVM patch to provide direct MSI injection
> >>> in future kernels so that we do not need to take this long path here
> >>> forever.
> >>>
> >>> Jan Kiszka (2):
> >>>   kvm: Introduce basic MSI support in-kernel irqchips
> >>>   KVM: x86: Wire up MSI support for in-kernel irqchip
> >>>
> >>>  hw/apic.c     |    3 +
> >>>  hw/kvm/apic.c |   33 ++++++++++-
> >>>  hw/pc.c       |    5 --
> >>>  kvm-all.c     |  171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>>  kvm.h         |    1 +
> >>>  5 files changed, 205 insertions(+), 8 deletions(-)
> >>>
> >>
> >> Anyone any comments? I think this series could open the door for
> >> kernel_irqchip=on as default in QEMU 1.1.
> >>
> >> Jan
> >>
> > 
> > For what this patch is trying to do, would adding a simple ioctl for
> > injecting a given message into guest be cleaner?
> 
> For sure, and I already proposed this in the past. I think we were only
> discussing the extensibility of such an IOCTL.

Yes. And the conclusion I think was that it's not very extensible
but a very good fit for what we want to do, right?
See Message-ID: <4EA66B99.3010205@redhat.com>

> Anyway, that won't help with existing kernels. That's why I'm proposing
> this userspace approach as an interim solution.

I guess we can just keep the userspace irqchip around?

> > Also, how would this support irqfd in the future? Will we have to
> > rip it all out and replace with per-device tracking that we
> > have today?
> 
> Irqfd and kvm device assignment will require additional interfaces (of
> the kvm core in QEMU) via which you will be able to request stable
> routes from such sources to specified MSIs. That will be widely
> orthogonal to what is done in these patches here.

Yes but not exactly as they will conflict for resources, right?
How do you plan to solve this?

> Upstream is not
> affected yet as it neither supports device assignment nor irqfds up to now.
> 
> Jan

Just to clarify: so in the end, we will need
to basically do what qemu-kvm does, as well?

> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Jan Kiszka]

On 2012-03-28 12:47, Michael S. Tsirkin wrote:
> On Wed, Mar 28, 2012 at 11:50:27AM +0200, Jan Kiszka wrote:
>> On 2012-03-28 11:45, Michael S. Tsirkin wrote:
>>> On Wed, Mar 28, 2012 at 09:13:22AM +0200, Jan Kiszka wrote:
>>>> On 2012-03-22 00:17, Jan Kiszka wrote:
>>>>> Some half a year ago when I posted my first attempt to refactor MSI
>>>>> for KVM support, we came to the conclusion that it might suffice to do
>>>>> transparent dynamic routing for user-space injected MSI messages. These
>>>>> two patches now implement such an approach for upstream.
>>>>>
>>>>> As QEMU does not yet include irqfd support (for vhost) or pci device
>>>>> assignment, this is already enough to enable MSI over the in-kernel
>>>>> irqchip. Still, this is only RFC as it is just lightly tested and should
>>>>> primarily collect feedback regarding the direction. If it's fine, I'd
>>>>> like to base further qemu-kvm refactorings and upstream preparations on
>>>>> top of such a series.
>>>>>
>>>>> Also, I'd like to reanimate my KVM patch to provide direct MSI injection
>>>>> in future kernels so that we do not need to take this long path here
>>>>> forever.
>>>>>
>>>>> Jan Kiszka (2):
>>>>>   kvm: Introduce basic MSI support in-kernel irqchips
>>>>>   KVM: x86: Wire up MSI support for in-kernel irqchip
>>>>>
>>>>>  hw/apic.c     |    3 +
>>>>>  hw/kvm/apic.c |   33 ++++++++++-
>>>>>  hw/pc.c       |    5 --
>>>>>  kvm-all.c     |  171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>>>  kvm.h         |    1 +
>>>>>  5 files changed, 205 insertions(+), 8 deletions(-)
>>>>>
>>>>
>>>> Anyone any comments? I think this series could open the door for
>>>> kernel_irqchip=on as default in QEMU 1.1.
>>>>
>>>> Jan
>>>>
>>>
>>> For what this patch is trying to do, would adding a simple ioctl for
>>> injecting a given message into guest be cleaner?
>>
>> For sure, and I already proposed this in the past. I think we were only
>> discussing the extensibility of such an IOCTL.
> 
> Yes. And the conclusion I think was that it's not very extensible
> but a very good fit for what we want to do, right?
> See Message-ID: <4EA66B99.3010205@redhat.com>

Cannot match this ID, but I guess the best is now to just leave a flags
and some padding fields in the struct for whatever may or may not come
in the future.

> 
>> Anyway, that won't help with existing kernels. That's why I'm proposing
>> this userspace approach as an interim solution.
> 
> I guess we can just keep the userspace irqchip around?

This is about the kernel IRQ chip support. We want to support it over
current kernels, not only 3.4 or even later.

> 
>>> Also, how would this support irqfd in the future? Will we have to
>>> rip it all out and replace with per-device tracking that we
>>> have today?
>>
>> Irqfd and kvm device assignment will require additional interfaces (of
>> the kvm core in QEMU) via which you will be able to request stable
>> routes from such sources to specified MSIs. That will be widely
>> orthogonal to what is done in these patches here.
> 
> Yes but not exactly as they will conflict for resources, right?
> How do you plan to solve this?

As done in my original series: If a static route requires a pseudo GSI
and there are none free, we simply flush the dynamic MSI routes.

> 
>> Upstream is not
>> affected yet as it neither supports device assignment nor irqfds up to now.
>>
>> Jan
> 
> Just to clarify: so in the end, we will need
> to basically do what qemu-kvm does, as well?

Basically yes, but with refactored interfaces. E.g. all pseudo GSI
management will be privatized in the KVM layer. And MSI[-X] interfaces
will be refactored to reduce the code you need in virtio and pci-assign
for propagating vector changes to the routing subsystem. Details
regarding this aren't settled yet, but it will be just an add-on to the
MSI injection path for fully emulated devices, ie. the topic of this series.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Michael S. Tsirkin]

On Wed, Mar 28, 2012 at 01:07:42PM +0200, Jan Kiszka wrote:
> On 2012-03-28 12:47, Michael S. Tsirkin wrote:
> > On Wed, Mar 28, 2012 at 11:50:27AM +0200, Jan Kiszka wrote:
> >> On 2012-03-28 11:45, Michael S. Tsirkin wrote:
> >>> On Wed, Mar 28, 2012 at 09:13:22AM +0200, Jan Kiszka wrote:
> >>>> On 2012-03-22 00:17, Jan Kiszka wrote:
> >>>>> Some half a year ago when I posted my first attempt to refactor MSI
> >>>>> for KVM support, we came to the conclusion that it might suffice to do
> >>>>> transparent dynamic routing for user-space injected MSI messages. These
> >>>>> two patches now implement such an approach for upstream.
> >>>>>
> >>>>> As QEMU does not yet include irqfd support (for vhost) or pci device
> >>>>> assignment, this is already enough to enable MSI over the in-kernel
> >>>>> irqchip. Still, this is only RFC as it is just lightly tested and should
> >>>>> primarily collect feedback regarding the direction. If it's fine, I'd
> >>>>> like to base further qemu-kvm refactorings and upstream preparations on
> >>>>> top of such a series.
> >>>>>
> >>>>> Also, I'd like to reanimate my KVM patch to provide direct MSI injection
> >>>>> in future kernels so that we do not need to take this long path here
> >>>>> forever.
> >>>>>
> >>>>> Jan Kiszka (2):
> >>>>>   kvm: Introduce basic MSI support in-kernel irqchips
> >>>>>   KVM: x86: Wire up MSI support for in-kernel irqchip
> >>>>>
> >>>>>  hw/apic.c     |    3 +
> >>>>>  hw/kvm/apic.c |   33 ++++++++++-
> >>>>>  hw/pc.c       |    5 --
> >>>>>  kvm-all.c     |  171 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>>>>  kvm.h         |    1 +
> >>>>>  5 files changed, 205 insertions(+), 8 deletions(-)
> >>>>>
> >>>>
> >>>> Anyone any comments? I think this series could open the door for
> >>>> kernel_irqchip=on as default in QEMU 1.1.
> >>>>
> >>>> Jan
> >>>>
> >>>
> >>> For what this patch is trying to do, would adding a simple ioctl for
> >>> injecting a given message into guest be cleaner?
> >>
> >> For sure, and I already proposed this in the past. I think we were only
> >> discussing the extensibility of such an IOCTL.
> > 
> > Yes. And the conclusion I think was that it's not very extensible
> > but a very good fit for what we want to do, right?
> > See Message-ID: <4EA66B99.3010205@redhat.com>
> 
> Cannot match this ID, but I guess the best is now to just leave a flags
> and some padding fields in the struct for whatever may or may not come
> in the future.
> 
> > 
> >> Anyway, that won't help with existing kernels. That's why I'm proposing
> >> this userspace approach as an interim solution.
> > 
> > I guess we can just keep the userspace irqchip around?
> 
> This is about the kernel IRQ chip support. We want to support it over
> current kernels, not only 3.4 or even later.
> 
> > 
> >>> Also, how would this support irqfd in the future? Will we have to
> >>> rip it all out and replace with per-device tracking that we
> >>> have today?
> >>
> >> Irqfd and kvm device assignment will require additional interfaces (of
> >> the kvm core in QEMU) via which you will be able to request stable
> >> routes from such sources to specified MSIs. That will be widely
> >> orthogonal to what is done in these patches here.
> > 
> > Yes but not exactly as they will conflict for resources, right?
> > How do you plan to solve this?
> 
> As done in my original series: If a static route requires a pseudo GSI
> and there are none free, we simply flush the dynamic MSI routes.

Right. So static routes take precedence. This means that in effect
we will have two APIs in qemu: for fast MSIs and for slow ones,
the advantage of the slow APIs being that they are easier to use,
right?

> > 
> >> Upstream is not
> >> affected yet as it neither supports device assignment nor irqfds up to now.
> >>
> >> Jan
> > 
> > Just to clarify: so in the end, we will need
> > to basically do what qemu-kvm does, as well?
> 
> Basically yes, but with refactored interfaces. E.g. all pseudo GSI
> management will be privatized in the KVM layer. And MSI[-X] interfaces
> will be refactored to reduce the code you need in virtio and pci-assign
> for propagating vector changes to the routing subsystem. Details
> regarding this aren't settled yet, but it will be just an add-on to the
> MSI injection path for fully emulated devices, ie. the topic of this series.
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Jan Kiszka]

On 2012-03-28 13:31, Michael S. Tsirkin wrote:
>>>>> Also, how would this support irqfd in the future? Will we have to
>>>>> rip it all out and replace with per-device tracking that we
>>>>> have today?
>>>>
>>>> Irqfd and kvm device assignment will require additional interfaces (of
>>>> the kvm core in QEMU) via which you will be able to request stable
>>>> routes from such sources to specified MSIs. That will be widely
>>>> orthogonal to what is done in these patches here.
>>>
>>> Yes but not exactly as they will conflict for resources, right?
>>> How do you plan to solve this?
>>
>> As done in my original series: If a static route requires a pseudo GSI
>> and there are none free, we simply flush the dynamic MSI routes.
> 
> Right. So static routes take precedence. This means that in effect
> we will have two APIs in qemu: for fast MSIs and for slow ones,
> the advantage of the slow APIs being that they are easier to use,
> right?

We will have two APIs depending on the source of the MSI. Special
sources are the exception while emulated ones are the majority. And for
the latter we should try very hard to keep things simple and clean.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Michael S. Tsirkin]

On Wed, Mar 28, 2012 at 01:36:15PM +0200, Jan Kiszka wrote:
> On 2012-03-28 13:31, Michael S. Tsirkin wrote:
> >>>>> Also, how would this support irqfd in the future? Will we have to
> >>>>> rip it all out and replace with per-device tracking that we
> >>>>> have today?
> >>>>
> >>>> Irqfd and kvm device assignment will require additional interfaces (of
> >>>> the kvm core in QEMU) via which you will be able to request stable
> >>>> routes from such sources to specified MSIs. That will be widely
> >>>> orthogonal to what is done in these patches here.
> >>>
> >>> Yes but not exactly as they will conflict for resources, right?
> >>> How do you plan to solve this?
> >>
> >> As done in my original series: If a static route requires a pseudo GSI
> >> and there are none free, we simply flush the dynamic MSI routes.
> > 
> > Right. So static routes take precedence. This means that in effect
> > we will have two APIs in qemu: for fast MSIs and for slow ones,
> > the advantage of the slow APIs being that they are easier to use,
> > right?
> 
> We will have two APIs depending on the source of the MSI. Special
> sources are the exception while emulated ones are the majority. And for
> the latter we should try very hard to keep things simple and clean.
> 
> Jan

I assume this means yes :) So how about we replace the hash table with a
single GSI reserved for this purpose, and use that for each interrupt?
This will work fine for slow paths such as hotplug controller, yes it
will be slow but *predictably* slow.

Fast path will use static GSIs like qemu-kvm does.


> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Jan Kiszka]

On 2012-03-28 17:43, Michael S. Tsirkin wrote:
> On Wed, Mar 28, 2012 at 01:36:15PM +0200, Jan Kiszka wrote:
>> On 2012-03-28 13:31, Michael S. Tsirkin wrote:
>>>>>>> Also, how would this support irqfd in the future? Will we have to
>>>>>>> rip it all out and replace with per-device tracking that we
>>>>>>> have today?
>>>>>>
>>>>>> Irqfd and kvm device assignment will require additional interfaces (of
>>>>>> the kvm core in QEMU) via which you will be able to request stable
>>>>>> routes from such sources to specified MSIs. That will be widely
>>>>>> orthogonal to what is done in these patches here.
>>>>>
>>>>> Yes but not exactly as they will conflict for resources, right?
>>>>> How do you plan to solve this?
>>>>
>>>> As done in my original series: If a static route requires a pseudo GSI
>>>> and there are none free, we simply flush the dynamic MSI routes.
>>>
>>> Right. So static routes take precedence. This means that in effect
>>> we will have two APIs in qemu: for fast MSIs and for slow ones,
>>> the advantage of the slow APIs being that they are easier to use,
>>> right?
>>
>> We will have two APIs depending on the source of the MSI. Special
>> sources are the exception while emulated ones are the majority. And for
>> the latter we should try very hard to keep things simple and clean.
>>
>> Jan
> 
> I assume this means yes :) So how about we replace the hash table with a
> single GSI reserved for this purpose, and use that for each interrupt?
> This will work fine for slow paths such as hotplug controller, yes it
> will be slow but *predictably* slow.

AHCI, HDA, virtio-block, and every other userspace MSI user will suffer
- I can't imagine you really want this. :)

> 
> Fast path will use static GSIs like qemu-kvm does.

Nope, qemu-kvm hooks deeply into the MSI layer to track vectors. I don't
believe we want this upstream. It also doesn't work for non-PCI MSI
(HPET on x86, try -global hpet.timers=4 -global hpet.msi=on with Linux
guests).

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Michael S. Tsirkin]

On Wed, Mar 28, 2012 at 06:00:03PM +0200, Jan Kiszka wrote:
> On 2012-03-28 17:43, Michael S. Tsirkin wrote:
> > On Wed, Mar 28, 2012 at 01:36:15PM +0200, Jan Kiszka wrote:
> >> On 2012-03-28 13:31, Michael S. Tsirkin wrote:
> >>>>>>> Also, how would this support irqfd in the future? Will we have to
> >>>>>>> rip it all out and replace with per-device tracking that we
> >>>>>>> have today?
> >>>>>>
> >>>>>> Irqfd and kvm device assignment will require additional interfaces (of
> >>>>>> the kvm core in QEMU) via which you will be able to request stable
> >>>>>> routes from such sources to specified MSIs. That will be widely
> >>>>>> orthogonal to what is done in these patches here.
> >>>>>
> >>>>> Yes but not exactly as they will conflict for resources, right?
> >>>>> How do you plan to solve this?
> >>>>
> >>>> As done in my original series: If a static route requires a pseudo GSI
> >>>> and there are none free, we simply flush the dynamic MSI routes.
> >>>
> >>> Right. So static routes take precedence. This means that in effect
> >>> we will have two APIs in qemu: for fast MSIs and for slow ones,
> >>> the advantage of the slow APIs being that they are easier to use,
> >>> right?
> >>
> >> We will have two APIs depending on the source of the MSI. Special
> >> sources are the exception while emulated ones are the majority. And for
> >> the latter we should try very hard to keep things simple and clean.
> >>
> >> Jan
> > 
> > I assume this means yes :) So how about we replace the hash table with a
> > single GSI reserved for this purpose, and use that for each interrupt?
> > This will work fine for slow paths such as hotplug controller, yes it
> > will be slow but *predictably* slow.
> 
> AHCI, HDA, virtio-block, and every other userspace MSI user will suffer
> - I can't imagine you really want this. :)

These should use static GSI routes or the new API if it exists.
Changing GSI routing when AHCI wants to send an interrupt
will cause performance trouble in unpredictable ways:
it triggers RCU write side and that can be *very* slow.

> > 
> > Fast path will use static GSIs like qemu-kvm does.
> 
> Nope, qemu-kvm hooks deeply into the MSI layer to track vectors. I don't
> believe we want this upstream. It also doesn't work for non-PCI MSI
> (HPET on x86, try -global hpet.timers=4 -global hpet.msi=on with Linux
> guests).
> 
> Jan

Yes I understand you want an API on top of this, with
some structure to abstract the ideas away from PCI.
But the principle is that we'll track GSIs at the device
and keep the mapping to vectors static.

> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Jan Kiszka]

On 2012-03-28 18:30, Michael S. Tsirkin wrote:
> On Wed, Mar 28, 2012 at 06:00:03PM +0200, Jan Kiszka wrote:
>> On 2012-03-28 17:43, Michael S. Tsirkin wrote:
>>> On Wed, Mar 28, 2012 at 01:36:15PM +0200, Jan Kiszka wrote:
>>>> On 2012-03-28 13:31, Michael S. Tsirkin wrote:
>>>>>>>>> Also, how would this support irqfd in the future? Will we have to
>>>>>>>>> rip it all out and replace with per-device tracking that we
>>>>>>>>> have today?
>>>>>>>>
>>>>>>>> Irqfd and kvm device assignment will require additional interfaces (of
>>>>>>>> the kvm core in QEMU) via which you will be able to request stable
>>>>>>>> routes from such sources to specified MSIs. That will be widely
>>>>>>>> orthogonal to what is done in these patches here.
>>>>>>>
>>>>>>> Yes but not exactly as they will conflict for resources, right?
>>>>>>> How do you plan to solve this?
>>>>>>
>>>>>> As done in my original series: If a static route requires a pseudo GSI
>>>>>> and there are none free, we simply flush the dynamic MSI routes.
>>>>>
>>>>> Right. So static routes take precedence. This means that in effect
>>>>> we will have two APIs in qemu: for fast MSIs and for slow ones,
>>>>> the advantage of the slow APIs being that they are easier to use,
>>>>> right?
>>>>
>>>> We will have two APIs depending on the source of the MSI. Special
>>>> sources are the exception while emulated ones are the majority. And for
>>>> the latter we should try very hard to keep things simple and clean.
>>>>
>>>> Jan
>>>
>>> I assume this means yes :) So how about we replace the hash table with a
>>> single GSI reserved for this purpose, and use that for each interrupt?
>>> This will work fine for slow paths such as hotplug controller, yes it
>>> will be slow but *predictably* slow.
>>
>> AHCI, HDA, virtio-block, and every other userspace MSI user will suffer
>> - I can't imagine you really want this. :)
> 
> These should use static GSI routes or the new API if it exists.

There will be an API to request an irqfd and associate it with a MSI
message and the same for an assigned device IRQ/MSI vector. But none for
userspace generated messages. That would mean hooking deep into the MSI
layer again - or even the devices themselves.

> Changing GSI routing when AHCI wants to send an interrupt
> will cause performance trouble in unpredictable ways:
> it triggers RCU write side and that can be *very* slow.

That's why we will have direct MSI injection for them. This here is just
to make it work without that feature in a reasonable, non-intrusive way.

If it really hurts that much, we need to invest more in avoiding cache
flushes. But I'm skeptical there is much to gain compared to the current
qemu-kvm model: every vector change that results in a route change
passes the RCU write side - and serializes other QEMU userspace exists.
That _is_ already a bottleneck. Every MSI IRQ balancing between CPUs in
the guest should trigger this e.g.

What I would really like to avoid is introducing invasive abstractions
and hooks to QEMU that optimize for a scenario that is obsolete mid to
long term.

> 
>>>
>>> Fast path will use static GSIs like qemu-kvm does.
>>
>> Nope, qemu-kvm hooks deeply into the MSI layer to track vectors. I don't
>> believe we want this upstream. It also doesn't work for non-PCI MSI
>> (HPET on x86, try -global hpet.timers=4 -global hpet.msi=on with Linux
>> guests).
>>
>> Jan
> 
> Yes I understand you want an API on top of this, with
> some structure to abstract the ideas away from PCI.
> But the principle is that we'll track GSIs at the device
> and keep the mapping to vectors static.

Devices will track irqfd objects or similar abstractions for device
assignment, not GSIs explicitly. Under the hood, there will be the GSI
stored, of course. That will be simpler to apply than the current
open-coded pattern.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Michael S. Tsirkin]

On Wed, Mar 28, 2012 at 06:53:01PM +0200, Jan Kiszka wrote:
> On 2012-03-28 18:30, Michael S. Tsirkin wrote:
> > On Wed, Mar 28, 2012 at 06:00:03PM +0200, Jan Kiszka wrote:
> >> On 2012-03-28 17:43, Michael S. Tsirkin wrote:
> >>> On Wed, Mar 28, 2012 at 01:36:15PM +0200, Jan Kiszka wrote:
> >>>> On 2012-03-28 13:31, Michael S. Tsirkin wrote:
> >>>>>>>>> Also, how would this support irqfd in the future? Will we have to
> >>>>>>>>> rip it all out and replace with per-device tracking that we
> >>>>>>>>> have today?
> >>>>>>>>
> >>>>>>>> Irqfd and kvm device assignment will require additional interfaces (of
> >>>>>>>> the kvm core in QEMU) via which you will be able to request stable
> >>>>>>>> routes from such sources to specified MSIs. That will be widely
> >>>>>>>> orthogonal to what is done in these patches here.
> >>>>>>>
> >>>>>>> Yes but not exactly as they will conflict for resources, right?
> >>>>>>> How do you plan to solve this?
> >>>>>>
> >>>>>> As done in my original series: If a static route requires a pseudo GSI
> >>>>>> and there are none free, we simply flush the dynamic MSI routes.
> >>>>>
> >>>>> Right. So static routes take precedence. This means that in effect
> >>>>> we will have two APIs in qemu: for fast MSIs and for slow ones,
> >>>>> the advantage of the slow APIs being that they are easier to use,
> >>>>> right?
> >>>>
> >>>> We will have two APIs depending on the source of the MSI. Special
> >>>> sources are the exception while emulated ones are the majority. And for
> >>>> the latter we should try very hard to keep things simple and clean.
> >>>>
> >>>> Jan
> >>>
> >>> I assume this means yes :) So how about we replace the hash table with a
> >>> single GSI reserved for this purpose, and use that for each interrupt?
> >>> This will work fine for slow paths such as hotplug controller, yes it
> >>> will be slow but *predictably* slow.
> >>
> >> AHCI, HDA, virtio-block, and every other userspace MSI user will suffer
> >> - I can't imagine you really want this. :)
> > 
> > These should use static GSI routes or the new API if it exists.
> 
> There will be an API to request an irqfd and associate it with a MSI
> message and the same for an assigned device IRQ/MSI vector. But none for
> userspace generated messages. That would mean hooking deep into the MSI
> layer again - or even the devices themselves.

What I had in mind is an API like

MSIVector *get_msi_vector(PCIDevice *)
put_msi_vector(MSIVector *)

and then devices just need to keep an array of these vectors
around. Is this really that bad?

> > Changing GSI routing when AHCI wants to send an interrupt
> > will cause performance trouble in unpredictable ways:
> > it triggers RCU write side and that can be *very* slow.
> 
> That's why we will have direct MSI injection for them. This here is just
> to make it work without that feature in a reasonable, non-intrusive way.
> 
> If it really hurts that much, we need to invest more in avoiding cache
> flushes. But I'm skeptical there is much to gain compared to the current
> qemu-kvm model: every vector change that results in a route change
> passes the RCU write side - and serializes other QEMU userspace exists.

Yes vector changes are slow but the cache would make route changes
on interrupt injection, as opposed to rebalancing.

> That _is_ already a bottleneck. Every MSI IRQ balancing between CPUs in
> the guest should trigger this e.g.
> 
> What I would really like to avoid is introducing invasive abstractions
> and hooks to QEMU that optimize for a scenario that is obsolete mid to
> long term.

If Avi adds the dynamic API, I'm fine with this hack as a fallback.
Let's see what happens ...

> > 
> >>>
> >>> Fast path will use static GSIs like qemu-kvm does.
> >>
> >> Nope, qemu-kvm hooks deeply into the MSI layer to track vectors. I don't
> >> believe we want this upstream. It also doesn't work for non-PCI MSI
> >> (HPET on x86, try -global hpet.timers=4 -global hpet.msi=on with Linux
> >> guests).
> >>
> >> Jan
> > 
> > Yes I understand you want an API on top of this, with
> > some structure to abstract the ideas away from PCI.
> > But the principle is that we'll track GSIs at the device
> > and keep the mapping to vectors static.
> 
> Devices will track irqfd objects or similar abstractions for device
> assignment, not GSIs explicitly. Under the hood, there will be the GSI
> stored, of course. That will be simpler to apply than the current
> open-coded pattern.
> 
> Jan
> 
> -- 
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/2] uq/master: Basic MSI support for in-kernel irqchip mode

[Jan Kiszka]

On 2012-03-28 19:06, Michael S. Tsirkin wrote:
> On Wed, Mar 28, 2012 at 06:53:01PM +0200, Jan Kiszka wrote:
>> On 2012-03-28 18:30, Michael S. Tsirkin wrote:
>>> On Wed, Mar 28, 2012 at 06:00:03PM +0200, Jan Kiszka wrote:
>>>> On 2012-03-28 17:43, Michael S. Tsirkin wrote:
>>>>> On Wed, Mar 28, 2012 at 01:36:15PM +0200, Jan Kiszka wrote:
>>>>>> On 2012-03-28 13:31, Michael S. Tsirkin wrote:
>>>>>>>>>>> Also, how would this support irqfd in the future? Will we have to
>>>>>>>>>>> rip it all out and replace with per-device tracking that we
>>>>>>>>>>> have today?
>>>>>>>>>>
>>>>>>>>>> Irqfd and kvm device assignment will require additional interfaces (of
>>>>>>>>>> the kvm core in QEMU) via which you will be able to request stable
>>>>>>>>>> routes from such sources to specified MSIs. That will be widely
>>>>>>>>>> orthogonal to what is done in these patches here.
>>>>>>>>>
>>>>>>>>> Yes but not exactly as they will conflict for resources, right?
>>>>>>>>> How do you plan to solve this?
>>>>>>>>
>>>>>>>> As done in my original series: If a static route requires a pseudo GSI
>>>>>>>> and there are none free, we simply flush the dynamic MSI routes.
>>>>>>>
>>>>>>> Right. So static routes take precedence. This means that in effect
>>>>>>> we will have two APIs in qemu: for fast MSIs and for slow ones,
>>>>>>> the advantage of the slow APIs being that they are easier to use,
>>>>>>> right?
>>>>>>
>>>>>> We will have two APIs depending on the source of the MSI. Special
>>>>>> sources are the exception while emulated ones are the majority. And for
>>>>>> the latter we should try very hard to keep things simple and clean.
>>>>>>
>>>>>> Jan
>>>>>
>>>>> I assume this means yes :) So how about we replace the hash table with a
>>>>> single GSI reserved for this purpose, and use that for each interrupt?
>>>>> This will work fine for slow paths such as hotplug controller, yes it
>>>>> will be slow but *predictably* slow.
>>>>
>>>> AHCI, HDA, virtio-block, and every other userspace MSI user will suffer
>>>> - I can't imagine you really want this. :)
>>>
>>> These should use static GSI routes or the new API if it exists.
>>
>> There will be an API to request an irqfd and associate it with a MSI
>> message and the same for an assigned device IRQ/MSI vector. But none for
>> userspace generated messages. That would mean hooking deep into the MSI
>> layer again - or even the devices themselves.
> 
> What I had in mind is an API like
> 
> MSIVector *get_msi_vector(PCIDevice *)
> put_msi_vector(MSIVector *)
> 
> and then devices just need to keep an array of these vectors
> around. Is this really that bad?

Yes, as the points to get and put means tracking what goes on in the
vectors beyond what is actually needed. The above is just the beginning.
And, again, it assumes that only PCI devices can send MSIs, which is not
true.

You may recall my first series which tried to hid a bit of this mess
behind the MSIRoutingCache concept. It was way more complex and invasive
than the new approach.

> 
>>> Changing GSI routing when AHCI wants to send an interrupt
>>> will cause performance trouble in unpredictable ways:
>>> it triggers RCU write side and that can be *very* slow.
>>
>> That's why we will have direct MSI injection for them. This here is just
>> to make it work without that feature in a reasonable, non-intrusive way.
>>
>> If it really hurts that much, we need to invest more in avoiding cache
>> flushes. But I'm skeptical there is much to gain compared to the current
>> qemu-kvm model: every vector change that results in a route change
>> passes the RCU write side - and serializes other QEMU userspace exists.
> 
> Yes vector changes are slow but the cache would make route changes
> on interrupt injection, as opposed to rebalancing.

What's the difference? It hits the whole VM (via the userspace
hypervisor part) at none-predictable points during runtime.

> 
>> That _is_ already a bottleneck. Every MSI IRQ balancing between CPUs in
>> the guest should trigger this e.g.
>>
>> What I would really like to avoid is introducing invasive abstractions
>> and hooks to QEMU that optimize for a scenario that is obsolete mid to
>> long term.
> 
> If Avi adds the dynamic API, I'm fine with this hack as a fallback.
> Let's see what happens ...

I'll refresh that KVM patch and give it another try.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux