From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH V5 12/12] xen/vm_event: Add RESUME option to vm_event_op domctl Date: Sat, 14 Feb 2015 00:10:29 +0100 Message-ID: References: <1423845203-18941-1-git-send-email-tamas.lengyel@zentific.com> <1423845203-18941-13-git-send-email-tamas.lengyel@zentific.com> <54DE7051.9030002@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54DE7051.9030002@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: "Tian, Kevin" , "wei.liu2@citrix.com" , Ian Campbell , Stefano Stabellini , Tim Deegan , Steven Maresca , "xen-devel@lists.xen.org" , Jan Beulich , "Dong, Eddie" , Andres Lagar-Cavilla , Jun Nakajima , "rshriram@cs.ubc.ca" , Keir Fraser , Daniel De Graaf , "yanghy@cn.fujitsu.com" , Ian Jackson List-Id: xen-devel@lists.xenproject.org On Fri, Feb 13, 2015 at 10:44 PM, Andrew Cooper wrote: > On 13/02/15 16:33, Tamas K Lengyel wrote: >> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h >> index 9c41f5d..334f60e 100644 >> --- a/xen/include/public/memory.h >> +++ b/xen/include/public/memory.h >> @@ -386,9 +386,8 @@ typedef struct xen_mem_paging_op xen_mem_paging_op_t; >> DEFINE_XEN_GUEST_HANDLE(xen_mem_paging_op_t); >> >> #define XENMEM_access_op 21 >> -#define XENMEM_access_op_resume 0 >> -#define XENMEM_access_op_set_access 1 >> -#define XENMEM_access_op_get_access 2 >> +#define XENMEM_access_op_set_access 0 >> +#define XENMEM_access_op_get_access 1 >> >> typedef enum { >> XENMEM_access_n, >> @@ -439,12 +438,11 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_access_op_t); >> #define XENMEM_sharing_op_nominate_gfn 0 >> #define XENMEM_sharing_op_nominate_gref 1 >> #define XENMEM_sharing_op_share 2 >> -#define XENMEM_sharing_op_resume 3 >> -#define XENMEM_sharing_op_debug_gfn 4 >> -#define XENMEM_sharing_op_debug_mfn 5 >> -#define XENMEM_sharing_op_debug_gref 6 >> -#define XENMEM_sharing_op_add_physmap 7 >> -#define XENMEM_sharing_op_audit 8 >> +#define XENMEM_sharing_op_debug_gfn 3 >> +#define XENMEM_sharing_op_debug_mfn 4 >> +#define XENMEM_sharing_op_debug_gref 5 >> +#define XENMEM_sharing_op_add_physmap 6 >> +#define XENMEM_sharing_op_audit 7 > > Hmm - I am not certain what our API/ABI constraints are in this case. > MEMOPs are not toolstack exclusive so lack a formal interface version, > but these bits are inside an #ifdef __XEN_TOOLS__ > > If we work on the assumption that there are not currently any > out-of-tree tools trying to use this interface, then its probably ok to > break the ABI now and get it right, perhaps even including an ABI > version parameter in the op itself if we wish to be flexible going forwards. I think there are some out-of-tree tools that make use if these. Andres did imply couple years ago that he works with proprietary software that uses the sharing system but he couldn't really tell much about it. > > I also notice that the structures have 32bit gfns baked into them, which > is going to need to be fixed at some point. I think that was just me in the first patch in the series.. > > ~Andrew > On a more general note, I'm not actually sure if we need this resume option on the domctl. Currently there are two methods ending up signaling to Xen to process the responses from the ring - one with the memops/domctl here, and one via event channels. I think the event channel method is sufficient so I'm not actually sure why these resume memops existed in the first place. It might be sufficient to just deprecate them altogether. Thanks, Tamas