Re: [Qemu-devel] [RFC][PATCH 28/45] qemu-kvm: msix: Drop tracking of used vectors

[Jan Kiszka]

On 2011-10-18 14:33, Michael S. Tsirkin wrote:
> On Tue, Oct 18, 2011 at 02:08:59PM +0200, Jan Kiszka wrote:
>> On 2011-10-18 13:58, Michael S. Tsirkin wrote:
>>> On Mon, Oct 17, 2011 at 09:28:12PM +0200, Jan Kiszka wrote:
>>>> On 2011-10-17 17:48, Michael S. Tsirkin wrote:
>>>>> On Mon, Oct 17, 2011 at 11:28:02AM +0200, Jan Kiszka wrote:
>>>>>> This optimization was only required to keep KVM route usage low. Now
>>>>>> that we solve that problem via lazy updates, we can drop the field. We
>>>>>> still need interfaces to clear pending vectors, though (and we have to
>>>>>> make use of them more broadly - but that's unrelated to this patch).
>>>>>>
>>>>>> Signed-off-by: Jan Kiszka <address@hidden>
>>>>>
>>>>> Lazy updates should be an implementation detail.
>>>>> IMO resource tracking of vectors makes sense
>>>>> as an API. Making devices deal with pending
>>>>> vectors as a concept, IMO, does not.
>>>>
>>>> There is really no use for tracking the vector lifecycle once we have
>>>> lazy updates (except for static routes). It's a way too invasive
>>>> concept, and it's not needed for anything but KVM.
>>>
>>> I think it's needed. The PCI spec states that when the device
>>> does not need an interrupt anymore, it should clear the pending
>>> bit.
>>
>> That should be done explicitly if it is required outside existing
>> clearing points. We already have that service, it's called
>> msix_clear_vector.
> 
> We do? I don't seem to see it upstream...

True. From the device's POV, MSI-X (and also MSI!) vectors are actually
level-triggered. So we should communicate the level to the MSI core and
not just the edge. Needs more fixing

> 
>> That alone does not justify msix_vector_use and all
>> the state and logic behind it IMHO.
> 
> To me it looks like an abstraction that solves both
> this problem and the resource allocation problem.
> Resources are actually limited BTW, this is not just
> a KVM thing. qemu.git currently lets guests decide
> what to do with them, but it might turn out to
> be benefitial to warn the management application
> that it is shooting itself in the foot.
> 
>>> The use/unuse is IMO a decent API for this,
>>> because it uses a familiar resource tracking concept.
>>> Exposing this knowledge of msix to devices seems
>>> like a worse API.
>>>
>>>>
>>>> If you want an example, check
>>>> http://permalink.gmane.org/gmane.comp.emulators.kvm.devel/70915 and
>>>> compare it to the changes done to hpet in this series.
>>>>
>>>> Jan
>>>>
>>>
>>> This seems to be a general argument that lazy updates are good?
>>> I have no real problem with them, besides the fact that
>>> we need an API to reserve space in the routing
>>> table so that device setup can fail upfront.
>>
>> That's not possible, even with used vectors, as devices change their
>> vector usage depending on how the guest configures the devices. If you
>> (pre-)allocate all possible vectors, you may run out of resources
>> earlier than needed actually.
> 
> This really depends, but please do look at how with virtio
> we report resource shortage to guest and let it fall back to
> level interrups. You seem to remove that capability.

To my understanding, virtio will be the exception as no other device
will have a chance to react on resource shortage while sending(!) an MSI
message.

> 
> I actually would not mind preallocating everything upfront which is much
> easier.  But with your patch we get a silent failure or a drastic
> slowdown which is much more painful IMO.

Again: did we already saw that limit? And where does it come from if not
from KVM?

> 
>> That's also why we do those data == 0
>> checks to skip used but unconfigured vectors.
>>
>> Jan
> 
> These checks work more or less by luck BTW. It's
> a hack which I hope lazy allocation will replace.

The check is still valid (for x86) when we have to use static routes
(device assignment, vhost). For lazy updates, it's obsolete, that's true.

Jan

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