Re: [Xen-devel] [PATCH 06/18] lib{xc, xl}: Seed grant tables with xenstore and console grants

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Thu, 2012-01-12 at 23:35 +0000, Daniel De Graaf wrote:
> @@ -275,6 +276,169 @@ int xc_dom_boot_image(struct xc_dom_image *dom)
>      return rc;
>  }
> 
> +static unsigned long xc_dom_gnttab_setup(xc_interface *xch, uint32_t domid)
> +{
> +    DECLARE_HYPERCALL;
> +    gnttab_setup_table_t setup_table;
> +    DECLARE_HYPERCALL_BUFFER(unsigned long, gmfnp);
> +    int rc;
> +       unsigned long gmfn;

There's a bunch of weird alignment issues in this patch. Presumably some
hard tabs have snuck in.

> +
> +    gmfnp = xc_hypercall_buffer_alloc(xch, gmfnp, sizeof(*gmfnp));
> +       if (gmfnp == NULL)
> +               return -1;

Alignment.

> +
> +    setup_table.dom = domid;
> +    setup_table.nr_frames = 1;
> +    set_xen_guest_handle(setup_table.frame_list, gmfnp);
> +    setup_table.status = 0;
> +
> +    hypercall.op = __HYPERVISOR_grant_table_op;
> +    hypercall.arg[0] = GNTTABOP_setup_table;
> +    hypercall.arg[1] = (unsigned long) &setup_table;
> +    hypercall.arg[2] = 1;
> +
> +    rc = do_xen_hypercall(xch, &hypercall);
> +       gmfn = *gmfnp;

Alignment. I'm going to stop pointing these out, I guess grep will find
them...

[...]
> +int xc_dom_gnttab_init(struct xc_dom_image *dom)
> +{
> +    unsigned long console_gmfn;
> +    unsigned long xenstore_gmfn;
> +    int autotranslated;
> +
> +    autotranslated = xc_dom_feature_translated(dom);
> +    console_gmfn = autotranslated ?
> +           dom->console_pfn : xc_dom_p2m_host(dom, dom->console_pfn);
> +    xenstore_gmfn = autotranslated ?
> +           dom->xenstore_pfn : xc_dom_p2m_host(dom, dom->xenstore_pfn);
> +
> +    return xc_dom_gnttab_seed(dom->xch, dom->guest_domid,
> +                              console_gmfn, xenstore_gmfn,
> +                              dom->console_domid, dom->xenstore_domid);

So on build we have can do the p2m lookup and hence use the PV version
of gnttab_seed, whereas on restore we don't have a p2m and hence can't?

The internals of xc_domain_restore do have the p2m though, so in
principal we could avoid the need for the hvm version and the
reintroduction of the remove_from_physmap by making xc_domain_restore
output the necessary mfns? I'm not sure it is worth it though.


> diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
> index 3fda6f8..8bee684 100644
> --- a/tools/libxc/xc_domain_restore.c
> +++ b/tools/libxc/xc_domain_restore.c
> @@ -1259,7 +1259,8 @@ static int apply_batch(xc_interface *xch, uint32_t dom, 
> struct restore_ctx *ctx,
> 
>  int xc_domain_restore(xc_interface *xch, int io_fd, uint32_t dom,
>                        unsigned int store_evtchn, unsigned long *store_mfn,
> -                      unsigned int console_evtchn, unsigned long 
> *console_mfn,
> +                      uint32_t store_domid, unsigned int console_evtchn,
> +                      unsigned long *console_mfn, uint32_t console_domid,
>                        unsigned int hvm, unsigned int pae, int superpages,
>                        int no_incr_generationid,
>                        unsigned long *vm_generationid_addr)
> @@ -2018,6 +2019,14 @@ int xc_domain_restore(xc_interface *xch, int io_fd, 
> uint32_t dom,
>          memcpy(ctx->live_p2m, ctx->p2m, dinfo->p2m_size * sizeof(xen_pfn_t));
>      munmap(ctx->live_p2m, P2M_FL_ENTRIES * PAGE_SIZE);
> 
> +    rc = xc_dom_gnttab_seed(xch, dom, *console_mfn, *store_mfn,
> +                            console_domid, store_domid);
> +    if (rc != 0)
> +    {
> +        ERROR("error seeding grant table");
> +        goto out;
> +    }
> +
>      DPRINTF("Domain ready to be built.\n");
>      rc = 0;
>      goto out;
> @@ -2076,6 +2085,14 @@ int xc_domain_restore(xc_interface *xch, int io_fd, 
> uint32_t dom,
>          goto out;
>      }
> 
> +    rc = xc_dom_gnttab_hvm_seed(xch, dom, *console_mfn, *store_mfn,
> +                                console_domid, store_domid);

Oh, we don't even need to return them from xc_domain_restore since we
could just translate from gfn to mfn right here (since we have a p2m in
hand) and call xc_dom_gnttab_seed. Or am I missing something?

> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index 5e39595..04db22f 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
[...]
> @@ -305,6 +312,8 @@ static int hvm_build_set_params(xc_interface *handle, 
> uint32_t domid,
>      xc_set_hvm_param(handle, domid, HVM_PARAM_NESTEDHVM, 
> info->u.hvm.nested_hvm);
>      xc_set_hvm_param(handle, domid, HVM_PARAM_STORE_EVTCHN, store_evtchn);
>      xc_set_hvm_param(handle, domid, HVM_PARAM_CONSOLE_EVTCHN, 
> console_evtchn);
> +
> +    xc_dom_gnttab_hvm_seed(handle, domid, *console_mfn, *store_mfn, 
> console_domid, store_domid);

Didn't this already happen in xc_linux_build_internal which was called
via xc_linux_build_mem?

[...]
Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel