From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v8 4/5] libxc: Provide set_xen_guest_handle_offset macro Date: Fri, 8 May 2015 16:56:13 +0100 Message-ID: <1431100573.2660.525.camel@citrix.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> <554CD92D.7090501@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <554CD92D.7090501@oracle.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: Boris Ostrovsky 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 Fri, 2015-05-08 at 11:41 -0400, Boris Ostrovsky wrote: > 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? How about refactoring such that this macro becomes an internal helper used by both set_xen_guest_handle and set_xen_guest_handle_offset and takes the (_hcbuf_hnd2)->hbuf or (_hcbuf_hnd2)->hbuf + ((sizeof(*_val)*(_off)) as appropriate? (iow don't implement set as set_offset(0))