Re: [PATCH] xen/p2m: Remove np2m-specific filter from generic p2m_flush_table

From: Tim Deegan <tim@xen.org>
To: Jan Beulich <JBeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Liang Li <liang.z.li@intel.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Matt Leinhos <matt@starlab.io>,
	Tamas KLengyel <tamas.lengyel@zentific.com>,
	Yang Zhang <yang.z.zhang@intel.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH] xen/p2m: Remove np2m-specific filter from generic p2m_flush_table
Date: Wed, 8 Feb 2017 10:02:19 +0000	[thread overview]
Message-ID: <20170208100219.GB54560@deinos.phlegethon.org> (raw)
In-Reply-To: <589078930200007800135547@prv-mh.provo.novell.com>

At 03:44 -0700 on 31 Jan (1485834259), Jan Beulich wrote:
> >>> On 30.01.17 at 16:17, <george.dunlap@citrix.com> wrote:
> > Commit 71bb7304e7a7a35ea6df4b0cedebc35028e4c159 added flushing of
> > nested p2m tables whenever the host p2m table changed.  Unfortunately
> > in the process, it added a filter to the generic p2m_flush_table()
> > function so that the p2m would only be flushed if it was being used as
> > a nested p2m.  This meant that the p2m was not being flushed at all
> > for altp2m callers.
> > 
> > Instead do the nested p2m filtering in p2m_flush_nestedp2m().

I think it would be better to fix the test in p2m_flush_table() so it
understands altp2m's use of tables.  That way we won't have to deal
with filtering at other call sites, as Jan points out.  Also, this:

> >      for ( i = 0; i < MAX_NESTEDP2M; i++ )
> > -        p2m_flush_table(d->arch.nested_p2m[i]);
> > +    {
> > +        struct p2m_domain *p2m = d->arch.nested_p2m[i];
> > +        if ( p2m->np2m_base != P2M_BASE_EADDR )
> > +            p2m_flush_table(p2m);
> > +    }

moves the check of np2m_base outside the lock.  That might be OK but
it's definitely a bit subtle.

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel