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

[Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>]

On Jo, 2018-06-28 at 08:40 -0600, Jan Beulich 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.

The number was with this patch so no correction. This was done with
xen-access write and I don't think it will make a difference if you
change the access to all the mem pages.

Alex

________________________
This email was scanned by Bitdefender
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel