All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Roger Pau Monne <roger.pau@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wl@xen.org>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH 2/3] x86/irq: don't disable domain MSI IRQ on force unbind
Date: Tue, 26 Jan 2021 15:52:54 +0100	[thread overview]
Message-ID: <7633ddb0-922b-0165-7e8c-d265786ef4aa@suse.com> (raw)
In-Reply-To: <20210126110606.21741-3-roger.pau@citrix.com>

On 26.01.2021 12:06, Roger Pau Monne wrote:
> When force unbinding a MSI the used IRQ would get added to the domain
> irq-pirq tree as -irq -pirq,

I think it's -pirq at index irq, i.e. I don't think IRQ gets
negated as far as the radix tree goes. info->arch.irq gets a
negative value stored, yes.

> thus preventing the same IRQ to be used by the domain.

Iirc this (answering your post-commit-message question here)
is for cleaning up _after_ the domain, i.e. there's no goal
to allow re-use of this IRQ. The comment ahead of
unmap_domain_pirq() validly says "The pirq should have been
unbound before this call." The only time we can't make
ourselves dependent upon this is when the guest is being
cleaned up. During normal operation I think we actually
_want_ to enforce correct behavior of the guest here.

> It's not clear to me why we add the irq as -irq -pirq to the irq-pirq
> domain tree on forced unbind, as allowing to bind the irq again should
> just work regardless of whether it has been previously forcefully
> unbound?

To continue from the above, see pirq_guest_unbind() where
we have

    if ( desc == NULL )
    {
        irq = -pirq->arch.irq;
        BUG_ON(irq <= 0);
        desc = irq_to_desc(irq);
        spin_lock_irq(&desc->lock);
        clear_domain_irq_pirq(d, irq, pirq);
    }

as the alternative to going the normal path through
__pirq_guest_unbind(). Again iirc that's to cover for the
unbind request arriving after the unmap one (i.e. having
caused the unmap to force-unbind the IRQ).

Jan


  reply	other threads:[~2021-01-26 14:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-26 11:06 [PATCH 0/3] x86/intr: interrupt related fixes Roger Pau Monne
2021-01-26 11:06 ` [PATCH 1/3] x86/irq: remove duplicated clear_domain_irq_pirq calls Roger Pau Monne
2021-01-26 14:38   ` Jan Beulich
2021-01-26 16:08     ` Roger Pau Monné
2021-01-26 16:13       ` Jan Beulich
2021-01-26 11:06 ` [PATCH 2/3] x86/irq: don't disable domain MSI IRQ on force unbind Roger Pau Monne
2021-01-26 14:52   ` Jan Beulich [this message]
2021-01-26 16:21     ` Roger Pau Monné
2021-01-27  8:59       ` Jan Beulich
2021-01-26 11:06 ` [PATCH 3/3] x86/vmsi: tolerate unsupported MSI address/data fields Roger Pau Monne
2021-01-26 15:17   ` Jan Beulich
2021-01-26 15:57     ` Roger Pau Monné

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7633ddb0-922b-0165-7e8c-d265786ef4aa@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.