From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 2/3] tools/libxc: Make the mem_access APIs generic Date: Fri, 11 Apr 2014 10:45:45 +0100 Message-ID: <5347B9C9.9050609@citrix.com> References: <1397188179-14606-1-git-send-email-aravindp@cisco.com> <1397188179-14606-3-git-send-email-aravindp@cisco.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WYY1u-0001dB-Id for xen-devel@lists.xenproject.org; Fri, 11 Apr 2014 09:45:50 +0000 In-Reply-To: <1397188179-14606-3-git-send-email-aravindp@cisco.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: Aravindh Puthiyaparambil Cc: xen-devel@lists.xenproject.org, Ian Jackson , Ian Campbell , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 11/04/14 04:49, Aravindh Puthiyaparambil wrote: > This patch does the following: > 1. Add new xc_[sg]et_mem_access APIs. > 2. Remove xc_hvm_[sg]et_mem_access() APIs. > > Signed-off-by: Aravindh Puthiyaparambil > Cc: Ian Jackson > Cc: Stefano Stabellini > Cc: Ian Campbell > > --- > Changes from the RFC version of the patch: > 1. Remove xc_mem_access_memop() wrapper. > 2. Remove xc_hvm_[sg]et_mem_access() APIs. > > Thanks, > Aravindh > > diff --git a/tools/libxc/xc_mem_access.c b/tools/libxc/xc_mem_access.c > index a50c145..0277ab3 100644 > --- a/tools/libxc/xc_mem_access.c > +++ b/tools/libxc/xc_mem_access.c > @@ -22,7 +22,7 @@ > */ > > #include "xc_private.h" > - > +#include > > int xc_mem_access_enable(xc_interface *xch, domid_t domain_id, > uint32_t *port) > @@ -47,12 +47,53 @@ int xc_mem_access_disable(xc_interface *xch, domid_t domain_id) > NULL); > } > > + Spurious whitespace change. > int xc_mem_access_resume(xc_interface *xch, domid_t domain_id, unsigned long gfn) What is this gfn parameter doing? It isn't used in the hypercall. > { > - return xc_mem_event_memop(xch, domain_id, > - XENMEM_access_op_resume, > - XENMEM_access_op, > - gfn, NULL); > + xen_mem_access_op_t mao; > + > + memset(&mao, 0, sizeof(mao)); > + mao.op = XENMEM_access_op_resume; > + mao.domid = domain_id; Please use structure initialisation for this. xen_mem_access_op_t mao = { .op = XENMEM_access_op_resume, .domid = domain_id }; > + > + return do_memory_op(xch, XENMEM_access_op, &mao, sizeof(mao)); > +} > + > +int xc_set_mem_access(xc_interface *xch, > + domid_t domain_id, > + xenmem_access_t access, > + uint64_t first_pfn, > + uint32_t nr) > +{ > + xen_mem_access_op_t mao; > + > + mao.op = XENMEM_access_op_set_access; > + mao.domid = domain_id; > + mao.access = access; > + mao.pfn = first_pfn; > + mao.nr = nr; > + > + return do_memory_op(xch, XENMEM_access_op, &mao, sizeof(mao)); > +} > + > +int xc_get_mem_access(xc_interface *xch, > + domid_t domain_id, > + uint64_t pfn, > + xenmem_access_t *access) > +{ > + xen_mem_access_op_t mao; > + int rc; > + > + mao.op = XENMEM_access_op_get_access; > + mao.domid = domain_id; > + mao.pfn = pfn; > + > + rc = do_memory_op(xch, XENMEM_access_op, &mao, sizeof(mao)); > + > + if ( !rc ) > + *access = mao.access; Surely you should only write access back in the case of success? ~Andrew