Re: [Qemu-devel] [Xen-devel] [XEN][RFC PATCH 09/15] xc: Add the hypercall for multiple servers

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2012-03-22 at 15:59 +0000, Julien Grall wrote:
> This patch add 5 hypercalls to register server, io range and PCI.
> 
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> ---
>  tools/libxc/xc_domain.c |  140 +++++++++++++++++++++++++++++++++++++++++++++++
>  tools/libxc/xenctrl.h   |   13 ++++
>  2 files changed, 153 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index d98e68b..8067397 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -1514,6 +1514,146 @@ int xc_domain_set_virq_handler(xc_interface *xch, uint32_t domid, int virq)
>      return do_domctl(xch, &domctl);
>  }
>  
> +int xc_hvm_register_ioreq_server(xc_interface *xch, domid_t dom, servid_t *id)
> +{
> +    DECLARE_HYPERCALL;
> +    DECLARE_HYPERCALL_BUFFER(xen_hvm_register_ioreq_server_t, arg);
> +    int rc = -1;
> +
> +    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof (*arg));
> +    if (!arg) {

Xen Coding style calls for
      if ( !arg )
      {

here and elsewhere in this patch.

> +        PERROR("Could not allocate memory for xc_hvm_register_ioreq_server hypercall");
> +        goto out;
> +    }
> +
> +    hypercall.op        = __HYPERVISOR_hvm_op;
> +    hypercall.arg[0]    = HVMOP_register_ioreq_server;
> +    hypercall.arg[1]    = HYPERCALL_BUFFER_AS_ARG(arg);
> +
> +    arg->domid = dom;
> +    rc = do_xen_hypercall(xch, &hypercall);
> +    *id = arg->id;

You could just return this if it's always +ve (vs -ve errors). Similarly
in xc_hvm_get_ioreq_server_buf_channel

> +
> +    xc_hypercall_buffer_free(xch, arg);
> +out:
> +    return rc;
> +}
> +
> +int xc_hvm_get_ioreq_server_buf_channel(xc_interface *xch, domid_t dom, servid_t id,
> +                                        unsigned int *channel)

channel should be evtchn_port_t, or if you decide to return it instead
evtchn_port_or_error_t.

> +{
> +    DECLARE_HYPERCALL;
> +    DECLARE_HYPERCALL_BUFFER(xen_hvm_get_ioreq_server_buf_channel_t, arg);
> +    int rc = -1;
> +
> +    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof (*arg));
> +    if (!arg) {
> +        PERROR("Could not allocate memory for xc_hvm_get_ioreq_servr_buf_channel");
> +        goto out;
> +    }
> +
> +    hypercall.op        = __HYPERVISOR_hvm_op;
> +    hypercall.arg[0]    = HVMOP_get_ioreq_server_buf_channel;
> +    hypercall.arg[1]    = HYPERCALL_BUFFER_AS_ARG(arg);
> +
> +    arg->domid = dom;
> +    arg->id = id;
> +    rc = do_xen_hypercall(xch, &hypercall);
> +    *channel = arg->channel;
> +
> +    xc_hypercall_buffer_free(xch, arg);
> +
> +out:
> +    return rc;
> +}
> +
> +int xc_hvm_map_io_range_to_ioreq_server(xc_interface *xch, domid_t dom, servid_t id,
> +                                        char is_mmio, uint64_t start, uint64_t end)

not sure char here  buys us anything, either bool or int would seem
fine.

