Re: [Xen-devel] [PATCH v8 4/5] libxc: Provide set_xen_guest_handle_offset macro

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

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 <boris.ostrovsky@xxxxxxxxxx>
> > ---
> >   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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel