"Michael S. Tsirkin" <mst@xxxxxxxxxx> wrote:
> When guest notifier is assigned, we set mask notifier,
> which will assign kvm irqfd.
> When guest notifier is unassigned, mask notifier is unset,
> which should unassign kvm irqfd.
>
> The way to do this is to call mask notifier telling it to mask the vector.
> This, unless vector is already masked which unassigns irqfd already.
>
> The logic in unassign was reversed, which left kvm irqfd assigned.
>
> This patch is qemu-kvm only as irqfd is not upstream.
>
> Signed-off-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> Reported-by: Amit Shah <amit.shah@xxxxxxxxxx>
> ---
> hw/msix.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/hw/msix.c b/hw/msix.c
> index 8f9a621..1398680 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -617,6 +617,7 @@ int msix_set_mask_notifier(PCIDevice *dev, unsigned vector, void *opaque)
> assert(opaque);
> assert(!dev->msix_mask_notifier_opaque[vector]);
>
> + /* Unmask the new notifier unless vector is masked. */
> if (msix_is_masked(dev, vector)) {
> return 0;
> }
> @@ -638,12 +639,13 @@ int msix_unset_mask_notifier(PCIDevice *dev, unsigned vector)
> assert(dev->msix_mask_notifier);
> assert(dev->msix_mask_notifier_opaque[vector]);
>
> + /* Mask the old notifier unless it is already masked. */
> if (msix_is_masked(dev, vector)) {
> return 0;
> }
> r = dev->msix_mask_notifier(dev, vector,
> dev->msix_mask_notifier_opaque[vector],
> - msix_is_masked(dev, vector));
> + !msix_is_masked(dev, vector));
Why don't put just a 1 here?
we have:
if (msix_is_masked())
return 0
r = msix_mask_notifier(....., !msix_is_masked());
i.e. at that point msix_is_masked() is false, or we really, really needs
locking.
Puttting a !foo, when we know that it needs to be an 1 looks strange.
Later, Juan.
PD. Yes, I already asked in a previous version to just have two
methods, mask/unmask. we now at call time which one we need.
--
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
