[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [v3 2/5] Qemu-Xen-vTPM: Xen frontend driver infrastructure



On Tue, 30 Dec 2014, Quan Xu wrote:
> This patch adds infrastructure for xen front drivers living in qemu,
> so drivers don't need to implement common stuff on their own.  It's
> mostly xenbus management stuff: some functions to access XenStore,
> setting up XenStore watches, callbacks on device discovery and state
> changes, and handle event channel between the virtual machines.
> 
> Call xen_fe_register() function to register XenDevOps, and make sure,
> XenDevOps's flags is DEVOPS_FLAG_FE, which is flag bit to point out
> the XenDevOps is Xen frontend.
> 
> --Changes in v3:
> -New xen_frontend.c file
> -Move xenbus_switch_state() to xen_frontend.c
> -Move xen_stubdom_be() to xenstore_fe_read_be_str()
> -Move *_stubdom_*() to *_fe_*()
> 
> Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx>
> ---
>  hw/xen/Makefile.objs         |   2 +-
>  hw/xen/xen_backend.c         |  11 +-
>  hw/xen/xen_frontend.c        | 323 
> +++++++++++++++++++++++++++++++++++++++++++
>  include/hw/xen/xen_backend.h |  14 ++
>  4 files changed, 348 insertions(+), 2 deletions(-)
>  create mode 100644 hw/xen/xen_frontend.c
> 
> diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs
> index a0ca0aa..b0bb065 100644
> --- a/hw/xen/Makefile.objs
> +++ b/hw/xen/Makefile.objs
> @@ -1,5 +1,5 @@
>  # xen backend driver support
> -common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
> +common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o 
> xen_frontend.o
>  
>  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
>  obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o 
> xen_pt_msi.o
> diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
> index b2cb22b..ad6e324 100644
> --- a/hw/xen/xen_backend.c
> +++ b/hw/xen/xen_backend.c
> @@ -275,7 +275,7 @@ static struct XenDevice *xen_be_get_xendev(const char 
> *type, int dom, int dev,
>  /*
>   * release xen backend device.
>   */
> -static struct XenDevice *xen_be_del_xendev(int dom, int dev)
> +struct XenDevice *xen_be_del_xendev(int dom, int dev)
>  {
>      struct XenDevice *xendev, *xnext;
>  
> @@ -681,6 +681,10 @@ static void xenstore_update(void *unused)
>      if (sscanf(vec[XS_WATCH_TOKEN], "fe:%" PRIxPTR, &ptr) == 1) {
>          xenstore_update_fe(vec[XS_WATCH_PATH], (void*)ptr);
>      }
> +    if (sscanf(vec[XS_WATCH_TOKEN], "stub:%" PRIxPTR ":%d:%" PRIxPTR,
> +               &type, &dom, &ops) == 3) {
> +        xenstore_fe_update(vec[XS_WATCH_PATH], (void *)type, dom, (void 
> *)ops);
> +    }

Why "stub:"? Where is it coming from?

I think that the xenstore_update function should be moved to a new file:
we don't want xen_backend.c to be handling any frontend updates.
Maybe you could create a new file, hw/xen/xen_pvdev.c?


>  cleanup:
>      free(vec);
> @@ -808,3 +812,8 @@ void xen_be_printf(struct XenDevice *xendev, int 
> msg_level, const char *fmt, ...
>      }
>      qemu_log_flush();
>  }
> +
> +void xen_qtail_insert_xendev(struct XenDevice *xendev)
> +{
> +    QTAILQ_INSERT_TAIL(&xendevs, xendev, next);
> +}

You should either move the xendevs queue to the new file xen_pvdev.c or
you should introduce a separate xendevs queue for frontends in
xen_frontend.c.


> diff --git a/hw/xen/xen_frontend.c b/hw/xen/xen_frontend.c
> new file mode 100644
> index 0000000..07ffc5c
> --- /dev/null
> +++ b/hw/xen/xen_frontend.c
> @@ -0,0 +1,323 @@
> +/*
> + * Xen frontend driver infrastructure
> + *
> + *  Copyright (c) 2014 Intel Corporation
> + *  Authors:
> + *    Quan Xu <quan.xu@xxxxxxxxx>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> <http://www.gnu.org/licenses/>
> + */
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdarg.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <sys/mman.h>
> +#include <sys/signal.h>
> +
> +#include "hw/hw.h"
> +#include "sysemu/char.h"
> +#include "qemu/log.h"
> +#include "hw/xen/xen_backend.h"
> +#include <xen/grant_table.h>
> +
> +/* private */
> +static int debug;
> +
> +/* ------------------------------------------------------------- */
> +/*Get backend with fe type|domid, try to write the backend-id when
> + *create virtual machine.
> + *
> + *[XenStore]
> + *
> + *Dom.ID = ""
> + *  device
> + *    vtpm = ""
> + *      0 = ""
> + *        backend-id = "ID"
> + *[..]
> + */
> +
> +char *xenstore_fe_read_be_str(const char *type, int dom, int dev)
> +{
> +    char *val, *domu;
> +    char path[XEN_BUFSIZE];
> +    unsigned int len, ival;
> +
> +    /*fe path*/
> +    domu = xs_get_domain_path(xenstore, dom);
> +    snprintf(path, sizeof(path), "%s/device/%s/%d/backend-id",
> +             domu, type, dev);
> +    g_free(domu);
> +
> +    val = xs_read(xenstore, 0, path, &len);
> +    if (!val || 1 != sscanf(val, "%d", &ival)) {
> +        g_free(val);
> +        return NULL;
> +    }
> +    g_free(val);
> +
> +    /*be path*/
> +    domu = xs_get_domain_path(xenstore, ival);
> +
> +    return domu;
> +}
> +
> +/*make sure, initialize the 'xendev->fe' in xendev->ops->init() or
> + * xendev->ops->initialize()
> + */
> +int xenbus_switch_state(struct XenDevice *xendev, enum xenbus_state xbus)
> +{
> +    xs_transaction_t xbt = XBT_NULL;
> +
> +    if (xendev->fe_state == xbus) {
> +        return 0;
> +    }
> +
> +    xendev->fe_state = xbus;
> +    if (xendev->fe == NULL) {
> +        xen_be_printf(NULL, 0, "xendev->fe is NULL\n");
> +        return -1;
> +    }
> +
> +retry_transaction:
> +    xbt = xs_transaction_start(xenstore);
> +    if (xbt == XBT_NULL) {
> +        goto abort_transaction;
> +    }
> +
> +    if (xenstore_write_int(xendev->fe, "state", xbus)) {
> +        goto abort_transaction;
> +    }
> +
> +    if (!xs_transaction_end(xenstore, xbt, 0)) {
> +        if (errno == EAGAIN) {
> +            goto retry_transaction;
> +        }
> +    }
> +
> +    return 0;
> +
> +abort_transaction:
> +    xs_transaction_end(xenstore, xbt, 1);
> +    return -1;
> +}
> +
> +void xenstore_fe_update(char *watch, char *type, int dom, struct XenDevOps 
> *ops)
> +{
> +    struct XenDevice *xendev;
> +    char path[XEN_BUFSIZE];
> +    char *ptr, *bepath;
> +    unsigned int len, dev;
> +
> +    if (!(ops->flags & DEVOPS_FLAG_FE)) {
> +        return;
> +    }
> +
> +    len = snprintf(path, sizeof(path), "backend/%s/%d", type, dom);
> +    ptr = strstr(watch, path);
> +    if (ptr == NULL) {
> +        return;
> +    }
> +
> +    if (sscanf(ptr+len, "/%u/%255s", &dev, path) != 2) {
> +        strcpy(path, "");
> +        if (sscanf(ptr+len, "/%u", &dev) != 1) {
> +            dev = -1;
> +        }
> +    }
> +
> +    if (dev == -1) {
> +        return;
> +    }
> +
> +    xendev = xen_fe_get_xendev(type, dom, dev, ops);
> +    if (xendev != NULL) {
> +        bepath = xs_read(xenstore, 0, xendev->be, &len);
> +        /*bepath is NULL, indicates that the backend is not running,
> +         *delete it.
> +         */
> +        if (bepath == NULL) {
> +            xen_be_del_xendev(dom, dev);
> +        } else {
> +            free(bepath);
> +            if (xendev->ops->backend_changed) {
> +                xendev->ops->backend_changed(xendev, path);
> +            }
> +        }
> +    }
> +}
> +
> +/*
> + * get xen fe device, allocate a new one if it doesn't exist.
> + */
> +struct XenDevice *xen_fe_get_xendev(const char *type, int dom, int dev,
> +                                    struct XenDevOps *ops)
> +{
> +    struct XenDevice *xendev;
> +    char *stub;
> +
> +    xendev = xen_be_find_xendev(type, dom, dev);
> +    if (xendev) {
> +        return xendev;
> +    }
> +
> +    /* init new xendev */
> +    xendev = g_malloc0(ops->size);
> +    xendev->type  = type;
> +    xendev->dom   = dom;
> +    xendev->dev   = dev;
> +    xendev->ops   = ops;
> +
> +    /*return if the ops->flags is not DEVOPS_FLAG_FE*/
> +    if (!(ops->flags & DEVOPS_FLAG_FE)) {
> +        return NULL;
> +    }
> +
> +    stub = xenstore_fe_read_be_str(xendev->type, xendev->dom, xendev->dev);
> +    snprintf(xendev->be, sizeof(xendev->be), "%s/backend/%s/%d/%d",
> +             stub, xendev->type, xendev->dom, xendev->dev);
> +    g_free(stub);
> +    snprintf(xendev->name, sizeof(xendev->name), "%s-%d",
> +             xendev->type, xendev->dev);
> +
> +    xendev->debug = debug;
> +    xendev->local_port = -1;
> +
> +    xendev->evtchndev = xen_xc_evtchn_open(NULL, 0);
> +    if (xendev->evtchndev == XC_HANDLER_INITIAL_VALUE) {
> +        xen_be_printf(NULL, 0, "can't open evtchn device\n");
> +        g_free(xendev);
> +        return NULL;
> +    }
> +    fcntl(xc_evtchn_fd(xendev->evtchndev), F_SETFD, FD_CLOEXEC);
> +
> +    if (ops->flags & DEVOPS_FLAG_NEED_GNTDEV) {
> +        xendev->gnttabdev = xen_xc_gnttab_open(NULL, 0);
> +        if (xendev->gnttabdev == XC_HANDLER_INITIAL_VALUE) {
> +            xen_be_printf(NULL, 0, "can't open gnttab device\n");
> +            xc_evtchn_close(xendev->evtchndev);
> +            g_free(xendev);
> +            return NULL;
> +        }
> +    } else {
> +        xendev->gnttabdev = XC_HANDLER_INITIAL_VALUE;
> +    }
> +
> +    xen_qtail_insert_xendev(xendev);
> +
> +    if (xendev->ops->alloc) {
> +        xendev->ops->alloc(xendev);
> +    }
> +
> +    return xendev;
> +}
> +
> +/* simplify QEMU side, a thread is running in Xen backend, which will
> + * connect frontend when the frontend is initialised. Call these initialised
> + * functions.
> + */
> +
> +static int xen_fe_check(struct XenDevice *xendev, uint32_t domid,
> +                        int handle)
> +{
> +    int rc = 0;
> +
> +    if (xendev->ops->init) {
> +        rc = xendev->ops->init(xendev);
> +    }
> +
> +    if (rc != 0) {
> +        xen_be_printf(xendev, 0, "xendev %s init error\n",
> +                      xendev->name);
> +       goto err;
> +    }
> +
> +    if (xendev->ops->initialise) {
> +        rc = xendev->ops->initialise(xendev);
> +    }
> +
> +    if (rc != 0) {
> +        xen_be_printf(xendev, 0, "xendev %s initialise error\n",
> +                      xendev->name);
> +       goto err;
> +    }

Shouldn't you be checking the backend status before setting the frontend
to connected?  Something like xen_be_try_connected?


> +    if (xendev->ops->connected) {
> +        xendev->ops->connected(xendev);
> +    }
> +
> +    return rc;
> +
> +err:
> +    xen_be_del_xendev(domid, handle);
> +    return -1;
> +}
> +
> +static int xenstore_fe_scan(const char *type, uint32_t domid,
> +                            struct XenDevOps *ops)
> +{
> +    struct XenDevice *xendev;
> +    char path[XEN_BUFSIZE], token[XEN_BUFSIZE];
> +    char *domu;
> +    unsigned int cdev, j;
> +    char **dev = NULL;
> +
> +    /*Xen frontend : /local/domain/ID */
> +    domu = xs_get_domain_path(xenstore, domid);
> +    snprintf(path, sizeof(path), "%s/device/%s",
> +             domu, type);
> +    free(domu);
> +    dev = xs_directory(xenstore, 0, path, &cdev);
> +    if (dev == NULL) {
> +        return 0;
> +    }
> +
> +    for (j = 0; j < cdev; j++) {
> +        xendev = xen_fe_get_xendev(type, domid, atoi(dev[j]), ops);
> +        if (xendev == NULL) {
> +            xen_be_printf(xendev, 0, "xendev is NULL.\n");
> +            continue;
> +        }
> +
> +        /* simplify QEMU side, a thread is running in Xen backend, which will
> +         * connect frontend when the frontend is initialised.
> +         */
> +        if (xen_fe_check(xendev, domid, atoi(dev[j])) < 0) {
> +            xen_be_printf(xendev, 0, "xendev fe_check error.\n");
> +            continue;
> +        }
> +
> +        /*setup watch*/
> +        snprintf(token, sizeof(token), "stub:%p:%d:%p",
> +                 type, domid, xendev->ops);
> +        if (!xs_watch(xenstore, xendev->be, token)) {
> +            xen_be_printf(xendev, 0, "xs_watch failed.\n");
> +            continue;
> +        }
> +    }
> +
> +    free(dev);
> +    return 0;
> +}
> +
> +int xen_fe_register(const char *type, struct XenDevOps *ops)
> +{
> +    return xenstore_fe_scan(type, xen_domid, ops);
> +}
> diff --git a/include/hw/xen/xen_backend.h b/include/hw/xen/xen_backend.h
> index 3b4125e..06e202f 100644
> --- a/include/hw/xen/xen_backend.h
> +++ b/include/hw/xen/xen_backend.h
> @@ -15,6 +15,8 @@ struct XenDevice;
>  #define DEVOPS_FLAG_NEED_GNTDEV   1
>  /* don't expect frontend doing correct state transitions (aka console quirk) 
> */
>  #define DEVOPS_FLAG_IGNORE_STATE  2
> +/*dev is frontend device*/
> +#define DEVOPS_FLAG_FE            4
>  
>  struct XenDevOps {
>      size_t    size;
> @@ -80,6 +82,8 @@ int xenstore_read_fe_uint64(struct XenDevice *xendev, const 
> char *node, uint64_t
>  const char *xenbus_strstate(enum xenbus_state state);
>  struct XenDevice *xen_be_find_xendev(const char *type, int dom, int dev);
>  void xen_be_check_state(struct XenDevice *xendev);
> +struct XenDevice *xen_be_del_xendev(int dom, int dev);
> +void xen_qtail_insert_xendev(struct XenDevice *xendev);
>  
>  /* xen backend driver bits */
>  int xen_be_init(void);
> @@ -91,6 +95,16 @@ int xen_be_send_notify(struct XenDevice *xendev);
>  void xen_be_printf(struct XenDevice *xendev, int msg_level, const char *fmt, 
> ...)
>      GCC_FMT_ATTR(3, 4);
>  
> +/*Xen frontend driver*/
> +int xen_fe_register(const char *type, struct XenDevOps *ops);
> +int xen_be_alloc_unbound(struct XenDevice *xendev, int dom, int remote_dom);
> +char *xenstore_fe_read_be_str(const char *type, int dom, int dev);
> +int xenbus_switch_state(struct XenDevice *xendev, enum xenbus_state xbus);
> +struct XenDevice *xen_fe_get_xendev(const char *type, int dom, int dev,
> +                                    struct XenDevOps *ops);
> +void xenstore_fe_update(char *watch, char *type, int dom,
> +                        struct XenDevOps *ops);
> +
>  /* actual backend drivers */
>  extern struct XenDevOps xen_console_ops;      /* xen_console.c     */
>  extern struct XenDevOps xen_kbdmouse_ops;     /* xen_framebuffer.c */
> -- 
> 1.8.3.2
> 

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.