Re: [Xen-devel] [PATCH v6 09/15] argo: implement the sendv op; evtchn: expose send_guest_global_virq

[Christopher Clark <christopher.w.clark@xxxxxxxxx>]

On Mon, Jan 28, 2019 at 3:29 AM Jan Beulich <JBeulich@xxxxxxxx> wrote:
>
> >>> On 24.01.19 at 03:04, <christopher.w.clark@xxxxxxxxx> wrote:
> > --- a/xen/include/public/argo.h
> > +++ b/xen/include/public/argo.h
> > @@ -46,6 +46,34 @@ typedef uint32_t xen_argo_port_t;
> >  /* gfn type: 64-bit on all architectures to aid avoiding a compat ABI */
> >  typedef uint64_t xen_argo_gfn_t;
> >
> > +/*
> > + * XEN_ARGO_MAXIOV : maximum number of iovs accepted in a single sendv.
> > + * Caution is required if this value is increased: this determines the 
> > size of
> > + * an array of xen_argo_iov_t structs on the hypervisor stack, so could 
> > cause
> > + * stack overflow if the value is too large.
> > + * The Linux Argo driver never passes more than two iovs.
> > + *
> > + * This value should also not exceed 128 to ensure that the total amount 
> > of data
> > + * posted in a single Argo sendv operation cannot exceed 2^31 bytes, to 
> > reduce
> > + * risk of integer overflow defects:
> > + * Each argo iov can hold ~ 2^24 bytes, so XEN_ARGO_MAXIOV <= 2^(31-24),
> > + * ie. keep XEN_ARGO_MAXIOV <= 128.
> > +*/
> > +#define XEN_ARGO_MAXIOV          8U
> > +
> > +DEFINE_XEN_GUEST_HANDLE(uint8_t);
>
> There's no need for this, you can simply use ...
>
> > +typedef struct xen_argo_iov
> > +{
> > +#ifdef XEN_GUEST_HANDLE_64
>
> Note that this is defined only when __XEN__ or __XEN_TOOLS__ are
> defined, i.e. not for an "ordinary" consumer. I'm afraid that - as
> said before - you won't get around some translation as long as you
> use any form of handles.

ack.
I've retained use of the handle, so have added compat handling in v7.

>
> > +    XEN_GUEST_HANDLE_64(uint8_t) iov_hnd;
>
> ... XEN_GUEST_HANDLE(uint8) here.
>
> > +#else
> > +    uint64_t iov_hnd;
>
> Clearly this is not a suitable alternative for a handle.
>
> > --- a/xen/include/xlat.lst
> > +++ b/xen/include/xlat.lst
> > @@ -152,3 +152,5 @@
> >  ?    argo_ring                       argo.h
> >  ?    argo_register_ring              argo.h
> >  ?    argo_unregister_ring            argo.h
> > +!    argo_iov                        argo.h
>
> I wasn't able to spot where what this line produces is actually used.
> As per above I think you will need to use it, but if in the end there's
> a different solution, then this should not be added without need.

ack, v7 uses the generated XLAT item for translation.

Christopher

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel