From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [RFC PATCH 06/10] Add vmport structs Date: Thu, 12 Dec 2013 23:10:09 +0000 Message-ID: <52AA4251.4050302@citrix.com> References: <1386875718-28166-1-git-send-email-dslutz@terremark.com> <1386875718-28166-7-git-send-email-dslutz@terremark.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1386875718-28166-7-git-send-email-dslutz@terremark.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Don Slutz , xen-devel@lists.xen.org Cc: Keir Fraser , Ian Campbell , Stefano Stabellini , Jun Nakajima , Eddie Dong , Ian Jackson , Jan Beulich , Boris Ostrovsky , Suravee Suthikulpanit List-Id: xen-devel@lists.xenproject.org On 12/12/2013 19:15, Don Slutz wrote: > From: Don Slutz > > Signed-off-by: Don Slutz > --- > xen/arch/x86/hvm/hvm.c | 59 +++++++++++++++++++++++++++++- > xen/include/asm-x86/hvm/domain.h | 4 +++ > xen/include/asm-x86/hvm/vmport.h | 77 ++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 139 insertions(+), 1 deletion(-) > create mode 100644 xen/include/asm-x86/hvm/vmport.h > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 38641c4..fa5d382 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -57,6 +57,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -536,6 +537,7 @@ static int handle_pvh_io( > int hvm_domain_initialise(struct domain *d) > { > int rc; > + long vmport_mem = 0; > > if ( !hvm_enabled ) > { > @@ -562,6 +564,7 @@ int hvm_domain_initialise(struct domain *d) > > spin_lock_init(&d->arch.hvm_domain.irq_lock); > spin_lock_init(&d->arch.hvm_domain.uc_lock); > + spin_lock_init(&d->arch.hvm_domain.vmport_lock); > > INIT_LIST_HEAD(&d->arch.hvm_domain.msixtbl_list); > spin_lock_init(&d->arch.hvm_domain.msixtbl_list_lock); > @@ -574,11 +577,47 @@ int hvm_domain_initialise(struct domain *d) > > d->arch.hvm_domain.params = xzalloc_array(uint64_t, HVM_NR_PARAMS); > d->arch.hvm_domain.io_handler = xmalloc(struct hvm_io_handler); > + d->arch.hvm_domain.vmport_data = xzalloc(struct vmport_state); > rc = -ENOMEM; > - if ( !d->arch.hvm_domain.params || !d->arch.hvm_domain.io_handler ) > + if ( !d->arch.hvm_domain.params || !d->arch.hvm_domain.io_handler || > + !d->arch.hvm_domain.vmport_data ) > goto fail1; > d->arch.hvm_domain.io_handler->num_slot = 0; > > + vmport_mem += sizeof(struct vmport_state); > + d->arch.hvm_domain.vmport_data->open_cookie = ('C' << 8) + 'S'; This is one place where multicharacter constants would really help. However, I suspect a change to add -Wno-multichar to the build might not go down too well. > + d->arch.hvm_domain.vmport_data->used_guestinfo = 10; > + > + for (rc = 0; rc < d->arch.hvm_domain.vmport_data->used_guestinfo; rc++) { Xen style - space inside brackets and braces lined up. > + d->arch.hvm_domain.vmport_data->guestinfo[rc] = xzalloc(vmport_guestinfo_t); xzalloc_array() of used_guestinfo entries, and needs to be checked for an allocation failure. > + vmport_mem += sizeof(vmport_guestinfo_t); > + } > + d->arch.hvm_domain.vmport_data->guestinfo[0]->key_len = 2; > + memcpy(d->arch.hvm_domain.vmport_data->guestinfo[0]->key_data, "ip", 2); > + > + gdprintk(XENLOG_DEBUG, "vmport_mem=%ld bytes (%ld KiB, %ld MiB)\n", > + vmport_mem, > + (vmport_mem + (1 << 10) - 1) >> 10, > + (vmport_mem + (1 << 20) - 1) >> 20); > + vmport_mem += sizeof(uint64_t) * HVM_NR_PARAMS; > + vmport_mem += sizeof(struct hvm_io_handler); > + gdprintk(XENLOG_DEBUG, "hvm overhead=%ld bytes (%ld KiB, %ld MiB)\n", > + vmport_mem, > + (vmport_mem + (1 << 10) - 1) >> 10, > + (vmport_mem + (1 << 20) - 1) >> 20); > + gdprintk(XENLOG_DEBUG, "tot_pages=%d bytes (%d KiB, %d MiB)\n", > + d->tot_pages, > + (d->tot_pages + (1 << 10) - 1) >> 10, > + (d->tot_pages + (1 << 20) - 1) >> 20); > + gdprintk(XENLOG_DEBUG, "max_pages=%d bytes (%d KiB, %d MiB)\n", > + d->max_pages, > + (d->max_pages + (1 << 10) - 1) >> 10, > + (d->max_pages + (1 << 20) - 1) >> 20); These two gdprintk()s do not appear to be related to the content of this patch. > + > +#if 0 > + vmport_flush(&d->arch.hvm_domain); > +#endif Is this stray debugging? > + > if ( is_pvh_domain(d) ) > { > register_portio_handler(d, 0, 0x10003, handle_pvh_io); > @@ -617,6 +656,15 @@ int hvm_domain_initialise(struct domain *d) > stdvga_deinit(d); > vioapic_deinit(d); > fail1: > + if (d->arch.hvm_domain.vmport_data) { > + struct vmport_state *vs = d->arch.hvm_domain.vmport_data; > + int idx; > + > + for (idx = 0; idx < vs->used_guestinfo; idx++) { > + xfree(vs->guestinfo[idx]); > + } > + } > + xfree(d->arch.hvm_domain.vmport_data); > xfree(d->arch.hvm_domain.io_handler); > xfree(d->arch.hvm_domain.params); > fail0: > @@ -626,6 +674,15 @@ int hvm_domain_initialise(struct domain *d) > > void hvm_domain_relinquish_resources(struct domain *d) > { > + if (d->arch.hvm_domain.vmport_data) { > + struct vmport_state *vs = d->arch.hvm_domain.vmport_data; > + int idx; > + > + for (idx = 0; idx < vs->used_guestinfo; idx++) { > + xfree(vs->guestinfo[idx]); > + } > + xfree(d->arch.hvm_domain.vmport_data); > + } > xfree(d->arch.hvm_domain.io_handler); > xfree(d->arch.hvm_domain.params); > > diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h > index b1e3187..0ca2778 100644 > --- a/xen/include/asm-x86/hvm/domain.h > +++ b/xen/include/asm-x86/hvm/domain.h > @@ -62,6 +62,10 @@ struct hvm_domain { > /* emulated irq to pirq */ > struct radix_tree_root emuirq_pirq; > > + /* VMware special port */ > + spinlock_t vmport_lock; > + struct vmport_state *vmport_data; > + Stray space between struct and vmport. > uint64_t *params; > > /* Memory ranges with pinned cache attributes. */ > diff --git a/xen/include/asm-x86/hvm/vmport.h b/xen/include/asm-x86/hvm/vmport.h > new file mode 100644 > index 0000000..180ddac > --- /dev/null > +++ b/xen/include/asm-x86/hvm/vmport.h > @@ -0,0 +1,77 @@ > +/* > + * vmport.h: HVM VMPORT emulation > + * > + * > + * Copyright (C) 2012 Verizon Corporation > + * > + * This file is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License Version 2 (GPLv2) > + * as published by the Free Software Foundation. > + * > + * This file 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 > + * General Public License for more details. . > + */ > + > +#ifndef __ASM_X86_HVM_VMPORT_H__ > +#define __ASM_X86_HVM_VMPORT_H__ > + > +/* Note: VMPORT_PORT and VMPORT_MAGIC is also defined as BDOOR_PORT > + * and BDOOR_MAGIC in backdoor_def.h Defined here so that other > + * parts of XEN can use it. > + */ > + > +#define VMPORT_PORT 0x5658 > +#define VMPORT_MAGIC 0x564D5868 You should include backdoor_def.h rather than having the same constant defined twice in the codebase. > + > +#define VMPORT_MAX_KEY_LEN 30 > +#define VMPORT_MAX_VAL_LEN 128 > +#define VMPORT_MAX_NUM_KEY 128 > + > +#define VMPORT_MAX_SEND_BUF ((22 + VMPORT_MAX_KEY_LEN + VMPORT_MAX_VAL_LEN + 3)/4) > +#define VMPORT_MAX_RECV_BUF ((2 + VMPORT_MAX_VAL_LEN + 3)/4) > +#define VMPORT_MAX_CHANS 4 > +#define VMPORT_MAX_BKTS 8 > + > + > +typedef struct vmport_guestinfo_ { Why the trailing underscores in the non-typedef'd name? > + uint8_t key_len; > + uint8_t val_len; > + char key_data[VMPORT_MAX_KEY_LEN]; > + char val_data[VMPORT_MAX_VAL_LEN]; > +} vmport_guestinfo_t; > + > +typedef struct vmport_bucket_ { > + uint16_t recv_len; > + uint16_t recv_idx; > + uint32_t recv_buf[VMPORT_MAX_RECV_BUF + 1]; > + uint8_t recv_slot; > +} vmport_bucket_t; > + > +typedef struct vmport_channel_ { > + unsigned long active_time; > + uint32_t chan_id; > + uint32_t cookie; > + uint32_t proto_num; > + uint16_t send_len; > + uint16_t send_idx; > + uint32_t send_buf[VMPORT_MAX_SEND_BUF + 1]; > + vmport_bucket_t recv_bkt[VMPORT_MAX_BKTS]; > + uint8_t recv_read; > + uint8_t recv_write; > +} vmport_channel_t; > + > +struct vmport_state { > + unsigned long ping_time; > + uint32_t open_cookie; > + uint32_t used_guestinfo; > + vmport_channel_t chans[VMPORT_MAX_CHANS]; > + vmport_guestinfo_t *guestinfo[VMPORT_MAX_NUM_KEY]; > +}; > + > +int vmport_ioport(int dir, int size, unsigned long data, struct cpu_user_regs *regs); > +void vmport_ctrl_send(struct hvm_domain *hd, char *msg, int slot); > +void vmport_flush(struct hvm_domain *hd); These declarations need to be in the same patch as defines them. ~Andrew > + > +#endif /* __ASM_X86_HVM_VMPORT_H__ */