Re: [Xen-devel] [PATCH v2 REPOST 05/12] tools/libxenctrl: use new xenforeignmemory API to seed grant table

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Aug 22, 2017 at 03:50:59PM +0100, Paul Durrant wrote:
> A previous patch added support for priv-mapping guest resources directly
> (rather than having to foreign-map, which requires P2M modification for
> HVM guests).
> 
> This patch makes use of the new API to seed the guest grant table unless
> the underlying infrastructure (i.e. privcmd) doesn't support it, in which
> case the old scheme is used.
> 
> NOTE: The call to xc_dom_gnttab_hvm_seed() in hvm_build_set_params() was
>       actually unnecessary, as the grant table has already been seeded
>       by a prior call to xc_dom_gnttab_init() made by libxl__build_dom().
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Acked-by: Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  tools/libxc/include/xc_dom.h        |   8 +--
>  tools/libxc/xc_dom_boot.c           | 102 
> ++++++++++++++++++++++++++++--------
>  tools/libxc/xc_sr_restore_x86_hvm.c |  10 ++--
>  tools/libxc/xc_sr_restore_x86_pv.c  |   2 +-
>  tools/libxl/libxl_dom.c             |   1 -
>  tools/python/xen/lowlevel/xc/xc.c   |   6 +--
>  6 files changed, 92 insertions(+), 37 deletions(-)
> 
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index ce47058c41..d6ca0a8680 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -323,12 +323,8 @@ void *xc_dom_boot_domU_map(struct xc_dom_image *dom, 
> xen_pfn_t pfn,
>  int xc_dom_boot_image(struct xc_dom_image *dom);
>  int xc_dom_compat_check(struct xc_dom_image *dom);
>  int xc_dom_gnttab_init(struct xc_dom_image *dom);
> -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> -                           xen_pfn_t console_gmfn,
> -                           xen_pfn_t xenstore_gmfn,
> -                           domid_t console_domid,
> -                           domid_t xenstore_domid);
> -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> +                       bool is_hvm,
>                         xen_pfn_t console_gmfn,
>                         xen_pfn_t xenstore_gmfn,
>                         domid_t console_domid,
> diff --git a/tools/libxc/xc_dom_boot.c b/tools/libxc/xc_dom_boot.c
> index c3b44dd399..fc3174ad7e 100644
> --- a/tools/libxc/xc_dom_boot.c
> +++ b/tools/libxc/xc_dom_boot.c
> @@ -280,11 +280,11 @@ static xen_pfn_t xc_dom_gnttab_setup(xc_interface *xch, 
> domid_t domid)
>      return gmfn;
>  }
>  
> -int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
> -                       xen_pfn_t console_gmfn,
> -                       xen_pfn_t xenstore_gmfn,
> -                       domid_t console_domid,
> -                       domid_t xenstore_domid)
> +static int compat_gnttab_seed(xc_interface *xch, domid_t domid,
> +                              xen_pfn_t console_gmfn,
> +                              xen_pfn_t xenstore_gmfn,
> +                              domid_t console_domid,
> +                              domid_t xenstore_domid)
>  {
>  
>      xen_pfn_t gnttab_gmfn;
> @@ -337,11 +337,11 @@ int xc_dom_gnttab_seed(xc_interface *xch, domid_t domid,
>      return 0;
>  }
>  
> -int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> -                           xen_pfn_t console_gpfn,
> -                           xen_pfn_t xenstore_gpfn,
> -                           domid_t console_domid,
> -                           domid_t xenstore_domid)
> +static int compat_gnttab_hvm_seed(xc_interface *xch, domid_t domid,
> +                                  xen_pfn_t console_gpfn,
> +                                  xen_pfn_t xenstore_gpfn,
> +                                  domid_t console_domid,
> +                                  domid_t xenstore_domid)
>  {
>      int rc;
>      xen_pfn_t scratch_gpfn;
> @@ -380,7 +380,7 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t 
> domid,
>          return -1;
>      }
>  
> -    rc = xc_dom_gnttab_seed(xch, domid,
> +    rc = compat_gnttab_seed(xch, domid,
>                              console_gpfn, xenstore_gpfn,
>                              console_domid, xenstore_domid);
>      if (rc != 0)
> @@ -405,18 +405,78 @@ int xc_dom_gnttab_hvm_seed(xc_interface *xch, domid_t 
> domid,
>      return 0;
>  }
>  
> -int xc_dom_gnttab_init(struct xc_dom_image *dom)
> +int xc_dom_gnttab_seed(xc_interface *xch, domid_t guest_domid,
> +                       bool is_hvm, xen_pfn_t console_gmfn,
> +                       xen_pfn_t xenstore_gmfn, domid_t console_domid,
> +                       domid_t xenstore_domid)
>  {
> -    if ( xc_dom_translated(dom) ) {
> -        return xc_dom_gnttab_hvm_seed(dom->xch, dom->guest_domid,
> -                                      dom->console_pfn, dom->xenstore_pfn,
> -                                      dom->console_domid, 
> dom->xenstore_domid);
> -    } else {
> -        return xc_dom_gnttab_seed(dom->xch, dom->guest_domid,
> -                                  xc_dom_p2m(dom, dom->console_pfn),
> -                                  xc_dom_p2m(dom, dom->xenstore_pfn),
> -                                  dom->console_domid, dom->xenstore_domid);
> +    xenforeignmemory_handle* fmem = xch->fmem;
> +    xenforeignmemory_resource_handle *fres;
> +    void *addr = NULL;
> +    grant_entry_v1_t *gnttab;
> +
> +    fres = xenforeignmemory_map_resource(fmem, guest_domid,
> +                                         XENMEM_resource_grant_table,
> +                                         0, 0, 1,
> +                                         &addr, PROT_READ | PROT_WRITE, 0);
> +    if ( !fres )
> +    {
> +        if ( errno == EOPNOTSUPP )
> +            return is_hvm ?
> +                compat_gnttab_hvm_seed(xch, guest_domid,
> +                                       console_gmfn, xenstore_gmfn,
> +                                       console_domid, xenstore_domid) :
> +                compat_gnttab_seed(xch, guest_domid,
> +                                   console_gmfn, xenstore_gmfn,
> +                                   console_domid, xenstore_domid);

Could be written as:

return (is_hvm ? compat_gnttab_hvm_seed : compat_gnttab_seed)
       (xch, guest_domid, console_gmfn, xenstore_gmfn, console_domid,
        xenstore_domid);

> +
> +        xc_dom_panic(xch, XC_INTERNAL_ERROR,
> +                     "%s: failed to acquire grant table "
> +                     "[errno=%d]\n",
> +                     __FUNCTION__, errno);
> +        return -1;
>      }
> +
> +    gnttab = addr;
> +
> +    if ( guest_domid != console_domid  && console_gmfn != -1)
                                        ^ extra space.
> +    {
> +        xc_dom_printf(xch, "%s: setting console pfn=0x%"PRI_xen_pfn,
> +                      __FUNCTION__, console_gmfn);
> +
> +        gnttab[GNTTAB_RESERVED_CONSOLE].flags = GTF_permit_access;
> +        gnttab[GNTTAB_RESERVED_CONSOLE].domid = console_domid;
> +        gnttab[GNTTAB_RESERVED_CONSOLE].frame = console_gmfn;
> +    }
> +
> +    if ( guest_domid != xenstore_domid && xenstore_gmfn != -1)
> +    {
> +        xc_dom_printf(xch, "%s: setting xenstore pfn=0x%"PRI_xen_pfn,
> +                      __FUNCTION__, xenstore_gmfn);
> +
> +        gnttab[GNTTAB_RESERVED_XENSTORE].flags = GTF_permit_access;
> +        gnttab[GNTTAB_RESERVED_XENSTORE].domid = xenstore_domid;
> +        gnttab[GNTTAB_RESERVED_XENSTORE].frame = xenstore_gmfn;
> +    }

The code above is already present in xc_dom_gnttab_seed (now renamed
to compat_gnttab_seed, isn't there anyway that you could re-use it?

Roger.

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