From patchwork Wed Mar 23 20:12:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konrad Rzeszutek Wilk X-Patchwork-Id: 8654101 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id C75789F38C for ; Wed, 23 Mar 2016 20:15:50 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C43A620121 for ; Wed, 23 Mar 2016 20:15:49 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AC376200D0 for ; Wed, 23 Mar 2016 20:15:48 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aip8x-0006Js-3x; Wed, 23 Mar 2016 20:12:39 +0000 Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aip8w-0006Jm-CM for xen-devel@lists.xenproject.org; Wed, 23 Mar 2016 20:12:38 +0000 Received: from [193.109.254.147] by server-15.bemta-14.messagelabs.com id DB/F6-02980-5B8F2F65; Wed, 23 Mar 2016 20:12:37 +0000 X-Env-Sender: konrad@char.us.oracle.com X-Msg-Ref: server-12.tower-27.messagelabs.com!1458763955!33388663!1 X-Originating-IP: [156.151.31.81] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTU2LjE1MS4zMS44MSA9PiAyODgzMzk=\n X-StarScan-Received: X-StarScan-Version: 8.11; banners=-,-,- X-VirusChecked: Checked Received: (qmail 24165 invoked from network); 23 Mar 2016 20:12:36 -0000 Received: from userp1040.oracle.com (HELO userp1040.oracle.com) (156.151.31.81) by server-12.tower-27.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 23 Mar 2016 20:12:36 -0000 Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u2NKCRAH023784 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 23 Mar 2016 20:12:27 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by aserv0021.oracle.com (8.13.8/8.13.8) with ESMTP id u2NKCQS0021488 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Wed, 23 Mar 2016 20:12:27 GMT Received: from abhmp0019.oracle.com (abhmp0019.oracle.com [141.146.116.25]) by aserv0122.oracle.com (8.13.8/8.13.8) with ESMTP id u2NKCQcn008375; Wed, 23 Mar 2016 20:12:26 GMT Received: from char.us.oracle.com (/10.137.176.158) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Wed, 23 Mar 2016 13:12:21 -0700 Received: by char.us.oracle.com (Postfix, from userid 1000) id B51F76A00A9; Wed, 23 Mar 2016 16:12:19 -0400 (EDT) Date: Wed, 23 Mar 2016 16:12:19 -0400 From: Konrad Rzeszutek Wilk To: Jan Beulich Message-ID: <20160323201219.GA31623@char.us.oracle.com> References: <1458064616-23101-1-git-send-email-konrad.wilk@oracle.com> <1458064616-23101-12-git-send-email-konrad.wilk@oracle.com> <56F289A002000078000DF9BD@prv-mh.provo.novell.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <56F289A002000078000DF9BD@prv-mh.provo.novell.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Cc: Keir Fraser , ross.lagerwall@citrix.com, andrew.cooper3@citrix.com, Ian Jackson , Tim Deegan , mpohlack@amazon.de, sasha.levin@oracle.com, xen-devel@lists.xenproject.org Subject: Re: [Xen-devel] [PATCH v4 11/34] xsplice: Design document X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Mar 23, 2016 at 05:18:39AM -0600, Jan Beulich wrote: > >>> On 15.03.16 at 18:56, wrote: > > +### XEN_SYSCTL_XSPLICE_LIST (2) > > + > > +Retrieve an array of abbreviated status and names of payloads that are > > loaded in the > > +hypervisor. > > + > > +The caller provides: > > + > > + * `version`. Initially (on first hypercall) *MUST* be zero. > > + * `idx` index iterator. On first call *MUST* be zero, subsequent calls varies. > > + * `nr` the max number of entries to populate. > > + * `pad` - *MUST* be zero. > > + * `status` virtual address of where to write `struct xen_xsplice_status` > > + structures. Caller *MUST* allocate up to `nr` of them. > > + * `name` - virtual address of where to write the unique name of the payload. > > + Caller *MUST* allocate up to `nr` of them. Each *MUST* be of > > + **XEN_XSPLICE_NAME_SIZE** size. > > + * `len` - virtual address of where to write the length of each unique name > > + of the payload. Caller *MUST* allocate up to `nr` of them. Each *MUST* be > > + of sizeof(uint32_t) (4 bytes). > > + > > +If the hypercall returns an positive number, it is the number (upto `nr` > > +provided to the hypercall) of the payloads returned, along with `nr` updated > > +with the number of remaining payloads, `version` updated (it may be the same > > +across hypercalls - if it varies the data is stale and further calls could > > +fail). The `status`, `name`, and `len`' are updated at their designed index > > +value (`idx`) with the returned value of data. > > + > > +If the hypercall returns -XEN_E2BIG the `nr` is too big and should be > > +lowered. > > + > > +If the hypercall returns an zero value there are no more payloads. > > + > > +Note that due to the asynchronous nature of hypercalls the control domain might > > +have added or removed a number of payloads making this information stale. It is > > +the responsibility of the toolstack to use the `version` field to check > > +between each invocation. if the version differs it should discard the stale > > +data and start from scratch. It is OK for the toolstack to use the new > > +`version` field. > > + > > +The `struct xen_xsplice_status` structure contains an status of payload which includes: > > + > > + * `status` - indicates the current status of the payload: > > + * *XSPLICE_STATUS_CHECKED* (1) loaded and the ELF payload safety checks passed. > > + * *XSPLICE_STATUS_APPLIED* (2) loaded, checked, and applied. > > + * No other value is possible. > > + * `rc` - -XEN_EXX type errors encountered while performing the last > > + XSPLICE_ACTION_* operation. The normal values can be zero or -XEN_EAGAIN which > > + respectively mean: success or operation in progress. Other values > > + imply an error occurred. If there is an error in `rc`, `status` will **NOT** > > + have changed. > > + > > +The structure is as follow: > > + > > +
> > +struct xen_sysctl_xsplice_list {  
> > +    uint32_t version;                       /* IN/OUT: Initially *MUST* be zero.  
> > +                                               On subsequent calls reuse value.  
> > +                                               If varies between calls, we are  
> > +                                             * getting stale data. */  
> > +    uint32_t idx;                           /* IN/OUT: Index into array. */ 
> > +    uint32_t nr;                            /* IN: How many status, names, and len  
> > +                                               should fill out.  
> > +                                               OUT: How many payloads left. */  
> 
> I think there's an ambiguity left in both the description above and
> the comments here: With idx required to be zero upon first
> invocation (which I'm not clear why that is), which parts of the

That is actually a stale design choice. Initially the "How many payloads left"
was going to be stamped in 'idx'. But it is now in 'nr'.

The value can be arbitrary, albeit on first invocation it should be 0 otherwise
you won't get 'nr' telling you how many payloads there left. Unless your
'idx' falls below the amount of payloads.

As in, say we have 20 payloads.
If the first hypercall for 'idx' has 30, then the hypercall will return -EINVAL.
If the first hypercall 'idx' has 19, then the hypercall will populate
->name,->len,->status, ->version and write ->nr with 1.

> three arrays get filled when idx is non-zero: [0, idx) or [nr, nr + idx)?

I am going to assume the you are filling the two /*IN*/ entries, so ->idx
and ->nr.

[0, idx]:

If there is data and the amount of payloads is greater than idx (0), and there
are no hypercall preemptions, then:

->nr = remaining amount
->version = version value
->name[0..idx]
->len[0..idx]
->status[0..idx]


[nr, nr + idx]:

If there is data and the amount of payloads is less than nr, then -EINVAL
is returned.

If there is data and the amount of payloads is greater than 'nr', and there
are no hypercall preemptions, then:

->nr = remaining amount
->version = version value
->name[nr..nr + idx]
->len[nr..nr+idx]
->status[nr..nr+idx]

Let me update the design doc to remove the /*IN*/ part about the 'idx'.


> 
> Jan
>

diff --git a/docs/misc/xsplice.markdown b/docs/misc/xsplice.markdown
index 6aa5a27..8252e6c 100644
--- a/docs/misc/xsplice.markdown
+++ b/docs/misc/xsplice.markdown
@@ -487,7 +487,9 @@ hypervisor.
 The caller provides:
 
  * `version`. Initially (on first hypercall) *MUST* be zero.
- * `idx` index iterator. On first call *MUST* be zero, subsequent calls varies.
+ * `idx` index iterator. The index into the hypervisor's payload count. It is
+    recommended that on first invocation zero be used so that `nr` (which the
+    hypervisor will update with the remaining payload count) be provided.
  * `nr` the max number of entries to populate.
  * `pad` - *MUST* be zero.
  * `status` virtual address of where to write `struct xen_xsplice_status`
@@ -538,9 +540,9 @@ struct xen_sysctl_xsplice_list {
                                                On subsequent calls reuse value.  
                                                If varies between calls, we are  
                                              * getting stale data. */  
-    uint32_t idx;                           /* IN/OUT: Index into array. */  
+    uint32_t idx;                           /* IN: Index into array. */  
     uint32_t nr;                            /* IN: How many status, names, and len  
-                                               should fill out.  
+                                               should be filled out.  
                                                OUT: How many payloads left. */  
     uint32_t pad;                           /* IN: Must be zero. */  
     XEN_GUEST_HANDLE_64(xen_xsplice_status_t) status;  /* OUT. Must have enough