From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH v4 11/16] Add live migration of VMware's hyper-call RPC Date: Fri, 12 Sep 2014 09:54:49 -0400 Message-ID: <5412FB29.9090800@oracle.com> References: <1410460610-14759-1-git-send-email-dslutz@verizon.com> <1410460610-14759-12-git-send-email-dslutz@verizon.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1410460610-14759-12-git-send-email-dslutz@verizon.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: Kevin Tian , Keir Fraser , Ian Campbell , Stefano Stabellini , Jun Nakajima , Eddie Dong , Ian Jackson , Tim Deegan , George Dunlap , Aravind Gopalakrishnan , Jan Beulich , Andrew Cooper , Suravee Suthikulpanit List-Id: xen-devel@lists.xenproject.org On 09/11/2014 02:36 PM, Don Slutz wrote: > The VMware's hyper-call state is included in live migration and > save/restore. Because the max size of the VMware guestinfo is > large, then data is compressed and expanded in the > vmport_save_domain_ctxt and vmport_load_domain_ctxt. > > Signed-off-by: Don Slutz > --- > xen/arch/x86/hvm/vmware/vmport_rpc.c | 302 ++++++++++++++++++++++++++++++++- > xen/include/public/arch-x86/hvm/save.h | 39 ++++- > 2 files changed, 339 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/hvm/vmware/vmport_rpc.c b/xen/arch/x86/hvm/vmware/vmport_rpc.c > index 695e58f..e25ed09 100644 > --- a/xen/arch/x86/hvm/vmware/vmport_rpc.c > +++ b/xen/arch/x86/hvm/vmware/vmport_rpc.c > @@ -39,7 +39,8 @@ > #include > #include > > -#include "public/arch-x86/hvm/vmporttype.h" > +#include > +#include > > #include "backdoor_def.h" > #include "guest_msg_def.h" > @@ -1270,6 +1271,305 @@ void vmport_rpc_deinit(struct domain *d) > xfree(vs); > } > > +/* save and restore functions */ > + > +static int vmport_save_domain_ctxt(struct domain *d, hvm_domain_context_t *h) > +{ > + struct hvm_vmport_context *ctxt; > + struct hvm_save_descriptor *desc; > + struct hvm_domain *hd = &d->arch.hvm_domain; > + struct vmport_state *vs = hd->vmport_data; > + char *p; > + unsigned int guestinfo_size = 0; > + unsigned int used_guestinfo = 0; > + unsigned int used_guestinfo_jumbo = 0; > + unsigned int chans_size; > + unsigned int i; > + > + /* Customized handling for entry since our entry is of variable length */ > + desc = (struct hvm_save_descriptor *)&h->data[h->cur]; > + if ( _hvm_init_entry(h, HVM_SAVE_CODE(VMPORT), 0, > + HVM_SAVE_LENGTH(VMPORT)) ) > + return 1; > + ctxt = (struct hvm_vmport_context *)&h->data[h->cur]; > + > + spin_lock(&hd->vmport_lock); > + > + ctxt->version = VMPORT_SAVE_VERSION; > + ctxt->ping_time = vs->ping_time; > + ctxt->open_cookie = vs->open_cookie; > + ctxt->used_guestinfo = vs->used_guestinfo; > + ctxt->used_guestinfo_jumbo = vs->used_guestinfo_jumbo; > + > + p = ctxt->u.packed.packed_data; > + > + for ( i = 0; i < VMPORT_MAX_CHANS; i++ ) > + { > + unsigned int j; > + unsigned int buf_max; > + > + ctxt->u.packed.chan_ctl[i].chan = vs->chans[i].ctl; > + buf_max = vs->chans[i].ctl.send_len; > + if ( buf_max > VMPORT_MAX_SEND_BUF * 4 ) > + buf_max = VMPORT_MAX_SEND_BUF * 4; > + memcpy(p, vs->chans[i].send_buf, buf_max); > + p += buf_max; > + for ( j = 0; j < VMPORT_MAX_BKTS; j++ ) > + { > + ctxt->u.packed.chan_ctl[i].recv[j] = vs->chans[i].recv_bkt[j].ctl; > + buf_max = vs->chans[i].recv_bkt[j].ctl.recv_len; > + if ( buf_max > VMPORT_MAX_RECV_BUF * 4 ) > + buf_max = VMPORT_MAX_RECV_BUF * 4; > + memcpy(p, vs->chans[i].recv_bkt[j].recv_buf, buf_max); > + p += buf_max; > + } > + ctxt->u.packed.chan_ctl[i].jumbo = vs->chans[i].jumbo_recv_bkt.ctl; > + buf_max = vs->chans[i].jumbo_recv_bkt.ctl.recv_len; > + if ( buf_max > VMPORT_MAX_RECV_JUMBO_BUF * 4 ) > + buf_max = VMPORT_MAX_RECV_JUMBO_BUF * 4; > + memcpy(p, vs->chans[i].jumbo_recv_bkt.recv_buf, buf_max); > + p += buf_max; > + } > + > + chans_size = p - ctxt->u.packed.packed_data; > + > + for ( i = 0; i < ctxt->used_guestinfo; i++ ) > + { > + vmport_guestinfo_t *vg = vs->guestinfo[i]; > + > + if ( vg && vg->key_len ) > + { > + guestinfo_size += sizeof(vg->key_len) + sizeof(vg->val_len) + > + vg->key_len + vg->val_len; > + used_guestinfo++; > + ASSERT(sizeof(vg->key_len) == 1); > + *p++ = (char) vg->key_len; > + ASSERT(sizeof(vg->val_len) == 1); > + *p++ = (char) vg->val_len; > + if ( vg->key_len ) You ASSERTed that vg->key_len is 1 so you may not need the 'if'. (And in NDEBUG version the worst that could happen is you do memcpy(,,0), which is safe). > + { > + memcpy(p, vg->key_data, vg->key_len); > + p += vg->key_len; > + if ( vg->val_len ) Same here. > + { > + memcpy(p, vg->val_data, vg->val_len); > + p += vg->val_len; > + } > + } > + } > + } > + ctxt->used_guestinfo = used_guestinfo; > + > + for ( i = 0; i < ctxt->used_guestinfo_jumbo; i++ ) > + { > + vmport_guestinfo_jumbo_t *vgj = > + vs->guestinfo_jumbo[i]; > + if ( vgj && vgj->key_len ) > + { > + guestinfo_size += sizeof(vgj->key_len) + sizeof(vgj->val_len) + > + vgj->key_len + vgj->val_len; > + used_guestinfo_jumbo++; > + ASSERT(sizeof(vgj->key_len) == 1); > + *p++ = (char) vgj->key_len; > + memcpy(p, &vgj->val_len, sizeof(vgj->val_len)); > + p += sizeof(vgj->val_len); > + if ( vgj->key_len ) And here. And do you need ASSERT(vgj->val_len ==1) as well? Also, In vmport_load_domain_ctxt() you don't seem to be making this assertions. I don't know whether this is on purpose. > + { > + memcpy(p, vgj->key_data, vgj->key_len); > + p += vgj->key_len; > + if ( vgj->val_len ) > + { > + memcpy(p, vgj->val_data, vgj->val_len); > + p += vgj->val_len; > + } > + } > + } > + } > + ctxt->used_guestinfo_jumbo = used_guestinfo_jumbo; > + > + ctxt->used_guestsize = guestinfo_size; > + > + spin_unlock(&hd->vmport_lock); > + > +#ifndef NDEBUG > + gdprintk(XENLOG_WARNING, "chans_size=%d guestinfo_size=%d, used=%ld\n", > + chans_size, guestinfo_size, > + p - ctxt->u.packed.packed_data); > +#endif > + ASSERT(p - ctxt->u.packed.packed_data == chans_size + guestinfo_size); > + ASSERT(desc->length >= p - (char *)ctxt); > + desc->length = p - (char *)ctxt; /* Fixup length to be right */ > + h->cur += desc->length; /* Do _hvm_write_entry */ > + ASSERT(guestinfo_size < desc->length); > + > + return 0; > +} > + > +static int vmport_load_domain_ctxt(struct domain *d, hvm_domain_context_t *h) > +{ > + struct hvm_vmport_context *ctxt; > + struct hvm_save_descriptor *desc; > + struct hvm_domain *hd = &d->arch.hvm_domain; > + struct vmport_state *vs = hd->vmport_data; > + unsigned int i; > + uint8_t key_len; > + uint16_t val_len; > + char *p; > + vmport_guestinfo_t *vg; > + vmport_guestinfo_jumbo_t *vgj; > + unsigned int loop_cnt; > + unsigned int guestinfo_size; > + unsigned int used_guestinfo; > + unsigned int used_guestinfo_jumbo; > + > + if ( !vs ) > + return -ENOMEM; > + > + /* Customized checking for entry since our entry is of variable length */ > + desc = (struct hvm_save_descriptor *)&h->data[h->cur]; > + if ( sizeof(*desc) > h->size - h->cur ) > + { > + printk(XENLOG_G_WARNING > + "HVM%d restore: not enough data left to read descriptor" > + "for type %lu\n", d->domain_id, > + HVM_SAVE_CODE(VMPORT)); > + return -1; Since in some cases you are returning proper error codes you should return them everywhere. -boris > + } > + if ( desc->length + sizeof(*desc) > h->size - h->cur ) > + { > + printk(XENLOG_G_WARNING > + "HVM%d restore: not enough data left to read %u bytes " > + "for type %lu\n", d->domain_id, desc->length, > + HVM_SAVE_CODE(VMPORT)); > + return -1; > + } > + if ( HVM_SAVE_CODE(VMPORT) != desc->typecode || > + (desc->length > HVM_SAVE_LENGTH(VMPORT)) ) > + { > + printk(XENLOG_G_WARNING > + "HVM%d restore mismatch: expected type %lu with max length %lu, " > + "saw type %u length %u\n", d->domain_id, HVM_SAVE_CODE(VMPORT), > + HVM_SAVE_LENGTH(VMPORT), > + desc->typecode, desc->length); > + return -1; > + } > + h->cur += sizeof(*desc); > + /* Checking finished */ > + > + ctxt = (struct hvm_vmport_context *)&h->data[h->cur]; > + h->cur += desc->length; > + > + if ( ctxt->version != VMPORT_SAVE_VERSION ) > + return -EINVAL; > + > + spin_lock(&hd->vmport_lock); > + > + vs->ping_time = ctxt->ping_time; > + vs->open_cookie = ctxt->open_cookie; > + vs->used_guestinfo = ctxt->used_guestinfo; > + vs->used_guestinfo_jumbo = ctxt->used_guestinfo_jumbo; > + guestinfo_size = ctxt->used_guestsize; > + used_guestinfo = ctxt->used_guestinfo; > + used_guestinfo_jumbo = ctxt->used_guestinfo_jumbo; > + > + p = ctxt->u.packed.packed_data; > + > + for ( i = 0; i < VMPORT_MAX_CHANS; i++ ) > + { > + unsigned int j; > + > + vs->chans[i].ctl = ctxt->u.packed.chan_ctl[i].chan; > + memcpy(vs->chans[i].send_buf, p, vs->chans[i].ctl.send_len); > + p += vs->chans[i].ctl.send_len; > + for ( j = 0; j < VMPORT_MAX_BKTS; j++ ) > + { > + vs->chans[i].recv_bkt[j].ctl = ctxt->u.packed.chan_ctl[i].recv[j]; > + memcpy(vs->chans[i].recv_bkt[j].recv_buf, p, > + vs->chans[i].recv_bkt[j].ctl.recv_len); > + p += vs->chans[i].recv_bkt[j].ctl.recv_len; > + } > + vs->chans[i].jumbo_recv_bkt.ctl = ctxt->u.packed.chan_ctl[i].jumbo; > + memcpy(vs->chans[i].jumbo_recv_bkt.recv_buf, p, > + vs->chans[i].jumbo_recv_bkt.ctl.recv_len); > + p += vs->chans[i].jumbo_recv_bkt.ctl.recv_len; > + } > + > + > + /* keep at least 10 total and 5 empty entries */ > + loop_cnt = (vs->used_guestinfo + 5) > 10 ? > + (vs->used_guestinfo + 5) : 10; > + for ( i = 0; i < loop_cnt; i++ ) > + { > + if ( !vs->guestinfo[i] ) > + { > + vs->guestinfo[i] = xzalloc(vmport_guestinfo_t); > + } > + if ( i < vs->used_guestinfo > + && guestinfo_size > 0 ) > + { > + key_len = (uint8_t)*p++; > + val_len = (uint8_t)*p++; > + guestinfo_size -= 2; > + if ( guestinfo_size >= key_len + val_len ) > + { > + vg = vs->guestinfo[i]; > + if ( key_len ) > + { > + vg->key_len = key_len; > + vg->val_len = val_len; > + memcpy(vg->key_data, p, key_len); > + p += key_len; > + memcpy(vg->val_data, p, val_len); > + p += val_len; > + guestinfo_size -= key_len + val_len; > + } > + } > + } > + } > + vs->used_guestinfo = loop_cnt; > + > + /* keep at least 2 total and 1 empty entries */ > + loop_cnt = (vs->used_guestinfo_jumbo + 1) > 2 ? > + (vs->used_guestinfo_jumbo + 1) : 2; > + for ( i = 0; i < loop_cnt; i++ ) > + { > + if ( !vs->guestinfo_jumbo[i] ) > + { > + vs->guestinfo_jumbo[i] = xzalloc(vmport_guestinfo_jumbo_t); > + } > + if ( i < vs->used_guestinfo_jumbo > + && guestinfo_size > 0 ) > + { > + key_len = (uint8_t)*p++; > + memcpy(&val_len, p, 2); > + p += 2; > + guestinfo_size -= 3; > + if ( guestinfo_size >= key_len + val_len ) > + { > + vgj = vs->guestinfo_jumbo[i]; > + if ( key_len ) > + { > + vgj->key_len = key_len; > + vgj->val_len = val_len; > + memcpy(vgj->key_data, p, key_len); > + p += key_len; > + memcpy(vgj->val_data, p, val_len); > + p += val_len; > + guestinfo_size -= key_len + val_len; > + } > + } > + } > + } > + vs->used_guestinfo_jumbo = loop_cnt; > + > + spin_unlock(&hd->vmport_lock); > + > + return 0; > +} > + > +HVM_REGISTER_SAVE_RESTORE(VMPORT, vmport_save_domain_ctxt, > + vmport_load_domain_ctxt, 1, HVMSR_PER_DOM); > + > /* > * Local variables: > * mode: C > diff --git a/xen/include/public/arch-x86/hvm/save.h b/xen/include/public/arch-x86/hvm/save.h > index 16d85a3..7cdcb8f 100644 > --- a/xen/include/public/arch-x86/hvm/save.h > +++ b/xen/include/public/arch-x86/hvm/save.h > @@ -26,6 +26,8 @@ > #ifndef __XEN_PUBLIC_HVM_SAVE_X86_H__ > #define __XEN_PUBLIC_HVM_SAVE_X86_H__ > > +#include "vmporttype.h" > + > /* > * Save/restore header: general info about the save file. > */ > @@ -610,9 +612,44 @@ struct hvm_msr { > > #define CPU_MSR_CODE 20 > > +/* > + * VMware context. > + */ > +struct hvm_vmport_context { > + uint32_t version; > + uint32_t used_guestsize; > + uint64_t ping_time; > + uint32_t open_cookie; > + uint32_t used_guestinfo; > + uint32_t used_guestinfo_jumbo; > + union { > + struct { > + vmport_channel_t chans[VMPORT_MAX_CHANS]; > + vmport_guestinfo_t guestinfo[VMPORT_MAX_NUM_KEY]; > + vmport_guestinfo_jumbo_t guestinfo_jumbo[VMPORT_MAX_NUM_JUMBO_KEY]; > + } full; > + struct { > + struct { > + vmport_channel_control_t chan; > + vmport_bucket_control_t recv[VMPORT_MAX_BKTS]; > + vmport_bucket_control_t jumbo; > + } chan_ctl[VMPORT_MAX_CHANS]; > +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L > + char packed_data[]; > +#elif defined(__GNUC__) > + char packed_data[0]; > +#else > + char packed_data[1 /* Variable length */]; > +#endif > + } packed; > + } u; > +}; > + > +DECLARE_HVM_SAVE_TYPE(VMPORT, 21, struct hvm_vmport_context); > + > /* > * Largest type-code in use > */ > -#define HVM_SAVE_CODE_MAX 20 > +#define HVM_SAVE_CODE_MAX 21 > > #endif /* __XEN_PUBLIC_HVM_SAVE_X86_H__ */