On Thu, Nov 03, 2011 at 12:42:55PM +0100, Jan Kiszka wrote:
> On 2011-11-02 21:11, Michael S. Tsirkin wrote:
> > MSIX spec requires that device can be operated with
> > all vectors masked, by polling pending bits.
> > Add APIs to recall an msix notification, and make polling
> > mode possible in virtio-pci by clearing the
> > pending bits and setting ISR appropriately on ISR read.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> > ---
> > hw/msix.c | 26 ++++++++++++++++++++++++++
> > hw/msix.h | 3 +++
> > hw/virtio-pci.c | 11 ++++++++++-
> > 3 files changed, 39 insertions(+), 1 deletions(-)
> >
> > diff --git a/hw/msix.c b/hw/msix.c
> > index 63b41b9..fe967c9 100644
> > --- a/hw/msix.c
> > +++ b/hw/msix.c
> > @@ -349,6 +349,32 @@ void msix_notify(PCIDevice *dev, unsigned vector)
> > stl_le_phys(address, data);
> > }
> >
> > +/* Recall outstanding MSI-X notifications for a vector, if possible.
> > + * Return true if any were outstanding. */
> > +bool msix_recall(PCIDevice *dev, unsigned vector)
> > +{
> > + bool ret;
> > + if (vector >= dev->msix_entries_nr)
> > + return false;
> > + ret = msix_is_pending(dev, vector);
> > + msix_clr_pending(dev, vector);
> > + return ret;
> > +}
>
> I would prefer to have a single API instead to clarify the tight relation:
>
> bool msi[x]_set_notify(PCIDevice *dev, unsigned vector, unsigned level)
>
> Would return true for level=1 if the message was either sent directly or
> queued (we could deliver false if it was already queued, but I see no
> use case for this yet).
It's a matter of taste: some people like functions with flags, some
prefer separate functions. I really prefer two functions.
But I agree it woulkd be better to have a name that makes it clear that
what we recall is a notification.
msix_notify_queue/msix_notify_dequeue?
> Also, I don't see the generic value of some msix_recall_all. I think
> it's better handled in a single loop over all vectors at caller site,
> clearing the individual interrupt reason bits on a per-vector basis
> there. msix_recall_all is only useful in the virtio case where you have
> one vector of reason A and all the rest of B. Once you had multiple
> reason C vectors as well, it would not help anymore.
>
> Jan
The reason I wanted to have it is to reduce the overhead this adds:
since PBA is packed, it's much faster to check whether any bits are set
than by going through them all, one by one. Typically all PBA
bits are clear ...
I agree it might not help non-virtio devices, but to me it looks like a
harmless little helper - what's the issue with it?
> --
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
