Re: [Xen-devel] [PATCH 15/18 V2]: PVH xen: grant related changes

[Tim Deegan <tim@xxxxxxx>]

At 18:03 -0700 on 15 Mar (1363370595), Mukesh Rathor wrote:
> +    /* PVH: Because we free the existing mfn in XENMEM_add_to_physmap during
> +     * map, we undo that here so the guest P2M (EPT/NPT) is consistent */

Thinking about it, it seems like it would be better for the guest to
handle this problem itself.  Either it should put its grant mappings in
otherwise unpopulated GFNs or it should remove and replace the existing
MFNs itself.  Having an alloc_domheap_page() here is weird because:
(a) although the memory is replaced its contents are not; and 
(b) if a guest does choose to put its grant maps in unpopulated slots,
    it won't want them populated as it unmaps (and will probably 
    hit the page == NULL error path here).

Cheers,

Tim

> +    if ( is_pvh_domain(d) ) {
> +        struct page_info *page = alloc_domheap_page(d, 0);
> +
> +        if ( page == NULL ) {
> +            gdprintk(XENLOG_ERR, "domid:%d Unable to alloc domheap page\n",
> +                     d->domain_id);
> +            goto out_err;
> +        }
> +        if ( guest_physmap_add_page(d, gfn, page_to_mfn(page), 0) != 0 ) {
> +            gdprintk(XENLOG_ERR, "Unable to add mfn to replace grant\n");
> +            goto out_err;
> +        }
> +    }
> +
>      put_gfn(d, gfn);
>      return GNTST_okay;
> +
> +out_err:
> +    put_gfn(d, gfn);
> +    return GNTST_general_error;
>  }
>  
>  int replace_grant_host_mapping(
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 3f97328..84ce267 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -721,7 +721,7 @@ __gnttab_map_grant_ref(
>  
>      double_gt_lock(lgt, rgt);
>  
> -    if ( !is_hvm_domain(ld) && need_iommu(ld) )
> +    if ( !is_hvm_or_pvh_domain(ld) && need_iommu(ld) )
>      {
>          unsigned int wrc, rdc;
>          int err = 0;
> @@ -932,7 +932,7 @@ __gnttab_unmap_common(
>              act->pin -= GNTPIN_hstw_inc;
>      }
>  
> -    if ( !is_hvm_domain(ld) && need_iommu(ld) )
> +    if ( !is_hvm_or_pvh_domain(ld) && need_iommu(ld) )
>      {
>          unsigned int wrc, rdc;
>          int err = 0;
> -- 
> 1.7.2.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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