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

[Alexandru Stefan ISAILA <aisaila@bitdefender.com>]

On Vi, 2018-06-22 at 09:51 -0600, Jan Beulich wrote:
> >
> > >
> > > >
> > > > On 18.06.18 at 17:17, <aisaila@bitdefender.com> wrote:
> > From: Isaila Alexandru <aisaila@bitdefender.com>
> >
> > 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.
>
> >
> > --- a/xen/arch/x86/mm/mem_access.c
> > +++ b/xen/arch/x86/mm/mem_access.c
> > @@ -221,6 +221,9 @@ bool p2m_mem_access_check(paddr_t gpa, unsigned
> > long gla,
> >          {
> >              req->u.mem_access.flags |= MEM_ACCESS_GLA_VALID;
> >              req->u.mem_access.gla = gla;
> > +        }
> > +        if ( npfec.gla_valid || cpu_has_svm )
> > +        {
> >
> >              if ( npfec.kind == npfec_kind_with_gla )
> You leave a bogusly placed blank line. Please put it ahead of the
> if()
> you add.
>
> >
> > @@ -112,8 +117,37 @@ static unsigned long p2m_type_to_flags(const
> > struct p2m_domain *p2m,
> >              flags |= _PAGE_PWT;
> >              ASSERT(!level);
> >          }
> > -        return flags | P2M_BASE_FLAGS | _PAGE_PCD;
> > +        flags |= P2M_BASE_FLAGS | _PAGE_PCD;
> > +        break;
> > +    }
> > +    switch (access)
> Coding style.
>
> >
> > +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.

Alex

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