Re: [Xen-devel] [PATCH 4/4] tools/dombuilder: Don't allocate dom->p2m_host[] for translated domains

[Jan Beulich <jbeulich@xxxxxxxx>]

On 17.12.2019 21:15, Andrew Cooper wrote:
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -123,16 +123,12 @@ struct xc_dom_image {
>  
>      /* other state info */
>      uint32_t f_active[XENFEAT_NR_SUBMAPS];
> +
>      /*
> -     * p2m_host maps guest physical addresses an offset from
> -     * rambase_pfn (see below) into gfns.

The removal of this part of the comment and ...

> -     * For a pure PV guest this means that it maps GPFNs into MFNs for
> -     * a hybrid guest this means that it maps GPFNs to GPFNS.
> -     *
> -     * Note that the input is offset by rambase.
> +     * pv_p2m is specific to x86 PV guests, and maps GFNs to MFNs.  It is
> +     * eventually copied into guest context.
>       */
> -    xen_pfn_t *p2m_host;
> +    xen_pfn_t *pv_p2m;
>  
>      /* physical memory
>       *
> @@ -433,9 +429,12 @@ static inline xen_pfn_t xc_dom_p2m(struct xc_dom_image 
> *dom, xen_pfn_t pfn)
>  {
>      if ( xc_dom_translated(dom) )
>          return pfn;
> -    if (pfn < dom->rambase_pfn || pfn >= dom->rambase_pfn + dom->total_pages)
> +
> +    /* x86 PV only now. */
> +    if ( pfn >= dom->total_pages )
>          return INVALID_MFN;
> -    return dom->p2m_host[pfn - dom->rambase_pfn];
> +
> +    return dom->pv_p2m[pfn];
>  }

... the dropping of all uses of rambase_pfn here make it look
like you're doing away with that concept altogether. Is this
really correct? If so, I guess you want to
- say a word in this regard in the description, the more that
  you don't fully get rid of this (the structure field and
  some uses elsewhere remain),
- drop/adjust the respective comment fragment next to the
  field a little further down in the structure.

> @@ -1245,11 +1245,11 @@ static int meminit_pv(struct xc_dom_image *dom)
>          return -EINVAL;
>      }
>  
> -    dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->p2m_size);
> -    if ( dom->p2m_host == NULL )
> +    dom->pv_p2m = xc_dom_malloc(dom, sizeof(xen_pfn_t) * dom->p2m_size);

Since you're touching the line anyway, perhaps better
sizeof(*dom->pv_p2m)?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel