[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 1/2] arm/mem_access: adjust check_and_get_page to not rely on current





On 12/12/16 07:47, Tamas K Lengyel wrote:


On Dec 12, 2016 00:42, "Jan Beulich" <JBeulich@xxxxxxxx
<mailto:JBeulich@xxxxxxxx>> wrote:

    >>> On 09.12.16 at 20:59, <tamas.lengyel@xxxxxxxxxxxx
    <mailto:tamas.lengyel@xxxxxxxxxxxx>> wrote:
    > --- a/xen/arch/arm/p2m.c
    > +++ b/xen/arch/arm/p2m.c
    > @@ -1461,7 +1461,8 @@ mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn)
    >   * we indeed found a conflicting mem_access setting.
    >   */
    >  static struct page_info*
    > -p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag)
    > +p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned long flag,
    > +                                  const struct vcpu *v)
    >  {
    >      long rc;
    >      paddr_t ipa;
    > @@ -1470,7 +1471,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;
    > +    struct p2m_domain *p2m = &v->domain->arch.p2m;
    >
    >      rc = gva_to_ipa(gva, &ipa, flag);
    >      if ( rc < 0 )
    > @@ -1482,7 +1483,7 @@ p2m_mem_access_check_and_get_page(vaddr_t
    gva, unsigned long flag)
    >       * We do this first as this is faster in the default case when no
    >       * permission is set on the page.
    >       */
    > -    rc = __p2m_get_mem_access(current->domain, gfn, &xma);
    > +    rc = __p2m_get_mem_access(v->domain, gfn, &xma);
    >      if ( rc < 0 )
    >          goto err;
    >
    > @@ -1546,7 +1547,7 @@ p2m_mem_access_check_and_get_page(vaddr_t
    gva, unsigned long flag)
    >
    >      page = mfn_to_page(mfn_x(mfn));
    >
    > -    if ( unlikely(!get_page(page, current->domain)) )
    > +    if ( unlikely(!get_page(page, v->domain)) )
    >          page = NULL;
    >
    >  err:

    Looking at these changes, wouldn't it be more reasonable to hand
    the function a struct domain *?


In its current state it might be but I believe with altp2m we will need
the vcpu struct here anyway.

Not only the altp2m. I keep mentioning a bigger issue, but it seems been ignored every time...

The translation VA to IPA (guest physical address) is done using hardware. If the underlying memory of the stage-1 page table is protected, so the translation will fail. Given that this function is used in hypercall to retrieve the page associated to a buffer, it means that it will not be possible to do hypercall when the page table used to find the buffer IPA has not been touched.

I believe this would require the vCPU in hand to fix this problem.

Regards,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.