From mboxrd@z Thu Jan  1 00:00:00 1970
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH 1/6] xc: use XENMEM_claim_pages hypercall
 during guest creation.
Date: Fri, 29 Mar 2013 09:12:21 -0400
Message-ID: <20130329131221.GD31356@phenom.dumpdata.com>
References: <1364417739-10121-1-git-send-email-konrad.wilk@oracle.com>
	<1364417739-10121-2-git-send-email-konrad.wilk@oracle.com>
	<20820.28271.651677.268634@mariner.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <20820.28271.651677.268634@mariner.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Dan Magenheimer <dan.magenheimer@oracle.com>, "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, Ian Campbell <Ian.Campbell@citrix.com>
List-Id: xen-devel@lists.xenproject.org

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@zakaz.uk.xensource.com>:
> 
>    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.