> +{
> +    DECLARE_HYPERCALL;
> +    DECLARE_HYPERCALL_BUFFER(xen_hvm_map_io_range_to_ioreq_server_t, arg);
> +    int rc = -1;
> +
> +    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof (*arg));
> +    if (!arg) {
> +        PERROR("Could not allocate memory for xc_hvm_map_io_range_to_ioreq_server hypercall");
> +        goto out;
> +    }
> +
> +    hypercall.op        = __HYPERVISOR_hvm_op;
> +    hypercall.arg[0]    = HVMOP_map_io_range_to_ioreq_server;
> +    hypercall.arg[1]    = HYPERCALL_BUFFER_AS_ARG(arg);
> +
> +    arg->domid = dom;
> +    arg->id = id;
> +    arg->is_mmio = is_mmio;
> +    arg->s = start;
> +    arg->e = end;
> +
> +    rc = do_xen_hypercall(xch, &hypercall);
> +
> +    xc_hypercall_buffer_free(xch, arg);
> +out:
> +    return rc;
> +}
> +
> +int xc_hvm_unmap_io_range_from_ioreq_server(xc_interface *xch, domid_t dom, servid_t id,
> +                                            char is_mmio, uint64_t addr)
> +{
> +    DECLARE_HYPERCALL;
> +    DECLARE_HYPERCALL_BUFFER(xen_hvm_unmap_io_range_from_ioreq_server_t, arg);
> +    int rc = -1;
> +
> +    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof (*arg));
> +    if (!arg) {
> +        PERROR("Could not allocate memory for xc_hvm_unmap_io_range_from_ioreq_server hypercall");
> +        goto out;
> +    }
> +
> +    hypercall.op        = __HYPERVISOR_hvm_op;
> +    hypercall.arg[0]    = HVMOP_unmap_io_range_from_ioreq_server;
> +    hypercall.arg[1]    = HYPERCALL_BUFFER_AS_ARG(arg);
> +
> +    arg->domid = dom;
> +    arg->id = id;
> +    arg->is_mmio = is_mmio;
> +    arg->addr = addr;
> +    rc = do_xen_hypercall(xch, &hypercall);
> +
> +    xc_hypercall_buffer_free(xch, arg);
> +out:
> +    return rc;
> +}
> +
> +int xc_hvm_register_pcidev(xc_interface *xch, domid_t dom, servid_t id,
> +                           uint16_t bdf)
> +{
> +    DECLARE_HYPERCALL;
> +    DECLARE_HYPERCALL_BUFFER(xen_hvm_register_pcidev_t, arg);
> +    int rc = -1;
> +
> +    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof (*arg));
> +    if (!arg)
> +    {
> +        PERROR("Could not allocate memory for xc_hvm_create_pci hypercall");
> +        goto out;
> +    }
> +
> +    hypercall.op        = __HYPERVISOR_hvm_op;
> +    hypercall.arg[0]    = HVMOP_register_pcidev;
> +    hypercall.arg[1]    = HYPERCALL_BUFFER_AS_ARG(arg);
> +
> +    arg->domid = dom;
> +    arg->id = id;
> +    arg->bdf = bdf;
> +    rc = do_xen_hypercall(xch, &hypercall);
> +
> +    xc_hypercall_buffer_free(xch, arg);
> +out:
> +    return rc;
> +}
> +
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 812e723..bcbfee5 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -1648,6 +1648,19 @@ void xc_clear_last_error(xc_interface *xch);
>  int xc_set_hvm_param(xc_interface *handle, domid_t dom, int param, unsigned long value);
>  int xc_get_hvm_param(xc_interface *handle, domid_t dom, int param, unsigned long *value);
>  
> +int xc_hvm_register_ioreq_server(xc_interface *xch, domid_t dom, unsigned int *id);
> +int xc_hvm_get_ioreq_server_buf_channel(xc_interface *xch, domid_t dom, servid_t id,
> +                                        unsigned int *channel);
> +int xc_hvm_map_io_range_to_ioreq_server(xc_interface *xch, domid_t dom, unsigned int id,
> +                                        char is_mmio, uint64_t start, uint64_t end);
> +int xc_hvm_unmap_io_range_from_ioreq_server(xc_interface *xch, domid_t dom, unsigned int id,
> +                                            char is_mmio, uint64_t addr);
> +/*
> + * Register a PCI device
> + */
> +int xc_hvm_register_pcidev(xc_interface *xch, domid_t dom, unsigned int id,
> +                           uint16_t bdf);
> +
>  /* IA64 specific, nvram save */
>  int xc_ia64_save_to_nvram(xc_interface *xch, uint32_t dom);
>