xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Don Slutz <dslutz@verizon.com>, xen-devel@lists.xen.org
Cc: Keir Fraser <keir@xen.org>,
	Ian Campbell <ian.campbell@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Eddie Dong <eddie.dong@intel.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Subject: Re: [RFC PATCH 10/10] Add VMware guest info access
Date: Fri, 13 Dec 2013 01:08:07 +0000	[thread overview]
Message-ID: <52AA5DF7.9040304@citrix.com> (raw)
In-Reply-To: <1386875718-28166-11-git-send-email-dslutz@terremark.com>

On 12/12/2013 19:15, Don Slutz wrote:
> From: Don Slutz <dslutz@verizon.com>
>
> Signed-off-by: Don Slutz <dslutz@verizon.com>
> ---
>  tools/libxc/xc_domain.c         | 112 ++++++++++++++++++++++++++++++
>  tools/libxc/xenctrl.h           |  24 +++++++
>  xen/arch/x86/hvm/hvm.c          | 148 ++++++++++++++++++++++++++++++++++++++++
>  xen/include/public/hvm/hvm_op.h |  18 +++++
>  4 files changed, 302 insertions(+)
>
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index 1ccafc5..0437c6f 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -1246,6 +1246,118 @@ int xc_get_hvm_param(xc_interface *handle, domid_t dom, int param, unsigned long
>      return rc;
>  }
>  
> +int xc_set_vmport_guest_info(xc_interface *handle,
> +                             domid_t dom,
> +                             unsigned int key_len,
> +                             char *key,
> +                             unsigned int val_len,
> +                             char *val)
> +{
> +    DECLARE_HYPERCALL;
> +    DECLARE_HYPERCALL_BUFFER(xen_hvm_vmport_guest_info_t, arg);
> +    int rc;
> +
> +    if ((key_len < 1) ||
> +        (key_len > VMPORT_GUEST_INFO_KEY_MAX) ||
> +        (val_len > VMPORT_GUEST_INFO_VAL_MAX)) {
> +        return -1;
> +    }
> +
> +    arg = xc_hypercall_buffer_alloc(handle, arg, sizeof(*arg));
> +    if ( arg == NULL )
> +        return -1;
> +
> +    hypercall.op     = __HYPERVISOR_hvm_op;
> +    hypercall.arg[0] = HVMOP_set_vmport_guest_info;
> +    hypercall.arg[1] = HYPERCALL_BUFFER_AS_ARG(arg);
> +    arg->domid = dom;
> +    arg->key_length = key_len;
> +    arg->value_length = val_len;
> +    memcpy(arg->data, key, key_len);
> +    memcpy(&arg->data[key_len], val, val_len);
> +    rc = do_xen_hypercall(handle, &hypercall);
> +    xc_hypercall_buffer_free(handle, arg);
> +    return rc;
> +}
> +
> +int xc_get_vmport_guest_info(xc_interface *handle,
> +                             domid_t dom,
> +                             unsigned int key_len,
> +                             char *key,
> +                             unsigned int val_max,
> +                             unsigned int *val_len,
> +                             char *val)
> +{
> +    DECLARE_HYPERCALL;
> +    DECLARE_HYPERCALL_BUFFER(xen_hvm_vmport_guest_info_t, arg);
> +    int rc;
> +
> +    if ((key_len < 1) ||
> +        (key_len > VMPORT_GUEST_INFO_KEY_MAX) )
> +        return -1;
> +
> +    arg = xc_hypercall_buffer_alloc(handle, arg, sizeof(*arg));
> +
> +    hypercall.op     = __HYPERVISOR_hvm_op;
> +    hypercall.arg[0] = HVMOP_get_vmport_guest_info;
> +    hypercall.arg[1] = HYPERCALL_BUFFER_AS_ARG(arg);
> +    arg->domid = dom;
> +    arg->key_length = key_len;
> +    arg->value_length = 0;
> +    *val_len = 0;
> +    memcpy(arg->data, key, key_len);
> +    rc = do_xen_hypercall(handle, &hypercall);
> +    if (rc == 0) {
> +        if (arg->value_length > val_max)
> +            arg->value_length = val_max;
> +        *val_len = arg->value_length;
> +        memcpy(val, &arg->data[key_len], arg->value_length);
> +    }
> +    xc_hypercall_buffer_free(handle, arg);
> +    return rc;
> +}
> +
> +int xc_fetch_vmport_guest_info(xc_interface *handle,
> +                               domid_t dom,
> +                               unsigned int idx,
> +                               unsigned int key_max,
> +                               unsigned int *key_len,
> +                               char *key,
> +                               unsigned int val_max,
> +                               unsigned int *val_len,
> +                               char *val)
> +{
> +    DECLARE_HYPERCALL;
> +    DECLARE_HYPERCALL_BUFFER(xen_hvm_vmport_guest_info_t, arg);
> +    int rc;
> +
> +    arg = xc_hypercall_buffer_alloc(handle, arg, sizeof(*arg));
> +
> +    hypercall.op     = __HYPERVISOR_hvm_op;
> +    hypercall.arg[0] = HVMOP_get_vmport_guest_info;
> +    hypercall.arg[1] = HYPERCALL_BUFFER_AS_ARG(arg);
> +    arg->domid = dom;
> +    arg->key_length = 0;
> +    arg->value_length = idx;
> +    *key_len = 0;
> +    *val_len = 0;
> +    rc = do_xen_hypercall(handle, &hypercall);
> +    if (rc == 0) {
> +        if (arg->key_length > key_max)
> +            arg->key_length = key_max;
> +        *key_len = arg->key_length;
> +        memcpy(key, arg->data, arg->key_length);
> +        if (arg->value_length > val_max)
> +            arg->value_length = val_max;
> +        *val_len = arg->value_length;
> +        memcpy(val,
> +               &arg->data[arg->key_length],
> +               arg->value_length);
> +    }
> +    xc_hypercall_buffer_free(handle, arg);
> +    return rc;
> +}
> +
>  int xc_domain_setdebugging(xc_interface *xch,
>                             uint32_t domid,
>                             unsigned int enable)
> diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h
> index 6e58ebe..6b22b3b 100644
> --- a/tools/libxc/xenctrl.h
> +++ b/tools/libxc/xenctrl.h
> @@ -1774,6 +1774,30 @@ 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_set_vmport_guest_info(xc_interface *handle,
> +                             domid_t dom,
> +                             unsigned int key_len,
> +                             char *key,
> +                             unsigned int val_len,
> +                             char *val);
> +int xc_get_vmport_guest_info(xc_interface *handle,
> +                             domid_t dom,
> +                             unsigned int key_len,
> +                             char *key,
> +                             unsigned int val_max,
> +                             unsigned int *val_len,
> +                             char *val);
> +int xc_fetch_vmport_guest_info(xc_interface *handle,
> +                               domid_t dom,
> +                               unsigned int idx,
> +                               unsigned int key_max,
> +                               unsigned int *key_len,
> +                               char *key,
> +                               unsigned int val_max,
> +                               unsigned int *val_len,
> +                               char *val);
> +
> +
>  /* HVM guest pass-through */
>  int xc_assign_device(xc_interface *xch,
>                       uint32_t domid,
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index a557272..c6f84fc 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4662,6 +4662,154 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
>          break;
>      }
>  
> +    case HVMOP_get_vmport_guest_info:
> +    case HVMOP_set_vmport_guest_info:
> +    {
> +        struct xen_hvm_vmport_guest_info a;
> +        struct domain *d;
> +        char *key = NULL;
> +        char *value = NULL;
> +        struct vmport_state *vs;
> +        int idx;
> +        vmport_guestinfo_t *add_slots[5];
> +        int num_slots = 0, num_free_slots = 0;
> +
> +        if ( copy_from_guest(&a, arg, 1) )
> +            return -EFAULT;
> +
> +        ASSERT(strlen("guestinfo.") == 10);

Debugging code?

> +#if VMPORT_MAX_KEY_LEN + 10 != VMPORT_GUEST_INFO_KEY_MAX
> +#error Need to adjust VMPORT_MAX_KEY_LEN & VMPORT_GUEST_INFO_KEY_MAX
> +#endif
> +#if VMPORT_MAX_VAL_LEN != VMPORT_GUEST_INFO_VAL_MAX
> +#error Need to adjust VMPORT_MAX_VAL_LEN & VMPORT_GUEST_INFO_VAL_MAX
> +#endif
> +        if ( a.key_length > strlen("guestinfo.") ) {

Xen braces style.

> +            if ( (unsigned long)a.key_length + (unsigned long)a.value_length > sizeof(a.data) )
> +                return -EINVAL;
> +            if ( memcmp(a.data, "guestinfo.", strlen("guestinfo.")) == 0 ) {
> +                key = &a.data[strlen("guestinfo.")];
> +                a.key_length -= strlen("guestinfo.");
> +            } else {
> +                key = &a.data[0];
> +            }
> +            value = key + a.key_length;
> +        } else if (a.key_length > 0) {
> +            if ( (unsigned long)a.key_length + (unsigned long)a.value_length > sizeof(a.data) )
> +                return -EINVAL;
> +            key = &a.data[0];
> +            if ( a.key_length > VMPORT_MAX_KEY_LEN )
> +                return -EINVAL;
> +            if ( a.value_length > VMPORT_MAX_VAL_LEN )
> +                return -EINVAL;
> +            value = key + a.key_length;
> +        } else if ( (a.key_length == 0) && (op == HVMOP_set_vmport_guest_info) ) {
> +            return -EINVAL;
> +        }
> +        d = rcu_lock_domain_by_any_id(a.domid);
> +        if ( d == NULL )
> +            return rc;
> +
> +        rc = -EINVAL;
> +        if ( !is_hvm_domain(d) )
> +            goto param_fail9;
> +
> +        rc = xsm_hvm_param(XSM_TARGET, d, op);
> +        if ( rc )
> +            goto param_fail9;
> +
> +        vs = d->arch.hvm_domain.vmport_data;
> +        if ((a.key_length == 0) && (a.value_length >= vs->used_guestinfo)) {
> +            rc = -E2BIG;
> +            goto param_fail9;
> +        }
> +        for (idx = 0; idx < vs->used_guestinfo; idx++) {
> +            if (vs->guestinfo[idx] &&
> +                (vs->guestinfo[idx]->key_len == 0))
> +                num_free_slots++;
> +        }
> +        if (num_free_slots < 5) {
> +            num_slots = 5 - num_free_slots;
> +            if (vs->used_guestinfo + num_slots > VMPORT_MAX_NUM_KEY)
> +                num_slots = VMPORT_MAX_NUM_KEY - vs->used_guestinfo;
> +            for (idx = 0; idx < num_slots; idx++)
> +                add_slots[idx] = xzalloc(vmport_guestinfo_t);
> +        }
> +
> +        spin_lock(&d->arch.hvm_domain.vmport_lock);
> +
> +        for (idx = 0; idx < num_slots; idx++)
> +            vs->guestinfo[vs->used_guestinfo + idx] = add_slots[idx];
> +        vs->used_guestinfo += num_slots;
> +
> +        if ( op == HVMOP_set_vmport_guest_info )
> +        {
> +            int free_idx = -1;
> +
> +            for (idx = 0; idx < vs->used_guestinfo; idx++) {
> +                if (!vs->guestinfo[idx]) {
> +                    gdprintk(XENLOG_WARNING, "idx=%d not allocated, but used_guestinfo=%d\n",
> +                             idx, vs->used_guestinfo);
> +                } else if ((vs->guestinfo[idx]->key_len == a.key_length) &&
> +                           (memcmp(key,
> +                                   vs->guestinfo[idx]->key_data,
> +                                   vs->guestinfo[idx]->key_len) == 0)) {
> +                    vs->guestinfo[idx]->val_len = a.value_length;
> +                    memcpy(vs->guestinfo[idx]->val_data, value, a.value_length);
> +                    break;

This break will skip the spin_unlock() on the vmport_lock, as will most
of the others by the looks of the code.

> +                } else if ((vs->guestinfo[idx]->key_len == 0) &&
> +                           (free_idx == -1)) {
> +                    free_idx = idx;
> +                }
> +            }
> +            if (idx >= vs->used_guestinfo) {
> +                if (free_idx == -1) {
> +                    rc = -EBUSY;
> +                } else {
> +                    vs->guestinfo[free_idx]->key_len = a.key_length;
> +                    memcpy(vs->guestinfo[free_idx]->key_data, key, a.key_length);
> +                    vs->guestinfo[free_idx]->val_len = a.value_length;
> +                    memcpy(vs->guestinfo[free_idx]->val_data, value, a.value_length);
> +                }
> +            }
> +        }
> +        else
> +        {
> +            if (a.key_length == 0) {
> +                idx = a.value_length;
> +                a.key_length = vs->guestinfo[idx]->key_len;
> +                memcpy(a.data, vs->guestinfo[idx]->key_data, a.key_length);
> +                a.value_length = vs->guestinfo[idx]->val_len;
> +                memcpy(&a.data[a.key_length],
> +                       vs->guestinfo[idx]->val_data,
> +                       a.value_length);
> +                rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
> +            } else {
> +                for (idx = 0; idx < vs->used_guestinfo; idx++) {
> +                    if ((vs->guestinfo[idx]->key_len == a.key_length) &&
> +                        (memcmp(key,
> +                                vs->guestinfo[idx]->key_data,
> +                                vs->guestinfo[idx]->key_len) == 0)) {
> +                        a.value_length = vs->guestinfo[idx]->val_len;
> +                        memcpy(value,
> +                               vs->guestinfo[idx]->val_data,
> +                               a.value_length);
> +                        rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
> +                        break;
> +                    }
> +                }
> +                if (idx >= vs->used_guestinfo) {
> +                    rc = -ENOENT;
> +                }
> +            }
> +        }
> +        spin_unlock(&d->arch.hvm_domain.vmport_lock);
> +
> +    param_fail9:
> +        rcu_unlock_domain(d);
> +        break;
> +    }
> +
>      default:
>      {
>          gdprintk(XENLOG_DEBUG, "Bad HVM op %ld.\n", op);
> diff --git a/xen/include/public/hvm/hvm_op.h b/xen/include/public/hvm/hvm_op.h
> index a9aab4b..a530903 100644
> --- a/xen/include/public/hvm/hvm_op.h
> +++ b/xen/include/public/hvm/hvm_op.h
> @@ -272,4 +272,22 @@ DEFINE_XEN_GUEST_HANDLE(xen_hvm_inject_msi_t);
>  
>  #endif /* defined(__XEN__) || defined(__XEN_TOOLS__) */
>  
> +/* Get/set vmport subcommands */
> +#define HVMOP_get_vmport_guest_info 17
> +#define HVMOP_set_vmport_guest_info 18

These probably want to be domctl hypercalls not HVMOPs

> +#define VMPORT_GUEST_INFO_KEY_MAX 40
> +#define VMPORT_GUEST_INFO_VAL_MAX 128
> +struct xen_hvm_vmport_guest_info {
> +    /* Domain to be accessed */
> +    domid_t   domid;
> +    /* key length */
> +    uint16_t   key_length;
> +    /* value length */
> +    uint16_t   value_length;
> +    /* key and value data */
> +    char      data[VMPORT_GUEST_INFO_KEY_MAX + VMPORT_GUEST_INFO_VAL_MAX];

This wants to be a GUEST_HANDLE

~Andrew

> +};
> +typedef struct xen_hvm_vmport_guest_info xen_hvm_vmport_guest_info_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_hvm_vmport_guest_info_t);
> +
>  #endif /* __XEN_PUBLIC_HVM_HVM_OP_H__ */

  reply	other threads:[~2013-12-13  1:08 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-12 19:15 [RFC PATCH 00/10] Xen VMware tools support Don Slutz
2013-12-12 19:15 ` [RFC PATCH 01/10] smbios: Add "plus VMware-Tools" to HVM_XS_SYSTEM_PRODUCT_NAME Don Slutz
2013-12-12 19:35   ` Olaf Hering
2013-12-12 22:07     ` Andrew Cooper
2013-12-13 18:03       ` Don Slutz
2013-12-12 19:15 ` [RFC PATCH 02/10] Add VMware HVM params Don Slutz
2013-12-12 22:32   ` Andrew Cooper
2013-12-13 18:12     ` Don Slutz
2013-12-13 10:52   ` Jan Beulich
2013-12-13 18:13     ` Don Slutz
2013-12-17 20:02   ` Konrad Rzeszutek Wilk
2013-12-19  0:47     ` Don Slutz
2013-12-12 19:15 ` [RFC PATCH 03/10] Add cpuid_vmware_leaves Don Slutz
2013-12-12 22:27   ` Andrew Cooper
2013-12-13 10:55     ` Jan Beulich
2013-12-13 13:38       ` Andrew Cooper
2013-12-13 18:55         ` Don Slutz
2013-12-16  8:13           ` Jan Beulich
2013-12-19  0:51             ` Don Slutz
2013-12-17 16:20     ` Don Slutz
2013-12-12 19:15 ` [RFC PATCH 04/10] tools: Add support for new HVM params Don Slutz
2013-12-12 22:36   ` Andrew Cooper
2013-12-13 23:23     ` Don Slutz
2013-12-12 19:15 ` [RFC PATCH 05/10] vmport: Add VMware provided include files Don Slutz
2013-12-17 20:22   ` Konrad Rzeszutek Wilk
2013-12-19  0:54     ` Don Slutz
2013-12-12 19:15 ` [RFC PATCH 06/10] Add vmport structs Don Slutz
2013-12-12 23:10   ` Andrew Cooper
2013-12-19  1:26     ` Don Slutz
2013-12-12 19:15 ` [RFC PATCH 07/10] Add new vmport code Don Slutz
2013-12-13  0:06   ` Andrew Cooper
2013-12-19  2:22     ` Don Slutz
2013-12-13 10:59   ` Jan Beulich
2013-12-19  2:25     ` Don Slutz
2013-12-17 20:36   ` Konrad Rzeszutek Wilk
2013-12-19  2:29     ` Don Slutz
2013-12-12 19:15 ` [RFC PATCH 08/10] connect vmport up Don Slutz
2013-12-13  0:51   ` Andrew Cooper
2013-12-19  2:53     ` Don Slutz
2013-12-13 15:46   ` Boris Ostrovsky
2013-12-19  3:45     ` Don Slutz
2013-12-17 20:37   ` Konrad Rzeszutek Wilk
2013-12-19  3:46     ` Don Slutz
2013-12-12 19:15 ` [RFC PATCH 09/10] libxl: Add VTPOWER, VTREBOOT and VTPING Don Slutz
2013-12-13  0:58   ` Andrew Cooper
2013-12-17 20:30   ` Konrad Rzeszutek Wilk
2013-12-12 19:15 ` [RFC PATCH 10/10] Add VMware guest info access Don Slutz
2013-12-13  1:08   ` Andrew Cooper [this message]
2013-12-13  5:32   ` Matthew Daley
2013-12-17 20:34   ` Konrad Rzeszutek Wilk
2013-12-17 19:03 ` [RFC PATCH 00/10] Xen VMware tools support Konrad Rzeszutek Wilk
2013-12-19  0:46   ` Don Slutz
2013-12-19  9:50     ` Ian Campbell
2013-12-19 14:08       ` Don Slutz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52AA5DF7.9040304@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=dslutz@verizon.com \
    --cc=eddie.dong@intel.com \
    --cc=ian.campbell@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=jun.nakajima@intel.com \
    --cc=keir@xen.org \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=suravee.suthikulpanit@amd.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).