Re: [Xen-devel] [RFC 13/22] xen/arm: p2m: Replace all usage of __p2m_lookup with p2m_get_entry

[Julien Grall <julien.grall@xxxxxxx>]

Hello Tamas,

On 28/07/2016 18:29, Tamas K Lengyel wrote:
> On Thu, Jul 28, 2016 at 8:51 AM, Julien Grall <julien.grall@xxxxxxx> wrote:
[...]

> > ---
> >  xen/arch/arm/p2m.c | 18 ++++--------------
> >  1 file changed, 4 insertions(+), 14 deletions(-)
> >
> > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > index 8676b9d..9a9c85c 100644
> > --- a/xen/arch/arm/p2m.c
> > +++ b/xen/arch/arm/p2m.c
> > @@ -398,24 +398,13 @@ out:
> >      return mfn;
> >  }
> >
> > -/*
> > - * Lookup the MFN corresponding to a domain's GFN.
> > - *
> > - * There are no processor functions to do a stage 2 only lookup therefore we
> > - * do a a software walk.
> > - */
> > -static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
> > -{
> > -    return p2m_get_entry(&d->arch.p2m, gfn, t, NULL, NULL);
> > -}
> > -
> >  mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
> >  {
> >      mfn_t ret;
> >      struct p2m_domain *p2m = &d->arch.p2m;
> >
> >      p2m_read_lock(p2m);
> > -    ret = __p2m_lookup(d, gfn, t);
> > +    ret = p2m_get_entry(p2m, gfn, t, NULL, NULL);
> >      p2m_read_unlock(p2m);
> >
> >      return ret;
> > @@ -679,7 +668,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
> >           * No setting was found in the Radix tree. Check if the
> >           * entry exists in the page-tables.
> >           */
> > -        mfn_t mfn = __p2m_lookup(d, gfn, NULL);
> > +        mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL);
> >
> >          if ( mfn_eq(mfn, INVALID_MFN) )
> >              return -ESRCH;
> > @@ -1595,6 +1584,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned 
> > long flag)
> >      xenmem_access_t xma;
> >      p2m_type_t t;
> >      struct page_info *page = NULL;
> > +    struct p2m_domain *p2m = &current->domain->arch.p2m;
> I think this would be a good time to change this and pass p2m as an
> input to p2m_mem_access_check_and_get_page. This would help with our
> altp2m series as well.
This function can only work with the current p2m because of the call to 
gva_to_ipa. So I don't think it is a good idea to pass the p2m in parameter.
If you want to pass the p2m in parameter, you have to context switch 
properly all the registers (i.e VTTBR_EL2, TTBR{0,1}_EL1 and SCTLR_EL1).
Actually, this function is buggy if memaccess has changed the permission 
on memory holding the stage-1 page-table. Because we are using the 
hardware to translate the VA -> PA, the translate may fail due to memaccess.
Regards,

--
Julien Grall

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