Re: [Xen-devel] [PATCH v2] x86/mm: Add mem access rights to NPT

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Jun 28, 2018 at 8:40 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
> >>> On 28.06.18 at 16:10, <aisaila@xxxxxxxxxxxxxxx> wrote:
> > On Vi, 2018-06-22 at 09:51 -0600, Jan Beulich wrote:
> >> > > > On 18.06.18 at 17:17, <aisaila@xxxxxxxxxxxxxxx> wrote:
> >> > From: Isaila Alexandru <aisaila@xxxxxxxxxxxxxxx>
> >> >
> >> > This patch adds access rights for the NPT pages. The access rights
> >> > are
> >> > saved in a radix tree with the root saved in p2m_domain.
> >> Sounds resource intensive. How many nodes would such a radix tree
> >> have
> >> on average?
> >
> > The average is around 1478890 for a machine with 4GB of ram.
>
> Is this with ...
>
> >> > +static void p2m_set_access(struct p2m_domain *p2m, unsigned long
> >> > gfn,
> >> > +                                      p2m_access_t a)
> >> > +{
> >> > +    int rc;
> >> > +
> >> > +    if ( p2m_access_rwx == a )
> >> > +        radix_tree_delete(&p2m->mem_access_settings, gfn);
> >> > +
> >> > +    rc = radix_tree_insert(&p2m->mem_access_settings, gfn,
> >> > +                           radix_tree_int_to_ptr(a));
> >> Is there an "else" missing above here? Otherwise why would you
> >> delete the node first?
> >
> > Yes it needs a else or a return. We plan not to have the rwx in the
> > tree so we ca save up some space.
>
> ... this corrected? Otherwise I'm tempted to say that the creation of
> this radix tree needs to be avoided by all means, as long as it's not
> really needed.

Having the radix tree created and present doesn't add much overhead. A
lookup on an empty radix tree is effectively just boils down to a NULL
check. So yes, when the page is rwx, there should be no setting in the
tree. Otherwise performance is going to decrease with the number of
pages having custom settings, but since this tree under "normal" use
won't have more then a couple hundred nodes at most this shouldn't
really be of concern. If performance is of essence, optimizing how
many pages are monitoring with mem_access is ultimately up to the
user.

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel