All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	George Dunlap <george.dunlap@citrix.com>
Subject: Re: [PATCH] x86/PoD: defer nested P2M flushes
Date: Tue, 19 Oct 2021 14:59:54 +0200	[thread overview]
Message-ID: <YW7BSjF3DD0jl74r@MacBook-Air-de-Roger.local> (raw)
In-Reply-To: <9e444a8b-58e9-ea37-0e22-474e430be5e5@suse.com>

On Tue, Oct 19, 2021 at 01:58:38PM +0200, Jan Beulich wrote:
> On 19.10.2021 12:41, Roger Pau Monné wrote:
> > On Mon, Oct 11, 2021 at 10:17:08AM +0200, Jan Beulich wrote:
> >> With NPT or shadow in use, the p2m_set_entry() -> p2m_pt_set_entry() ->
> >> write_p2m_entry() -> p2m_flush_nestedp2m() call sequence triggers a lock
> >> order violation when the PoD lock is held around it. Hence such flushing
> >> needs to be deferred. Steal the approach from p2m_change_type_range().
> >>
> >> Similarly for EPT I think ept_set_entry() -> ept_sync_domain() ->
> >> ept_sync_domain_prepare() -> p2m_flush_nestedp2m() is affected.
> > 
> > I'm slightly worried by this path because it doesn't seem to
> > acknowledge defer_nested_flush.
> 
> Oh, yes. Iirc I did look at that logic, write down the remark, and
> then forgot to add the conditional to ept_sync_domain_prepare().
> The interactions with the real (host) flush are kind of ugly there,
> though - we then further depend on the ->defer_flush / ->need_flush
> logic, which is EPT-only. But I think I've convinced myself that
> this ought to work out.
> 
> > Maybe the flag should be checked by
> > p2m_flush_nestedp2m instead of leaving it to the callers?
> 
> I'm not sure this would be a good idea, as there are more callers.

We should maybe add an ASSERT to p2m_flush_nestedp2m to make sure it's
not called with defer_nested_flush being set then, or else it's
possible for new callers of p2m_flush_nestedp2m to forget checking
defer_nested_flush.

Thanks, Roger.


  reply	other threads:[~2021-10-19 13:00 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-11  8:17 [PATCH] x86/PoD: defer nested P2M flushes Jan Beulich
2021-10-18  8:34 ` Ping: " Jan Beulich
2021-10-19  8:09 ` Roger Pau Monné
2021-10-19  8:17   ` Jan Beulich
2021-10-19  8:19     ` Jan Beulich
2021-10-19 10:39       ` Roger Pau Monné
2021-10-19 10:50         ` Jan Beulich
2021-10-19 10:41 ` Roger Pau Monné
2021-10-19 11:58   ` Jan Beulich
2021-10-19 12:59     ` Roger Pau Monné [this message]
2021-10-19 13:14       ` Jan Beulich
2021-10-19 13:36         ` 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=YW7BSjF3DD0jl74r@MacBook-Air-de-Roger.local \
    --to=roger.pau@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=jbeulich@suse.com \
    --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.