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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.01.19 at 03:04, <christopher.w.clark@xxxxxxxxx> wrote:
> --- a/xen/common/argo.c
> +++ b/xen/common/argo.c
> @@ -32,16 +32,24 @@
>  #ifdef CONFIG_COMPAT
>  #include <compat/argo.h>
>  CHECK_argo_addr;
> +#undef CHECK_argo_addr
> +#define CHECK_argo_addr struct xen_argo_addr
>  CHECK_argo_register_ring;
>  CHECK_argo_ring;
>  CHECK_argo_unregister_ring;
> +CHECK_argo_send_addr;
>  #endif



> --- 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.

> +    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.

Jan



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