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

[Jan Kiszka <jan.kiszka@siemens.com>]

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