Re: [Xen-devel] [PATCH] x86/mm: drop redundant domain parameter from get_page_from_gfn_p2m()

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 21/06/17 11:12, Jan Beulich wrote:
> It can always be read from the passed p2m. Take the opportunity and
> also rename the function, making the "p2m" suffix a prefix, to match
> other p2m functions, and convert the "gfn" parameter's type.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Looks good, thanks:

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>



> 
> --- a/xen/arch/x86/mm/hap/guest_walk.c
> +++ b/xen/arch/x86/mm/hap/guest_walk.c
> @@ -55,13 +55,13 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
>      void *top_map;
>      p2m_type_t p2mt;
>      walk_t gw;
> -    unsigned long top_gfn;
> +    gfn_t top_gfn;
>      struct page_info *top_page;
>  
>      /* Get the top-level table's MFN */
> -    top_gfn = cr3 >> PAGE_SHIFT;
> -    top_page = get_page_from_gfn_p2m(p2m->domain, p2m, top_gfn,
> -                                     &p2mt, NULL, P2M_ALLOC | P2M_UNSHARE);
> +    top_gfn = _gfn(cr3 >> PAGE_SHIFT);
> +    top_page = p2m_get_page_from_gfn(p2m, top_gfn, &p2mt, NULL,
> +                                     P2M_ALLOC | P2M_UNSHARE);
>      if ( p2m_is_paging(p2mt) )
>      {
>          ASSERT(p2m_is_hostp2m(p2m));
> @@ -100,8 +100,9 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
>      {
>          gfn_t gfn = guest_walk_to_gfn(&gw);
>          struct page_info *page;
> -        page = get_page_from_gfn_p2m(p2m->domain, p2m, gfn_x(gfn), &p2mt,
> -                                     NULL, P2M_ALLOC | P2M_UNSHARE);
> +
> +        page = p2m_get_page_from_gfn(p2m, gfn, &p2mt, NULL,
> +                                     P2M_ALLOC | P2M_UNSHARE);
>          if ( page )
>              put_page(page);
>          if ( p2m_is_paging(p2mt) )
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -472,8 +472,8 @@ void __put_gfn(struct p2m_domain *p2m, u
>  }
>  
>  /* Atomically look up a GFN and take a reference count on the backing page. 
> */
> -struct page_info *get_page_from_gfn_p2m(
> -    struct domain *d, struct p2m_domain *p2m, unsigned long gfn,
> +struct page_info *p2m_get_page_from_gfn(
> +    struct p2m_domain *p2m, gfn_t gfn,
>      p2m_type_t *t, p2m_access_t *a, p2m_query_t q)
>  {
>      struct page_info *page = NULL;
> @@ -489,7 +489,7 @@ struct page_info *get_page_from_gfn_p2m(
>      {
>          /* Fast path: look up and get out */
>          p2m_read_lock(p2m);
> -        mfn = __get_gfn_type_access(p2m, gfn, t, a, 0, NULL, 0);
> +        mfn = __get_gfn_type_access(p2m, gfn_x(gfn), t, a, 0, NULL, 0);
>          if ( p2m_is_any_ram(*t) && mfn_valid(mfn)
>               && !((q & P2M_UNSHARE) && p2m_is_shared(*t)) )
>          {
> @@ -497,11 +497,12 @@ struct page_info *get_page_from_gfn_p2m(
>              if ( unlikely(p2m_is_foreign(*t)) )
>              {
>                  struct domain *fdom = page_get_owner_and_reference(page);
> -                ASSERT(fdom != d);
> +
> +                ASSERT(fdom != p2m->domain);
>                  if ( fdom == NULL )
>                      page = NULL;
>              }
> -            else if ( !get_page(page, d) &&
> +            else if ( !get_page(page, p2m->domain) &&
>                        /* Page could be shared */
>                        (!p2m_is_shared(*t) || !get_page(page, dom_cow)) )
>                  page = NULL;
> @@ -517,14 +518,14 @@ struct page_info *get_page_from_gfn_p2m(
>      }
>  
>      /* Slow path: take the write lock and do fixups */
> -    mfn = get_gfn_type_access(p2m, gfn, t, a, q, NULL);
> +    mfn = get_gfn_type_access(p2m, gfn_x(gfn), t, a, q, NULL);
>      if ( p2m_is_ram(*t) && mfn_valid(mfn) )
>      {
>          page = mfn_to_page(mfn);
> -        if ( !get_page(page, d) )
> +        if ( !get_page(page, p2m->domain) )
>              page = NULL;
>      }
> -    put_gfn(d, gfn);
> +    put_gfn(p2m->domain, gfn_x(gfn));
>  
>      return page;
>  }
> @@ -1900,7 +1901,7 @@ void *map_domain_gfn(struct p2m_domain *
>      }
>  
>      /* Translate the gfn, unsharing if shared. */
> -    page = get_page_from_gfn_p2m(p2m->domain, p2m, gfn_x(gfn), p2mt, NULL, 
> q);
> +    page = p2m_get_page_from_gfn(p2m, gfn, p2mt, NULL, q);
>      if ( p2m_is_paging(*p2mt) )
>      {
>          ASSERT(p2m_is_hostp2m(p2m));
> --- a/xen/include/asm-x86/p2m.h
> +++ b/xen/include/asm-x86/p2m.h
> @@ -465,9 +465,7 @@ static inline mfn_t get_gfn_query_unlock
>   * and should be used by any path that intends to write to the backing page.
>   * Returns NULL if the page is not backed by RAM.
>   * The caller is responsible for calling put_page() afterwards. */
> -struct page_info *get_page_from_gfn_p2m(struct domain *d,
> -                                        struct p2m_domain *p2m,
> -                                        unsigned long gfn,
> +struct page_info *p2m_get_page_from_gfn(struct p2m_domain *p2m, gfn_t gfn,
>                                          p2m_type_t *t, p2m_access_t *a,
>                                          p2m_query_t q);
>  
> @@ -477,7 +475,7 @@ static inline struct page_info *get_page
>      struct page_info *page;
>  
>      if ( paging_mode_translate(d) )
> -        return get_page_from_gfn_p2m(d, p2m_get_hostp2m(d), gfn, t, NULL, q);
> +        return p2m_get_page_from_gfn(p2m_get_hostp2m(d), _gfn(gfn), t, NULL, 
> q);
>  
>      /* Non-translated guests see 1-1 RAM / MMIO mappings everywhere */
>      if ( t )
> 
> 


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