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

["Michael S. Tsirkin" <mst@redhat.com>]

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 <jan.kiszka@siemens.com>
> >>>
> >>> 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...

> 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.

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.

> 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.

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