From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v8 4/5] libxc: Provide set_xen_guest_handle_offset macro Date: Fri, 08 May 2015 11:41:33 -0400 Message-ID: <554CD92D.7090501@oracle.com> References: <1430936102-3465-1-git-send-email-boris.ostrovsky@oracle.com> <1430936102-3465-5-git-send-email-boris.ostrovsky@oracle.com> <1431097073.2660.492.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1431097073.2660.492.camel@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: Ian Campbell Cc: elena.ufimtseva@oracle.com, wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, dario.faggioli@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, jbeulich@suse.com, keir@xen.org, dgdegra@tycho.nsa.gov List-Id: xen-devel@lists.xenproject.org On 05/08/2015 10:57 AM, Ian Campbell wrote: > On Wed, 2015-05-06 at 14:15 -0400, Boris Ostrovsky wrote: >> Add set_xen_guest_handle_offset() macro that can be used for setting >> xen_guest_handle to an offset into hypercall buffer. >> >> Signed-off-by: Boris Ostrovsky >> --- >> tools/libxc/include/xenctrl.h | 11 ++++++++--- >> 1 files changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h >> index 8c8eae6..4e6f62c 100644 >> --- a/tools/libxc/include/xenctrl.h >> +++ b/tools/libxc/include/xenctrl.h >> @@ -322,16 +322,21 @@ typedef struct xc_hypercall_buffer xc_hypercall_buffer_t; >> * Set a xen_guest_handle in a type safe manner, ensuring that the >> * data pointer has been correctly allocated. >> */ >> -#undef set_xen_guest_handle >> -#define set_xen_guest_handle(_hnd, _val) \ >> +#undef set_xen_guest_handle_offset > This isn't strictly speaking needed since you aren't trying to override > a version of this interface provided by the hypervisor headers. > >> +#define set_xen_guest_handle_offset(_hnd, _val, _off) \ >> do { \ >> xc_hypercall_buffer_t _hcbuf_hnd1; \ >> typeof(XC__HYPERCALL_BUFFER_NAME(_val)) *_hcbuf_hnd2 = \ >> HYPERCALL_BUFFER(_val); \ >> (void) (&_hcbuf_hnd1 == _hcbuf_hnd2); \ >> - set_xen_guest_handle_raw(_hnd, (_hcbuf_hnd2)->hbuf); \ >> + set_xen_guest_handle_raw(_hnd, \ >> + (_hcbuf_hnd2)->hbuf + (_off)); \ > The hypervisor side guest_handle_add_offset equivalents operate in > multiples of the type. If we can arrange that here too then I think that > would be good. > > I think that means > + (_hcbuf_hnd2)->hbuf + ((sizeof(*_val)*(_off))); \ > > Then the caller becomes e.g. > > set_xen_guest_handle_offset(sysctl.u.pcitopoinfo.devs, devs, processed); > > which is much nicer IMHO. That's what I started with. This, however, breaks at least two things: -- Callers that pass HYPERCALL_BUFFER_NULL to set_xen_guest_handle(). This I think can be fixed by declaring a void *HYPERCALL_BUFFER_NULL (currenty HYPERCALL_BUFFER_NULL is just a syntactic building block) -- DECLARE_NAMED_HYPERCALL_BOUNCE. This makes _val a non-existing symbol (it creates a xc__hypercall_buffer_val). That I am not sure I know how to fix. Add type size filed to struct xc_hypercall_buffer? -boris