Re: [Xen-devel] [PATCH 1/6] xc: use XENMEM_claim_pages hypercall during guest creation.

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Thu, Mar 28, 2013 at 04:23:11PM +0000, Ian Jackson wrote:
> Konrad Rzeszutek Wilk writes ("[PATCH 1/6] xc: use XENMEM_claim_pages 
> hypercall during guest creation."):
> > We add an extra parameter to the structures passed to the
> > PV routine (arch_setup_meminit) and HVM routine (setup_guest)
> > that determines whether the claim hypercall is to be done.
> 
> This looks plausible to me, except that you seem to have missed a
> comment of Ian Campbell's on the hypercall buffers.
> 
> > +int xc_domain_claim_pages(xc_interface *xch,
> > +                               uint32_t domid,
> > +                               unsigned long nr_pages)
> > +{
> > +    int err;
> > +    struct xen_memory_reservation reservation = {
> > +        .nr_extents   = nr_pages,
> > +        .extent_order = 0,
> > +        .mem_flags    = 0, /* no flags */
> > +        .domid        = domid
> > +    };
> > +
> > +    set_xen_guest_handle(reservation.extent_start, HYPERCALL_BUFFER_NULL);
> 
> In response to which Ian C wrote in
> <1363170195.32410.124.camel@xxxxxxxxxxxxxxxxxxxxxx>:
> 
>    This is unused? I think you just need:
>      set_xen_guest_handle(reservation.extent_start,HYPERCALL_BUFFER_NULL);
>    and drop the declaration of the bounce above.

I think that is what I did?

The original patch (v11 posting) had this:

[Also available here: 
http://xenbits.xen.org/gitweb/?p=people/konradwilk/xen.git;a=blobdiff;f=tools/libxc/xc_domain.c;h=af7ef66c041652309b44f0437ab402a4dfa18ad7;hp=480ce91500dd4e90a420e0407387205f76128752;hb=2430df20d51ad1a53a47831396ba6257f2e732ec;hpb=1a5757996a197abb5660d159fba843eb5e7aa5af
 in the claim.v11 branch]

int xc_domain_claim_pages(xc_interface *xch,
+                               uint32_t domid,
+                               unsigned long nr_pages,
+                               unsigned int claim_flag)
+{
+    int err;
+    xen_pfn_t *extent_start = NULL;
+    DECLARE_HYPERCALL_BOUNCE(extent_start, 0, XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
+    struct xen_memory_reservation reservation = {
+        .nr_extents   = nr_pages,
+        .extent_order = 0,
+        .mem_flags    = claim_flag,
+        .domid        = domid
+    };
+
+    set_xen_guest_handle(reservation.extent_start, extent_start);
+
+    err = do_memory_op(xch, XENMEM_claim_pages, &reservation, 
sizeof(reservation));
+    return err;
+}

And he suggested that I drop the bounce and just use the BUFFER_NULL.
The patch I posted (v12 and this v13) does this:

int xc_domain_claim_pages(xc_interface *xch,
+                               uint32_t domid,
+                               unsigned long nr_pages)
+{
+    int err;
+    struct xen_memory_reservation reservation = {
+        .nr_extents   = nr_pages,
+        .extent_order = 0,
+        .mem_flags    = 0, /* no flags */
+        .domid        = domid
+    };
+
+    set_xen_guest_handle(reservation.extent_start, HYPERCALL_BUFFER_NULL);
+
+    err = do_memory_op(xch, XENMEM_claim_pages, &reservation, 
sizeof(reservation));
+    /* Ignore it if the hypervisor does not support the call. */
+    if (err == -1 && errno == ENOSYS)
+        err = errno = 0;
+    return err;
+}

Which I believe does what he suggested? I also added the check for err and errno
as he suggested in another review.

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