On Wed, Oct 06, 2010 at 04:05:38PM -0600, Alex Williamson wrote:
> On Wed, 2010-10-06 at 23:44 +0200, Michael S. Tsirkin wrote:
> > On Wed, Oct 06, 2010 at 11:24:24AM -0600, Alex Williamson wrote:
> > > You could always keep the functions as separate wrapper callers of the
> > > common function so you only need to keep true = unset, false = set
> > > straight in one place. Thanks,
> >
> >
> > Just to show why it does not work, I did exactly this: as you see the
> > code is shorter but the true/false magic gets spread: it was in 2
> > places, (set/unset) now it is in 4 places and it is within the loop, in
> > code that is more complex.
>
> You seem to have missed the wrapper function. I'm simply suggesting
> something like this:
Good idea. I tweaked the code a bit more and came up with this:
I think I will keep this as an incremental patch just to
keep diffs small.
msix: factor out mask notifier code
Move some common code handling msix mask notifiers to a function.
Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
diff --git a/hw/msix.c b/hw/msix.c
index 3d4dd61..5f66d20 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -583,40 +583,26 @@ void msix_unuse_all_vectors(PCIDevice *dev)
msix_free_irq_entries(dev);
}
-static int msix_set_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
+/* Invoke the notifier if vector entry is used and unmasked. */
+static int msix_notify_if_unmasked(PCIDevice *dev, unsigned vector, int masked)
{
- int r = 0;
- if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector])
- return 0;
-
assert(dev->msix_mask_notifier);
-
- /* Unmask the new notifier unless vector is masked. */
- if (!msix_is_masked(dev, vector)) {
- r = dev->msix_mask_notifier(dev, vector, false);
- if (r < 0) {
- return r;
- }
+ if (!dev->msix_entry_used[vector] || msix_is_masked(dev, vector)) {
+ return 0;
}
- return r;
+ return dev->msix_mask_notifier(dev, vector, masked);
}
-static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
+static int msix_set_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
{
- int r = 0;
- if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector])
- return 0;
-
- assert(dev->msix_mask_notifier);
+ /* Notifier has been set. Invoke it on unmasked vectors. */
+ return msix_notify_if_unmasked(dev, vector, 0);
+}
- /* Mask the old notifier unless it is already masked. */
- if (!msix_is_masked(dev, vector)) {
- r = dev->msix_mask_notifier(dev, vector, true);
- if (r < 0) {
- return r;
- }
- }
- return r;
+static int msix_unset_mask_notifier_for_vector(PCIDevice *dev, unsigned vector)
+{
+ /* Notifier will be unset. Invoke it to mask unmasked entries. */
+ return msix_notify_if_unmasked(dev, vector, 1);
}
int msix_set_mask_notifier(PCIDevice *dev, msix_mask_notifier_func f)
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
